All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 0/6] hw/nvme patches
@ 2021-07-11 20:27 Klaus Jensen
  2021-07-11 20:27 ` [PULL 1/6] hw/nvme: remove NvmeCtrl parameter from ns setup/check functions Klaus Jensen
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Klaus Jensen @ 2021-07-11 20:27 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell
  Cc: Klaus Jensen, Keith Busch, qemu-block, Klaus Jensen

From: Klaus Jensen <k.jensen@samsung.com>

Hi Pater,

The following changes since commit 9516034d05a8c71ef157a59f525e4c4f7ed79827:

  Merge remote-tracking branch 'remotes/cminyard/tags/for-qemu-6.1-2' into staging (2021-07-11 14:32:49 +0100)

are available in the Git repository at:

  git://git.infradead.org/qemu-nvme.git tags/nvme-next-pull-request

for you to fetch changes up to 9cc1a34ec4fe7e89e44e460dcf50159e40962e59:

  hw/nvme: fix controller hot unplugging (2021-07-11 21:50:22 +0200)

----------------------------------------------------------------
hw/nvme patches

* new PMR test (Gollu Appalanaidu)
* pmr/sgl mapping fix (Padmakar Kalghatgi)
* hotplug fixes (me)

----------------------------------------------------------------

Gollu Appalanaidu (1):
  tests/qtest/nvme-test: add persistent memory region test

Klaus Jensen (4):
  hw/nvme: remove NvmeCtrl parameter from ns setup/check functions
  hw/nvme: mark nvme-subsys non-hotpluggable
  hw/nvme: unregister controller with subsystem at exit
  hw/nvme: fix controller hot unplugging

Padmakar Kalghatgi (1):
  hw/nvme: error handling for too many mappings

 hw/nvme/nvme.h          | 18 ++++++------
 hw/nvme/ctrl.c          | 27 ++++++++++++++----
 hw/nvme/ns.c            | 55 ++++++++++++++++++++++++-------------
 hw/nvme/subsys.c        |  9 ++++++
 tests/qtest/nvme-test.c | 61 ++++++++++++++++++++++++++++++++++++++++-
 hw/nvme/trace-events    |  1 +
 6 files changed, 137 insertions(+), 34 deletions(-)

-- 
2.32.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PULL 1/6] hw/nvme: remove NvmeCtrl parameter from ns setup/check functions
  2021-07-11 20:27 [PULL 0/6] hw/nvme patches Klaus Jensen
@ 2021-07-11 20:27 ` Klaus Jensen
  2021-07-11 20:27 ` [PULL 2/6] hw/nvme: mark nvme-subsys non-hotpluggable Klaus Jensen
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Klaus Jensen @ 2021-07-11 20:27 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell
  Cc: Klaus Jensen, Keith Busch, Hannes Reinecke, qemu-block, Klaus Jensen

From: Klaus Jensen <k.jensen@samsung.com>

The nvme_ns_setup and nvme_ns_check_constraints should not depend on the
controller state. Refactor and remove it.

Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/nvme.h |  2 +-
 hw/nvme/ctrl.c |  2 +-
 hw/nvme/ns.c   | 37 ++++++++++++++++++-------------------
 3 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 56f8eceed2ad..0868359a1e86 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -246,7 +246,7 @@ static inline void nvme_aor_dec_active(NvmeNamespace *ns)
 }
 
 void nvme_ns_init_format(NvmeNamespace *ns);
-int nvme_ns_setup(NvmeCtrl *n, NvmeNamespace *ns, Error **errp);
+int nvme_ns_setup(NvmeNamespace *ns, Error **errp);
 void nvme_ns_drain(NvmeNamespace *ns);
 void nvme_ns_shutdown(NvmeNamespace *ns);
 void nvme_ns_cleanup(NvmeNamespace *ns);
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 629b0d38c2a2..dd1801510032 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -6498,7 +6498,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
         ns = &n->namespace;
         ns->params.nsid = 1;
 
-        if (nvme_ns_setup(n, ns, errp)) {
+        if (nvme_ns_setup(ns, errp)) {
             return;
         }
 
diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 4275c3db6301..3c4f5b8c714a 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -346,8 +346,7 @@ static void nvme_zoned_ns_shutdown(NvmeNamespace *ns)
     assert(ns->nr_open_zones == 0);
 }
 
-static int nvme_ns_check_constraints(NvmeCtrl *n, NvmeNamespace *ns,
-                                     Error **errp)
+static int nvme_ns_check_constraints(NvmeNamespace *ns, Error **errp)
 {
     if (!ns->blkconf.blk) {
         error_setg(errp, "block backend not configured");
@@ -366,20 +365,6 @@ static int nvme_ns_check_constraints(NvmeCtrl *n, NvmeNamespace *ns,
         return -1;
     }
 
-    if (!n->subsys) {
-        if (ns->params.detached) {
-            error_setg(errp, "detached requires that the nvme device is "
-                       "linked to an nvme-subsys device");
-            return -1;
-        }
-
-        if (ns->params.shared) {
-            error_setg(errp, "shared requires that the nvme device is "
-                       "linked to an nvme-subsys device");
-            return -1;
-        }
-    }
-
     if (ns->params.zoned) {
         if (ns->params.max_active_zones) {
             if (ns->params.max_open_zones > ns->params.max_active_zones) {
@@ -411,9 +396,9 @@ static int nvme_ns_check_constraints(NvmeCtrl *n, NvmeNamespace *ns,
     return 0;
 }
 
-int nvme_ns_setup(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
+int nvme_ns_setup(NvmeNamespace *ns, Error **errp)
 {
-    if (nvme_ns_check_constraints(n, ns, errp)) {
+    if (nvme_ns_check_constraints(ns, errp)) {
         return -1;
     }
 
@@ -465,7 +450,21 @@ static void nvme_ns_realize(DeviceState *dev, Error **errp)
     uint32_t nsid = ns->params.nsid;
     int i;
 
-    if (nvme_ns_setup(n, ns, errp)) {
+    if (!n->subsys) {
+        if (ns->params.detached) {
+            error_setg(errp, "detached requires that the nvme device is "
+                       "linked to an nvme-subsys device");
+            return;
+        }
+
+        if (ns->params.shared) {
+            error_setg(errp, "shared requires that the nvme device is "
+                       "linked to an nvme-subsys device");
+            return;
+        }
+    }
+
+    if (nvme_ns_setup(ns, errp)) {
         return;
     }
 
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PULL 2/6] hw/nvme: mark nvme-subsys non-hotpluggable
  2021-07-11 20:27 [PULL 0/6] hw/nvme patches Klaus Jensen
  2021-07-11 20:27 ` [PULL 1/6] hw/nvme: remove NvmeCtrl parameter from ns setup/check functions Klaus Jensen
@ 2021-07-11 20:27 ` Klaus Jensen
  2021-07-11 20:27 ` [PULL 3/6] hw/nvme: unregister controller with subsystem at exit Klaus Jensen
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Klaus Jensen @ 2021-07-11 20:27 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell
  Cc: Klaus Jensen, Keith Busch, Hannes Reinecke, qemu-block, Klaus Jensen

From: Klaus Jensen <k.jensen@samsung.com>

We currently lack the infrastructure to handle subsystem hotplugging, so
disable it.

Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/subsys.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/nvme/subsys.c b/hw/nvme/subsys.c
index 192223d17ca1..dc7a96862f37 100644
--- a/hw/nvme/subsys.c
+++ b/hw/nvme/subsys.c
@@ -61,6 +61,7 @@ static void nvme_subsys_class_init(ObjectClass *oc, void *data)
 
     dc->realize = nvme_subsys_realize;
     dc->desc = "Virtual NVMe subsystem";
+    dc->hotpluggable = false;
 
     device_class_set_props(dc, nvme_subsystem_props);
 }
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PULL 3/6] hw/nvme: unregister controller with subsystem at exit
  2021-07-11 20:27 [PULL 0/6] hw/nvme patches Klaus Jensen
  2021-07-11 20:27 ` [PULL 1/6] hw/nvme: remove NvmeCtrl parameter from ns setup/check functions Klaus Jensen
  2021-07-11 20:27 ` [PULL 2/6] hw/nvme: mark nvme-subsys non-hotpluggable Klaus Jensen
@ 2021-07-11 20:27 ` Klaus Jensen
  2021-07-11 20:27 ` [PULL 4/6] hw/nvme: error handling for too many mappings Klaus Jensen
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Klaus Jensen @ 2021-07-11 20:27 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell
  Cc: Klaus Jensen, Keith Busch, Hannes Reinecke, qemu-block, Klaus Jensen

From: Klaus Jensen <k.jensen@samsung.com>

Make sure the controller is unregistered from the subsystem when device
is removed.

Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/nvme.h   | 1 +
 hw/nvme/ctrl.c   | 4 ++++
 hw/nvme/subsys.c | 5 +++++
 3 files changed, 10 insertions(+)

diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 0868359a1e86..c4065467d877 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -50,6 +50,7 @@ typedef struct NvmeSubsystem {
 } NvmeSubsystem;
 
 int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp);
+void nvme_subsys_unregister_ctrl(NvmeSubsystem *subsys, NvmeCtrl *n);
 
 static inline NvmeCtrl *nvme_subsys_ctrl(NvmeSubsystem *subsys,
                                          uint32_t cntlid)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index dd1801510032..90e3ee2b70ee 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -6523,6 +6523,10 @@ static void nvme_exit(PCIDevice *pci_dev)
         nvme_ns_cleanup(ns);
     }
 
+    if (n->subsys) {
+        nvme_subsys_unregister_ctrl(n->subsys, n);
+    }
+
     g_free(n->cq);
     g_free(n->sq);
     g_free(n->aer_reqs);
diff --git a/hw/nvme/subsys.c b/hw/nvme/subsys.c
index dc7a96862f37..92caa604a280 100644
--- a/hw/nvme/subsys.c
+++ b/hw/nvme/subsys.c
@@ -32,6 +32,11 @@ int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp)
     return cntlid;
 }
 
+void nvme_subsys_unregister_ctrl(NvmeSubsystem *subsys, NvmeCtrl *n)
+{
+    subsys->ctrls[n->cntlid] = NULL;
+}
+
 static void nvme_subsys_setup(NvmeSubsystem *subsys)
 {
     const char *nqn = subsys->params.nqn ?
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PULL 4/6] hw/nvme: error handling for too many mappings
  2021-07-11 20:27 [PULL 0/6] hw/nvme patches Klaus Jensen
                   ` (2 preceding siblings ...)
  2021-07-11 20:27 ` [PULL 3/6] hw/nvme: unregister controller with subsystem at exit Klaus Jensen
@ 2021-07-11 20:27 ` Klaus Jensen
  2021-07-11 20:27 ` [PULL 5/6] tests/qtest/nvme-test: add persistent memory region test Klaus Jensen
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Klaus Jensen @ 2021-07-11 20:27 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell
  Cc: Klaus Jensen, Keith Busch, Padmakar Kalghatgi, qemu-block, Klaus Jensen

From: Padmakar Kalghatgi <p.kalghatgi@samsung.com>

If the number of PRP/SGL mappings exceed 1024, reads and writes will
fail because of an internal QEMU limitation of max 1024 vectors.

Signed-off-by: Padmakar Kalghatgi <p.kalghatgi@samsung.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
[k.jensen: changed the error message to be more generic]
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ctrl.c       | 13 +++++++++++++
 hw/nvme/trace-events |  1 +
 2 files changed, 14 insertions(+)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 90e3ee2b70ee..ead7531bde5e 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -623,6 +623,10 @@ static uint16_t nvme_map_addr(NvmeCtrl *n, NvmeSg *sg, hwaddr addr, size_t len)
             return NVME_INVALID_USE_OF_CMB | NVME_DNR;
         }
 
+        if (sg->iov.niov + 1 > IOV_MAX) {
+            goto max_mappings_exceeded;
+        }
+
         if (cmb) {
             return nvme_map_addr_cmb(n, &sg->iov, addr, len);
         } else {
@@ -634,9 +638,18 @@ static uint16_t nvme_map_addr(NvmeCtrl *n, NvmeSg *sg, hwaddr addr, size_t len)
         return NVME_INVALID_USE_OF_CMB | NVME_DNR;
     }
 
+    if (sg->qsg.nsg + 1 > IOV_MAX) {
+        goto max_mappings_exceeded;
+    }
+
     qemu_sglist_add(&sg->qsg, addr, len);
 
     return NVME_SUCCESS;
+
+max_mappings_exceeded:
+    NVME_GUEST_ERR(pci_nvme_ub_too_many_mappings,
+                   "number of mappings exceed 1024");
+    return NVME_INTERNAL_DEV_ERROR | NVME_DNR;
 }
 
 static inline bool nvme_addr_is_dma(NvmeCtrl *n, hwaddr addr)
diff --git a/hw/nvme/trace-events b/hw/nvme/trace-events
index f9a1f14e2638..430eeb395b24 100644
--- a/hw/nvme/trace-events
+++ b/hw/nvme/trace-events
@@ -199,3 +199,4 @@ pci_nvme_ub_db_wr_invalid_cqhead(uint32_t qid, uint16_t new_head) "completion qu
 pci_nvme_ub_db_wr_invalid_sq(uint32_t qid) "submission queue doorbell write for nonexistent queue, sqid=%"PRIu32", ignoring"
 pci_nvme_ub_db_wr_invalid_sqtail(uint32_t qid, uint16_t new_tail) "submission queue doorbell write value beyond queue size, sqid=%"PRIu32", new_head=%"PRIu16", ignoring"
 pci_nvme_ub_unknown_css_value(void) "unknown value in cc.css field"
+pci_nvme_ub_too_many_mappings(void) "too many prp/sgl mappings"
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PULL 5/6] tests/qtest/nvme-test: add persistent memory region test
  2021-07-11 20:27 [PULL 0/6] hw/nvme patches Klaus Jensen
                   ` (3 preceding siblings ...)
  2021-07-11 20:27 ` [PULL 4/6] hw/nvme: error handling for too many mappings Klaus Jensen
@ 2021-07-11 20:27 ` Klaus Jensen
  2021-07-11 20:27 ` [PULL 6/6] hw/nvme: fix controller hot unplugging Klaus Jensen
  2021-07-12 14:51 ` [PULL 0/6] hw/nvme patches Peter Maydell
  6 siblings, 0 replies; 9+ messages in thread
From: Klaus Jensen @ 2021-07-11 20:27 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell
  Cc: Klaus Jensen, Keith Busch, Gollu Appalanaidu, qemu-block, Klaus Jensen

From: Gollu Appalanaidu <anaidu.gollu@samsung.com>

This will test the PMR functionality.

Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
[k.jensen: replaced memory-backend-file with memory-backend-ram]
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 tests/qtest/nvme-test.c | 61 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/nvme-test.c b/tests/qtest/nvme-test.c
index d32c953a3824..47e757d7e2af 100644
--- a/tests/qtest/nvme-test.c
+++ b/tests/qtest/nvme-test.c
@@ -13,6 +13,7 @@
 #include "libqos/libqtest.h"
 #include "libqos/qgraph.h"
 #include "libqos/pci.h"
+#include "include/block/nvme.h"
 
 typedef struct QNvme QNvme;
 
@@ -66,12 +67,65 @@ static void nvmetest_oob_cmb_test(void *obj, void *data, QGuestAllocator *alloc)
     g_assert_cmpint(qpci_io_readl(pdev, bar, cmb_bar_size - 1), !=, 0x44332211);
 }
 
+static void nvmetest_pmr_reg_test(void *obj, void *data, QGuestAllocator *alloc)
+{
+    QNvme *nvme = obj;
+    QPCIDevice *pdev = &nvme->dev;
+    QPCIBar pmr_bar, nvme_bar;
+    uint32_t pmrcap, pmrsts;
+
+    qpci_device_enable(pdev);
+    pmr_bar = qpci_iomap(pdev, 4, NULL);
+
+    /* Without Enabling PMRCTL check bar enablemet */
+    qpci_io_writel(pdev, pmr_bar, 0, 0xccbbaa99);
+    g_assert_cmpint(qpci_io_readb(pdev, pmr_bar, 0), !=, 0x99);
+    g_assert_cmpint(qpci_io_readw(pdev, pmr_bar, 0), !=, 0xaa99);
+
+    /* Map NVMe Bar Register to Enable the Mem Region */
+    nvme_bar = qpci_iomap(pdev, 0, NULL);
+
+    pmrcap = qpci_io_readl(pdev, nvme_bar, 0xe00);
+    g_assert_cmpint(NVME_PMRCAP_RDS(pmrcap), ==, 0x1);
+    g_assert_cmpint(NVME_PMRCAP_WDS(pmrcap), ==, 0x1);
+    g_assert_cmpint(NVME_PMRCAP_BIR(pmrcap), ==, 0x4);
+    g_assert_cmpint(NVME_PMRCAP_PMRWBM(pmrcap), ==, 0x2);
+    g_assert_cmpint(NVME_PMRCAP_CMSS(pmrcap), ==, 0x1);
+
+    /* Enable PMRCTRL */
+    qpci_io_writel(pdev, nvme_bar, 0xe04, 0x1);
+
+    qpci_io_writel(pdev, pmr_bar, 0, 0x44332211);
+    g_assert_cmpint(qpci_io_readb(pdev, pmr_bar, 0), ==, 0x11);
+    g_assert_cmpint(qpci_io_readw(pdev, pmr_bar, 0), ==, 0x2211);
+    g_assert_cmpint(qpci_io_readl(pdev, pmr_bar, 0), ==, 0x44332211);
+
+    pmrsts = qpci_io_readl(pdev, nvme_bar, 0xe08);
+    g_assert_cmpint(NVME_PMRSTS_NRDY(pmrsts), ==, 0x0);
+
+    /* Disable PMRCTRL */
+    qpci_io_writel(pdev, nvme_bar, 0xe04, 0x0);
+
+    qpci_io_writel(pdev, pmr_bar, 0, 0x88776655);
+    g_assert_cmpint(qpci_io_readb(pdev, pmr_bar, 0), !=, 0x55);
+    g_assert_cmpint(qpci_io_readw(pdev, pmr_bar, 0), !=, 0x6655);
+    g_assert_cmpint(qpci_io_readl(pdev, pmr_bar, 0), !=, 0x88776655);
+
+    pmrsts = qpci_io_readl(pdev, nvme_bar, 0xe08);
+    g_assert_cmpint(NVME_PMRSTS_NRDY(pmrsts), ==, 0x1);
+
+    qpci_iounmap(pdev, nvme_bar);
+    qpci_iounmap(pdev, pmr_bar);
+}
+
 static void nvme_register_nodes(void)
 {
     QOSGraphEdgeOptions opts = {
         .extra_device_opts = "addr=04.0,drive=drv0,serial=foo",
         .before_cmd_line = "-drive id=drv0,if=none,file=null-co://,"
-                           "file.read-zeroes=on,format=raw",
+                           "file.read-zeroes=on,format=raw "
+                           "-object memory-backend-ram,id=pmr0,"
+                           "share=on,size=8",
     };
 
     add_qpci_address(&opts, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });
@@ -83,6 +137,11 @@ static void nvme_register_nodes(void)
     qos_add_test("oob-cmb-access", "nvme", nvmetest_oob_cmb_test, &(QOSGraphTestOptions) {
         .edge.extra_device_opts = "cmb_size_mb=2"
     });
+
+    qos_add_test("pmr-test-access", "nvme", nvmetest_pmr_reg_test,
+                 &(QOSGraphTestOptions) {
+        .edge.extra_device_opts = "pmrdev=pmr0"
+    });
 }
 
 libqos_init(nvme_register_nodes);
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PULL 6/6] hw/nvme: fix controller hot unplugging
  2021-07-11 20:27 [PULL 0/6] hw/nvme patches Klaus Jensen
                   ` (4 preceding siblings ...)
  2021-07-11 20:27 ` [PULL 5/6] tests/qtest/nvme-test: add persistent memory region test Klaus Jensen
@ 2021-07-11 20:27 ` Klaus Jensen
  2021-07-12 14:51 ` [PULL 0/6] hw/nvme patches Peter Maydell
  6 siblings, 0 replies; 9+ messages in thread
From: Klaus Jensen @ 2021-07-11 20:27 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell
  Cc: Klaus Jensen, Keith Busch, Hannes Reinecke, qemu-block, Klaus Jensen

From: Klaus Jensen <k.jensen@samsung.com>

Prior to this patch the nvme-ns devices are always children of the
NvmeBus owned by the NvmeCtrl. This causes the namespaces to be
unrealized when the parent device is removed. However, when subsystems
are involved, this is not what we want since the namespaces may be
attached to other controllers as well.

This patch adds an additional NvmeBus on the subsystem device. When
nvme-ns devices are realized, if the parent controller device is linked
to a subsystem, the parent bus is set to the subsystem one instead. This
makes sure that namespaces are kept alive and not unrealized.

Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/nvme.h   | 15 ++++++++-------
 hw/nvme/ctrl.c   | 14 ++++++--------
 hw/nvme/ns.c     | 18 ++++++++++++++++++
 hw/nvme/subsys.c |  3 +++
 4 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index c4065467d877..83ffabade4cf 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -33,12 +33,20 @@ QEMU_BUILD_BUG_ON(NVME_MAX_NAMESPACES > NVME_NSID_BROADCAST - 1);
 typedef struct NvmeCtrl NvmeCtrl;
 typedef struct NvmeNamespace NvmeNamespace;
 
+#define TYPE_NVME_BUS "nvme-bus"
+OBJECT_DECLARE_SIMPLE_TYPE(NvmeBus, NVME_BUS)
+
+typedef struct NvmeBus {
+    BusState parent_bus;
+} NvmeBus;
+
 #define TYPE_NVME_SUBSYS "nvme-subsys"
 #define NVME_SUBSYS(obj) \
     OBJECT_CHECK(NvmeSubsystem, (obj), TYPE_NVME_SUBSYS)
 
 typedef struct NvmeSubsystem {
     DeviceState parent_obj;
+    NvmeBus     bus;
     uint8_t     subnqn[256];
 
     NvmeCtrl      *ctrls[NVME_MAX_CONTROLLERS];
@@ -365,13 +373,6 @@ typedef struct NvmeCQueue {
     QTAILQ_HEAD(, NvmeRequest) req_list;
 } NvmeCQueue;
 
-#define TYPE_NVME_BUS "nvme-bus"
-#define NVME_BUS(obj) OBJECT_CHECK(NvmeBus, (obj), TYPE_NVME_BUS)
-
-typedef struct NvmeBus {
-    BusState parent_bus;
-} NvmeBus;
-
 #define TYPE_NVME "nvme"
 #define NVME(obj) \
         OBJECT_CHECK(NvmeCtrl, (obj), TYPE_NVME)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index ead7531bde5e..2f0524e12a36 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -6527,16 +6527,14 @@ static void nvme_exit(PCIDevice *pci_dev)
 
     nvme_ctrl_reset(n);
 
-    for (i = 1; i <= NVME_MAX_NAMESPACES; i++) {
-        ns = nvme_ns(n, i);
-        if (!ns) {
-            continue;
+    if (n->subsys) {
+        for (i = 1; i <= NVME_MAX_NAMESPACES; i++) {
+            ns = nvme_ns(n, i);
+            if (ns) {
+                ns->attached--;
+            }
         }
 
-        nvme_ns_cleanup(ns);
-    }
-
-    if (n->subsys) {
         nvme_subsys_unregister_ctrl(n->subsys, n);
     }
 
diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 3c4f5b8c714a..b7cf1494e75b 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -441,6 +441,15 @@ void nvme_ns_cleanup(NvmeNamespace *ns)
     }
 }
 
+static void nvme_ns_unrealize(DeviceState *dev)
+{
+    NvmeNamespace *ns = NVME_NS(dev);
+
+    nvme_ns_drain(ns);
+    nvme_ns_shutdown(ns);
+    nvme_ns_cleanup(ns);
+}
+
 static void nvme_ns_realize(DeviceState *dev, Error **errp)
 {
     NvmeNamespace *ns = NVME_NS(dev);
@@ -462,6 +471,14 @@ static void nvme_ns_realize(DeviceState *dev, Error **errp)
                        "linked to an nvme-subsys device");
             return;
         }
+    } else {
+        /*
+         * If this namespace belongs to a subsystem (through a link on the
+         * controller device), reparent the device.
+         */
+        if (!qdev_set_parent_bus(dev, &subsys->bus.parent_bus, errp)) {
+            return;
+        }
     }
 
     if (nvme_ns_setup(ns, errp)) {
@@ -552,6 +569,7 @@ static void nvme_ns_class_init(ObjectClass *oc, void *data)
 
     dc->bus_type = TYPE_NVME_BUS;
     dc->realize = nvme_ns_realize;
+    dc->unrealize = nvme_ns_unrealize;
     device_class_set_props(dc, nvme_ns_props);
     dc->desc = "Virtual NVMe namespace";
 }
diff --git a/hw/nvme/subsys.c b/hw/nvme/subsys.c
index 92caa604a280..93c35950d69d 100644
--- a/hw/nvme/subsys.c
+++ b/hw/nvme/subsys.c
@@ -50,6 +50,9 @@ static void nvme_subsys_realize(DeviceState *dev, Error **errp)
 {
     NvmeSubsystem *subsys = NVME_SUBSYS(dev);
 
+    qbus_create_inplace(&subsys->bus, sizeof(NvmeBus), TYPE_NVME_BUS, dev,
+                        dev->id);
+
     nvme_subsys_setup(subsys);
 }
 
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PULL 0/6] hw/nvme patches
  2021-07-11 20:27 [PULL 0/6] hw/nvme patches Klaus Jensen
                   ` (5 preceding siblings ...)
  2021-07-11 20:27 ` [PULL 6/6] hw/nvme: fix controller hot unplugging Klaus Jensen
@ 2021-07-12 14:51 ` Peter Maydell
  2021-07-12 16:54   ` Klaus Jensen
  6 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2021-07-12 14:51 UTC (permalink / raw)
  To: Klaus Jensen; +Cc: Keith Busch, Klaus Jensen, QEMU Developers, Qemu-block

On Sun, 11 Jul 2021 at 21:27, Klaus Jensen <its@irrelevant.dk> wrote:
>
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Hi Pater,
>
> The following changes since commit 9516034d05a8c71ef157a59f525e4c4f7ed79827:
>
>   Merge remote-tracking branch 'remotes/cminyard/tags/for-qemu-6.1-2' into staging (2021-07-11 14:32:49 +0100)
>
> are available in the Git repository at:
>
>   git://git.infradead.org/qemu-nvme.git tags/nvme-next-pull-request
>
> for you to fetch changes up to 9cc1a34ec4fe7e89e44e460dcf50159e40962e59:
>
>   hw/nvme: fix controller hot unplugging (2021-07-11 21:50:22 +0200)
>
> ----------------------------------------------------------------
> hw/nvme patches
>
> * new PMR test (Gollu Appalanaidu)
> * pmr/sgl mapping fix (Padmakar Kalghatgi)
> * hotplug fixes (me)
>
> ----------------------------------------------------------------
>
> Gollu Appalanaidu (1):
>   tests/qtest/nvme-test: add persistent memory region test
>
> Klaus Jensen (4):
>   hw/nvme: remove NvmeCtrl parameter from ns setup/check functions
>   hw/nvme: mark nvme-subsys non-hotpluggable
>   hw/nvme: unregister controller with subsystem at exit
>   hw/nvme: fix controller hot unplugging
>
> Padmakar Kalghatgi (1):
>   hw/nvme: error handling for too many mappings

Hi; this failed an assertion on two of the travis CI jobs and
then got timed-out for not producing any more output:

https://app.travis-ci.com/gitlab/qemu-project/qemu/jobs/523720897
https://app.travis-ci.com/gitlab/qemu-project/qemu/jobs/523720898

ERROR:../tests/qtest/nvme-test.c:89:nvmetest_pmr_reg_test: assertion
failed (NVME_PMRCAP_RDS(pmrcap) == 0x1): (0 == 1)

ERROR qtest-ppc64/qos-test - Bail out!
ERROR:../tests/qtest/nvme-test.c:89:nvmetest_pmr_reg_test: assertion
failed (NVME_PMRCAP_RDS(pmrcap) == 0x1): (0 == 1)

No output has been received in the last 10m0s, this potentially
indicates a stalled build or something wrong with the build itself.



These jobs both run on s390, and my ppc64be and s390 hosts
both hung during "make check", so I think you have an
issue on big-endian hosts somewhere.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PULL 0/6] hw/nvme patches
  2021-07-12 14:51 ` [PULL 0/6] hw/nvme patches Peter Maydell
@ 2021-07-12 16:54   ` Klaus Jensen
  0 siblings, 0 replies; 9+ messages in thread
From: Klaus Jensen @ 2021-07-12 16:54 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Keith Busch, Klaus Jensen, QEMU Developers, Qemu-block

[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]

On Jul 12 15:51, Peter Maydell wrote:
> On Sun, 11 Jul 2021 at 21:27, Klaus Jensen <its@irrelevant.dk> wrote:
> >
> > From: Klaus Jensen <k.jensen@samsung.com>
> >
> > Hi Pater,
> >
> > The following changes since commit 9516034d05a8c71ef157a59f525e4c4f7ed79827:
> >
> >   Merge remote-tracking branch 'remotes/cminyard/tags/for-qemu-6.1-2' into staging (2021-07-11 14:32:49 +0100)
> >
> > are available in the Git repository at:
> >
> >   git://git.infradead.org/qemu-nvme.git tags/nvme-next-pull-request
> >
> > for you to fetch changes up to 9cc1a34ec4fe7e89e44e460dcf50159e40962e59:
> >
> >   hw/nvme: fix controller hot unplugging (2021-07-11 21:50:22 +0200)
> >
> > ----------------------------------------------------------------
> > hw/nvme patches
> >
> > * new PMR test (Gollu Appalanaidu)
> > * pmr/sgl mapping fix (Padmakar Kalghatgi)
> > * hotplug fixes (me)
> >
> > ----------------------------------------------------------------
> >
> > Gollu Appalanaidu (1):
> >   tests/qtest/nvme-test: add persistent memory region test
> >
> > Klaus Jensen (4):
> >   hw/nvme: remove NvmeCtrl parameter from ns setup/check functions
> >   hw/nvme: mark nvme-subsys non-hotpluggable
> >   hw/nvme: unregister controller with subsystem at exit
> >   hw/nvme: fix controller hot unplugging
> >
> > Padmakar Kalghatgi (1):
> >   hw/nvme: error handling for too many mappings
> 
> Hi; this failed an assertion on two of the travis CI jobs and
> then got timed-out for not producing any more output:
> 
> https://app.travis-ci.com/gitlab/qemu-project/qemu/jobs/523720897
> https://app.travis-ci.com/gitlab/qemu-project/qemu/jobs/523720898
> 
> ERROR:../tests/qtest/nvme-test.c:89:nvmetest_pmr_reg_test: assertion
> failed (NVME_PMRCAP_RDS(pmrcap) == 0x1): (0 == 1)
> 
> ERROR qtest-ppc64/qos-test - Bail out!
> ERROR:../tests/qtest/nvme-test.c:89:nvmetest_pmr_reg_test: assertion
> failed (NVME_PMRCAP_RDS(pmrcap) == 0x1): (0 == 1)
> 
> No output has been received in the last 10m0s, this potentially
> indicates a stalled build or something wrong with the build itself.
> 
> 
> 
> These jobs both run on s390, and my ppc64be and s390 hosts
> both hung during "make check", so I think you have an
> issue on big-endian hosts somewhere.
> 

Thanks Peter,

I'll look into it!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-07-12 17:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-11 20:27 [PULL 0/6] hw/nvme patches Klaus Jensen
2021-07-11 20:27 ` [PULL 1/6] hw/nvme: remove NvmeCtrl parameter from ns setup/check functions Klaus Jensen
2021-07-11 20:27 ` [PULL 2/6] hw/nvme: mark nvme-subsys non-hotpluggable Klaus Jensen
2021-07-11 20:27 ` [PULL 3/6] hw/nvme: unregister controller with subsystem at exit Klaus Jensen
2021-07-11 20:27 ` [PULL 4/6] hw/nvme: error handling for too many mappings Klaus Jensen
2021-07-11 20:27 ` [PULL 5/6] tests/qtest/nvme-test: add persistent memory region test Klaus Jensen
2021-07-11 20:27 ` [PULL 6/6] hw/nvme: fix controller hot unplugging Klaus Jensen
2021-07-12 14:51 ` [PULL 0/6] hw/nvme patches Peter Maydell
2021-07-12 16:54   ` Klaus Jensen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.