diff mbox series

[RFC,V3,7/8] hw/block/nvme: add 'detached' param not to attach namespace

Message ID 20210119170147.19657-8-minwoo.im.dev@gmail.com
State New
Headers show
Series hw/block/nvme: support multi-path for ctrl/ns | expand

Commit Message

Minwoo Im Jan. 19, 2021, 5:01 p.m. UTC
Introduced 'detached' parameter to nvme-ns device.  If given, the
namespace will not be attached to controller(s) in the subsystem.  If
'subsys' is not given with this option, it should be provided with 'bus'
which is for private namespace.

This patch also introduced 'ctrls_bitmap' in NvmeNamespace instance to
represent which controler id(cntlid) is attached to this namespace
device.  A namespace can be attached to multiple controllers in a
subsystem so that this bitmap maps those two relationships.

The ctrls_bitmap bitmap should not be accessed directly, but through the
helpers introduced in this patch: nvme_ns_is_attached(),
nvme_ns_attach(), nvme_ns_detach().

Note that this patch made identify namespace list data not hold
non-attached namespace ID in nvme_identify_nslist.  Currently, this
command handler is for CNS 0x2(Active) and 0x10(Allocated) both.  The
next patch will introduce a handler for later on.

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 hw/block/nvme-ns.c     |  9 +++++++++
 hw/block/nvme-ns.h     |  6 ++++++
 hw/block/nvme-subsys.c |  2 ++
 hw/block/nvme.c        | 31 ++++++++++++++++++++++++++++++-
 hw/block/nvme.h        | 15 +++++++++++++++
 5 files changed, 62 insertions(+), 1 deletion(-)

Comments

Klaus Jensen Jan. 19, 2021, 6:25 p.m. UTC | #1
On Jan 20 02:01, Minwoo Im wrote:
> Introduced 'detached' parameter to nvme-ns device.  If given, the
> namespace will not be attached to controller(s) in the subsystem.  If
> 'subsys' is not given with this option, it should be provided with 'bus'
> which is for private namespace.
> 
> This patch also introduced 'ctrls_bitmap' in NvmeNamespace instance to
> represent which controler id(cntlid) is attached to this namespace
> device.  A namespace can be attached to multiple controllers in a
> subsystem so that this bitmap maps those two relationships.
> 
> The ctrls_bitmap bitmap should not be accessed directly, but through the
> helpers introduced in this patch: nvme_ns_is_attached(),
> nvme_ns_attach(), nvme_ns_detach().
> 
> Note that this patch made identify namespace list data not hold
> non-attached namespace ID in nvme_identify_nslist.  Currently, this
> command handler is for CNS 0x2(Active) and 0x10(Allocated) both.  The
> next patch will introduce a handler for later on.
> 
> Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
> ---
>  hw/block/nvme-ns.c     |  9 +++++++++
>  hw/block/nvme-ns.h     |  6 ++++++
>  hw/block/nvme-subsys.c |  2 ++
>  hw/block/nvme.c        | 31 ++++++++++++++++++++++++++++++-
>  hw/block/nvme.h        | 15 +++++++++++++++
>  5 files changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
> index 073f65e49cac..70d42c24065c 100644
> --- a/hw/block/nvme-ns.c
> +++ b/hw/block/nvme-ns.c
> @@ -17,6 +17,7 @@
>  #include "qemu/cutils.h"
>  #include "qemu/log.h"
>  #include "qemu/error-report.h"
> +#include "qemu/hbitmap.h"

Isn't the HBitmap slightly overkill? Can qemu/bitmap.h suffice?

>  #include "hw/block/block.h"
>  #include "hw/pci/pci.h"
>  #include "sysemu/sysemu.h"
Minwoo Im Jan. 20, 2021, 12:47 a.m. UTC | #2
> Isn't the HBitmap slightly overkill? Can qemu/bitmap.h suffice?

Definitely, yes, I think.  Current patch series supoprt up to 32
controllers so I think qemu/bitmap.h is enough for us.

Will update the bitmap operations in the next series.
diff mbox series

Patch

diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
index 073f65e49cac..70d42c24065c 100644
--- a/hw/block/nvme-ns.c
+++ b/hw/block/nvme-ns.c
@@ -17,6 +17,7 @@ 
 #include "qemu/cutils.h"
 #include "qemu/log.h"
 #include "qemu/error-report.h"
+#include "qemu/hbitmap.h"
 #include "hw/block/block.h"
 #include "hw/pci/pci.h"
 #include "sysemu/sysemu.h"
@@ -304,6 +305,11 @@  static int nvme_ns_check_constraints(NvmeNamespace *ns, Error **errp)
     return 0;
 }
 
+static void nvme_ns_init_state(NvmeNamespace *ns)
+{
+    ns->ctrls_bitmap = hbitmap_alloc(NVME_SUBSYS_MAX_CTRLS, 0);
+}
+
 int nvme_ns_setup(NvmeNamespace *ns, Error **errp)
 {
     if (nvme_ns_check_constraints(ns, errp)) {
@@ -314,6 +320,8 @@  int nvme_ns_setup(NvmeNamespace *ns, Error **errp)
         return -1;
     }
 
+    nvme_ns_init_state(ns);
+
     if (nvme_ns_init(ns, errp)) {
         return -1;
     }
@@ -381,6 +389,7 @@  static Property nvme_ns_props[] = {
     DEFINE_BLOCK_PROPERTIES(NvmeNamespace, blkconf),
     DEFINE_PROP_LINK("subsys", NvmeNamespace, subsys, TYPE_NVME_SUBSYS,
                      NvmeSubsystem *),
+    DEFINE_PROP_BOOL("detached", NvmeNamespace, params.detached, false),
     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 929e78861903..ad2f55931d1b 100644
--- a/hw/block/nvme-ns.h
+++ b/hw/block/nvme-ns.h
@@ -26,6 +26,7 @@  typedef struct NvmeZone {
 } NvmeZone;
 
 typedef struct NvmeNamespaceParams {
+    bool     detached;
     uint32_t nsid;
     QemuUUID uuid;
 
@@ -48,6 +49,11 @@  typedef struct NvmeNamespace {
     uint8_t      csi;
 
     NvmeSubsystem   *subsys;
+    /*
+     * Whether this namespace is attached to a controller or not.  This bitmap
+     * is based on controller id.  This is valid only in case 'subsys' != NULL.
+     */
+    HBitmap         *ctrls_bitmap;
 
     NvmeIdNsZoned   *id_ns_zoned;
     NvmeZone        *zone_array;
diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
index e7efdcae7d0d..32ad8ef2825a 100644
--- a/hw/block/nvme-subsys.c
+++ b/hw/block/nvme-subsys.c
@@ -11,6 +11,7 @@ 
 #include "qemu/uuid.h"
 #include "qemu/iov.h"
 #include "qemu/cutils.h"
+#include "qemu/hbitmap.h"
 #include "qapi/error.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-core.h"
@@ -20,6 +21,7 @@ 
 #include "sysemu/sysemu.h"
 #include "hw/pci/pci.h"
 #include "nvme.h"
+#include "nvme-ns.h"
 #include "nvme-subsys.h"
 
 int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 06bccf1b9e9e..2b2c07b36c2b 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -26,7 +26,7 @@ 
  *              ,subsys=<subsys_id> \
  *      -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\
  *              zoned=<true|false[optional]>, \
- *              subsys=<subsys_id>
+ *              subsys=<subsys_id>,detached=<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
@@ -77,6 +77,13 @@ 
  *   controllers in the subsystem. Otherwise, `bus` must be given to attach
  *   this namespace to a specified single controller as a non-shared namespace.
  *
+ * - `detached`
+ *   Not to attach the namespace device to controllers in the NVMe subsystem
+ *   during boot-up.  If not given, namespaces are all attached to all
+ *   controllers in the subsystem by default.
+ *   It's mutual exclusive with 'bus' paraemter.  It's only valid in case
+ *   'subsys' is provided.
+ *
  * Setting `zoned` to true selects Zoned Command Set at the namespace.
  * In this case, the following namespace properties are available to configure
  * zoned operation:
@@ -2906,6 +2913,11 @@  static uint16_t nvme_identify_nslist(NvmeCtrl *n, NvmeRequest *req)
         if (ns->params.nsid <= min_nsid) {
             continue;
         }
+
+        if (!nvme_ns_is_attached(n, ns)) {
+            continue;
+        }
+
         list_ptr[j++] = cpu_to_le32(ns->params.nsid);
         if (j == data_len / sizeof(uint32_t)) {
             break;
@@ -4146,6 +4158,19 @@  static void nvme_init_state(NvmeCtrl *n)
     n->aer_reqs = g_new0(NvmeRequest *, n->params.aerl + 1);
 }
 
+static int nvme_attach_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
+{
+    if (nvme_ns_is_attached(n, ns)) {
+        error_setg(errp,
+                   "namespace %d is already attached to controller %d",
+                   nvme_nsid(ns), n->cntlid);
+        return -1;
+    }
+
+    nvme_ns_attach(n, ns);
+    return 0;
+}
+
 int nvme_register_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
 {
     uint32_t nsid = nvme_nsid(ns);
@@ -4179,6 +4204,10 @@  int nvme_register_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
 
     n->namespaces[nsid - 1] = ns;
 
+    if (!ns->params.detached) {
+        return nvme_attach_namespace(n, ns, errp);
+    }
+
     return 0;
 }
 
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index c158cc873b59..582e6d4e8c40 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -181,6 +181,21 @@  static inline NvmeNamespace *nvme_ns(NvmeCtrl *n, uint32_t nsid)
     return n->namespaces[nsid - 1];
 }
 
+static inline bool nvme_ns_is_attached(NvmeCtrl *n, NvmeNamespace *ns)
+{
+    return hbitmap_get(ns->ctrls_bitmap, n->cntlid);
+}
+
+static inline void nvme_ns_attach(NvmeCtrl *n, NvmeNamespace *ns)
+{
+    hbitmap_set(ns->ctrls_bitmap, n->cntlid, 1);
+}
+
+static inline void nvme_ns_detach(NvmeCtrl *n, NvmeNamespace *ns)
+{
+    hbitmap_reset(ns->ctrls_bitmap, n->cntlid, 1);
+}
+
 static inline NvmeCQueue *nvme_cq(NvmeRequest *req)
 {
     NvmeSQueue *sq = req->sq;