Hi Aaron, On Wed, Aug 21, 2019 at 8:34 AM Aaron Williams <[email protected]> wrote: > > When large writes take place I saw a Samsung > EVO 970+ return a status value of 0x13, PRP > Offset Invalid. I tracked this down to the > improper handling of PRP entries. The blocks > the PRP entries are placed in cannot cross a > page boundary and thus should be allocated on > page boundaries. This is how the Linux kernel > driver works. > > With this patch, the PRP pool is allocated on > a page boundary and other than the very first > allocation, the pool size is a multiple of > the page size. Each page can hold (4096 / 8) - 1 > entries since the last entry must point to the > next page in the pool.
Please write more words in a line, about 70 characters in a line. > > Signed-off-by: Aaron Williams <[email protected]> > --- > drivers/nvme/nvme.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c > index 7008a54a6d..ae64459edf 100644 > --- a/drivers/nvme/nvme.c > +++ b/drivers/nvme/nvme.c > @@ -75,6 +75,8 @@ static int nvme_setup_prps(struct nvme_dev *dev, u64 *prp2, > int length = total_len; > int i, nprps; > length -= (page_size - offset); > + u32 prps_per_page = (page_size >> 3) - 1; > + u32 num_pages; nits: please move these 2 above the line "length -= (page_size - offset);" > > if (length <= 0) { > *prp2 = 0; > @@ -90,15 +92,16 @@ static int nvme_setup_prps(struct nvme_dev *dev, u64 > *prp2, > } > > nprps = DIV_ROUND_UP(length, page_size); > + num_pages = (nprps + prps_per_page - 1) / prps_per_page; use DIV_ROUND_UP() > > if (nprps > dev->prp_entry_num) { I think we should adjust nprps before the comparison here. nprps += num_pages - 1; > free(dev->prp_pool); > - dev->prp_pool = malloc(nprps << 3); > + dev->prp_pool = memalign(page_size, num_pages * page_size); Then we need only do: dev->prp_pool = memalign(page_size, nprps << 3)? > if (!dev->prp_pool) { > printf("Error: malloc prp_pool fail\n"); > return -ENOMEM; > } > - dev->prp_entry_num = nprps; > + dev->prp_entry_num = ((page_size >> 3) - 1) * num_pages; and no need to change this line, but we need change the while (nprps) {} loop. > } > > prp_pool = dev->prp_pool; > @@ -791,7 +794,7 @@ static int nvme_probe(struct udevice *udev) > } > memset(ndev->queues, 0, NVME_Q_NUM * sizeof(struct nvme_queue *)); > > - ndev->prp_pool = malloc(MAX_PRP_POOL); > + ndev->prp_pool = memalign(1 << 12, MAX_PRP_POOL); I think we need use ndev->pagesize instead of (1 << 12), but ndev->pagesize is initialized in nvme_configure_admin_queue(), so we need move the initialization of ndev->pagesize out of that function and do it before the memalign() here. > if (!ndev->prp_pool) { > ret = -ENOMEM; > printf("Error: %s: Out of memory!\n", udev->name); > -- Regards, Bin _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
