* [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device
2021-01-21 22:09 [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
@ 2021-01-21 22:09 ` Minwoo Im
2021-01-21 22:52 ` Keith Busch
2021-01-21 22:09 ` [PATCH V4 2/6] hw/block/nvme: support to map controller to a subsystem Minwoo Im
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 22:09 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 | 63 ++++++++++++++++++++++++++++++++++++++++++
hw/block/nvme-subsys.h | 25 +++++++++++++++++
hw/block/nvme.c | 3 ++
4 files changed, 92 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..f1dc71d588d9
--- /dev/null
+++ b/hw/block/nvme-subsys.c
@@ -0,0 +1,63 @@
+/*
+ * 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)
+{
+ char *subnqn;
+
+ subnqn = g_strdup_printf("nqn.2019-08.org.qemu:%s", subsys->parent_obj.id);
+ strpadcpy((char *)subsys->subnqn, sizeof(subsys->subnqn), subnqn, '\0');
+ g_free(subnqn);
+}
+
+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] 13+ messages in thread* Re: [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device
2021-01-21 22:09 ` [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device Minwoo Im
@ 2021-01-21 22:52 ` Keith Busch
2021-01-21 23:40 ` Minwoo Im
0 siblings, 1 reply; 13+ messages in thread
From: Keith Busch @ 2021-01-21 22:52 UTC (permalink / raw)
To: Minwoo Im; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
On Fri, Jan 22, 2021 at 07:09:03AM +0900, Minwoo Im wrote:
> +static void nvme_subsys_setup(NvmeSubsystem *subsys)
> +{
> + char *subnqn;
> +
> + subnqn = g_strdup_printf("nqn.2019-08.org.qemu:%s", subsys->parent_obj.id);
> + strpadcpy((char *)subsys->subnqn, sizeof(subsys->subnqn), subnqn, '\0');
> + g_free(subnqn);
Instead of the duplication and copy, you could format the string
directly into the destination:
snprintf(subsys->subnqn, sizeof(subsys->subnqn), "nqn.2019-08.org.qemu:%s",
subsys->parent_obj.id);
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device
2021-01-21 22:52 ` Keith Busch
@ 2021-01-21 23:40 ` Minwoo Im
0 siblings, 0 replies; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 23:40 UTC (permalink / raw)
To: Keith Busch; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
On 21-01-21 14:52:02, Keith Busch wrote:
> On Fri, Jan 22, 2021 at 07:09:03AM +0900, Minwoo Im wrote:
> > +static void nvme_subsys_setup(NvmeSubsystem *subsys)
> > +{
> > + char *subnqn;
> > +
> > + subnqn = g_strdup_printf("nqn.2019-08.org.qemu:%s", subsys->parent_obj.id);
> > + strpadcpy((char *)subsys->subnqn, sizeof(subsys->subnqn), subnqn, '\0');
> > + g_free(subnqn);
>
> Instead of the duplication and copy, you could format the string
> directly into the destination:
>
> snprintf(subsys->subnqn, sizeof(subsys->subnqn), "nqn.2019-08.org.qemu:%s",
> subsys->parent_obj.id);
Oh, Thanks. Will fix it!
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V4 2/6] hw/block/nvme: support to map controller to a subsystem
2021-01-21 22:09 [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
2021-01-21 22:09 ` [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device Minwoo Im
@ 2021-01-21 22:09 ` Minwoo Im
2021-01-21 23:03 ` Keith Busch
2021-01-21 22:09 ` [PATCH V4 3/6] hw/block/nvme: add CMIC enum value for Identify Controller Minwoo Im
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 22:09 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, 29 insertions(+), 4 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index aabccdf36f4b..ab0531492ddd 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -23,6 +23,7 @@
* max_ioqpairs=<N[optional]>, \
* aerl=<N[optional]>, aer_max_queued=<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,25 @@ 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;
+ char *subnqn;
+
+ if (!subsys) {
+ 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);
+ } 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 +4477,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 +4565,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] 13+ messages in thread* Re: [PATCH V4 2/6] hw/block/nvme: support to map controller to a subsystem
2021-01-21 22:09 ` [PATCH V4 2/6] hw/block/nvme: support to map controller to a subsystem Minwoo Im
@ 2021-01-21 23:03 ` Keith Busch
2021-01-21 23:41 ` Minwoo Im
0 siblings, 1 reply; 13+ messages in thread
From: Keith Busch @ 2021-01-21 23:03 UTC (permalink / raw)
To: Minwoo Im; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
On Fri, Jan 22, 2021 at 07:09:04AM +0900, Minwoo Im wrote:
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -23,6 +23,7 @@
> * max_ioqpairs=<N[optional]>, \
> * aerl=<N[optional]>, aer_max_queued=<N[optional]>, \
> * mdts=<N[optional]>,zoned.append_size_limit=<N[optional]> \
> + * ,subsys=<subsys_id> \
For consistency, the ',' goes in the preceeding line.
> * -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\
> * zoned=<true|false[optional]>
> * -device nvme-subsys,id=<subsys_id>
> @@ -4404,11 +4412,25 @@ 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;
> + char *subnqn;
> +
> + if (!subsys) {
> + 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);
snprintf(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);
> + }
> +}
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH V4 2/6] hw/block/nvme: support to map controller to a subsystem
2021-01-21 23:03 ` Keith Busch
@ 2021-01-21 23:41 ` Minwoo Im
0 siblings, 0 replies; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 23:41 UTC (permalink / raw)
To: Keith Busch; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
On 21-01-21 15:03:38, Keith Busch wrote:
> On Fri, Jan 22, 2021 at 07:09:04AM +0900, Minwoo Im wrote:
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -23,6 +23,7 @@
> > * max_ioqpairs=<N[optional]>, \
> > * aerl=<N[optional]>, aer_max_queued=<N[optional]>, \
> > * mdts=<N[optional]>,zoned.append_size_limit=<N[optional]> \
> > + * ,subsys=<subsys_id> \
>
> For consistency, the ',' goes in the preceeding line.
I have no idea what happened here :(. Will fix it. Thanks!
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V4 3/6] hw/block/nvme: add CMIC enum value for Identify Controller
2021-01-21 22:09 [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
2021-01-21 22:09 ` [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device Minwoo Im
2021-01-21 22:09 ` [PATCH V4 2/6] hw/block/nvme: support to map controller to a subsystem Minwoo Im
@ 2021-01-21 22:09 ` Minwoo Im
2021-01-21 22:09 ` [PATCH V4 4/6] hw/block/nvme: support for multi-controller in subsystem Minwoo Im
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 22:09 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] 13+ messages in thread* [PATCH V4 4/6] hw/block/nvme: support for multi-controller in subsystem
2021-01-21 22:09 [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
` (2 preceding siblings ...)
2021-01-21 22:09 ` [PATCH V4 3/6] hw/block/nvme: add CMIC enum value for Identify Controller Minwoo Im
@ 2021-01-21 22:09 ` Minwoo Im
2021-01-21 23:17 ` Keith Busch
2021-01-21 22:09 ` [PATCH V4 5/6] hw/block/nvme: add NMIC enum value for Identify Namespace Minwoo Im
2021-01-21 22:09 ` [PATCH V4 6/6] hw/block/nvme: support for shared namespace in subsystem Minwoo Im
5 siblings, 1 reply; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 22:09 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 | 34 ++++++++++++++++++++++++++++++++--
hw/block/nvme.h | 1 +
4 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
index f1dc71d588d9..a01003136b12 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)
{
char *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 ab0531492ddd..225f0d3f3a27 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -4427,16 +4427,21 @@ static void nvme_init_subnqn(NvmeCtrl *n)
}
}
-static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
+static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t cntlid)
{
NvmeIdCtrl *id = &n->id_ctrl;
uint8_t *pci_conf = pci_dev->config;
+ n->cntlid = cntlid;
+
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));
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 = cntlid;
+
id->rab = 6;
id->ieee[0] = 0x00;
id->ieee[1] = 0x02;
@@ -4483,6 +4488,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);
@@ -4497,11 +4506,28 @@ 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;
+ }
+
+ return cntlid;
+}
+
static void nvme_realize(PCIDevice *pci_dev, Error **errp)
{
NvmeCtrl *n = NVME(pci_dev);
NvmeNamespace *ns;
Error *local_err = NULL;
+ int cntlid;
nvme_check_constraints(n, &local_err);
if (local_err) {
@@ -4517,7 +4543,11 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
return;
}
- nvme_init_ctrl(n, pci_dev);
+ cntlid = nvme_init_subsys(n, errp);
+ if (cntlid < 0) {
+ return;
+ }
+ nvme_init_ctrl(n, pci_dev, cntlid);
/* setup a namespace if the controller drive property was given */
if (n->namespace.blkconf.blk) {
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] 13+ messages in thread* Re: [PATCH V4 4/6] hw/block/nvme: support for multi-controller in subsystem
2021-01-21 22:09 ` [PATCH V4 4/6] hw/block/nvme: support for multi-controller in subsystem Minwoo Im
@ 2021-01-21 23:17 ` Keith Busch
2021-01-21 23:41 ` Minwoo Im
0 siblings, 1 reply; 13+ messages in thread
From: Keith Busch @ 2021-01-21 23:17 UTC (permalink / raw)
To: Minwoo Im; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
On Fri, Jan 22, 2021 at 07:09:06AM +0900, Minwoo Im wrote:
> -static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
> +static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t cntlid)
> {
> NvmeIdCtrl *id = &n->id_ctrl;
> uint8_t *pci_conf = pci_dev->config;
>
> + n->cntlid = cntlid;
I don't think 'cntlid' is important enough to be a function parameter.
You can just set it within the 'NvmeCtrl' struct before calling this
function like all the other properties.
> @@ -4517,7 +4543,11 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> return;
> }
>
> - nvme_init_ctrl(n, pci_dev);
> + cntlid = nvme_init_subsys(n, errp);
> + if (cntlid < 0) {
error_propogate();
> + return;
> + }
> + nvme_init_ctrl(n, pci_dev, cntlid);
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH V4 4/6] hw/block/nvme: support for multi-controller in subsystem
2021-01-21 23:17 ` Keith Busch
@ 2021-01-21 23:41 ` Minwoo Im
0 siblings, 0 replies; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 23:41 UTC (permalink / raw)
To: Keith Busch; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz
On 21-01-21 15:17:16, Keith Busch wrote:
> On Fri, Jan 22, 2021 at 07:09:06AM +0900, Minwoo Im wrote:
> > -static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
> > +static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t cntlid)
> > {
> > NvmeIdCtrl *id = &n->id_ctrl;
> > uint8_t *pci_conf = pci_dev->config;
> >
> > + n->cntlid = cntlid;
>
> I don't think 'cntlid' is important enough to be a function parameter.
> You can just set it within the 'NvmeCtrl' struct before calling this
> function like all the other properties.
Okay. Rather than adding a parameter to this function,
nvme_init_subsys() may take this job to assign cntlid to the controller
instance first. Let me fix one!
> > @@ -4517,7 +4543,11 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> > return;
> > }
> >
> > - nvme_init_ctrl(n, pci_dev);
> > + cntlid = nvme_init_subsys(n, errp);
> > + if (cntlid < 0) {
>
> error_propogate();
Thanks for catching this.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V4 5/6] hw/block/nvme: add NMIC enum value for Identify Namespace
2021-01-21 22:09 [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
` (3 preceding siblings ...)
2021-01-21 22:09 ` [PATCH V4 4/6] hw/block/nvme: support for multi-controller in subsystem Minwoo Im
@ 2021-01-21 22:09 ` Minwoo Im
2021-01-21 22:09 ` [PATCH V4 6/6] hw/block/nvme: support for shared namespace in subsystem Minwoo Im
5 siblings, 0 replies; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 22:09 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] 13+ messages in thread* [PATCH V4 6/6] hw/block/nvme: support for shared namespace in subsystem
2021-01-21 22:09 [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
` (4 preceding siblings ...)
2021-01-21 22:09 ` [PATCH V4 5/6] hw/block/nvme: add NMIC enum value for Identify Namespace Minwoo Im
@ 2021-01-21 22:09 ` Minwoo Im
5 siblings, 0 replies; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 22:09 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 a01003136b12..e7efdcae7d0d 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)
{
char *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 225f0d3f3a27..b123514c6262 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] 13+ messages in thread