Hi, Devkate
If the last entry in the first Prp list points to the last 4K data range rather
than a next Prp list, how does h/w distinguish the last entry is a pointer to
next Prp list or a valid entry to buffer?
I am consulting NVMe org and if I have some feedbacks I will get back to you.
Thanks
Feng
From: Baban Devkate [mailto:baban.devk...@seagate.com]
Sent: Tuesday, April 14, 2015 13:20
To: Tian, Feng
Cc: baban...@gmail.com; edk2-devel@lists.sourceforge.net
Subject: Re: MdeModulePkg:Fix required in NvmExpressDxe driver
Feng, you were correct that we don't need the 2nd Prp list for such case( case
3) and the last entry in the first Prp list points to last 4K range rather than
a pointer to next Prp List). Which is inline with NVMe spec.
Excerpt from NVM Express 1.1, section 4.3:
The first PRP entry contained within the command may have a non-zero offset
within the memory page. The first PRP List entry (i.e. the first pointer to a
memory page containing additional PRP entries) that if present is contained in
the PRP Entry 2 location within the command, shall be Qword aligned and may
also have a non-zero offset within the memory page. All other PRP and PRP List
entries shall have a memory page offset of 0h, i.e. the entries are memory page
aligned based on the value in CC.MPS. The last entry within a memory page, as
indicated by the memory page size in the CC.MPS field, shall be a PRP List
pointer if there is more than a single memory page of data to be transferred.
PRP Lists shall be minimally sized with packed entries starting with entry 0.
If more PRP List pages are required, then the last entry of the PRP List page
is a pointer to the next PRP List page. The total number of PRP entries is
implied by the command parameters and memory page size.
So with the latest patch, case 4 will generate error.
4) Offset = 0KB, Transfer Length = 4K * 1024 blocks = 4MB
Prp1 = 0K to 4KB
Prp2 = pointer to a Prp List which is followed by another Prp List.
The first Prp List contains 511 entries to read data from offset 4KB to offset
2MB and last one entry is a pointer to 2nd Prp List.
Second Prp List contains next 511 entries to read data from offset 2MB to
offset 4MB-4K and last one entry is a pointer to 3rd Prp List.
But as per NVMe spec, pointer to 3rd Prp List will read data from offset
4MB-4KB to 4MB since there is no more than single memory page data to be
transferred.
So I suggest we should go with below fix which will cater both aligned and
unaligned accesses.
Prp = NvmeCreatePrpList (PciIo, PhyAddr, ((Offset!= 0) ?
EFI_SIZE_TO_PAGES(Offset + Bytes) – 1: EFI_SIZE_TO_PAGES(Offset + Bytes) ),
&PrpListHost, &PrpListNo, &MapPrpList);
On Fri, Apr 10, 2015 at 11:20 AM, Tian, Feng
<feng.t...@intel.com<mailto:feng.t...@intel.com>> wrote:
Got your point now.
My original thought for the 3rd case is we only need a Prp list which contains
512 entries to describe the whole 2M data buffer (I thought we don’t need the
2nd Prp list for such case and the last entry in the first Prp list points to
last 4K range rather than a pointer to next Prp List).
Looks like my original thought is wrong as the h/w will have no way to
distinguish the last entry is a pointer to next Prp list or a valid entry to
buffer.
I made a little adjustment on your proposal. Please help review the attachment.
I think this way may be better to correct callee’s algorithm to create those
right Prp lists, do you agree?
Thanks
Feng
From: Baban Devkate
[mailto:baban.devk...@seagate.com<mailto:baban.devk...@seagate.com>]
Sent: Thursday, April 9, 2015 5:56 PM
To: Tian, Feng; baban...@gmail.com<mailto:baban...@gmail.com>
Cc: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: Re: MdeModulePkg:Fix required in NvmExpressDxe driver
+ adding self
On Thu, Apr 9, 2015 at 3:19 PM, Baban Devkate
<baban.devk...@seagate.com<mailto:baban.devk...@seagate.com>> wrote:
Let's take one more sample......
3) Offset = 0KB, Transfer Length = 4K * 513 blocks = 2MB + 4K
Prp1 = 0K to 4KB
Prp2 = pointer to a Prp List which is followed by another Prp List.
The first Prp List contains 511 entries to read data from offset 4KB to offset
2MB. the last one entry is a pointer to 2nd Prp List. The second Prp List
should contain 1 entry to read data from offset 2MB to offset 2MB +4k.
So for the third sample, EFI_SIZE_TO_PAGES(Offset + Bytes) – 1 = 512(i.e
Pages=512). In NvmeCreatePrpList(), PrpListNo = 1 for third sample.
*PrpListNo =1 with Remainder=0 when Pages=512 and PrpEntryNo=512
and this means only first Prp Lists entries will get populated and second Prp
List will not come into picture at all.
So the 2MB + 4K is requested data but 2MB is the actual payload.
We should have two cases each for Offset =0 and !=0.
So the fix should be like this:
Prp = NvmeCreatePrpList (PciIo, PhyAddr, ((Offset!= 0) ?
EFI_SIZE_TO_PAGES(Offset + Bytes) – 1: EFI_SIZE_TO_PAGES(Offset + Bytes) ),
&PrpListHost, &PrpListNo, &MapPrpList);
On Thu, Apr 9, 2015 at 1:26 PM, Tian, Feng
<feng.t...@intel.com<mailto:feng.t...@intel.com>> wrote:
Hi, Devkate
I agree the first one is a bug and need to be fixed like your proposal.
For the second one, sorry I couldn’t catch your point.
When the requested buffer size is larger than 2M, it means we need multiple PRP
list pages.
For example, when block size is 4K, memory page size is 4K
1) Offset = 0B, Transfer Length = 4K * 512 blocks = 2MB
Prp1 = 0 to 4KB
Prp2 = pointer to a Prp List which contains 511 entries to read data from
offset 4KB to offset 2MB.
2) Offset = 2KB, Transfer Length = 4K * 513 blocks = 2MB + 4K
Prp1 = 2K to 4KB
Prp2 = pointer to a Prp List which is followed by another Prp List.
The first Prp List contains 511 entries to read data from offset 4KB to offset
2MB. the last one entry is a pointer to 2nd Prp List. The second Prp List
contains 2 entries to read data from offset 2MB to offset 2MB +4k and from
offset 2MB + 4K to offset 2MB + 6K.
So for the first sample, EFI_SIZE_TO_PAGES(Offset + Bytes) – 1 = 511. For the
second sample, EFI_SIZE_TO_PAGES(Offset + Bytes) – 1 = 513. In
NvmeCreatePrpList(), PrpListNo = 1 for first sample and PrpListNo = 2 for
second sample.
So do I misunderstand something?
Thanks
Feng
From: Baban Devkate
[mailto:baban.devk...@seagate.com<mailto:baban.devk...@seagate.com>]
Sent: Thursday, April 09, 2015 09:37
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Cc: Tian, Feng
Subject: MdeModulePkg:Fix required in NvmExpressDxe driver
Hi,
Commit Message
============
MdeModulePkg: Fixed Pointer truncation and incorrect bytes passed for PRP list
creation.
There were two issues in NvmExpressPassthru.c(
MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c)-
1.PRP list entries were zeroed out as "PrpListBase" was getting truncated to
UINT8, changed to UINT64.
Old code- PrpListBase = *(UINT8*)PrpListHost + PrpListIndex * EFI_PAGE_SIZE;
New code- PrpListBase = *(UINT64*)PrpListHost + PrpListIndex * EFI_PAGE_SIZE;
2. Number of bytes passed to PRP list creation function corrected.
Actual payload length will not match requested whenever nested PRP list is
required. With current 1024 max transfer size and 4K Blocksize if the
requested buffer is greater than 2MB then nested PRP list will be required and
NvmeCreatePrpList (.., Pages-1 , ..) will result into above failure.
Old code-
Prp = NvmeCreatePrpList (PciIo, PhyAddr, EFI_SIZE_TO_PAGES(Offset + Bytes) - 1,
&PrpListHost, &PrpListNo, &MapPrpList);
New code-
Prp = NvmeCreatePrpList (PciIo, PhyAddr, EFI_SIZE_TO_PAGES(Offset + Bytes),
&PrpListHost, &PrpListNo, &MapPrpList);
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: baban.devk...@seagate.com<mailto:baban.devk...@seagate.com>
Name-Baban Devkate
email-baban.devk...@seagate.com<mailto:email-baban.devk...@seagate.com>
------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
