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

Commit cd59f50ab017 ("hw/nvme: always initialize a subsystem") causes
the controller to always set the CMIC.MCTRS ("Multiple Controllers")
bit. While spec-compliant, this is a deviation from the previous
behavior where this was only set if an nvme-subsys device was explicitly
created (to configure a subsystem with multiple controllers/namespaces).

Revert the behavior to only set CMIC.MCTRS if an nvme-subsys device is
created explicitly.

Reported-by: Alan Adamson <alan.adamson@oracle.com>
Fixes: cd59f50ab017 ("hw/nvme: always initialize a subsystem")
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ctrl.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 8de900ef8aca9b510b072892f9f82c01acee4f7d..0637e29ec9fdcfe65a97b8bdcff7851091096d44 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -8780,7 +8780,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
     uint8_t *pci_conf = pci_dev->config;
     uint64_t cap = ldq_le_p(&n->bar.cap);
     NvmeSecCtrlEntry *sctrl = nvme_sctrl(n);
-    uint32_t ctratt;
+    uint32_t ctratt = le32_to_cpu(id->ctratt);
     uint16_t oacs;
 
     memcpy(n->cse.acs, nvme_cse_acs_default, sizeof(n->cse.acs));
@@ -8798,10 +8798,11 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
 
     id->oaes = cpu_to_le32(NVME_OAES_NS_ATTR);
 
-    ctratt = NVME_CTRATT_ELBAS;
+    ctratt |= NVME_CTRATT_ELBAS;
     if (n->params.ctratt.mem) {
         ctratt |= NVME_CTRATT_MEM;
     }
+    id->ctratt = cpu_to_le32(ctratt);
 
     id->rab = 6;
 
@@ -8884,17 +8885,6 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
     id->psd[0].enlat = cpu_to_le32(0x10);
     id->psd[0].exlat = cpu_to_le32(0x4);
 
-    id->cmic |= NVME_CMIC_MULTI_CTRL;
-    ctratt |= NVME_CTRATT_ENDGRPS;
-
-    id->endgidmax = cpu_to_le16(0x1);
-
-    if (n->subsys->endgrp.fdp.enabled) {
-        ctratt |= NVME_CTRATT_FDPS;
-    }
-
-    id->ctratt = cpu_to_le32(ctratt);
-
     NVME_CAP_SET_MQES(cap, n->params.mqes);
     NVME_CAP_SET_CQR(cap, 1);
     NVME_CAP_SET_TO(cap, 0xf);
@@ -8927,6 +8917,20 @@ static int nvme_init_subsys(NvmeCtrl *n, Error **errp)
         }
 
         n->subsys = NVME_SUBSYS(dev);
+    } else {
+        NvmeIdCtrl *id = &n->id_ctrl;
+        uint32_t ctratt = le32_to_cpu(id->ctratt);
+
+        id->cmic |= NVME_CMIC_MULTI_CTRL;
+        ctratt |= NVME_CTRATT_ENDGRPS;
+
+        id->endgidmax = cpu_to_le16(0x1);
+
+        if (n->subsys->endgrp.fdp.enabled) {
+            ctratt |= NVME_CTRATT_FDPS;
+        }
+
+        id->ctratt = cpu_to_le32(ctratt);
     }
 
     cntlid = nvme_subsys_register_ctrl(n, errp);

-- 
2.47.2

On 6/3/25 5:59 AM, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Commit cd59f50ab017 ("hw/nvme: always initialize a subsystem") causes
> the controller to always set the CMIC.MCTRS ("Multiple Controllers")
> bit. While spec-compliant, this is a deviation from the previous
> behavior where this was only set if an nvme-subsys device was explicitly
> created (to configure a subsystem with multiple controllers/namespaces).
>
> Revert the behavior to only set CMIC.MCTRS if an nvme-subsys device is
> created explicitly.
>
> Reported-by: Alan Adamson <alan.adamson@oracle.com>
> Fixes: cd59f50ab017 ("hw/nvme: always initialize a subsystem")
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---

Reviewed-by: Alan Adamson <alan.adamson@oracle.com>

Tested-by: Alan Adamson <alan.adamson@oracle.com>


                                 Before 
Fix                                      With Klaus's Fix
subsys with 1 controller        # nvme id-ctrl /dev/nvme0n1 | grep 
cmic         # nvme id-ctrl /dev/nvme0n1 | grep cmic
                                 cmic      : 
0x2                                 cmic      : 0x2

subsys with 2 controllers       # nvme id-ctrl /dev/nvme0n1 | grep 
cmic         # nvme id-ctrl /dev/nvme0n1 | grep cmic
                                 cmic      : 
0x2                                 cmic      : 0x2
                                 # nvme id-ctrl /dev/nvme0n2 | grep 
cmic         # nvme id-ctrl /dev/nvme0n2 | grep cmic
                                 cmic      : 
0x2                                 cmic      : 0x2

no subsys with 2 controllers    # nvme id-ctrl /dev/nvme0n1 | grep 
cmic         # nvme id-ctrl /dev/nvme0n1 | grep cmic
                                 cmic      : 
0x2                                 cmic      : 0x0
                                 # nvme id-ctrl /dev/nvme1n1 | grep 
cmic         # nvme id-ctrl /dev/nvme1n1 | grep cmic
                                 cmic      : 
0x2                                 cmic      : 0x0


A bug in the Linux Atomic Write feature resulted in the 
atomic_write_max_bytes being set
to zero when cmic=0x2.

# uname -a
Linux localhost.localdomain 6.15.0-rc3+ #1 SMP PREEMPT_DYNAMIC Tue Apr 
22 14:34:09 PDT 2025 x86_64 x86_64 x86_64 GNU/Linux
# nvme id-ctrl /dev/nvme0n1 | grep cmic
cmic      : 0
# cat /sys/block/nvme0n1/queue/atomic_write_max_bytes
0

With this fix:

# nvme id-ctrl /dev/nvme0n1
cmic      : 0
# cat /sys/block/nvme0n1/queue/atomic_write_max_bytes
8192