<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 12 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
@font-face
{font-family:"Tw Cen MT";
panose-1:2 11 6 2 2 1 4 2 6 3;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri","sans-serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.MsoPlainText, li.MsoPlainText, div.MsoPlainText
{mso-style-priority:99;
mso-style-link:"Plain Text Char";
margin:0in;
margin-bottom:.0001pt;
font-size:10.5pt;
font-family:Consolas;}
p.MsoAcetate, li.MsoAcetate, div.MsoAcetate
{mso-style-priority:99;
mso-style-link:"Balloon Text Char";
margin:0in;
margin-bottom:.0001pt;
font-size:8.0pt;
font-family:"Tahoma","sans-serif";}
span.PlainTextChar
{mso-style-name:"Plain Text Char";
mso-style-priority:99;
mso-style-link:"Plain Text";
font-family:Consolas;}
span.BalloonTextChar
{mso-style-name:"Balloon Text Char";
mso-style-priority:99;
mso-style-link:"Balloon Text";
font-family:"Tahoma","sans-serif";}
span.EmailStyle21
{mso-style-type:personal;
font-family:"Calibri","sans-serif";
color:#1F497D;}
span.EmailStyle22
{mso-style-type:personal;
font-family:"Calibri","sans-serif";
color:#1F497D;}
span.EmailStyle23
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">Hi Carolyn,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">Thank you for your feedback. It is a good idea to eliminate confusion over the currently-named routine –
</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif"">IoCompletionDpcRoutine -</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"> which does indeed imply running as a DPC.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">We could either create a new function, as you suggest or alternatively, we could just rename
</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif"">IoCompletionDpcRoutine
<span style="color:#1F497D">directly to for example,</span></span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"> “</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";background:yellow;mso-highlight:yellow">ProcessIoCompletions</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif"">”</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">
or just “</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";background:yellow;mso-highlight:yellow">IoCompletionRoutine</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif"">”</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">
and then indicate at the top of that routine that it can either be scheduled to run as a DPC or called directly, depending on context. If we take the word “Dpc” out of it, I think it might eliminate all potential confusion.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">If it is ok with you, I think renaming
</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif"">IoCompletionDpcRoutine</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"> to no longer imply the method by which its invoked might be preferable
as this would also cover/clarify the case where, in dump mode, </span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif"">NVMeIsrMsix</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"> is currently calling
</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif"">IoCompletionDpcRoutine</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"> directly.
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">If you still prefer a new routine, to cover both cases above, the name should imply the direct call nature of the function rather than polled mode per se
since, if we call it from the ISR it could lead to the opposite confusion – a function that implies no ISR involvement being called from the ISR itself. We could:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">Create new function called
</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif"">ImmediateCompletionProcessing
</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">(or some other name that implies non-deferred processing) which in turn would call</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif""> IoCompletionDpcRoutine.</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">But again, I think renaming
</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif"">IoCompletionDpcRoutine
</span><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">is better because no matter what we name a new function, if it still calls a routine with the word “Dpc” in it, it will still have the potential to create confusion in
(especially new) readers minds.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">Thanks,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">Judy<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Foster, Carolyn D [mailto:carolyn.d.foster@intel.com]
<br>
<b>Sent:</b> Tuesday, October 28, 2014 4:24 PM<br>
<b>To:</b> Alex Chang; Judy Brock-SSI; suman.p@samsung.com; nvmewin@lists.openfabrics.org<br>
<b>Subject:</b> RE: Samsung Patch for Bus Reset Enhancements<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span style="color:#1F497D">Hi Alex, Judy and Suman,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">I have completed my testing of the proposed patch and verified that it works. I also agree with Judy’s comments below. Since we’re now calling a function that is normally called in a DPC, for clarity, I would
like to see a new function that calls the DPC routine instead. You could give the new function a name that indicates that it will handle command completions in polled mode. Then that new function would call the DPC routine directly, and the new function
would be called from RunningStartAttempt. This way it doesn’t look like the RunningStartAttempt routine is doing anything with DPCs.
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Thanks,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Carolyn<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Alex Chang [<a href="mailto:Alex.Chang@pmcs.com">mailto:Alex.Chang@pmcs.com</a>]
<br>
<b>Sent:</b> Monday, October 27, 2014 8:57 PM<br>
<b>To:</b> Judy Brock-SSI; Foster, Carolyn D; <a href="mailto:suman.p@samsung.com">
suman.p@samsung.com</a>; <a href="mailto:nvmewin@lists.openfabrics.org">nvmewin@lists.openfabrics.org</a><br>
<b>Subject:</b> RE: Samsung Patch for Bus Reset Enhancements<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span style="color:#1F497D">Hi Carolyn,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Please let us know what you think.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Thanks,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Alex<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Judy Brock-SSI [<a href="mailto:judy.brock@ssi.samsung.com">mailto:judy.brock@ssi.samsung.com</a>]
<br>
<b>Sent:</b> Thursday, October 23, 2014 6:45 PM<br>
<b>To:</b> Foster, Carolyn D; Alex Chang; <a href="mailto:suman.p@samsung.com">suman.p@samsung.com</a>;
<a href="mailto:nvmewin@lists.openfabrics.org">nvmewin@lists.openfabrics.org</a><br>
<b>Subject:</b> RE: Samsung Patch for Bus Reset Enhancements<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoPlainText">Hi Carolyn,<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Replies inline below in <span style="color:#1F497D">blue</span>.<o:p></o:p></p>
<p class="MsoPlainText">Thanks,<o:p></o:p></p>
<p class="MsoPlainText">Judy<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">-----Original Message-----<br>
From: <a href="mailto:nvmewin-bounces@lists.openfabrics.org">nvmewin-bounces@lists.openfabrics.org</a> [<a href="mailto:nvmewin-bounces@lists.openfabrics.org">mailto:nvmewin-bounces@lists.openfabrics.org</a>] On Behalf Of Foster, Carolyn D<br>
Sent: Wednesday, October 22, 2014 4:29 PM<br>
To: Alex Chang; <a href="mailto:suman.p@samsung.com">suman.p@samsung.com</a>; <a href="mailto:nvmewin@lists.openfabrics.org">
nvmewin@lists.openfabrics.org</a><br>
Cc: <a href="mailto:cgps@samsung.com">cgps@samsung.com</a><br>
Subject: Re: [nvmewin] Samsung Patch for Bus Reset Enhancements<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Hi Suman,<o:p></o:p></p>
<p class="MsoPlainText"><span style="color:black"><o:p> </o:p></span></p>
<p class="MsoPlainText" style="margin-left:.5in">I have some feedback in addition to Alex's comments. I believe there is an issue with the loop that was added to NVMeRunningStartAttempt. The issue is that IoCompletionDpcRoutine was never meant to be called
directly. It was architected and designed to always run from a DPC.<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">[Judy]<o:p></o:p></span></p>
<p class="MsoPlainText"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"> That’s because at runtime, we don’t want to be doing time-consuming request-completion work in the ISR. Therefore the work is offloaded to
a DPC which runs at a lower IRQL. However, the work we need to do to process cmd completions is fixed - there is actually no innate architectural design impediment in the routine itself to calling this routine directly in the two scenarios our patch addresses
– i.e., those situations where by architectural definition we are expected to finish all work before returning to the caller (and in our case, that includes sending and completing multiple commands in our init state machine ).. Those scenarios are the two
that Suman listed in the change notes:<o:p></o:p></span></p>
<p class="MsoPlainText" style="margin-left:.5in"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">a) NVMeResetBus
<o:p></o:p></span></p>
<p class="MsoPlainText" style="text-indent:.5in"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">b) NVMeAdapterControl-> ScsiRestartAdapter<o:p></o:p></span></p>
<p class="MsoPlainText" style="text-indent:.5in"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoPlainText"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">By design, we don’t want to schedule a DPC to handle completions for the commands generated by the init state machine in these 2 reset paths – we want
to poll. That’s why we make the direct call instead.<o:p></o:p></span></p>
<p class="MsoPlainText"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoPlainText" style="margin-left:.5in"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">
</span>It's possible that a command from the init state machine could generate an interrupt and run the IoCompletionDpcRoutine before it can be called in RunningStartAttempt.
<o:p></o:p></p>
<p class="MsoPlainText"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">[Judy]<o:p></o:p></span></p>
<p class="MsoPlainText"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoPlainText"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">This can’t happen.
<o:p></o:p></span></p>
<p class="MsoPlainText"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">If an interrupt is generated on behalf of a command from the init state machine during the first scenario above (NVMeResetBus), the hwResetInProg flag
at the top of the ISR causes us to return immediately:<o:p></o:p></span></p>
<p class="MsoPlainText"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal" style="margin-left:.5in;text-autospace:none"><span style="font-size:8.0pt;font-family:"Courier New"">NVMeIsrMsix (<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-left:.5in;text-autospace:none"><span style="font-size:8.0pt;font-family:"Courier New"">…<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-left:.5in;text-autospace:none"><span style="font-size:8.0pt;font-family:"Courier New"">
<span style="color:blue">if</span> (pAE->hwResetInProg)<o:p></o:p></span></p>
<p class="MsoPlainText" style="margin-left:.5in"><span style="font-size:8.0pt;font-family:"Courier New"">
<span style="color:blue">return</span> TRUE;<o:p></o:p></span></p>
<p class="MsoPlainText" style="margin-left:.5in"><span style="font-size:8.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoPlainText"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">The second scenario above (NVMeAdapterControl-> ScsiRestartAdapter) is not interrupt-driven by definition. That is, at the time it is called, interrupts
aren’t enabled. But even if it they were, the hwResetInProg flag would catch it.</span><o:p></o:p></p>
<p class="MsoPlainText" style="margin-left:.5in"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoPlainText" style="margin-left:.5in">A better solution would be to have a loop similar to the one at the end of NVMePassiveInitialize where RunningStartAttempt is called, and is followed by a loop that waits for the state machine to complete.
<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">[Judy] This is actually the first approach we took and were intending to use but we found it didn’t work. The reason was the loop you refer to is periodic
timer-driven but the timer was not getting scheduled in the NVMeAdapterControl-> ScsiRestartAdapter path as there is no timer available at that point. The reason this is not an issue for the current OFA driver is because we launch the state machine but then
return from the call to NVMeAdapterControl and let the state machine run asynchronously and complete outside of that context (violates the spec).</span><o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText" style="margin-left:.5in">As the patch is currently written I am not comfortable approving it.This change to wait for the state machine's completion could be made in the new ReinitializeController function, and then you wouldn't need
the changes to RunningStartAttempt or any of the polledmode code. <o:p></o:p></p>
<p class="MsoPlainText"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoPlainText"><span style="font-size:10.0pt;font-family:"Tw Cen MT","sans-serif";color:#1F497D">[Judy] The approach you propose will not work for the reason explained above. Again, we too had first hoped it would but it won’t. Hence we went to a polled-mode
model. Since we have to finish all work before returning anyway and since reset bus is not a performance path, there is no downside to polling.<o:p></o:p></span></p>
<p class="MsoPlainText"><span style="color:black"><o:p> </o:p></span></p>
<p class="MsoPlainText"><span style="color:black"><o:p> </o:p></span></p>
<p class="MsoPlainText">Thanks,<o:p></o:p></p>
<p class="MsoPlainText">Carolyn<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">-----Original Message-----<o:p></o:p></p>
<p class="MsoPlainText">From: <a href="mailto:nvmewin-bounces@lists.openfabrics.org">
<span style="color:windowtext;text-decoration:none">nvmewin-bounces@lists.openfabrics.org</span></a> [<a href="mailto:nvmewin-bounces@lists.openfabrics.org"><span style="color:windowtext;text-decoration:none">mailto:nvmewin-bounces@lists.openfabrics.org</span></a>]
On Behalf Of Alex Chang<o:p></o:p></p>
<p class="MsoPlainText">Sent: Tuesday, October 21, 2014 1:20 PM<o:p></o:p></p>
<p class="MsoPlainText">To: <a href="mailto:suman.p@samsung.com"><span style="color:windowtext;text-decoration:none">suman.p@samsung.com</span></a>;
<a href="mailto:nvmewin@lists.openfabrics.org"><span style="color:windowtext;text-decoration:none">nvmewin@lists.openfabrics.org</span></a><o:p></o:p></p>
<p class="MsoPlainText">Cc: <a href="mailto:cgps@samsung.com"><span style="color:windowtext;text-decoration:none">cgps@samsung.com</span></a><o:p></o:p></p>
<p class="MsoPlainText">Subject: Re: [nvmewin] Samsung Patch for Bus Reset Enhancements<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Hi Suman,<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">(1) There is a call of StorPortResume(pAE) in Line2434 of nvmestd.c, which is redundant because, when NextDriverState is NVMeStartComplete, in the end of NVMeRunning, StorPortResume had been called already.<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">(2) To comply with our agreed coding style and make the logic easier, may I suggest changing Line#184 of nvmestat.c to:<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">if (pAE->ntldrDump == FALSE) {<o:p></o:p></p>
<p class="MsoPlainText"> if (pAE->polledMode == FALSE) {<o:p></o:p></p>
<p class="MsoPlainText"> NVMeRunning(pAE);<o:p></o:p></p>
<p class="MsoPlainText"> } else {<o:p></o:p></p>
<p class="MsoPlainText"> /* <o:p></o:p></p>
<p class="MsoPlainText"> * we poll if we're launching the reinit state machine from HwStorResetBus<o:p></o:p></p>
<p class="MsoPlainText"> * or HwStorAdapterControl->ScsiRestartAdapter path<o:p></o:p></p>
<p class="MsoPlainText"> */<o:p></o:p></p>
<p class="MsoPlainText"> NVMeRunning(pAE);<o:p></o:p></p>
<p class="MsoPlainText"> /* TO val is based on CAP register plus a few, 5, seconds to init post RDY */<o:p></o:p></p>
<p class="MsoPlainText"> passiveTimeout = pAE->uSecCrtlTimeout + (STORPORT_TIMER_CB_us * MICRO_TO_SEC);<o:p></o:p></p>
<p class="MsoPlainText"> ...<o:p></o:p></p>
<p class="MsoPlainText"> return (pAE->DriverState.NextDriverState == NVMeStartComplete) ? TRUE : FALSE;<o:p></o:p></p>
<p class="MsoPlainText"> }<o:p></o:p></p>
<p class="MsoPlainText">} else {<o:p></o:p></p>
<p class="MsoPlainText"> PRES_MAPPING_TBL pRMT = &pAE->ResMapTbl;<o:p></o:p></p>
<p class="MsoPlainText"> .....<o:p></o:p></p>
<p class="MsoPlainText">}<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Thank you!<o:p></o:p></p>
<p class="MsoPlainText">Alex<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">From: SUMAN PRAKASH B [<a href="mailto:suman.p@samsung.com"><span style="color:windowtext;text-decoration:none">mailto:suman.p@samsung.com</span></a>]<o:p></o:p></p>
<p class="MsoPlainText">Sent: Wednesday, October 15, 2014 6:00 AM<o:p></o:p></p>
<p class="MsoPlainText">To: <a href="mailto:nvmewin@lists.openfabrics.org"><span style="color:windowtext;text-decoration:none">nvmewin@lists.openfabrics.org</span></a><o:p></o:p></p>
<p class="MsoPlainText">Cc: Alex Chang; <a href="mailto:cgps@samsung.com"><span style="color:windowtext;text-decoration:none">cgps@samsung.com</span></a><o:p></o:p></p>
<p class="MsoPlainText">Subject: Samsung Patch for Bus Reset Enhancements<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Content-Type: text/plain; charset=UTF-8<o:p></o:p></p>
<p class="MsoPlainText">Content-Transfer-Encoding: 8bit<o:p></o:p></p>
<p class="MsoPlainText">Date: %%SENT_DATE%%<o:p></o:p></p>
<p class="MsoPlainText">Subject: Suspect Message Quarantined <o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">WARNING: The virus scanner was unable to scan an attachment in an email message sent to you. This attachment could possibly contain viruses or other malicious programs. The attachment could not be scanned for the following reasons:<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">%%DESC%%<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">The full message and the attachment have been stored in the quarantine.<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">The identifier for this message is '%%QID%%'.<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Access the quarantine at: <o:p></o:p></p>
<p class="MsoPlainText"><a href="https://puremessage.pmc-sierra.bc.ca:28443/"><span style="color:windowtext;text-decoration:none">https://puremessage.pmc-sierra.bc.ca:28443/</span></a><o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">For more information on PMC's Anti-Spam system:<o:p></o:p></p>
<p class="MsoPlainText"><a href="http://pmc-intranet/wiki/index.php/Outlook:Anti-Spam_FAQ"><span style="color:windowtext;text-decoration:none">http://pmc-intranet/wiki/index.php/Outlook:Anti-Spam_FAQ</span></a><o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">IT Services<o:p></o:p></p>
<p class="MsoPlainText">PureMessage Admin <o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Hi Everyone,<o:p></o:p></p>
<p class="MsoPlainText"> <o:p></o:p></p>
<p class="MsoPlainText">We have a patch for the Bus Reset Enhancements.<o:p></o:p></p>
<p class="MsoPlainText">Please find attached the source code. The password is samsung123<o:p></o:p></p>
<p class="MsoPlainText"> <o:p></o:p></p>
<p class="MsoPlainText">Please find the change description below - <o:p></o:p></p>
<p class="MsoPlainText"> <o:p></o:p></p>
<p class="MsoPlainText">1. There are multiple paths in the driver that reset the controller and execute the initialization state machine. Our patch is not concerned with the majority of those paths. Aside from a few additional isolated modifications, our patch
focuses on the two paths that are supposed to be synchronous -i.e. they should not return to caller until all work is completed - but which currently are not so. They are:<o:p></o:p></p>
<p class="MsoPlainText">a) NVMeResetBus (and)<o:p></o:p></p>
<p class="MsoPlainText">b) NVMeAdapterControl-> ScsiRestartAdapter We have introduced a new routine NVMeReInitializeController(), which will be invoked from NVMeReseBus() and NVMeAdapterControl() - ScsiRestartAdapter. This routine will reset and initialize
the controller and then complete the requests. It will not return until the initialization state machine is complete.<o:p></o:p></p>
<p class="MsoPlainText">We disallow processing of any SRB in NVMeStartIo() when NextDriverState != NVMeStateComplete. In this way we direct the PowerUp operations to be executed in NVMeAdapterControl() - ScsiRestartAdapter only. When resuming from hibernation
for example, NVMeStartio() will not process the POWER SRB. Instead, the Power Up operations will be invoked in NVMeAdapterControl()->ScsiRestartAdapter.<o:p></o:p></p>
<p class="MsoPlainText"> <o:p></o:p></p>
<p class="MsoPlainText">Additionally , Miniport drivers should disregard requests to reset the bus when ntldrDump is set to TRUE in NvmeResetBus(). But current implementation processes this request.<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">2. When pAE->ntldrDump is TRUE, in the NVMeMapCore2Queue() routine, the pPGT value is NULL. Hence a BSOD occurs when executing ULONG coreNum = (ULONG)(pPN->Number + pPGT->BaseProcessor). We fixed the problem by moving access to pPGT
when ntldrDump is FALSE.<o:p></o:p></p>
<p class="MsoPlainText"> <o:p></o:p></p>
<p class="MsoPlainText">3. In ProcessIo(), when IoStatus is set to NOT_SUBMITTED, the SRB is not completed. Due to this, a BSOD was occuring when executing WHCK test "DP WLK - Hot-Add - Device test". We fixed the problem by changing the code to complete SRB
when IoStatus is NOT_SUBMITTED.<o:p></o:p></p>
<p class="MsoPlainText"> <o:p></o:p></p>
<p class="MsoPlainText">4. We changed the use of StorPortBusy()/StorPortReady() to StorPortPause()/StorPortResume(), since StorPortBusy() will not prevent new IOS from coming in once the current ones in the driver have been completed.<o:p></o:p></p>
<p class="MsoPlainText"> <o:p></o:p></p>
<p class="MsoPlainText"> <o:p></o:p></p>
<p class="MsoPlainText">Tested the following on Win7 and Windows 2012R2.<o:p></o:p></p>
<p class="MsoPlainText">- WHCK<o:p></o:p></p>
<p class="MsoPlainText">- Install/Uninstall, Enable/Disable, FS Format<o:p></o:p></p>
<p class="MsoPlainText">- Hibernation/Resume, Sleep/Resume<o:p></o:p></p>
<p class="MsoPlainText">- IOmeter<o:p></o:p></p>
<p class="MsoPlainText"> <o:p></o:p></p>
<p class="MsoPlainText">Thanks,<o:p></o:p></p>
<p class="MsoPlainText">Suman<o:p></o:p></p>
<p class="MsoPlainText"> <o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">_______________________________________________<o:p></o:p></p>
<p class="MsoPlainText">nvmewin mailing list<o:p></o:p></p>
<p class="MsoPlainText"><a href="mailto:nvmewin@lists.openfabrics.org"><span style="color:windowtext;text-decoration:none">nvmewin@lists.openfabrics.org</span></a><o:p></o:p></p>
<p class="MsoPlainText"><a href="http://lists.openfabrics.org/mailman/listinfo/nvmewin"><span style="color:windowtext;text-decoration:none">http://lists.openfabrics.org/mailman/listinfo/nvmewin</span></a><o:p></o:p></p>
<p class="MsoPlainText">_______________________________________________<o:p></o:p></p>
<p class="MsoPlainText">nvmewin mailing list<o:p></o:p></p>
<p class="MsoPlainText"><a href="mailto:nvmewin@lists.openfabrics.org"><span style="color:windowtext;text-decoration:none">nvmewin@lists.openfabrics.org</span></a><o:p></o:p></p>
<p class="MsoPlainText"><a href="http://lists.openfabrics.org/mailman/listinfo/nvmewin"><span style="color:windowtext;text-decoration:none">http://lists.openfabrics.org/mailman/listinfo/nvmewin</span></a><o:p></o:p></p>
</div>
</body>
</html>