[nvmewin] HGST Changes to Enable WMI, Driver Trace and various fixes

Tue Sep 30 16:06:57 PDT 2014

Hi Tom,

1. Your change in function SntiHardCodeCacheModePage (nvmesnti.c) overturns
the fix made in last patch.
2. Function SetWmiDataBlock (nvmeWmi.c & .h) is never used. It can be
removed.

Other than these 2, the patch looks fine.

Thanks
Parag Sheth

On Tue, Sep 30, 2014 at 8:18 AM, Alex Chang <Alex.Chang at pmcs.com> wrote:

>  Thank you, Parag.
>
> I know we allocate one extra page intentionally. When people don’t know
> the trick, the original order simply avoids some concern.
>
>
>
> Alex
>
>
>
> *From:* Thomas Freeman [mailto:thomas.freeman at hgst.com]
> *Sent:* Tuesday, September 30, 2014 7:50 AM
> *To:* Parag Sheth; Alex Chang
> *Cc:* nvmewin at lists.openfabrics.org
> *Subject:* RE: [nvmewin] HGST Changes to Enable WMI, Driver Trace and
> various fixes
>
>
>
> Thank you Paraq… I like your explanation.
>
> In addition to your explanation, I’d also say the drivers uses pCplQStart
> -> (pCplQstart + queueSize -1) for the completion queue.
>
> That is also the address range used in that memset. If those address are
> wrong, then the CompletionQueue itself is wrong.
>
>
>
> Alex…. Yes, I’ll change the RtlZeroMemory to memset. I’m not sure why I
> didn’t use memset to begin with.
>
>
>
> Tom Freeman
>
>
>
> Software Engineer, Device Manager and Driver Development
>
> HGST, a Western Digital company
>
> Thomas.Freeman at hgst.com
>
> 507-322-2311
>
>
>
> *From:* Parag Sheth [mailto:parag.sheth at seagate.com
> <parag.sheth at seagate.com>]
> *Sent:* Monday, September 29, 2014 8:32 PM
> *To:* Alex Chang
> *Cc:* Thomas Freeman; nvmewin at lists.openfabrics.org
> *Subject:* Re: [nvmewin] HGST Changes to Enable WMI, Driver Trace and
> various fixes
>
>
>
> Hi Alex,
>
>
>
> Tom's change related to your point # 2 seems right.
>
>
>
> Here is my explanation.
>
> - While allocating memory for submission queue + completion queue +
> command entries, there is 1 additional memory page allocated.
>
> - Because this allocation is page aligned to begin with, sub queue start
> is always page aligned.
>
> - making completion queue page aligned might use 1 additionally allocated
> page.
>
> - command entries need not be page aligned.
>
> - And all these 3 things are initialized in the above order.
>
>
>
> So I think clearing memory for completion queue must be done only after
> page align macro. Let me know if I am missing anything here.
>
>
>
> Thanks
>
> Parag Sheth
>
>
>
> On Mon, Sep 29, 2014 at 11:18 AM, Alex Chang <Alex.Chang at pmcs.com> wrote:
>
>   Hi Tom,
>
>
>
> I have the following comments after browsing the changes you’ve made:
>
> 1.       In line 184 of nvmestd.c, can RtlZeroMemory be replaced by
> memset ?
>
> 2.       In line 1195 of nvmeinit.c, you moved down memset after page
> align macro for completion queue start. In case of the original completion
> queue start wasn’t page aligned, memset will clear the memory it shouldn’t.
>
>
>
> Thanks,
>
> Alex
>
>
>
> *From:* nvmewin-bounces at lists.openfabrics.org [mailto:
> nvmewin-bounces at lists.openfabrics.org] *On Behalf Of *Thomas Freeman
> *Sent:* Wednesday, September 24, 2014 11:55 AM
> *To:* nvmewin at lists.openfabrics.org
> *Subject:* [nvmewin] HGST Changes to Enable WMI, Driver Trace and various
> fixes
>
>
>
> 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
>
>
>
> The attached zip file contains source code, project files and
> instructional documents for WMI and Driver tracing.
>
> The password is “hgst1234”
>
>
>
> We tested the changes on w7, w8 and w8.1 – 64-bit
>
> Testing included sdstress, iometer, quick and slow format: MBR and GPT and
> scsi compliance.
>
>
>
>
>
> Here is a description of the various changes:
>
>
>
> *WMI *(WindowsWmiDesc.pdf in the attached zip file gives details on using
> the change)
>
> nvme.vcxproj - Menu for "Mof Compiler"
>
>                 -Create Binary Mof File = "../../source/%(FileName).bmf"
>
>                 -WMI Syntax Check = Yes (-WMI)
>
>    menu for Linker->Input
>
>                 -added $(DDK_LIB_PATH)\scsiwmi.lib
>
>    directly in the file. This directive causes VS to create nvmeMofData.h:
>
>   <ItemGroup>
>
>     <Wmimofck Include="../../source/nvmeMofData.bmf">
>
>       <HeaderOuputFile>"$(IntDir)nvmeMofData.h"</HeaderOuputFile>
>
>       <AdditionalOptions>-u -h"$(IntDir)nvmeMofData.h"</AdditionalOptions>
>
>     </Wmimofck>
>
>   </ItemGroup>
>
>
>
>
>
> nvme.rc - Added "mofResource" statement
>
> nvmeMofData.bmf - Visual studio generates this file on every compile.
> Exists in the "source" directory.
>
> nvmeMofData.mof – Used to define WMI obects. Exists in the "source"
> directory
>
> nvmeMofData.h - Visual studio generates during compile. It is created
> based on nvmeMofData.mof. It exists in the project's intermediate directory
> (e.g. nvme/x64/Win7Debug).
>
>
>
> nvmeStd.c
>
>                 -NVMeFindAdapter - indicate that the driver is a
> wmiProvider and initialize the wmiContext.
>
>                 -NVMeBuildIo - Allow calls to WMI
>
>         -NVMeStartIo - Allow calls to WMI
>
>
>
> nvmeStd.h
>
>                 -WmiLibContext - Added this structure to the device
> extension. The WmiLibContext structure provides registration information
> for a miniport driver's data and event blocks and defines entry points for
> the miniport driver's HwScsiWmiXxx callback routines.
>
>                 -WmiReqContext - Added this structure to the srb
> extension. When processing a WMI command, this structure is used to save
> context information for that particular command.
>
>
>
> precomp.h
>
>                 -added guiddef.h, scsiwmi.h, initguid.h, nvmeMofData.h,
> nmeWmi.h
>
>
>
>
>
> *WPP - Driver tracing *(WindowsTraceCollection.pdf in the attached zip
> file gives details on using the change)
>
> Relied heavily on the document "Using Storage Tracing Support In Miniport
> Drivers" -
> http://download.microsoft.com/download/8/e/8/8e85288f-b718-47d9-a7b2-af19d4c8f031/Storage-tracing-for-miniports.doc
>
>
>
> nvme.vcxproj - in the Menu for "Wpp Tracing"
>
>                 -Func to Gen Trace = StorStorPortDebugPrint(LEVEL,MSG,...)
>
>                 -Run Wpp Tracing = Yes
>
>                 -Scan Configuration Data = ..\..\source\nvme_tracing.h
>
>                 -Specify Template File = {km-StorDefault.tpl}*.tmh
>
>                 -Trace Kernel-Mode Components = Yes (-km)
>
>
>
> All c-files: Added include for "nvmeinit.tmh". A Visual Studio generated
> file to support driver tracing.
>
>
>
> nvmeStd.c - create a global variable for storing the trace context
> information. At cleanup time, the Port Driver calls a cleanup routine that
> uses this global variable.
>
>                 -DriverEntry - Include code to initialize driver tracing
> through wpp.
>
>                 -WppCleanupRoutine - Added code to be called by the port
> driver to clean up driver tracing.
>
> nvmeStd.h -
>
>                 -add function prototype for Wpp Cleanup routine.
>
>         -Changed values for debug print levels to match values in
> evntrace.h
>
>     INFO = TRACE_LEVEL_INFORMATION,
>
>     WARNING = TRACE_LEVEL_WARNING,
>
>     ERROR = TRACE_LEVEL_ERROR,
>
>     TRACE = TRACE_LEVEL_VERBOSE
>
>
>
> precomp.h
>
>                 -added nvme_tracing.h
>
>
>
>
>
> *Various Fixes*
>
> nvme.vcxproj
>
>                 -Changed c/c++ compiler warning level to W3/WX
>
> nvmeSnti.c
>
>    SntiTranslateReadCapacity16 - The caller’s response buffer was being
> cleared and set up without regard to its size. I put in changes to allocate
> a read capacity buffer on the stack. Once that temporary buffer was filled
> in, I used the response buffer size to determine how much data to copy into
> the caller’s response buffer.
>
> NOTE: It appears this same issue occurs in the handling of other scsi
> commands (e.g. in SntiTranslateDeviceIdentificationPage, if the response
> buffer was too short, setting the fields in pIdDescriptor would overrun
> the response buffer). This may be a fix to be implemented in the future.
> The issue in SntiTranslateReadCapacity16 issue was seen during testing. The
> potential issue with SntiTranslateDeviceIdentificationPage has not
> occurred in our testing.
>
>
>
>
>
>    SnitTranslateUnmap
>
>                 -Changes to ensure the DataSetManagement buffer was always
> 16-byte aligned.
>
>                 -Use prp2 to allow the prplist to cross a page boundary
>
>                 -When building the dsm-deallocate command, cdw10->NR was
> used to count the number of ranges - starting with the value 1. Since NR is
> a zero-based value, the count was decremented when command building was
> completed. Unfortunately, when 256 ranges are specified, NR has a value of
> 0 before decrementing. The value of 0 prevented the code from decrementing
> NR. A change was made to use a local uint16 variable to cound the number of
> ranges.
>
>     SntiTranslateWriteBuffer - The existing code set dword10 as a 1-based
> value. A change was made to correctly set this as a zero-based value.
>
>     SntiTranslateLogSense - A pointer to the data buffer is saved in the
> srb extension. A change was made to also save the data buffer length in the
> srb extension. The value is used when the data buffer is freed.
>
>
>
> nvmeStd.h
>
>                 -Increase size of dsmBuffer to allow for 16-byte alignment.
>
>         -add dataBufferSize to store size of data buffer allocated for log
> sense command.
>
>
>
>
>
>
>
> Tom Freeman
>
>
>
> Software Engineer, Device Manager and Driver Development
>
> HGST, a Western Digital company
>
> Thomas.Freeman at hgst.com
>
> 507-322-2311
>
>
>
>
> _______________________________________________
> nvmewin mailing list
> nvmewin at lists.openfabrics.org
> http://lists.openfabrics.org/mailman/listinfo/nvmewin
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20140930/2f8cd6bf/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.gif
Type: image/gif
Size: 1117 bytes
Desc: not available
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20140930/2f8cd6bf/attachment.gif>