Re: [U-Boot] [PATCH] nvme: use page-aligned buffer for identify command

On Mon, Oct 14, 2019 at 7:10 PM Patrick Wildt patrick@blueri.se wrote:

Change the stack-allocated buffer for the identification command to explicitly allocate page-aligned buffers. Even though the spec seems to allow having admin queue commands on non page-aligned buffers, it seems to not be possible on my i.MX8MQ board with a a Silicon Power P34A80. Since all of the NVMe drivers I have seen always do admin commands on a page-aligned buffer, which does work on my system, it makes sense for us to do that as well.

Signed-off-by: Patrick Wildt patrick@blueri.se

drivers/nvme/nvme.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c index ee6b581d9e..2444e0270f 100644 --- a/drivers/nvme/nvme.c +++ b/drivers/nvme/nvme.c @@ -580,14 +580,19 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)

static int nvme_get_info_from_identify(struct nvme_dev *dev) {

•   ALLOC_CACHE_ALIGN_BUFFER(char, buf, sizeof(struct nvme_id_ctrl));
  

•   struct nvme_id_ctrl *ctrl = (struct nvme_id_ctrl *)buf;
  

•   struct nvme_id_ctrl *ctrl;
    int ret;
    int shift = NVME_CAP_MPSMIN(dev->cap) + 12;
  
  

•   ctrl = memalign(4096, sizeof(struct nvme_id_ctrl));
  

Please use ndev->page_size instead of 4096.

•   if (!ctrl)
  

•           return -ENOMEM;
  

•   ret = nvme_identify(dev, 0, 1, (dma_addr_t)(long)ctrl);
  

•   if (ret)
  

•   if (ret) {
  

•           free(ctrl);
            return -EIO;
  

•   }
  
    dev->nn = le32_to_cpu(ctrl->nn);
    dev->vwc = ctrl->vwc;
  

@@ -618,6 +623,7 @@ static int nvme_get_info_from_identify(struct nvme_dev *dev) dev->max_transfer_shift = 20; }

•   free(ctrl);
    return 0;
  

}

@@ -658,16 +664,21 @@ static int nvme_blk_probe(struct udevice *udev) struct blk_desc *desc = dev_get_uclass_platdata(udev); struct nvme_ns *ns = dev_get_priv(udev); u8 flbas;

•   ALLOC_CACHE_ALIGN_BUFFER(char, buf, sizeof(struct nvme_id_ns));
  

•   struct nvme_id_ns *id = (struct nvme_id_ns *)buf;
    struct pci_child_platdata *pplat;
  

•   struct nvme_id_ns *id;
  

•   id = memalign(4096, sizeof(struct nvme_id_ns));
  

ditto

•   if (!id)
  

•           return -ENOMEM;
  
    memset(ns, 0, sizeof(*ns));
    ns->dev = ndev;
    /* extract the namespace id from the block device name */
    ns->ns_id = trailing_strtol(udev->name) + 1;
  

•   if (nvme_identify(ndev, ns->ns_id, 0, (dma_addr_t)(long)id))
  

•   if (nvme_identify(ndev, ns->ns_id, 0, (dma_addr_t)(long)id)) {
  

•           free(id);
            return -EIO;
  

•   }
  
    memcpy(&ns->eui64, &id->eui64, sizeof(id->eui64));
    flbas = id->flbas & NVME_NS_FLBAS_LBA_MASK;
  

@@ -686,6 +697,7 @@ static int nvme_blk_probe(struct udevice *udev) memcpy(desc->product, ndev->serial, sizeof(ndev->serial)); memcpy(desc->revision, ndev->firmware_rev, sizeof(ndev->firmware_rev));

•   free(id);
    return 0;
  

}

Regards, Bin