[nvmewin] Samsung Patch for Random bug fixes - Resubmit

Parag Sheth parag.sheth at seagate.com
Wed Sep 17 11:40:31 PDT 2014


Hi Alex,

Yes I did review the changes and they look good. Although I was not able to
test them as my test system is down. But looking at the changes - there
should not be any problem.

Thanks
Parag SHeth

On Wed, Sep 17, 2014 at 8:01 AM, Alex Chang <Alex.Chang at pmcs.com> wrote:

>   Hi Parag and Carolyn,
>
> I am finalizing the tests on the patch now. If you approve it, please let
> me know as soon as possible.
>
> Thank you!
>
> Alex
>
>
>
> *From:* nvmewin-bounces at lists.openfabrics.org [mailto:
> nvmewin-bounces at lists.openfabrics.org] *On Behalf Of *Alex Chang
> *Sent:* Friday, September 12, 2014 7:50 AM
> *To:* suman.p at samsung.com; nvmewin at lists.openfabrics.org
> *Cc:* cpgs .
> *Subject:* Re: [nvmewin] Samsung Patch for Random bug fixes - Resubmit
>
>
>
> Dear all,
>
> Please review/test the patch and provide your feedback at your earliest
> convenience. I plan to collect approvals next Wednesday.
>
> Thank you!
>
> Alex
>
>
>
> *From:* SUMAN PRAKASH B [mailto:suman.p at samsung.com <suman.p at samsung.com>]
>
> *Sent:* Friday, September 12, 2014 4:57 AM
> *To:* Alex Chang; nvmewin at lists.openfabrics.org
> *Cc:* cpgs .
> *Subject:* Re: RE: RE: 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/ <https://urldefense.proofpoint.com/v2/url?u=https-3A__puremessage.pmc-2Dsierra.bc.ca-3A28443_&d=AAMGaQ&c=IGDlg0lD0b-nebmJJ0Kp8A&r=QOwFo5M7MYyQeT06CcSuSQHSUdSO20xC9GZe6-T9Svk&m=j6D7ZI8F3Aa9SQlZWxJK4uUxNG1P29bbv3b3SZpOxbE&s=tq7M8f-oPeZVYvjGNNTq322d0WalZMV6p2FU2xeCIpk&e=>
>
>
>
> For more information on PMC's Anti-Spam system:
>
> http://pmc-intranet/wiki/index.php/Outlook:Anti-Spam_FAQ <https://urldefense.proofpoint.com/v2/url?u=http-3A__pmc-2Dintranet_wiki_index.php_Outlook-3AAnti-2DSpam-5FFAQ&d=AAMGaQ&c=IGDlg0lD0b-nebmJJ0Kp8A&r=QOwFo5M7MYyQeT06CcSuSQHSUdSO20xC9GZe6-T9Svk&m=j6D7ZI8F3Aa9SQlZWxJK4uUxNG1P29bbv3b3SZpOxbE&s=Q5riHJJfzY_BVBFZv84irm_GeHx7qM7e8zusalxhl9Q&e=>
>
>
>
> IT Services
>
> PureMessage Admin
>
>
>
> Hi Alex,
>
>
>
> I have revised the patch with the following 2 corrections -
>
>
>
> a. nvmesnti.c - castings of PSCSI_REQUEST_BLOCK for
> StorPortGetScatterGatherList are missing in the following
> lines 1727,2068,5225,5285,5396,5464 and 5635.
>
> b. nvmestd.c - In line#1083, the casting should be
> “PSTOR_DEVICE_CAPABILITIES_EX” instead of “PSTOR_DEVICE_CAPABILITIES”.
>
>
>
> Please find attached the revised patch. Password is *samsung123*.
>
>
>
> Thanks all for reviewing.
>
>
>
> Regards,
>
> Suman
>
>
>
> ------- *Original Message* -------
>
> *Sender* : Alex Chang<Alex.Chang at pmcs.com>
>
> *Date* : Sep 12, 2014 00:45 (GMT+05:00)
>
> *Title* : RE: RE: Samsung Patch for Random bug fixes - Resubmit
>
>
>
> <!--[if mso 9]-->
>
> Hi Suman,
>
> If there is no more feedback by the end of today, could you please revise
> your patch and send it out sometime tomorrow?
>
> Thank you very much,
>
> Alex
>
>
>
> *From:* SUMAN PRAKASH B [mailto:suman.p at samsung.com <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
>
>
>
> <!--[if mso 9]-->
>
> 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 <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/ <https://urldefense.proofpoint.com/v2/url?u=https-3A__puremessage.pmc-2Dsierra.bc.ca-3A28443_&d=AAMGaQ&c=IGDlg0lD0b-nebmJJ0Kp8A&r=QOwFo5M7MYyQeT06CcSuSQHSUdSO20xC9GZe6-T9Svk&m=j6D7ZI8F3Aa9SQlZWxJK4uUxNG1P29bbv3b3SZpOxbE&s=tq7M8f-oPeZVYvjGNNTq322d0WalZMV6p2FU2xeCIpk&e=>
>
>
>
> For more information on PMC's Anti-Spam system:
>
> http://pmc-intranet/wiki/index.php/Outlook:Anti-Spam_FAQ <https://urldefense.proofpoint.com/v2/url?u=http-3A__pmc-2Dintranet_wiki_index.php_Outlook-3AAnti-2DSpam-5FFAQ&d=AAMGaQ&c=IGDlg0lD0b-nebmJJ0Kp8A&r=QOwFo5M7MYyQeT06CcSuSQHSUdSO20xC9GZe6-T9Svk&m=j6D7ZI8F3Aa9SQlZWxJK4uUxNG1P29bbv3b3SZpOxbE&s=Q5riHJJfzY_BVBFZv84irm_GeHx7qM7e8zusalxhl9Q&e=>
>
>
>
> 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
> <https://urldefense.proofpoint.com/v2/url?u=http-3A__msdn.microsoft.com_en-2Dus_library_windows_hardware_ff557365-28v-3Dvs.85-29.aspx&d=AAMGaQ&c=IGDlg0lD0b-nebmJJ0Kp8A&r=QOwFo5M7MYyQeT06CcSuSQHSUdSO20xC9GZe6-T9Svk&m=j6D7ZI8F3Aa9SQlZWxJK4uUxNG1P29bbv3b3SZpOxbE&s=FhLWLl-tNiGAVue2YGP_dV7ckSkFkAPa3QKNCetm2QE&e=>
>
> 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
>
>
>
>
>
>
>
>
>
>    [image: Image removed by sender.]
>
> _______________________________________________
> nvmewin mailing list
> nvmewin at lists.openfabrics.org
>
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.openfabrics.org_mailman_listinfo_nvmewin&d=AAICAg&c=IGDlg0lD0b-nebmJJ0Kp8A&r=QOwFo5M7MYyQeT06CcSuSQHSUdSO20xC9GZe6-T9Svk&m=j6D7ZI8F3Aa9SQlZWxJK4uUxNG1P29bbv3b3SZpOxbE&s=fcwM8_-rqRoV3MS8sC6SAATWyb_X3bUXyS32PXOQqCE&e=
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20140917/93ce6829/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.gif
Type: image/gif
Size: 13168 bytes
Desc: not available
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20140917/93ce6829/attachment.gif>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image002.jpg
Type: image/jpeg
Size: 823 bytes
Desc: not available
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20140917/93ce6829/attachment.jpg>


More information about the nvmewin mailing list