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

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

[Patrick Wildt]

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));
+	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));
+	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;
 }
 
-- 
2.23.0

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

[Bin Meng]

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

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

[Patrick Wildt]

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>
---
Changes for v2:
   - use dev->page_size instead of hardcoded value

 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..dee92b613d 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(dev->page_size, sizeof(struct nvme_id_ctrl));
+	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(ndev->page_size, sizeof(struct nvme_id_ns));
+	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;
 }
 
-- 
2.23.0

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

[Bin Meng]

On Wed, Oct 16, 2019 at 2:42 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>
> ---
> Changes for v2:
>    - use dev->page_size instead of hardcoded value
>
>  drivers/nvme/nvme.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

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

[Tom Rini]

On Wed, Oct 16, 2019 at 08:42:04AM +0200, Patrick Wildt 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>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191101/bebcd92f/attachment.sig>