[PATCH 0/2] nvme fixes

[Gerd Hoffmann]

Gerd Hoffmann (2):
  nvme: improve namespace allocation
  nvme: drive desc should not include the newline

src/hw/nvme-int.h |  1 -
 src/hw/nvme.c     | 44 ++++++++++++++++++--------------------------
 2 files changed, 18 insertions(+), 27 deletions(-)

[PATCH 1/2] nvme: improve namespace allocation

[Gerd Hoffmann]

Instead of allocating a big array upfront go probe the namespaces and
only allocate an nvme_namespace struct for those namespaces which are
actually active.

Signed-off-by: Gerd Hoffmann 
---
 src/hw/nvme-int.h |  1 -
 src/hw/nvme.c     | 40 ++++++++++++++++------------------------
 2 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/src/hw/nvme-int.h b/src/hw/nvme-int.h
index 7947b29c9660..a4c1555cc058 100644
--- a/src/hw/nvme-int.h
+++ b/src/hw/nvme-int.h
@@ -103,7 +103,6 @@ struct nvme_ctrl {
     struct nvme_cq admin_cq;

u32 ns_count;
-    struct nvme_namespace *ns;

struct nvme_sq io_sq;
     struct nvme_cq io_cq;
diff --git a/src/hw/nvme.c b/src/hw/nvme.c
index f26b811e88bd..f64eba880a83 100644
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -234,11 +234,9 @@ nvme_admin_identify_ns(struct nvme_ctrl *ctrl, u32 ns_id)
 }

static void
-nvme_probe_ns(struct nvme_ctrl *ctrl, struct nvme_namespace *ns, u32 ns_id,
-              u8 mdts)
+nvme_probe_ns(struct nvme_ctrl *ctrl, u32 ns_idx, u8 mdts)
 {
-    ns->ctrl  = ctrl;
-    ns->ns_id = ns_id;
+    u32 ns_id = ns_idx + 1;

struct nvme_identify_ns *id = nvme_admin_identify_ns(ctrl, ns_id);
     if (!id) {
@@ -254,12 +252,21 @@ nvme_probe_ns(struct nvme_ctrl *ctrl, struct nvme_namespace *ns, u32 ns_id,
         goto free_buffer;
     }

-    ns->lba_count = id->nsze;
-    if (!ns->lba_count) {
+    if (!id->nsze) {
         dprintf(2, "NVMe NS %u is inactive.\n", ns_id);
         goto free_buffer;
     }

+    struct nvme_namespace *ns = malloc_fseg(sizeof(*ns));
+    if (!ns) {
+        warn_noalloc();
+        goto free_buffer;
+    }
+    memset(ns, 0, sizeof(*ns));
+    ns->ctrl  = ctrl;
+    ns->ns_id = ns_id;
+    ns->lba_count = id->nsze;
+
     struct nvme_lba_format *fmt = &id->lbaf[current_lba_format];

ns->block_size    = 1U << fmt->lbads;
@@ -269,10 +276,11 @@ nvme_probe_ns(struct nvme_ctrl *ctrl, struct nvme_namespace *ns, u32 ns_id,
         /* If we see devices that trigger this path, we need to increase our
            buffer size. */
         warn_internalerror();
+        free(ns);
         goto free_buffer;
     }

-    ns->drive.cntl_id   = ns - ctrl->ns;
+    ns->drive.cntl_id   = ns_idx;
     ns->drive.removable = 0;
     ns->drive.type      = DTYPE_NVME;
     ns->drive.blksize   = ns->block_size;
@@ -527,13 +535,6 @@ nvme_create_io_queues(struct nvme_ctrl *ctrl)
     return -1;
 }

-static void
-nvme_destroy_io_queues(struct nvme_ctrl *ctrl)
-{
-    nvme_destroy_sq(&ctrl->io_sq);
-    nvme_destroy_cq(&ctrl->io_cq);
-}
-
 /* Waits for CSTS.RDY to match rdy. Returns 0 on success. */
 static int
 nvme_wait_csts_rdy(struct nvme_ctrl *ctrl, unsigned rdy)
@@ -627,24 +628,15 @@ nvme_controller_enable(struct nvme_ctrl *ctrl)
         goto err_destroy_admin_sq;
     }

-    ctrl->ns = malloc_fseg(sizeof(*ctrl->ns) * ctrl->ns_count);
-    if (!ctrl->ns) {
-        warn_noalloc();
-        goto err_destroy_ioq;
-    }
-    memset(ctrl->ns, 0, sizeof(*ctrl->ns) * ctrl->ns_count);
-
     /* Populate namespace IDs */
     int ns_idx;
     for (ns_idx = 0; ns_idx < ctrl->ns_count; ns_idx++) {
-        nvme_probe_ns(ctrl, &ctrl->ns[ns_idx], ns_idx + 1, identify->mdts);
+        nvme_probe_ns(ctrl, ns_idx, identify->mdts);
     }

dprintf(3, "NVMe initialization complete!\n");
     return 0;

- err_destroy_ioq:
-    nvme_destroy_io_queues(ctrl);
  err_destroy_admin_sq:
     nvme_destroy_sq(&ctrl->admin_sq);
  err_destroy_admin_cq:

[PATCH 2/2] nvme: drive desc should not include the newline

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 src/hw/nvme.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/hw/nvme.c b/src/hw/nvme.c
index f64eba880a83..746e4682d318 100644
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -297,11 +297,11 @@ nvme_probe_ns(struct nvme_ctrl *ctrl, u32 ns_idx, u8 mdts)
     ns->dma_buffer = zalloc_page_aligned(&ZoneHigh, NVME_PAGE_SIZE);

char *desc = znprintf(MAXDESCSIZE, "NVMe NS %u: %llu MiB (%llu %u-byte "
-                          "blocks + %u-byte metadata)\n",
+                          "blocks + %u-byte metadata)",
                           ns_id, (ns->lba_count * ns->block_size) >> 20,
                           ns->lba_count, ns->block_size, ns->metadata_size);

-    dprintf(3, "%s", desc);
+    dprintf(3, "%s\n", desc);
     boot_add_hd(&ns->drive, desc, bootprio_find_pci_device(ctrl->pci));

free_buffer:

[Kevin O'Connor]

On Wed, May 26, 2021 at 09:49:55AM +0200, Gerd Hoffmann wrote:
> Gerd Hoffmann (2):
>
> nvme: improve namespace allocation
>
> nvme: drive desc should not include the newline

Thanks.  It looks fine to me.

-Kevin

[Cole Robinson]

On 5/26/21 3:49 AM, Gerd Hoffmann wrote:
> Gerd Hoffmann (2):
>
> nvme: improve namespace allocation
>
> nvme: drive desc should not include the newline
>
> src/hw/nvme-int.h |  1 -
>
> src/hw/nvme.c     | 44 ++++++++++++++++++--------------------------
>
> 2 files changed, 18 insertions(+), 27 deletions(-)

Thanks, this fixes https://bugzilla.redhat.com/show_bug.cgi?id=1963255
for me

- Cole