* [PATCH V6 1/6] hw/block/nvme: introduce nvme-subsys device
2021-01-24 2:54 [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
@ 2021-01-24 2:54 ` Minwoo Im
2021-01-25 19:58 ` Klaus Jensen
2021-01-24 2:54 ` [PATCH V6 2/6] hw/block/nvme: support to map controller to a subsystem Minwoo Im
` (7 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Minwoo Im @ 2021-01-24 2:54 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Keith Busch, Klaus Jensen, Minwoo Im, Kevin Wolf, Max Reitz
To support multi-path in QEMU NVMe device model, We need to have NVMe
subsystem hierarchy to map controllers and namespaces to a NVMe
subsystem.
This patch introduced a simple nvme-subsys device model. The subsystem
will be prepared with subsystem NQN with <subsys_id> provided in
nvme-subsys device:
ex) -device nvme-subsys,id=subsys0: nqn.2019-08.org.qemu:subsys0
Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
hw/block/meson.build | 2 +-
hw/block/nvme-subsys.c | 60 ++++++++++++++++++++++++++++++++++++++++++
hw/block/nvme-subsys.h | 25 ++++++++++++++++++
hw/block/nvme.c | 3 +++
4 files changed, 89 insertions(+), 1 deletion(-)
create mode 100644 hw/block/nvme-subsys.c
create mode 100644 hw/block/nvme-subsys.h
diff --git a/hw/block/meson.build b/hw/block/meson.build
index 602ca6c8541d..83ea2d37978d 100644
--- a/hw/block/meson.build
+++ b/hw/block/meson.build
@@ -13,7 +13,7 @@ softmmu_ss.add(when: 'CONFIG_SSI_M25P80', if_true: files('m25p80.c'))
softmmu_ss.add(when: 'CONFIG_SWIM', if_true: files('swim.c'))
softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
softmmu_ss.add(when: 'CONFIG_SH4', if_true: files('tc58128.c'))
-softmmu_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('nvme.c', 'nvme-ns.c'))
+softmmu_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('nvme.c', 'nvme-ns.c', 'nvme-subsys.c'))
specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: files('vhost-user-blk.c'))
diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
new file mode 100644
index 000000000000..aa82911b951c
--- /dev/null
+++ b/hw/block/nvme-subsys.c
@@ -0,0 +1,60 @@
+/*
+ * QEMU NVM Express Subsystem: nvme-subsys
+ *
+ * Copyright (c) 2021 Minwoo Im <minwoo.im.dev@gmail.com>
+ *
+ * This code is licensed under the GNU GPL v2. Refer COPYING.
+ */
+
+#include "qemu/units.h"
+#include "qemu/osdep.h"
+#include "qemu/uuid.h"
+#include "qemu/iov.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/qdev-core.h"
+#include "hw/block/block.h"
+#include "block/aio.h"
+#include "block/accounting.h"
+#include "sysemu/sysemu.h"
+#include "hw/pci/pci.h"
+#include "nvme.h"
+#include "nvme-subsys.h"
+
+static void nvme_subsys_setup(NvmeSubsystem *subsys)
+{
+ snprintf((char *)subsys->subnqn, sizeof(subsys->subnqn),
+ "nqn.2019-08.org.qemu:%s", subsys->parent_obj.id);
+}
+
+static void nvme_subsys_realize(DeviceState *dev, Error **errp)
+{
+ NvmeSubsystem *subsys = NVME_SUBSYS(dev);
+
+ nvme_subsys_setup(subsys);
+}
+
+static void nvme_subsys_class_init(ObjectClass *oc, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(oc);
+
+ set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+
+ dc->realize = nvme_subsys_realize;
+ dc->desc = "Virtual NVMe subsystem";
+}
+
+static const TypeInfo nvme_subsys_info = {
+ .name = TYPE_NVME_SUBSYS,
+ .parent = TYPE_DEVICE,
+ .class_init = nvme_subsys_class_init,
+ .instance_size = sizeof(NvmeSubsystem),
+};
+
+static void nvme_subsys_register_types(void)
+{
+ type_register_static(&nvme_subsys_info);
+}
+
+type_init(nvme_subsys_register_types)
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
new file mode 100644
index 000000000000..40f06a4c7db0
--- /dev/null
+++ b/hw/block/nvme-subsys.h
@@ -0,0 +1,25 @@
+/*
+ * QEMU NVM Express Subsystem: nvme-subsys
+ *
+ * Copyright (c) 2021 Minwoo Im <minwoo.im.dev@gmail.com>
+ *
+ * This code is licensed under the GNU GPL v2. Refer COPYING.
+ */
+
+#ifndef NVME_SUBSYS_H
+#define NVME_SUBSYS_H
+
+#define TYPE_NVME_SUBSYS "nvme-subsys"
+#define NVME_SUBSYS(obj) \
+ OBJECT_CHECK(NvmeSubsystem, (obj), TYPE_NVME_SUBSYS)
+
+#define NVME_SUBSYS_MAX_CTRLS 32
+
+typedef struct NvmeCtrl NvmeCtrl;
+typedef struct NvmeNamespace NvmeNamespace;
+typedef struct NvmeSubsystem {
+ DeviceState parent_obj;
+ uint8_t subnqn[256];
+} NvmeSubsystem;
+
+#endif /* NVME_SUBSYS_H */
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 21aec90637fa..aabccdf36f4b 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -25,6 +25,7 @@
* mdts=<N[optional]>,zoned.append_size_limit=<N[optional]> \
* -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\
* zoned=<true|false[optional]>
+ * -device nvme-subsys,id=<subsys_id>
*
* Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
* offset 0 in BAR2 and supports only WDS, RDS and SQS for now. By default, the
@@ -38,6 +39,8 @@
*
* The PMR will use BAR 4/5 exclusively.
*
+ * To place controller(s) and namespace(s) to a subsystem, then provide
+ * nvme-subsys device as above.
*
* nvme device parameters
* ~~~~~~~~~~~~~~~~~~~~~~
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V6 1/6] hw/block/nvme: introduce nvme-subsys device
2021-01-24 2:54 ` [PATCH V6 1/6] hw/block/nvme: introduce nvme-subsys device Minwoo Im
@ 2021-01-25 19:58 ` Klaus Jensen
0 siblings, 0 replies; 20+ messages in thread
From: Klaus Jensen @ 2021-01-25 19:58 UTC (permalink / raw)
To: Minwoo Im; +Cc: Keith Busch, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 2709 bytes --]
On Jan 24 11:54, Minwoo Im wrote:
> To support multi-path in QEMU NVMe device model, We need to have NVMe
> subsystem hierarchy to map controllers and namespaces to a NVMe
> subsystem.
>
> This patch introduced a simple nvme-subsys device model. The subsystem
> will be prepared with subsystem NQN with <subsys_id> provided in
> nvme-subsys device:
>
> ex) -device nvme-subsys,id=subsys0: nqn.2019-08.org.qemu:subsys0
>
> Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
> ---
> hw/block/meson.build | 2 +-
> hw/block/nvme-subsys.c | 60 ++++++++++++++++++++++++++++++++++++++++++
> hw/block/nvme-subsys.h | 25 ++++++++++++++++++
> hw/block/nvme.c | 3 +++
> 4 files changed, 89 insertions(+), 1 deletion(-)
> create mode 100644 hw/block/nvme-subsys.c
> create mode 100644 hw/block/nvme-subsys.h
>
> diff --git a/hw/block/meson.build b/hw/block/meson.build
> index 602ca6c8541d..83ea2d37978d 100644
> --- a/hw/block/meson.build
> +++ b/hw/block/meson.build
> @@ -13,7 +13,7 @@ softmmu_ss.add(when: 'CONFIG_SSI_M25P80', if_true: files('m25p80.c'))
> softmmu_ss.add(when: 'CONFIG_SWIM', if_true: files('swim.c'))
> softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
> softmmu_ss.add(when: 'CONFIG_SH4', if_true: files('tc58128.c'))
> -softmmu_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('nvme.c', 'nvme-ns.c'))
> +softmmu_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('nvme.c', 'nvme-ns.c', 'nvme-subsys.c'))
>
> specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
> specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: files('vhost-user-blk.c'))
> diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
> new file mode 100644
> index 000000000000..aa82911b951c
> --- /dev/null
> +++ b/hw/block/nvme-subsys.c
> @@ -0,0 +1,60 @@
> +/*
> + * QEMU NVM Express Subsystem: nvme-subsys
> + *
> + * Copyright (c) 2021 Minwoo Im <minwoo.im.dev@gmail.com>
> + *
> + * This code is licensed under the GNU GPL v2. Refer COPYING.
> + */
> +
> +#include "qemu/units.h"
> +#include "qemu/osdep.h"
> +#include "qemu/uuid.h"
> +#include "qemu/iov.h"
> +#include "qemu/cutils.h"
> +#include "qapi/error.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/qdev-core.h"
> +#include "hw/block/block.h"
> +#include "block/aio.h"
> +#include "block/accounting.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/pci/pci.h"
> +#include "nvme.h"
> +#include "nvme-subsys.h"
> +
This can be slimmed down to
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "hw/block/block.h"
#include "sysemu/sysemu.h"
#include "hw/pci/pci.h"
#include "nvme.h"
#include "nvme-subsys.h"
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V6 2/6] hw/block/nvme: support to map controller to a subsystem
2021-01-24 2:54 [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
2021-01-24 2:54 ` [PATCH V6 1/6] hw/block/nvme: introduce nvme-subsys device Minwoo Im
@ 2021-01-24 2:54 ` Minwoo Im
2021-01-24 2:54 ` [PATCH V6 3/6] hw/block/nvme: add CMIC enum value for Identify Controller Minwoo Im
` (6 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Minwoo Im @ 2021-01-24 2:54 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Keith Busch, Klaus Jensen, Minwoo Im, Kevin Wolf, Max Reitz
nvme controller(nvme) can be mapped to a NVMe subsystem(nvme-subsys).
This patch maps a controller to a subsystem by adding a parameter
'subsys' to the nvme device.
To map a controller to a subsystem, we need to put nvme-subsys first and
then maps the subsystem to the controller:
-device nvme-subsys,id=subsys0
-device nvme,serial=foo,id=nvme0,subsys=subsys0
If 'subsys' property is not given to the nvme controller, then subsystem
NQN will be created with serial (e.g., 'foo' in above example),
Otherwise, it will be based on subsys id (e.g., 'subsys0' in above
example).
Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
hw/block/nvme.c | 30 +++++++++++++++++++++++++-----
hw/block/nvme.h | 3 +++
2 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index aabccdf36f4b..b525fca14103 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -22,7 +22,8 @@
* [pmrdev=<mem_backend_file_id>,] \
* max_ioqpairs=<N[optional]>, \
* aerl=<N[optional]>, aer_max_queued=<N[optional]>, \
- * mdts=<N[optional]>,zoned.append_size_limit=<N[optional]> \
+ * mdts=<N[optional]>,zoned.append_size_limit=<N[optional]>, \
+ * subsys=<subsys_id> \
* -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\
* zoned=<true|false[optional]>
* -device nvme-subsys,id=<subsys_id>
@@ -44,6 +45,13 @@
*
* nvme device parameters
* ~~~~~~~~~~~~~~~~~~~~~~
+ * - `subsys`
+ * NVM Subsystem device. If given, a subsystem NQN will be initialized with
+ * <subsys_id> given. Otherwise, <serial> will be taken for subsystem NQN.
+ * Also, it will enable multi controller capability represented in Identify
+ * Controller data structure in CMIC (Controller Multi-path I/O and Namesapce
+ * Sharing Capabilities), if given.
+ *
* - `aerl`
* The Asynchronous Event Request Limit (AERL). Indicates the maximum number
* of concurrently outstanding Asynchronous Event Request commands support
@@ -4404,11 +4412,23 @@ static int nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
return 0;
}
+static void nvme_init_subnqn(NvmeCtrl *n)
+{
+ NvmeSubsystem *subsys = n->subsys;
+ NvmeIdCtrl *id = &n->id_ctrl;
+
+ if (!subsys) {
+ snprintf((char *)id->subnqn, sizeof(id->subnqn),
+ "nqn.2019-08.org.qemu:%s", n->params.serial);
+ } else {
+ pstrcpy((char *)id->subnqn, sizeof(id->subnqn), (char*)subsys->subnqn);
+ }
+}
+
static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
{
NvmeIdCtrl *id = &n->id_ctrl;
uint8_t *pci_conf = pci_dev->config;
- char *subnqn;
id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
@@ -4455,9 +4475,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
id->sgls = cpu_to_le32(NVME_CTRL_SGLS_SUPPORT_NO_ALIGN |
NVME_CTRL_SGLS_BITBUCKET);
- subnqn = g_strdup_printf("nqn.2019-08.org.qemu:%s", n->params.serial);
- strpadcpy((char *)id->subnqn, sizeof(id->subnqn), subnqn, '\0');
- g_free(subnqn);
+ nvme_init_subnqn(n);
id->psd[0].mp = cpu_to_le16(0x9c4);
id->psd[0].enlat = cpu_to_le32(0x10);
@@ -4545,6 +4563,8 @@ static Property nvme_props[] = {
DEFINE_BLOCK_PROPERTIES(NvmeCtrl, namespace.blkconf),
DEFINE_PROP_LINK("pmrdev", NvmeCtrl, pmr.dev, TYPE_MEMORY_BACKEND,
HostMemoryBackend *),
+ DEFINE_PROP_LINK("subsys", NvmeCtrl, subsys, TYPE_NVME_SUBSYS,
+ NvmeSubsystem *),
DEFINE_PROP_STRING("serial", NvmeCtrl, params.serial),
DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, params.cmb_size_mb, 0),
DEFINE_PROP_UINT32("num_queues", NvmeCtrl, params.num_queues, 0),
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index dee6092bd45f..04d4684601fd 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -2,6 +2,7 @@
#define HW_NVME_H
#include "block/nvme.h"
+#include "nvme-subsys.h"
#include "nvme-ns.h"
#define NVME_MAX_NAMESPACES 256
@@ -170,6 +171,8 @@ typedef struct NvmeCtrl {
uint8_t zasl;
+ NvmeSubsystem *subsys;
+
NvmeNamespace namespace;
NvmeNamespace *namespaces[NVME_MAX_NAMESPACES];
NvmeSQueue **sq;
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH V6 3/6] hw/block/nvme: add CMIC enum value for Identify Controller
2021-01-24 2:54 [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
2021-01-24 2:54 ` [PATCH V6 1/6] hw/block/nvme: introduce nvme-subsys device Minwoo Im
2021-01-24 2:54 ` [PATCH V6 2/6] hw/block/nvme: support to map controller to a subsystem Minwoo Im
@ 2021-01-24 2:54 ` Minwoo Im
2021-01-24 2:54 ` [PATCH V6 4/6] hw/block/nvme: support for multi-controller in subsystem Minwoo Im
` (5 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Minwoo Im @ 2021-01-24 2:54 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Keith Busch, Klaus Jensen, Minwoo Im, Kevin Wolf, Max Reitz
Added Controller Multi-path I/O and Namespace Sharing Capabilities
(CMIC) field to support multi-controller in the following patches.
This field is in Identify Controller data structure in [76].
Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
include/block/nvme.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/block/nvme.h b/include/block/nvme.h
index e4b918064df9..d6415a869c1c 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -1034,6 +1034,10 @@ enum NvmeIdCtrlLpa {
NVME_LPA_EXTENDED = 1 << 2,
};
+enum NvmeIdCtrlCmic {
+ NVME_CMIC_MULTI_CTRL = 1 << 1,
+};
+
#define NVME_CTRL_SQES_MIN(sqes) ((sqes) & 0xf)
#define NVME_CTRL_SQES_MAX(sqes) (((sqes) >> 4) & 0xf)
#define NVME_CTRL_CQES_MIN(cqes) ((cqes) & 0xf)
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH V6 4/6] hw/block/nvme: support for multi-controller in subsystem
2021-01-24 2:54 [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
` (2 preceding siblings ...)
2021-01-24 2:54 ` [PATCH V6 3/6] hw/block/nvme: add CMIC enum value for Identify Controller Minwoo Im
@ 2021-01-24 2:54 ` Minwoo Im
2021-01-25 18:03 ` Klaus Jensen
2021-01-24 2:54 ` [PATCH V6 5/6] hw/block/nvme: add NMIC enum value for Identify Namespace Minwoo Im
` (4 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Minwoo Im @ 2021-01-24 2:54 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Keith Busch, Klaus Jensen, Minwoo Im, Kevin Wolf, Max Reitz
We have nvme-subsys and nvme devices mapped together. To support
multi-controller scheme to this setup, controller identifier(id) has to
be managed. Earlier, cntlid(controller id) used to be always 0 because
we didn't have any subsystem scheme that controller id matters.
This patch introduced 'cntlid' attribute to the nvme controller
instance(NvmeCtrl) and make it allocated by the nvme-subsys device
mapped to the controller. If nvme-subsys is not given to the
controller, then it will always be 0 as it was.
Added 'ctrls' array in the nvme-subsys instance to manage attached
controllers to the subsystem with a limit(32). This patch didn't take
list for the controllers to make it seamless with nvme-ns device.
Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
hw/block/nvme-subsys.c | 21 +++++++++++++++++++++
hw/block/nvme-subsys.h | 4 ++++
hw/block/nvme.c | 29 +++++++++++++++++++++++++++++
hw/block/nvme.h | 1 +
4 files changed, 55 insertions(+)
diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
index aa82911b951c..e9d61c993c90 100644
--- a/hw/block/nvme-subsys.c
+++ b/hw/block/nvme-subsys.c
@@ -22,6 +22,27 @@
#include "nvme.h"
#include "nvme-subsys.h"
+int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp)
+{
+ NvmeSubsystem *subsys = n->subsys;
+ int cntlid;
+
+ for (cntlid = 0; cntlid < ARRAY_SIZE(subsys->ctrls); cntlid++) {
+ if (!subsys->ctrls[cntlid]) {
+ break;
+ }
+ }
+
+ if (cntlid == ARRAY_SIZE(subsys->ctrls)) {
+ error_setg(errp, "no more free controller id");
+ return -1;
+ }
+
+ subsys->ctrls[cntlid] = n;
+
+ return cntlid;
+}
+
static void nvme_subsys_setup(NvmeSubsystem *subsys)
{
snprintf((char *)subsys->subnqn, sizeof(subsys->subnqn),
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
index 40f06a4c7db0..4eba50d96a1d 100644
--- a/hw/block/nvme-subsys.h
+++ b/hw/block/nvme-subsys.h
@@ -20,6 +20,10 @@ typedef struct NvmeNamespace NvmeNamespace;
typedef struct NvmeSubsystem {
DeviceState parent_obj;
uint8_t subnqn[256];
+
+ NvmeCtrl *ctrls[NVME_SUBSYS_MAX_CTRLS];
} NvmeSubsystem;
+int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp);
+
#endif /* NVME_SUBSYS_H */
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index b525fca14103..7138389be4bd 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -4435,6 +4435,9 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
strpadcpy((char *)id->fr, sizeof(id->fr), "1.0", ' ');
strpadcpy((char *)id->sn, sizeof(id->sn), n->params.serial, ' ');
+
+ id->cntlid = cpu_to_le16(n->cntlid);
+
id->rab = 6;
id->ieee[0] = 0x00;
id->ieee[1] = 0x02;
@@ -4481,6 +4484,10 @@ 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);
+ if (n->subsys) {
+ id->cmic |= NVME_CMIC_MULTI_CTRL;
+ }
+
NVME_CAP_SET_MQES(n->bar.cap, 0x7ff);
NVME_CAP_SET_CQR(n->bar.cap, 1);
NVME_CAP_SET_TO(n->bar.cap, 0xf);
@@ -4495,6 +4502,24 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
n->bar.intmc = n->bar.intms = 0;
}
+static int nvme_init_subsys(NvmeCtrl *n, Error **errp)
+{
+ int cntlid;
+
+ if (!n->subsys) {
+ return 0;
+ }
+
+ cntlid = nvme_subsys_register_ctrl(n, errp);
+ if (cntlid < 0) {
+ return -1;
+ }
+
+ n->cntlid = cntlid;
+
+ return 0;
+}
+
static void nvme_realize(PCIDevice *pci_dev, Error **errp)
{
NvmeCtrl *n = NVME(pci_dev);
@@ -4515,6 +4540,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
return;
}
+ if (nvme_init_subsys(n, errp)) {
+ error_propagate(errp, local_err);
+ return;
+ }
nvme_init_ctrl(n, pci_dev);
/* setup a namespace if the controller drive property was given */
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 04d4684601fd..b8f5f2d6ffb8 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -134,6 +134,7 @@ typedef struct NvmeCtrl {
NvmeBus bus;
BlockConf conf;
+ uint16_t cntlid;
bool qs_created;
uint32_t page_size;
uint16_t page_bits;
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V6 4/6] hw/block/nvme: support for multi-controller in subsystem
2021-01-24 2:54 ` [PATCH V6 4/6] hw/block/nvme: support for multi-controller in subsystem Minwoo Im
@ 2021-01-25 18:03 ` Klaus Jensen
2021-01-25 18:11 ` Keith Busch
0 siblings, 1 reply; 20+ messages in thread
From: Klaus Jensen @ 2021-01-25 18:03 UTC (permalink / raw)
To: Minwoo Im; +Cc: Keith Busch, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 1658 bytes --]
On Jan 24 11:54, Minwoo Im wrote:
> We have nvme-subsys and nvme devices mapped together. To support
> multi-controller scheme to this setup, controller identifier(id) has to
> be managed. Earlier, cntlid(controller id) used to be always 0 because
> we didn't have any subsystem scheme that controller id matters.
>
> This patch introduced 'cntlid' attribute to the nvme controller
> instance(NvmeCtrl) and make it allocated by the nvme-subsys device
> mapped to the controller. If nvme-subsys is not given to the
> controller, then it will always be 0 as it was.
>
> Added 'ctrls' array in the nvme-subsys instance to manage attached
> controllers to the subsystem with a limit(32). This patch didn't take
> list for the controllers to make it seamless with nvme-ns device.
>
> Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
> ---
> hw/block/nvme-subsys.c | 21 +++++++++++++++++++++
> hw/block/nvme-subsys.h | 4 ++++
> hw/block/nvme.c | 29 +++++++++++++++++++++++++++++
> hw/block/nvme.h | 1 +
> 4 files changed, 55 insertions(+)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index b525fca14103..7138389be4bd 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -4481,6 +4484,10 @@ 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);
>
> + if (n->subsys) {
> + id->cmic |= NVME_CMIC_MULTI_CTRL;
> + }
Since multiple controllers show up with a PCIe port of their own, do we
need to set bit 0 (NVME_CMIC_MULTI_PORT?) as well? Or am I
misunderstanding that bit?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V6 4/6] hw/block/nvme: support for multi-controller in subsystem
2021-01-25 18:03 ` Klaus Jensen
@ 2021-01-25 18:11 ` Keith Busch
2021-01-26 0:52 ` Minwoo Im
0 siblings, 1 reply; 20+ messages in thread
From: Keith Busch @ 2021-01-25 18:11 UTC (permalink / raw)
To: Klaus Jensen; +Cc: Kevin Wolf, Minwoo Im, qemu-devel, qemu-block, Max Reitz
On Mon, Jan 25, 2021 at 07:03:32PM +0100, Klaus Jensen wrote:
> On Jan 24 11:54, Minwoo Im wrote:
> > We have nvme-subsys and nvme devices mapped together. To support
> > multi-controller scheme to this setup, controller identifier(id) has to
> > be managed. Earlier, cntlid(controller id) used to be always 0 because
> > we didn't have any subsystem scheme that controller id matters.
> >
> > This patch introduced 'cntlid' attribute to the nvme controller
> > instance(NvmeCtrl) and make it allocated by the nvme-subsys device
> > mapped to the controller. If nvme-subsys is not given to the
> > controller, then it will always be 0 as it was.
> >
> > Added 'ctrls' array in the nvme-subsys instance to manage attached
> > controllers to the subsystem with a limit(32). This patch didn't take
> > list for the controllers to make it seamless with nvme-ns device.
> >
> > Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
> > ---
> > hw/block/nvme-subsys.c | 21 +++++++++++++++++++++
> > hw/block/nvme-subsys.h | 4 ++++
> > hw/block/nvme.c | 29 +++++++++++++++++++++++++++++
> > hw/block/nvme.h | 1 +
> > 4 files changed, 55 insertions(+)
> >
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index b525fca14103..7138389be4bd 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -4481,6 +4484,10 @@ 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);
> >
> > + if (n->subsys) {
> > + id->cmic |= NVME_CMIC_MULTI_CTRL;
> > + }
>
> Since multiple controllers show up with a PCIe port of their own, do we
> need to set bit 0 (NVME_CMIC_MULTI_PORT?) as well? Or am I
> misunderstanding that bit?
AIUI, if you report this MULTI_PORT bit, then each PCI device in the
subsystem needs to report a different "Port Number" in their PCIe Link
Capabilities register. I don't think we can manipulate that value from
the nvme "device", but I also don't know what a host should do with this
information even if we could. So, I think it's safe to leave it at 0.
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V6 4/6] hw/block/nvme: support for multi-controller in subsystem
2021-01-25 18:11 ` Keith Busch
@ 2021-01-26 0:52 ` Minwoo Im
2021-01-26 17:57 ` Keith Busch
0 siblings, 1 reply; 20+ messages in thread
From: Minwoo Im @ 2021-01-26 0:52 UTC (permalink / raw)
To: Keith Busch; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
On 21-01-25 10:11:43, Keith Busch wrote:
> On Mon, Jan 25, 2021 at 07:03:32PM +0100, Klaus Jensen wrote:
> > On Jan 24 11:54, Minwoo Im wrote:
> > > We have nvme-subsys and nvme devices mapped together. To support
> > > multi-controller scheme to this setup, controller identifier(id) has to
> > > be managed. Earlier, cntlid(controller id) used to be always 0 because
> > > we didn't have any subsystem scheme that controller id matters.
> > >
> > > This patch introduced 'cntlid' attribute to the nvme controller
> > > instance(NvmeCtrl) and make it allocated by the nvme-subsys device
> > > mapped to the controller. If nvme-subsys is not given to the
> > > controller, then it will always be 0 as it was.
> > >
> > > Added 'ctrls' array in the nvme-subsys instance to manage attached
> > > controllers to the subsystem with a limit(32). This patch didn't take
> > > list for the controllers to make it seamless with nvme-ns device.
> > >
> > > Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
> > > ---
> > > hw/block/nvme-subsys.c | 21 +++++++++++++++++++++
> > > hw/block/nvme-subsys.h | 4 ++++
> > > hw/block/nvme.c | 29 +++++++++++++++++++++++++++++
> > > hw/block/nvme.h | 1 +
> > > 4 files changed, 55 insertions(+)
> > >
> > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > > index b525fca14103..7138389be4bd 100644
> > > --- a/hw/block/nvme.c
> > > +++ b/hw/block/nvme.c
> > > @@ -4481,6 +4484,10 @@ 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);
> > >
> > > + if (n->subsys) {
> > > + id->cmic |= NVME_CMIC_MULTI_CTRL;
> > > + }
> >
> > Since multiple controllers show up with a PCIe port of their own, do we
> > need to set bit 0 (NVME_CMIC_MULTI_PORT?) as well? Or am I
> > misunderstanding that bit?
>
> AIUI, if you report this MULTI_PORT bit, then each PCI device in the
> subsystem needs to report a different "Port Number" in their PCIe Link
> Capabilities register. I don't think we can manipulate that value from
> the nvme "device", but I also don't know what a host should do with this
> information even if we could. So, I think it's safe to leave it at 0.
AFAIK, If we leave it to 0, kernel will not allocate disk for multi-path
case (e.g., nvmeXcYnZ).
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V6 4/6] hw/block/nvme: support for multi-controller in subsystem
2021-01-26 0:52 ` Minwoo Im
@ 2021-01-26 17:57 ` Keith Busch
2021-01-26 20:44 ` Minwoo Im
0 siblings, 1 reply; 20+ messages in thread
From: Keith Busch @ 2021-01-26 17:57 UTC (permalink / raw)
To: Minwoo Im; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
On Tue, Jan 26, 2021 at 09:52:48AM +0900, Minwoo Im wrote:
> On 21-01-25 10:11:43, Keith Busch wrote:
> > On Mon, Jan 25, 2021 at 07:03:32PM +0100, Klaus Jensen wrote:
> > > On Jan 24 11:54, Minwoo Im wrote:
> > > > We have nvme-subsys and nvme devices mapped together. To support
> > > > multi-controller scheme to this setup, controller identifier(id) has to
> > > > be managed. Earlier, cntlid(controller id) used to be always 0 because
> > > > we didn't have any subsystem scheme that controller id matters.
> > > >
> > > > This patch introduced 'cntlid' attribute to the nvme controller
> > > > instance(NvmeCtrl) and make it allocated by the nvme-subsys device
> > > > mapped to the controller. If nvme-subsys is not given to the
> > > > controller, then it will always be 0 as it was.
> > > >
> > > > Added 'ctrls' array in the nvme-subsys instance to manage attached
> > > > controllers to the subsystem with a limit(32). This patch didn't take
> > > > list for the controllers to make it seamless with nvme-ns device.
> > > >
> > > > Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
> > > > ---
> > > > hw/block/nvme-subsys.c | 21 +++++++++++++++++++++
> > > > hw/block/nvme-subsys.h | 4 ++++
> > > > hw/block/nvme.c | 29 +++++++++++++++++++++++++++++
> > > > hw/block/nvme.h | 1 +
> > > > 4 files changed, 55 insertions(+)
> > > >
> > > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > > > index b525fca14103..7138389be4bd 100644
> > > > --- a/hw/block/nvme.c
> > > > +++ b/hw/block/nvme.c
> > > > @@ -4481,6 +4484,10 @@ 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);
> > > >
> > > > + if (n->subsys) {
> > > > + id->cmic |= NVME_CMIC_MULTI_CTRL;
> > > > + }
> > >
> > > Since multiple controllers show up with a PCIe port of their own, do we
> > > need to set bit 0 (NVME_CMIC_MULTI_PORT?) as well? Or am I
> > > misunderstanding that bit?
> >
> > AIUI, if you report this MULTI_PORT bit, then each PCI device in the
> > subsystem needs to report a different "Port Number" in their PCIe Link
> > Capabilities register. I don't think we can manipulate that value from
> > the nvme "device", but I also don't know what a host should do with this
> > information even if we could. So, I think it's safe to leave it at 0.
>
> AFAIK, If we leave it to 0, kernel will not allocate disk for multi-path
> case (e.g., nvmeXcYnZ).
Kernel only checks for MULTI_CTRL. It doesn't do anything with MULTI_PORT.
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V6 4/6] hw/block/nvme: support for multi-controller in subsystem
2021-01-26 17:57 ` Keith Busch
@ 2021-01-26 20:44 ` Minwoo Im
0 siblings, 0 replies; 20+ messages in thread
From: Minwoo Im @ 2021-01-26 20:44 UTC (permalink / raw)
To: Keith Busch; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
On 21-01-26 09:57:23, Keith Busch wrote:
> On Tue, Jan 26, 2021 at 09:52:48AM +0900, Minwoo Im wrote:
> > On 21-01-25 10:11:43, Keith Busch wrote:
> > > On Mon, Jan 25, 2021 at 07:03:32PM +0100, Klaus Jensen wrote:
> > > > On Jan 24 11:54, Minwoo Im wrote:
> > > > > We have nvme-subsys and nvme devices mapped together. To support
> > > > > multi-controller scheme to this setup, controller identifier(id) has to
> > > > > be managed. Earlier, cntlid(controller id) used to be always 0 because
> > > > > we didn't have any subsystem scheme that controller id matters.
> > > > >
> > > > > This patch introduced 'cntlid' attribute to the nvme controller
> > > > > instance(NvmeCtrl) and make it allocated by the nvme-subsys device
> > > > > mapped to the controller. If nvme-subsys is not given to the
> > > > > controller, then it will always be 0 as it was.
> > > > >
> > > > > Added 'ctrls' array in the nvme-subsys instance to manage attached
> > > > > controllers to the subsystem with a limit(32). This patch didn't take
> > > > > list for the controllers to make it seamless with nvme-ns device.
> > > > >
> > > > > Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
> > > > > ---
> > > > > hw/block/nvme-subsys.c | 21 +++++++++++++++++++++
> > > > > hw/block/nvme-subsys.h | 4 ++++
> > > > > hw/block/nvme.c | 29 +++++++++++++++++++++++++++++
> > > > > hw/block/nvme.h | 1 +
> > > > > 4 files changed, 55 insertions(+)
> > > > >
> > > > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > > > > index b525fca14103..7138389be4bd 100644
> > > > > --- a/hw/block/nvme.c
> > > > > +++ b/hw/block/nvme.c
> > > > > @@ -4481,6 +4484,10 @@ 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);
> > > > >
> > > > > + if (n->subsys) {
> > > > > + id->cmic |= NVME_CMIC_MULTI_CTRL;
> > > > > + }
> > > >
> > > > Since multiple controllers show up with a PCIe port of their own, do we
> > > > need to set bit 0 (NVME_CMIC_MULTI_PORT?) as well? Or am I
> > > > misunderstanding that bit?
> > >
> > > AIUI, if you report this MULTI_PORT bit, then each PCI device in the
> > > subsystem needs to report a different "Port Number" in their PCIe Link
> > > Capabilities register. I don't think we can manipulate that value from
> > > the nvme "device", but I also don't know what a host should do with this
> > > information even if we could. So, I think it's safe to leave it at 0.
> >
> > AFAIK, If we leave it to 0, kernel will not allocate disk for multi-path
> > case (e.g., nvmeXcYnZ).
>
> Kernel only checks for MULTI_CTRL. It doesn't do anything with MULTI_PORT.
Please forgive me that I took this discussion as MULTI_CTRL rather than
MULTI_PORT. Please ignore this noise ;)
Thanks!
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V6 5/6] hw/block/nvme: add NMIC enum value for Identify Namespace
2021-01-24 2:54 [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
` (3 preceding siblings ...)
2021-01-24 2:54 ` [PATCH V6 4/6] hw/block/nvme: support for multi-controller in subsystem Minwoo Im
@ 2021-01-24 2:54 ` Minwoo Im
2021-01-24 2:54 ` [PATCH V6 6/6] hw/block/nvme: support for shared namespace in subsystem Minwoo Im
` (3 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Minwoo Im @ 2021-01-24 2:54 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Keith Busch, Klaus Jensen, Minwoo Im, Kevin Wolf, Max Reitz
Added Namespace Multi-path I/O and Namespace Sharing Capabilities (NMIC)
field to support shared namespace from controller(s).
This field is in Identify Namespace data structure in [30].
Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
include/block/nvme.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/block/nvme.h b/include/block/nvme.h
index d6415a869c1c..ad68cdc2b92d 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -1203,6 +1203,10 @@ enum NvmeNsIdentifierType {
NVME_NIDT_CSI = 0x04,
};
+enum NvmeIdNsNmic {
+ NVME_NMIC_NS_SHARED = 1 << 0,
+};
+
enum NvmeCsi {
NVME_CSI_NVM = 0x00,
NVME_CSI_ZONED = 0x02,
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH V6 6/6] hw/block/nvme: support for shared namespace in subsystem
2021-01-24 2:54 [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
` (4 preceding siblings ...)
2021-01-24 2:54 ` [PATCH V6 5/6] hw/block/nvme: add NMIC enum value for Identify Namespace Minwoo Im
@ 2021-01-24 2:54 ` Minwoo Im
2021-01-25 19:53 ` Klaus Jensen
2021-01-25 20:29 ` [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns Klaus Jensen
` (2 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Minwoo Im @ 2021-01-24 2:54 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Keith Busch, Klaus Jensen, Minwoo Im, Kevin Wolf, Max Reitz
nvme-ns device is registered to a nvme controller device during the
initialization in nvme_register_namespace() in case that 'bus' property
is given which means it's mapped to a single controller.
This patch introduced a new property 'subsys' just like the controller
device instance did to map a namespace to a NVMe subsystem.
If 'subsys' property is given to the nvme-ns device, it will belong to
the specified subsystem and will be attached to all controllers in that
subsystem by enabling shared namespace capability in NMIC(Namespace
Multi-path I/O and Namespace Capabilities) in Identify Namespace.
Usage:
-device nvme-subsys,id=subsys0
-device nvme,serial=foo,id=nvme0,subsys=subsys0
-device nvme,serial=bar,id=nvme1,subsys=subsys0
-device nvme,serial=baz,id=nvme2,subsys=subsys0
-device nvme-ns,id=ns1,drive=<drv>,nsid=1,subsys=subsys0 # Shared
-device nvme-ns,id=ns2,drive=<drv>,nsid=2,bus=nvme2 # Non-shared
In the above example, 'ns1' will be shared to 'nvme0' and 'nvme1' in
the same subsystem. On the other hand, 'ns2' will be attached to the
'nvme2' only as a private namespace in that subsystem.
All the namespace with 'subsys' parameter will attach all controllers in
the subsystem to the namespace by default.
Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
hw/block/nvme-ns.c | 23 ++++++++++++++++++-----
hw/block/nvme-ns.h | 7 +++++++
hw/block/nvme-subsys.c | 25 +++++++++++++++++++++++++
hw/block/nvme-subsys.h | 3 +++
hw/block/nvme.c | 10 +++++++++-
5 files changed, 62 insertions(+), 6 deletions(-)
diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
index 62b25cf69bfa..9b493f2ead03 100644
--- a/hw/block/nvme-ns.c
+++ b/hw/block/nvme-ns.c
@@ -63,6 +63,10 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
id_ns->npda = id_ns->npdg = npdg - 1;
+ if (nvme_ns_shared(ns)) {
+ id_ns->nmic |= NVME_NMIC_NS_SHARED;
+ }
+
return 0;
}
@@ -365,16 +369,25 @@ static void nvme_ns_realize(DeviceState *dev, Error **errp)
return;
}
- if (nvme_register_namespace(n, ns, errp)) {
- error_propagate_prepend(errp, local_err,
- "could not register namespace: ");
- return;
+ if (ns->subsys) {
+ if (nvme_subsys_register_ns(ns, errp)) {
+ error_propagate_prepend(errp, local_err,
+ "could not setup namespace to subsys: ");
+ return;
+ }
+ } else {
+ if (nvme_register_namespace(n, ns, errp)) {
+ error_propagate_prepend(errp, local_err,
+ "could not register namespace: ");
+ return;
+ }
}
-
}
static Property nvme_ns_props[] = {
DEFINE_BLOCK_PROPERTIES(NvmeNamespace, blkconf),
+ DEFINE_PROP_LINK("subsys", NvmeNamespace, subsys, TYPE_NVME_SUBSYS,
+ NvmeSubsystem *),
DEFINE_PROP_UINT32("nsid", NvmeNamespace, params.nsid, 0),
DEFINE_PROP_UUID("uuid", NvmeNamespace, params.uuid),
DEFINE_PROP_BOOL("zoned", NvmeNamespace, params.zoned, false),
diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h
index 293ac990e3f6..929e78861903 100644
--- a/hw/block/nvme-ns.h
+++ b/hw/block/nvme-ns.h
@@ -47,6 +47,8 @@ typedef struct NvmeNamespace {
const uint32_t *iocs;
uint8_t csi;
+ NvmeSubsystem *subsys;
+
NvmeIdNsZoned *id_ns_zoned;
NvmeZone *zone_array;
QTAILQ_HEAD(, NvmeZone) exp_open_zones;
@@ -77,6 +79,11 @@ static inline uint32_t nvme_nsid(NvmeNamespace *ns)
return -1;
}
+static inline bool nvme_ns_shared(NvmeNamespace *ns)
+{
+ return !!ns->subsys;
+}
+
static inline NvmeLBAF *nvme_ns_lbaf(NvmeNamespace *ns)
{
NvmeIdNs *id_ns = &ns->id_ns;
diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
index e9d61c993c90..641de33e99fc 100644
--- a/hw/block/nvme-subsys.c
+++ b/hw/block/nvme-subsys.c
@@ -43,6 +43,31 @@ int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp)
return cntlid;
}
+int nvme_subsys_register_ns(NvmeNamespace *ns, Error **errp)
+{
+ NvmeSubsystem *subsys = ns->subsys;
+ NvmeCtrl *n;
+ int i;
+
+ if (subsys->namespaces[nvme_nsid(ns)]) {
+ error_setg(errp, "namespace %d already registerd to subsy %s",
+ nvme_nsid(ns), subsys->parent_obj.id);
+ return -1;
+ }
+
+ subsys->namespaces[nvme_nsid(ns)] = ns;
+
+ for (i = 0; i < ARRAY_SIZE(subsys->ctrls); i++) {
+ n = subsys->ctrls[i];
+
+ if (n && nvme_register_namespace(n, ns, errp)) {
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
static void nvme_subsys_setup(NvmeSubsystem *subsys)
{
snprintf((char *)subsys->subnqn, sizeof(subsys->subnqn),
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
index 4eba50d96a1d..ccf6a71398d3 100644
--- a/hw/block/nvme-subsys.h
+++ b/hw/block/nvme-subsys.h
@@ -14,6 +14,7 @@
OBJECT_CHECK(NvmeSubsystem, (obj), TYPE_NVME_SUBSYS)
#define NVME_SUBSYS_MAX_CTRLS 32
+#define NVME_SUBSYS_MAX_NAMESPACES 32
typedef struct NvmeCtrl NvmeCtrl;
typedef struct NvmeNamespace NvmeNamespace;
@@ -22,8 +23,10 @@ typedef struct NvmeSubsystem {
uint8_t subnqn[256];
NvmeCtrl *ctrls[NVME_SUBSYS_MAX_CTRLS];
+ NvmeNamespace *namespaces[NVME_SUBSYS_MAX_NAMESPACES];
} NvmeSubsystem;
int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp);
+int nvme_subsys_register_ns(NvmeNamespace *ns, Error **errp);
#endif /* NVME_SUBSYS_H */
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 7138389be4bd..8259dbf48ec0 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -25,7 +25,8 @@
* mdts=<N[optional]>,zoned.append_size_limit=<N[optional]>, \
* subsys=<subsys_id> \
* -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\
- * zoned=<true|false[optional]>
+ * zoned=<true|false[optional]>, \
+ * subsys=<subsys_id>
* -device nvme-subsys,id=<subsys_id>
*
* Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
@@ -70,6 +71,13 @@
* data size being in effect. By setting this property to 0, users can make
* ZASL to be equal to MDTS. This property only affects zoned namespaces.
*
+ * nvme namespace device parameters
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * - `subsys`
+ * NVM Subsystem device. If given, this namespace will be attached to all
+ * controllers in the subsystem. Otherwise, `bus` must be given to attach
+ * this namespace to a specified single controller as a non-shared namespace.
+ *
* Setting `zoned` to true selects Zoned Command Set at the namespace.
* In this case, the following namespace properties are available to configure
* zoned operation:
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V6 6/6] hw/block/nvme: support for shared namespace in subsystem
2021-01-24 2:54 ` [PATCH V6 6/6] hw/block/nvme: support for shared namespace in subsystem Minwoo Im
@ 2021-01-25 19:53 ` Klaus Jensen
0 siblings, 0 replies; 20+ messages in thread
From: Klaus Jensen @ 2021-01-25 19:53 UTC (permalink / raw)
To: Minwoo Im; +Cc: Keith Busch, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 7476 bytes --]
On Jan 24 11:54, Minwoo Im wrote:
> nvme-ns device is registered to a nvme controller device during the
> initialization in nvme_register_namespace() in case that 'bus' property
> is given which means it's mapped to a single controller.
>
> This patch introduced a new property 'subsys' just like the controller
> device instance did to map a namespace to a NVMe subsystem.
>
> If 'subsys' property is given to the nvme-ns device, it will belong to
> the specified subsystem and will be attached to all controllers in that
> subsystem by enabling shared namespace capability in NMIC(Namespace
> Multi-path I/O and Namespace Capabilities) in Identify Namespace.
>
> Usage:
>
> -device nvme-subsys,id=subsys0
> -device nvme,serial=foo,id=nvme0,subsys=subsys0
> -device nvme,serial=bar,id=nvme1,subsys=subsys0
> -device nvme,serial=baz,id=nvme2,subsys=subsys0
> -device nvme-ns,id=ns1,drive=<drv>,nsid=1,subsys=subsys0 # Shared
> -device nvme-ns,id=ns2,drive=<drv>,nsid=2,bus=nvme2 # Non-shared
>
> In the above example, 'ns1' will be shared to 'nvme0' and 'nvme1' in
> the same subsystem. On the other hand, 'ns2' will be attached to the
> 'nvme2' only as a private namespace in that subsystem.
>
> All the namespace with 'subsys' parameter will attach all controllers in
> the subsystem to the namespace by default.
>
> Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
> ---
> hw/block/nvme-ns.c | 23 ++++++++++++++++++-----
> hw/block/nvme-ns.h | 7 +++++++
> hw/block/nvme-subsys.c | 25 +++++++++++++++++++++++++
> hw/block/nvme-subsys.h | 3 +++
> hw/block/nvme.c | 10 +++++++++-
> 5 files changed, 62 insertions(+), 6 deletions(-)
>
> diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
> index 62b25cf69bfa..9b493f2ead03 100644
> --- a/hw/block/nvme-ns.c
> +++ b/hw/block/nvme-ns.c
> @@ -63,6 +63,10 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
>
> id_ns->npda = id_ns->npdg = npdg - 1;
>
> + if (nvme_ns_shared(ns)) {
> + id_ns->nmic |= NVME_NMIC_NS_SHARED;
> + }
> +
> return 0;
> }
>
> @@ -365,16 +369,25 @@ static void nvme_ns_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> - if (nvme_register_namespace(n, ns, errp)) {
> - error_propagate_prepend(errp, local_err,
> - "could not register namespace: ");
> - return;
> + if (ns->subsys) {
> + if (nvme_subsys_register_ns(ns, errp)) {
> + error_propagate_prepend(errp, local_err,
> + "could not setup namespace to subsys: ");
> + return;
> + }
> + } else {
> + if (nvme_register_namespace(n, ns, errp)) {
> + error_propagate_prepend(errp, local_err,
> + "could not register namespace: ");
> + return;
> + }
> }
> -
> }
>
> static Property nvme_ns_props[] = {
> DEFINE_BLOCK_PROPERTIES(NvmeNamespace, blkconf),
> + DEFINE_PROP_LINK("subsys", NvmeNamespace, subsys, TYPE_NVME_SUBSYS,
> + NvmeSubsystem *),
> DEFINE_PROP_UINT32("nsid", NvmeNamespace, params.nsid, 0),
> DEFINE_PROP_UUID("uuid", NvmeNamespace, params.uuid),
> DEFINE_PROP_BOOL("zoned", NvmeNamespace, params.zoned, false),
> diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h
> index 293ac990e3f6..929e78861903 100644
> --- a/hw/block/nvme-ns.h
> +++ b/hw/block/nvme-ns.h
> @@ -47,6 +47,8 @@ typedef struct NvmeNamespace {
> const uint32_t *iocs;
> uint8_t csi;
>
> + NvmeSubsystem *subsys;
> +
> NvmeIdNsZoned *id_ns_zoned;
> NvmeZone *zone_array;
> QTAILQ_HEAD(, NvmeZone) exp_open_zones;
> @@ -77,6 +79,11 @@ static inline uint32_t nvme_nsid(NvmeNamespace *ns)
> return -1;
> }
>
> +static inline bool nvme_ns_shared(NvmeNamespace *ns)
> +{
> + return !!ns->subsys;
> +}
> +
> static inline NvmeLBAF *nvme_ns_lbaf(NvmeNamespace *ns)
> {
> NvmeIdNs *id_ns = &ns->id_ns;
> diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
> index e9d61c993c90..641de33e99fc 100644
> --- a/hw/block/nvme-subsys.c
> +++ b/hw/block/nvme-subsys.c
> @@ -43,6 +43,31 @@ int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp)
> return cntlid;
> }
>
> +int nvme_subsys_register_ns(NvmeNamespace *ns, Error **errp)
> +{
> + NvmeSubsystem *subsys = ns->subsys;
> + NvmeCtrl *n;
> + int i;
> +
> + if (subsys->namespaces[nvme_nsid(ns)]) {
> + error_setg(errp, "namespace %d already registerd to subsy %s",
Small typo.
"registerEd to subsySTEM"
> + nvme_nsid(ns), subsys->parent_obj.id);
> + return -1;
> + }
> +
> + subsys->namespaces[nvme_nsid(ns)] = ns;
> +
> + for (i = 0; i < ARRAY_SIZE(subsys->ctrls); i++) {
> + n = subsys->ctrls[i];
> +
> + if (n && nvme_register_namespace(n, ns, errp)) {
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +
> static void nvme_subsys_setup(NvmeSubsystem *subsys)
> {
> snprintf((char *)subsys->subnqn, sizeof(subsys->subnqn),
> diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
> index 4eba50d96a1d..ccf6a71398d3 100644
> --- a/hw/block/nvme-subsys.h
> +++ b/hw/block/nvme-subsys.h
> @@ -14,6 +14,7 @@
> OBJECT_CHECK(NvmeSubsystem, (obj), TYPE_NVME_SUBSYS)
>
> #define NVME_SUBSYS_MAX_CTRLS 32
> +#define NVME_SUBSYS_MAX_NAMESPACES 32
>
> typedef struct NvmeCtrl NvmeCtrl;
> typedef struct NvmeNamespace NvmeNamespace;
> @@ -22,8 +23,10 @@ typedef struct NvmeSubsystem {
> uint8_t subnqn[256];
>
> NvmeCtrl *ctrls[NVME_SUBSYS_MAX_CTRLS];
> + NvmeNamespace *namespaces[NVME_SUBSYS_MAX_NAMESPACES];
> } NvmeSubsystem;
>
> int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp);
> +int nvme_subsys_register_ns(NvmeNamespace *ns, Error **errp);
>
> #endif /* NVME_SUBSYS_H */
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 7138389be4bd..8259dbf48ec0 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -25,7 +25,8 @@
> * mdts=<N[optional]>,zoned.append_size_limit=<N[optional]>, \
> * subsys=<subsys_id> \
> * -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\
> - * zoned=<true|false[optional]>
> + * zoned=<true|false[optional]>, \
> + * subsys=<subsys_id>
> * -device nvme-subsys,id=<subsys_id>
> *
> * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
> @@ -70,6 +71,13 @@
> * data size being in effect. By setting this property to 0, users can make
> * ZASL to be equal to MDTS. This property only affects zoned namespaces.
> *
> + * nvme namespace device parameters
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + * - `subsys`
> + * NVM Subsystem device. If given, this namespace will be attached to all
> + * controllers in the subsystem. Otherwise, `bus` must be given to attach
> + * this namespace to a specified single controller as a non-shared namespace.
> + *
> * Setting `zoned` to true selects Zoned Command Set at the namespace.
> * In this case, the following namespace properties are available to configure
> * zoned operation:
> --
> 2.17.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns
2021-01-24 2:54 [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
` (5 preceding siblings ...)
2021-01-24 2:54 ` [PATCH V6 6/6] hw/block/nvme: support for shared namespace in subsystem Minwoo Im
@ 2021-01-25 20:29 ` Klaus Jensen
2021-01-26 0:53 ` Minwoo Im
2021-01-26 20:39 ` Keith Busch
2021-02-04 18:31 ` Klaus Jensen
8 siblings, 1 reply; 20+ messages in thread
From: Klaus Jensen @ 2021-01-25 20:29 UTC (permalink / raw)
To: Minwoo Im; +Cc: Keith Busch, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 5774 bytes --]
On Jan 24 11:54, Minwoo Im wrote:
> Hello,
>
> This is sixth patch series for the support of NVMe subsystem scheme with
> multi-controller and namespace sharing in a subsystem.
>
> This version has a fix in nvme_init_ctrl() when 'cntlid' is set to the
> Identify Controller data structure by making it by cpu_to_le16() as
> Keith reviewed.
>
> Here's test result with a simple 'nvme list -v' command from this model:
>
> -device nvme-subsys,id=subsys0 \
> -device nvme,serial=foo,id=nvme0,subsys=subsys0 \
> -device nvme,serial=bar,id=nvme1,subsys=subsys0 \
> -device nvme,serial=baz,id=nvme2,subsys=subsys0 \
> -device nvme-ns,id=ns1,drive=drv10,nsid=1,subsys=subsys0 \
> -device nvme-ns,id=ns2,drive=drv11,nsid=2,bus=nvme2 \
> \
> -device nvme,serial=qux,id=nvme3 \
> -device nvme-ns,id=ns3,drive=drv12,nsid=3,bus=nvme3 \
> \
> -device nvme-subsys,id=subsys1 \
> -device nvme,serial=quux,id=nvme4,subsys=subsys1 \
> -device nvme-ns,id=ns4,drive=drv13,nsid=1,subsys=subsys1,zoned=true \
>
> root@vm:~/work# nvme list -v
> NVM Express Subsystems
>
> Subsystem Subsystem-NQN Controllers
> ---------------- ------------------------------------------------------------------------------------------------ ----------------
> nvme-subsys1 nqn.2019-08.org.qemu:subsys0 nvme0, nvme1, nvme2
> nvme-subsys3 nqn.2019-08.org.qemu:qux nvme3
> nvme-subsys4 nqn.2019-08.org.qemu:subsys1 nvme4
>
> NVM Express Controllers
>
> Device SN MN FR TxPort Address Subsystem Namespaces
> -------- -------------------- ---------------------------------------- -------- ------ -------------- ------------ ----------------
> nvme0 foo QEMU NVMe Ctrl 1.0 pcie 0000:00:06.0 nvme-subsys1 nvme1c0n1
> nvme1 bar QEMU NVMe Ctrl 1.0 pcie 0000:00:07.0 nvme-subsys1 nvme1c1n1
> nvme2 baz QEMU NVMe Ctrl 1.0 pcie 0000:00:08.0 nvme-subsys1 nvme1c2n1, nvme1c2n2
> nvme3 qux QEMU NVMe Ctrl 1.0 pcie 0000:00:09.0 nvme-subsys3 nvme3n1
> nvme4 quux QEMU NVMe Ctrl 1.0 pcie 0000:00:0a.0 nvme-subsys4 nvme4c4n1
>
> NVM Express Namespaces
>
> Device NSID Usage Format Controllers
> ------------ -------- -------------------------- ---------------- ----------------
> nvme1n1 1 134.22 MB / 134.22 MB 512 B + 0 B nvme0, nvme1, nvme2
> nvme1n2 2 268.44 MB / 268.44 MB 512 B + 0 B nvme2
> nvme3n1 3 268.44 MB / 268.44 MB 512 B + 0 B nvme3
> nvme4n1 1 268.44 MB / 268.44 MB 512 B + 0 B nvme4
>
> Thanks,
>
> Since V5:
> - Fix endianness for 'cntlid' in Identify Controller data structure.
> (Keith)
>
> Since V4:
> - Code clean-up to snprintf rather than duplicating it and copy.
> (Keith)
> - Documentation for 'subsys' clean-up. (Keith)
> - Remove 'cntlid' param from nvme_init_ctrl(). (Keith)
> - Put error_propagate() in nvme_realize(). (Keith)
>
> Since RFC V3:
> - Exclude 'deatched' scheme from this series. This will be covered in
> the next series by covering all the ns-related admin commands
> including ZNS and ns-mgmt. (Niklas)
> - Rebased on nvme-next.
> - Remove RFC tag from this V4.
>
> Since RFC V2:
> - Rebased on nvme-next branch with trivial patches from the previous
> version(V2) applied. (Klaus)
> - Fix enumeration type name convention with NvmeIdNs prefix. (Klaus)
> - Put 'cntlid' to NvmeCtrl instance in nvme_init_ctrl() which was
> missed in V2.
> - Added 'detached' parameter to nvme-ns device to decide whether to
> attach or not to controller(s) in the subsystem. (Klaus)
> - Implemented Identify Active Namespace ID List aprt from Identify
> Allocated Namespace ID List by removing fall-thru statement.
>
> Since RFC V1:
> - Updated namespace sharing scheme to be based on nvme-subsys
> hierarchy.
>
> Minwoo Im (6):
> hw/block/nvme: introduce nvme-subsys device
> hw/block/nvme: support to map controller to a subsystem
> hw/block/nvme: add CMIC enum value for Identify Controller
> hw/block/nvme: support for multi-controller in subsystem
> hw/block/nvme: add NMIC enum value for Identify Namespace
> hw/block/nvme: support for shared namespace in subsystem
>
> hw/block/meson.build | 2 +-
> hw/block/nvme-ns.c | 23 +++++++--
> hw/block/nvme-ns.h | 7 +++
> hw/block/nvme-subsys.c | 106 +++++++++++++++++++++++++++++++++++++++++
> hw/block/nvme-subsys.h | 32 +++++++++++++
> hw/block/nvme.c | 72 +++++++++++++++++++++++++---
> hw/block/nvme.h | 4 ++
> include/block/nvme.h | 8 ++++
> 8 files changed, 242 insertions(+), 12 deletions(-)
> create mode 100644 hw/block/nvme-subsys.c
> create mode 100644 hw/block/nvme-subsys.h
>
Thanks Minwoo! This really is great stuff.
Notwithstanding the nitpicks I've pointed out for [1/6] and [6/6] (no
need to v7 for those), take my
Tested-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns
2021-01-25 20:29 ` [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns Klaus Jensen
@ 2021-01-26 0:53 ` Minwoo Im
0 siblings, 0 replies; 20+ messages in thread
From: Minwoo Im @ 2021-01-26 0:53 UTC (permalink / raw)
To: Klaus Jensen; +Cc: Keith Busch, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
On 21-01-25 21:29:58, Klaus Jensen wrote:
> On Jan 24 11:54, Minwoo Im wrote:
> > Hello,
> >
> > This is sixth patch series for the support of NVMe subsystem scheme with
> > multi-controller and namespace sharing in a subsystem.
> >
> > This version has a fix in nvme_init_ctrl() when 'cntlid' is set to the
> > Identify Controller data structure by making it by cpu_to_le16() as
> > Keith reviewed.
> >
> > Here's test result with a simple 'nvme list -v' command from this model:
> >
> > -device nvme-subsys,id=subsys0 \
> > -device nvme,serial=foo,id=nvme0,subsys=subsys0 \
> > -device nvme,serial=bar,id=nvme1,subsys=subsys0 \
> > -device nvme,serial=baz,id=nvme2,subsys=subsys0 \
> > -device nvme-ns,id=ns1,drive=drv10,nsid=1,subsys=subsys0 \
> > -device nvme-ns,id=ns2,drive=drv11,nsid=2,bus=nvme2 \
> > \
> > -device nvme,serial=qux,id=nvme3 \
> > -device nvme-ns,id=ns3,drive=drv12,nsid=3,bus=nvme3 \
> > \
> > -device nvme-subsys,id=subsys1 \
> > -device nvme,serial=quux,id=nvme4,subsys=subsys1 \
> > -device nvme-ns,id=ns4,drive=drv13,nsid=1,subsys=subsys1,zoned=true \
> >
> > root@vm:~/work# nvme list -v
> > NVM Express Subsystems
> >
> > Subsystem Subsystem-NQN Controllers
> > ---------------- ------------------------------------------------------------------------------------------------ ----------------
> > nvme-subsys1 nqn.2019-08.org.qemu:subsys0 nvme0, nvme1, nvme2
> > nvme-subsys3 nqn.2019-08.org.qemu:qux nvme3
> > nvme-subsys4 nqn.2019-08.org.qemu:subsys1 nvme4
> >
> > NVM Express Controllers
> >
> > Device SN MN FR TxPort Address Subsystem Namespaces
> > -------- -------------------- ---------------------------------------- -------- ------ -------------- ------------ ----------------
> > nvme0 foo QEMU NVMe Ctrl 1.0 pcie 0000:00:06.0 nvme-subsys1 nvme1c0n1
> > nvme1 bar QEMU NVMe Ctrl 1.0 pcie 0000:00:07.0 nvme-subsys1 nvme1c1n1
> > nvme2 baz QEMU NVMe Ctrl 1.0 pcie 0000:00:08.0 nvme-subsys1 nvme1c2n1, nvme1c2n2
> > nvme3 qux QEMU NVMe Ctrl 1.0 pcie 0000:00:09.0 nvme-subsys3 nvme3n1
> > nvme4 quux QEMU NVMe Ctrl 1.0 pcie 0000:00:0a.0 nvme-subsys4 nvme4c4n1
> >
> > NVM Express Namespaces
> >
> > Device NSID Usage Format Controllers
> > ------------ -------- -------------------------- ---------------- ----------------
> > nvme1n1 1 134.22 MB / 134.22 MB 512 B + 0 B nvme0, nvme1, nvme2
> > nvme1n2 2 268.44 MB / 268.44 MB 512 B + 0 B nvme2
> > nvme3n1 3 268.44 MB / 268.44 MB 512 B + 0 B nvme3
> > nvme4n1 1 268.44 MB / 268.44 MB 512 B + 0 B nvme4
> >
> > Thanks,
> >
> > Since V5:
> > - Fix endianness for 'cntlid' in Identify Controller data structure.
> > (Keith)
> >
> > Since V4:
> > - Code clean-up to snprintf rather than duplicating it and copy.
> > (Keith)
> > - Documentation for 'subsys' clean-up. (Keith)
> > - Remove 'cntlid' param from nvme_init_ctrl(). (Keith)
> > - Put error_propagate() in nvme_realize(). (Keith)
> >
> > Since RFC V3:
> > - Exclude 'deatched' scheme from this series. This will be covered in
> > the next series by covering all the ns-related admin commands
> > including ZNS and ns-mgmt. (Niklas)
> > - Rebased on nvme-next.
> > - Remove RFC tag from this V4.
> >
> > Since RFC V2:
> > - Rebased on nvme-next branch with trivial patches from the previous
> > version(V2) applied. (Klaus)
> > - Fix enumeration type name convention with NvmeIdNs prefix. (Klaus)
> > - Put 'cntlid' to NvmeCtrl instance in nvme_init_ctrl() which was
> > missed in V2.
> > - Added 'detached' parameter to nvme-ns device to decide whether to
> > attach or not to controller(s) in the subsystem. (Klaus)
> > - Implemented Identify Active Namespace ID List aprt from Identify
> > Allocated Namespace ID List by removing fall-thru statement.
> >
> > Since RFC V1:
> > - Updated namespace sharing scheme to be based on nvme-subsys
> > hierarchy.
> >
> > Minwoo Im (6):
> > hw/block/nvme: introduce nvme-subsys device
> > hw/block/nvme: support to map controller to a subsystem
> > hw/block/nvme: add CMIC enum value for Identify Controller
> > hw/block/nvme: support for multi-controller in subsystem
> > hw/block/nvme: add NMIC enum value for Identify Namespace
> > hw/block/nvme: support for shared namespace in subsystem
> >
> > hw/block/meson.build | 2 +-
> > hw/block/nvme-ns.c | 23 +++++++--
> > hw/block/nvme-ns.h | 7 +++
> > hw/block/nvme-subsys.c | 106 +++++++++++++++++++++++++++++++++++++++++
> > hw/block/nvme-subsys.h | 32 +++++++++++++
> > hw/block/nvme.c | 72 +++++++++++++++++++++++++---
> > hw/block/nvme.h | 4 ++
> > include/block/nvme.h | 8 ++++
> > 8 files changed, 242 insertions(+), 12 deletions(-)
> > create mode 100644 hw/block/nvme-subsys.c
> > create mode 100644 hw/block/nvme-subsys.h
> >
>
> Thanks Minwoo! This really is great stuff.
>
> Notwithstanding the nitpicks I've pointed out for [1/6] and [6/6] (no
> need to v7 for those), take my
>
> Tested-by: Klaus Jensen <k.jensen@samsung.com>
> Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Thanks Klaus!
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns
2021-01-24 2:54 [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
` (6 preceding siblings ...)
2021-01-25 20:29 ` [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns Klaus Jensen
@ 2021-01-26 20:39 ` Keith Busch
2021-01-26 21:41 ` Minwoo Im
2021-02-04 18:31 ` Klaus Jensen
8 siblings, 1 reply; 20+ messages in thread
From: Keith Busch @ 2021-01-26 20:39 UTC (permalink / raw)
To: Minwoo Im; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
This came out looking cleaner than I had initially expected. Thanks for
seeing this feature through!
Reviewed-by: Keith Busch <kbusch@kernel.org>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns
2021-01-26 20:39 ` Keith Busch
@ 2021-01-26 21:41 ` Minwoo Im
0 siblings, 0 replies; 20+ messages in thread
From: Minwoo Im @ 2021-01-26 21:41 UTC (permalink / raw)
To: Keith Busch; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
On 21-01-27 05:39:29, Keith Busch wrote:
> This came out looking cleaner than I had initially expected. Thanks for
> seeing this feature through!
>
> Reviewed-by: Keith Busch <kbusch@kernel.org>
Thanks Keith for the review!
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns
2021-01-24 2:54 [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
` (7 preceding siblings ...)
2021-01-26 20:39 ` Keith Busch
@ 2021-02-04 18:31 ` Klaus Jensen
2021-02-04 18:32 ` Klaus Jensen
8 siblings, 1 reply; 20+ messages in thread
From: Klaus Jensen @ 2021-02-04 18:31 UTC (permalink / raw)
To: Minwoo Im; +Cc: Keith Busch, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]
On Jan 24 11:54, Minwoo Im wrote:
>
> Minwoo Im (6):
> hw/block/nvme: introduce nvme-subsys device
> hw/block/nvme: support to map controller to a subsystem
> hw/block/nvme: add CMIC enum value for Identify Controller
> hw/block/nvme: support for multi-controller in subsystem
> hw/block/nvme: add NMIC enum value for Identify Namespace
> hw/block/nvme: support for shared namespace in subsystem
>
> hw/block/meson.build | 2 +-
> hw/block/nvme-ns.c | 23 +++++++--
> hw/block/nvme-ns.h | 7 +++
> hw/block/nvme-subsys.c | 106 +++++++++++++++++++++++++++++++++++++++++
> hw/block/nvme-subsys.h | 32 +++++++++++++
> hw/block/nvme.c | 72 +++++++++++++++++++++++++---
> hw/block/nvme.h | 4 ++
> include/block/nvme.h | 8 ++++
> 8 files changed, 242 insertions(+), 12 deletions(-)
> create mode 100644 hw/block/nvme-subsys.c
> create mode 100644 hw/block/nvme-subsys.h
>
Thanks Minwoo, this was a super awesome series.
Applied to nvme-next!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns
2021-02-04 18:31 ` Klaus Jensen
@ 2021-02-04 18:32 ` Klaus Jensen
0 siblings, 0 replies; 20+ messages in thread
From: Klaus Jensen @ 2021-02-04 18:32 UTC (permalink / raw)
To: Minwoo Im; +Cc: Keith Busch, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]
On Feb 4 19:31, Klaus Jensen wrote:
> On Jan 24 11:54, Minwoo Im wrote:
> >
> > Minwoo Im (6):
> > hw/block/nvme: introduce nvme-subsys device
> > hw/block/nvme: support to map controller to a subsystem
> > hw/block/nvme: add CMIC enum value for Identify Controller
> > hw/block/nvme: support for multi-controller in subsystem
> > hw/block/nvme: add NMIC enum value for Identify Namespace
> > hw/block/nvme: support for shared namespace in subsystem
> >
> > hw/block/meson.build | 2 +-
> > hw/block/nvme-ns.c | 23 +++++++--
> > hw/block/nvme-ns.h | 7 +++
> > hw/block/nvme-subsys.c | 106 +++++++++++++++++++++++++++++++++++++++++
> > hw/block/nvme-subsys.h | 32 +++++++++++++
> > hw/block/nvme.c | 72 +++++++++++++++++++++++++---
> > hw/block/nvme.h | 4 ++
> > include/block/nvme.h | 8 ++++
> > 8 files changed, 242 insertions(+), 12 deletions(-)
> > create mode 100644 hw/block/nvme-subsys.c
> > create mode 100644 hw/block/nvme-subsys.h
> >
>
> Thanks Minwoo, this was a super awesome series.
>
> Applied to nvme-next!
By the way, we are really cluttering up hw/block now, so I'm gonna
submit a patch based on the one that Philippe did to move the device
into hw/nvme.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread