[PULL 5/5] hw/nvme: fix firmware boot path when nvme-ns has a bootindex

[Klaus Jensen <its@irrelevant.dk>]

From: Luigi Leonardi <leonardi@redhat.com>

The correct way to register an NVMe namespace in fw_boot_order is to use
the NVMe controller as the device pointer with the namespace ID appended
as a suffix, producing:

  /pci@i0cf8/pci1b36,0010@4/namespace@1,0

This is how the controller-level bootindex works.
Explicit nvme-ns devices instead register the nvme-ns device itself,
which is wrong: realize() reparents the namespace onto the NVMe subsystem
bus, disconnecting it from the controller in the bus hierarchy.
qdev_get_fw_dev_path() therefore produces a path without the controller's
address:

  /nvme-ns/namespace@1,0

Fix by re-registering in realize(), once the controller and final nsid
are known, using the controller as the device pointer with the namespace
suffix. The old entry (registered against the nvme-ns device) must be
removed first since add_boot_device_path() only removes entries matching
the same device pointer, and leaving both would cause a
duplicate-bootindex error.

device_add_bootindex_property() cannot be called in realize() because
QEMU parses command-line properties before realize() runs, so the
bootindex property must exist by instance_init() time. The final nsid is
not yet known then either as it's assigned in realize().

The fix is restricted to non-shared namespaces: for shared namespaces
there is no single controller to register the boot path against.

Also add the symmetric unrealize() cleanup so that hot-unplugging a
non-shared nvme-ns with a bootindex removes the fw_boot_order entry that
was registered against the controller in realize().

Reported-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ns.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index de15921b613b..2ae26a9acbd0 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -792,6 +792,10 @@ static void nvme_ns_unrealize(DeviceState *dev)
     nvme_ns_shutdown(ns);
     nvme_ns_cleanup(ns);
 
+    if (!ns->params.shared && ns->bootindex >= 0) {
+        del_boot_device_path(DEVICE(ns->ctrl), ns->bootindex_suffix);
+    }
+
     /* Symmetric with nvme_ns_realize() which sets subsys->namespaces[nsid]. */
     if (subsys && nsid && subsys->namespaces[nsid] == ns) {
         subsys->namespaces[nsid] = NULL;
@@ -956,6 +960,19 @@ static void nvme_ns_realize(DeviceState *dev, Error **errp)
 
     if (!ns->params.shared) {
         ns->ctrl = n;
+
+        /*
+         * Register the boot device path using the NVMe controller
+         * so the OFW path includes the controller's PCI address:
+         *   /pci@i0cf8/pci1b36,0010@<slot>,0/namespace@<nsid>,0
+         */
+        if (ns->bootindex >= 0) {
+            del_boot_device_path(dev, NULL);
+            snprintf(ns->bootindex_suffix, sizeof(ns->bootindex_suffix),
+                     "/namespace@%" PRIu32 ",0", nsid);
+            add_boot_device_path(ns->bootindex, DEVICE(n),
+                                 ns->bootindex_suffix);
+        }
     }
 }
 
@@ -1182,10 +1199,8 @@ static void nvme_ns_instance_init(Object *obj)
 {
     NvmeNamespace *ns = NVME_NS(obj);
 
-    sprintf(ns->bootindex_suffix, "/namespace@%" PRIu32 ",0", ns->params.nsid);
-
     device_add_bootindex_property(obj, &ns->bootindex, "bootindex",
-                                  ns->bootindex_suffix, DEVICE(obj));
+                                  NULL, DEVICE(obj));
 }
 
 static const TypeInfo nvme_ns_info = {
-- 
2.53.0