[nvmewin] Samsung Patch for Random bug fixes - Resubmit

Tue Sep 9 17:21:14 PDT 2014

Hi all,
If you haven’t review/test the patch, please do so and provide your feedback at your earliest convenience such that Suman can revise patch.
Thank you!
Alex

From: SUMAN PRAKASH B [mailto:suman.p at samsung.com]
Sent: Monday, September 08, 2014 4:26 AM
To: Alex Chang; nvmewin at lists.openfabrics.org
Cc: cpgs .
Subject: Re: RE: Samsung Patch for Random bug fixes - Resubmit

Hi Alex,

Is it OK if I take these 2 observations as review comments and update the Samsung patch once I get review comments from others too?

Thanks,

Suman

------- Original Message -------

Sender : Alex Chang<Alex.Chang at pmcs.com>

Date : Sep 06, 2014 01:53 (GMT+05:00)

Title : RE: Samsung Patch for Random bug fixes - Resubmit

Hi Suman,
I diffed between your patch and the most current source and found the castings of PSCSI_REQUEST_BLOCK I added for StorPortGetScatterGatherList are missing in the following lines of nvmesnti.c:
1727,2068,5225,5285,5396,5464 and 5635.
The reason I added the casting is to fix warning level issue that we need to use W3 and treat warnings as errors. Could you please add them back and make sure you added changes on top of the most current sources?
In line#1083 of nvmestd.c, the casting should be “PSTOR_DEVICE_CAPABILITIES_EX” instead of “PSTOR_DEVICE_CAPABILITIES”. Again, if you changes the warning level to W3 and treat warnings as errors, you will see the compiling error.
Thanks,
Alex

From: SUMAN PRAKASH B [mailto:suman.p at samsung.com]
Sent: Friday, September 05, 2014 6:56 AM
To: nvmewin at lists.openfabrics.org
Cc: Alex Chang; cpgs at samsung.com
Subject: Samsung Patch for Random bug fixes - Resubmit

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

Date: %%SENT_DATE%%

Subject: Suspect Message Quarantined

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:

%%DESC%%

The full message and the attachment have been stored in the quarantine.

The identifier for this message is '%%QID%%'.

Access the quarantine at:

https://puremessage.pmc-sierra.bc.ca:28443/

For more information on PMC's Anti-Spam system:

http://pmc-intranet/wiki/index.php/Outlook:Anti-Spam_FAQ

IT Services

PureMessage Admin

Hi Everyone,

We have a patch for the following random bug fixes. We have merged our changes over OFA Revision 106.

1) Handling return value for NVMeWaitForCtrlRDY. NVMeWaitForCtrlRDY should have a return value of type BOOLEAN that can be checked to see if it was successful or not.

2) Handling return value for NVMeCompleteCmd. NVMeCompleteCmd should have a return value that can be checked to see if it was successful or not. Right now, wherever it’s called from the code forges ahead regardless of whether it succeeded or failed.

3) Fixing the NVMeResetAdapter issues to check if CC.EN is set to 1 before setting to 0.
The routine NVMeResetAdapter() sets CC.EN to 0 without ever checking to make sure that CSTS.RDY is set to ‘1’ first. This check has to be included in this routine. Since it is not, there are many paths in the driver where there is no prior check for this condition:
a) NVMeInitAdminQueues -> NVMeEnableAdapter -> NVMeResetAdapter
b) NVMeNormalShutdown -> NVMeResetAdapter
c) NVMeAdapterControlPowerDown -> NVMeResetAdapter
d) NVMeSynchronizeReset -> NVMeResetAdapter

4) Memory corruption while creating inquiry response data -  In SntiTranslateStandardInquiryPage(), the following line of code is accessing a field way past the end of STANDARD_INQUIRY_LENGTH (36 bytes): pStdInquiry->Reserved3[0]  = RESERVED_FIELD;

5) Eliminate NVMeWaitOnReady and use only NVMeWaitForCtrlRDY. There is redundancy in the new routine NVMeWaitForCtrlRDY() and the old routine NVMeWaitOnReady(). We don’t need both – we can get rid of the old routine.

6) Fixed the RecoveryDpcRoutine to avoid redundant setting of CC.EN bit to 0. The code does not need to set CC.EN to ‘0’ and then wait for CSTS.RDY to become 0 because right after it does so, it calls NVMeResetAdapter which does the exact same thing.

7) Write Buffer implementation correction.
a. Calculation of dword10 for DOWNLOAD_SAVE_ACTIVATE and DOWNLOAD_SAVE_DEFER_ACTIVATE is modified.
b. Set the SRB Status value for NVMe command failure condition in SntiCompletionCallbackRoutine.
c. Prepare the Firmware Activate Command and issue the command in case of DOWNLOAD_SAVE_ACTIVATE mode in SntiTranslateWriteBufferResponse.
d. Handled the scenario of SNTI_SEQUENCE_IN_PROGRESS for Write Data Buffer command.

8) Enabling Eject option in the taskbar.

9) StorPortGetScatterGatherList() - The second parameter pSrb is typecasted to PSCSI_REQUEST_BLOCK. But as per msdn, for Win8/8.1 kernel, it will be PSTORAGE_REQUEST_BLOCK. So it is OK to just pass pSrb which is already either PSCSI_REQUEST_BLOCK or PSTORAGE_REQUEST_BLOCK.

10) SntiTranslateReadCapacity10() - When the disk capacity is more than 2TB, according to the SCSI command spec (SBC-3), READ CAPACITY (10) command should return 0xFFFF FFFF as "Returned Logical Block Address" if lba is greater than the DWORD unit. This specification is, of course, considered in SCSI translation code. However 'minus 1' code which should not be applied in this case is executed. This makes device capacity to be 2TB (0xFFFF FFFE x 0x200 = 2047.99GB).

11) SntiTranslateRequestSense() - No need to separately get pSenseData for win7 and Win8. The GET_DATA_BUFFER will be defined to (SrbGetDataBuffer((PVOID)pSrb)) for Win8 and (pSrb)->DataBuffer for win7.

12) SntiBuildGetLogPageCmd() - The numDwords for ERROR_INFORMATION, SMART_HEALTH_INFORMATION and FIRMWARE_SLOT_INFORMATION is corrected to be number of dwords - 1 value.

13) NVMeProcessPublicIoctl() - For IOCTL_SCSI_MINIPORT_READ_SMART_THRESHOLDS, the pSrbExt->nvmeSqeUnit.CDW10 value is corrected to be number of dwords - 1 value.

14) With OFA Revision 106 driver, the SCSI Compliance test is failing in our setup (please find attached the scsicompliance test failure log)
a) MODE SELECT 6: MODE SENSE (6) Checking Caching Mode Page Length.
b) MODE SELECT 10: MODE SENSE (10) Checking Caching Mode Page Length.
c) ASSERTION: INQUIRY Checking Identification Descriptors in VPD page 0x83.

for fixing a) and b), we have to use PMODE_CACHING_PAGE for Win7 kernel and PMODE_CACHING_PAGE_EX for Win8/8.1 kernel. We have corrected this in this patch.
for c), the implementation is not as per NVMe-SCSI translation specification. But even if we follow the specification, we don t get a uniqueid. So Judy has proposed to change the SCSI Name format so that we get unique id for each device. This proposal in pending for 30 day review. Once review is complete and approved, We will include the modifications in the next Samsung patch.

15) Invalid return value for NVMeAdapterControl
NVMeAdapterControl miniport routine -  sometimes returns illegal value. WDK specifies that the driver must always return ScsiAdapterControlSuccess. However, depending on execution, the driver may currently return ScsiAdapterControlUnsuccessful for ScsiStopAdapter and ScsiRestartAdapter control types.
As per msdn – http://msdn.microsoft.com/en-us/library/windows/hardware/ff557365(v=vs.85).aspx

16) Handling StartIo deadlock when MultipleCoresToSingleQueueFlag is set.

Unit tests:
Tested the following on Win7 and Windows 2012R2.

-       Executed NVMe SCSI compliance tests

-       Executed IOmeter

-       Install/Uninstall, Enable/Disable driver, FS Format/hibernation.

-       Hot plug

Thanks,

Suman

[cid:image001.gif at 01CFCC52.52249EF0]

[Image removed by sender.]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20140910/09bf7088/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ~WRD000.jpg
Type: image/jpeg
Size: 823 bytes
Desc: ~WRD000.jpg
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20140910/09bf7088/attachment.jpg>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.gif
Type: image/gif
Size: 13168 bytes
Desc: image001.gif
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20140910/09bf7088/attachment.gif>