[nvmewin] numberOfPrpEntries patch for review

[Knoblaugh, Rick]

LSI is good with it as well.

Thanks,

       -RIck

From: nvmewin-bounces at lists.openfabrics.org [mailto:nvmewin-bounces at lists.openfabrics.org] On Behalf Of Chang, Alex
Sent: Monday, June 04, 2012 5:01 PM
To: Murray, Kris R; nvmewin at lists.openfabrics.org
Subject: Re: [nvmewin] numberOfPrpEntries patch for review

IDT is fine with the fix.

Thanks,
Alex

________________________________
From: nvmewin-bounces at lists.openfabrics.org [mailto:nvmewin-bounces at lists.openfabrics.org] On Behalf Of Murray, Kris R
Sent: Monday, June 04, 2012 11:15 AM
To: nvmewin at lists.openfabrics.org
Subject: [nvmewin] ***UNCHECKED*** numberOfPrpEntries patch for review
All,

Summary:
I found a bug in the ProcessIo step for copying the PRP list. From the SNTI code, the variable numberOfPrpEntries in the SRB Extension was built as the total number of PRPs for the NVMe command. However, the code to copy over the PRP list uses this variable unaltered. This creates a bug where one too many PRP entries are copied. This bug hasn't surfaced yet because creating an NVMe command with the maximum number of PRP entries in the list hasn't been tested yet.

See NVMe_PRP_fix.zip for source. Password: NVMe1234

Changes:
nvmeStd.h :

-        The numberofPrpEntries variable in the SRB Extension is updated with a comment that clarifies that it holds the total number of PRPs, not just the PRP entries in the list

nvmeIo.c:

-        Instead of checking for a valid entry 0 in the list, the number of entries is checked

-        The amount of memory to copy is decreased by the size of 1 entry to account for the PRP that is in PRP1 of the command.

Code Sample:
nvmeStd.h
/* Temp PRP List */
UINT64                       prpList[MAX_TX_SIZE / PAGE_SIZE];

/* Keep track of the total number of PRPs */
UINT32                       numberOfPrpEntries;


nvmeIo.c
if (pSrbExtension->numberOfPrpEntries > 2) {
    pNvmeCmd->PRP2 = pCmdInfo->prpListPhyAddr.QuadPart;


   /* Copy the PRP list pointed to by PRP2....

    * Size of the copy is total num of PRPs -1 because

    * PRP1 is not in the PRP list pointed to by PRP2

    */
    StorPortMoveMemory((PVOID)pCmdInfo->pPRPList,
                       (PVOID)&pSrbExtension->prpList[0],
                       ((pSrbExtension->numberOfPrpEntries - 1) * sizeof(UINT64)));
}

Testing (Logs attached):

-        IOMeter: PASS - See result.csv

-        PCMark7: PASS - See NVMe.pcmark-7-result

-        SCSI Compliance 2.0: 77 PASS, 36 WARN, 2 FAIL - See scsi_comp.txt

-        SD Stress: PASS - See SDSTRESS.LOG

Thanks,
Kris Murray
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20120604/1cca58f0/attachment.html>