From 5ad7d0174e8e5f3e5af27719d4fc5f6a93efa1f2 Mon Sep 17 00:00:00 2001
From: Klaus Jensen <k.jensen@samsung.com>
Date: Tue, 23 Mar 2021 09:03:10 +0100
Subject: [PATCH 01/10] hw/block/nvme: fix pi constraint check
Protection Information can only be enabled if there is at least 8 bytes
of metadata.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
Reviewed-by: Keith Busch <kbusch@kernel.org>
---
hw/block/nvme-ns.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
index 7f8d139a86..ca04ee1bac 100644
--- a/hw/block/nvme-ns.c
+++ b/hw/block/nvme-ns.c
@@ -394,7 +394,7 @@ static int nvme_ns_check_constraints(NvmeNamespace *ns, Error **errp)
return -1;
}
- if (ns->params.pi && !ns->params.ms) {
+ if (ns->params.pi && ns->params.ms < 8) {
error_setg(errp, "at least 8 bytes of metadata required to enable "
"protection information");
return -1;
From 349bf41d592cf59e0438ad79e247395b1af04c4f Mon Sep 17 00:00:00 2001
From: Klaus Jensen <k.jensen@samsung.com>
Date: Tue, 23 Mar 2021 15:10:54 +0100
Subject: [PATCH 02/10] hw/block/nvme: fix missing string representation for ns
attachment
Add the missing nvme_adm_opc_str entry for the Namespace Attachment
command.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
Reviewed-by: Keith Busch <kbusch@kernel.org>
---
hw/block/nvme.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 5b0031b11d..9edc86d79e 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -86,6 +86,7 @@ static inline const char *nvme_adm_opc_str(uint8_t opc)
case NVME_ADM_CMD_SET_FEATURES: return "NVME_ADM_CMD_SET_FEATURES";
case NVME_ADM_CMD_GET_FEATURES: return "NVME_ADM_CMD_GET_FEATURES";
case NVME_ADM_CMD_ASYNC_EV_REQ: return "NVME_ADM_CMD_ASYNC_EV_REQ";
+ case NVME_ADM_CMD_NS_ATTACHMENT: return "NVME_ADM_CMD_NS_ATTACHMENT";
case NVME_ADM_CMD_FORMAT_NVM: return "NVME_ADM_CMD_FORMAT_NVM";
default: return "NVME_ADM_CMD_UNKNOWN";
}
From dae8be368e7b9ad36bc8af1cd365d28b199a6502 Mon Sep 17 00:00:00 2001
From: Klaus Jensen <k.jensen@samsung.com>
Date: Wed, 24 Mar 2021 10:11:55 +0100
Subject: [PATCH 03/10] hw/block/nvme: fix the nsid 'invalid' value
The `nvme_nsid()` function returns '-1' (FFFFFFFFh) when the given
namespace is NULL. Since FFFFFFFFh is actually a valid namespace
identifier (the "broadcast" value), change this to be '0' since that
actually *is* the invalid value.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
Reviewed-by: Keith Busch <kbusch@kernel.org>
---
hw/block/nvme-ns.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h
index 9ab7894fc8..82340c4b25 100644
--- a/hw/block/nvme-ns.h
+++ b/hw/block/nvme-ns.h
@@ -96,7 +96,7 @@ static inline uint32_t nvme_nsid(NvmeNamespace *ns)
return ns->params.nsid;
}
- return -1;
+ return 0;
}
static inline bool nvme_ns_shared(NvmeNamespace *ns)
From f447f92c886576ebc7e2354e3af443760966e514 Mon Sep 17 00:00:00 2001
From: Klaus Jensen <k.jensen@samsung.com>
Date: Tue, 23 Mar 2021 22:12:36 +0100
Subject: [PATCH 04/10] hw/block/nvme: fix warning about legacy namespace
configuration
Remove the unused BlockConf from the controller structure and remove the
noop constraint checking.
Device works just fine with both legacy drive parameter namespace and
nvme-ns namespace definitions.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
---
hw/block/nvme.c | 5 -----
hw/block/nvme.h | 1 -
2 files changed, 6 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 7244534a89..5911adcf57 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -5805,11 +5805,6 @@ static void nvme_check_constraints(NvmeCtrl *n, Error **errp)
params->max_ioqpairs = params->num_queues - 1;
}
- if (n->conf.blk) {
- warn_report("drive property is deprecated; "
- "please use an nvme-ns device instead");
- }
-
if (params->max_ioqpairs < 1 ||
params->max_ioqpairs > NVME_MAX_IOQPAIRS) {
error_setg(errp, "max_ioqpairs must be between 1 and %d",
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 9edc86d79e..8d1806cc94 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -166,7 +166,6 @@ typedef struct NvmeCtrl {
NvmeBar bar;
NvmeParams params;
NvmeBus bus;
- BlockConf conf;
uint16_t cntlid;
bool qs_created;
From 9b8671ed43eed95e43ea97528e761810177d09a3 Mon Sep 17 00:00:00 2001
From: Klaus Jensen <k.jensen@samsung.com>
Date: Tue, 23 Mar 2021 22:42:56 +0100
Subject: [PATCH 05/10] hw/block/nvme: update dmsrl limit on namespace
detachment
The Non-MDTS DMSRL limit must be recomputed when namespaces are
detached.
Fixes: 645ce1a70cb6 ("hw/block/nvme: support namespace attachment command")
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
Reviewed-by: Keith Busch <kbusch@kernel.org>
---
hw/block/nvme.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5911adcf57..0e5ba27c46 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -4868,6 +4868,21 @@ static uint16_t nvme_aer(NvmeCtrl *n, NvmeRequest *req)
return NVME_NO_COMPLETE;
}
+static void nvme_update_dmrsl(NvmeCtrl *n)
+{
+ int nsid;
+
+ for (nsid = 1; nsid <= NVME_MAX_NAMESPACES; nsid++) {
+ NvmeNamespace *ns = nvme_ns(n, nsid);
+ if (!ns) {
+ continue;
+ }
+
+ n->dmrsl = MIN_NON_ZERO(n->dmrsl,
+ BDRV_REQUEST_MAX_BYTES / nvme_l2b(ns, 1));
+ }
+}
+
static void __nvme_select_ns_iocs(NvmeCtrl *n, NvmeNamespace *ns);
static uint16_t nvme_ns_attachment(NvmeCtrl *n, NvmeRequest *req)
{
@@ -4917,6 +4932,8 @@ static uint16_t nvme_ns_attachment(NvmeCtrl *n, NvmeRequest *req)
}
nvme_ns_detach(ctrl, ns);
+
+ nvme_update_dmrsl(ctrl);
}
/*
From e5489356349a400774c1bda60dbf78cff211e009 Mon Sep 17 00:00:00 2001
From: Klaus Jensen <k.jensen@samsung.com>
Date: Tue, 23 Mar 2021 12:43:24 +0100
Subject: [PATCH 06/10] hw/block/nvme: fix handling of private namespaces
Prior to this patch, if a private nvme-ns device (that is, a namespace
that is not linked to a subsystem) is wired up to an nvme-subsys linked
nvme controller device, the device fails to verify that the namespace id
is unique within the subsystem. NVM Express v1.4b, Section 6.1.6 ("NSID
and Namespace Usage") states that because the device supports Namespace
Management, "NSIDs *shall* be unique within the NVM subsystem".
Additionally, prior to this patch, private namespaces are not known to
the subsystem and the namespace is considered exclusive to the
controller with which it is initially wired up to. However, this is not
the definition of a private namespace; per Section 1.6.33 ("private
namespace"), a private namespace is just a namespace that does not
support multipath I/O or namespace sharing, which means "that it is only
able to be attached to one controller at a time".
Fix this by always allocating namespaces in the subsystem (if one is
linked to the controller), regardless of the shared/private status of
the namespace. Whether or not the namespace is shareable is controlled
by a new `shared` nvme-ns parameter.
Finally, this fix allows the nvme-ns `subsys` parameter to be removed,
since the `shared` parameter now serves the purpose of attaching the
namespace to all controllers in the subsystem upon device realization.
It is invalid to have an nvme-ns namespace device with a linked
subsystem without the parent nvme controller device also being linked to
one and since the nvme-ns devices will unconditionally be "attached" (in
QEMU terms that is) to an nvme controller device through an NvmeBus, the
nvme-ns namespace device can always get a reference to the subsystem of
the controller it is explicitly (using 'bus=' parameter) or implicitly
attaching to.
Fixes: e570768566b3 ("hw/block/nvme: support for shared namespace in subsystem")
Cc: Minwoo Im <minwoo.im.dev@gmail.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
hw/block/nvme-ns.c | 76 +++++++++++++++++++++----
hw/block/nvme-ns.h | 10 +---
hw/block/nvme-subsys.c | 28 ----------
hw/block/nvme-subsys.h | 7 +--
hw/block/nvme.c | 123 ++++++++++++++---------------------------
hw/block/nvme.h | 39 +------------
hw/block/trace-events | 1 -
include/block/nvme.h | 1 +
8 files changed, 115 insertions(+), 170 deletions(-)
diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
index ca04ee1bac..7bb618f182 100644
--- a/hw/block/nvme-ns.c
+++ b/hw/block/nvme-ns.c
@@ -73,7 +73,7 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
/* support DULBE and I/O optimization fields */
id_ns->nsfeat |= (0x4 | 0x10);
- if (nvme_ns_shared(ns)) {
+ if (ns->params.shared) {
id_ns->nmic |= NVME_NMIC_NS_SHARED;
}
@@ -387,7 +387,8 @@ static void nvme_zoned_ns_shutdown(NvmeNamespace *ns)
assert(ns->nr_open_zones == 0);
}
-static int nvme_ns_check_constraints(NvmeNamespace *ns, Error **errp)
+static int nvme_ns_check_constraints(NvmeCtrl *n, NvmeNamespace *ns,
+ Error **errp)
{
if (!ns->blkconf.blk) {
error_setg(errp, "block backend not configured");
@@ -400,12 +401,32 @@ static int nvme_ns_check_constraints(NvmeNamespace *ns, Error **errp)
return -1;
}
+ if (ns->params.nsid > NVME_MAX_NAMESPACES) {
+ error_setg(errp, "invalid namespace id (must be between 0 and %d)",
+ NVME_MAX_NAMESPACES);
+ return -1;
+ }
+
+ if (!n->subsys) {
+ if (ns->params.detached) {
+ error_setg(errp, "detached requires that the nvme device is "
+ "linked to an nvme-subsys device");
+ return -1;
+ }
+
+ if (ns->params.shared) {
+ error_setg(errp, "shared requires that the nvme device is "
+ "linked to an nvme-subsys device");
+ return -1;
+ }
+ }
+
return 0;
}
-int nvme_ns_setup(NvmeNamespace *ns, Error **errp)
+int nvme_ns_setup(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
{
- if (nvme_ns_check_constraints(ns, errp)) {
+ if (nvme_ns_check_constraints(n, ns, errp)) {
return -1;
}
@@ -453,27 +474,62 @@ static void nvme_ns_realize(DeviceState *dev, Error **errp)
NvmeNamespace *ns = NVME_NS(dev);
BusState *s = qdev_get_parent_bus(dev);
NvmeCtrl *n = NVME(s->parent);
+ NvmeSubsystem *subsys = n->subsys;
+ uint32_t nsid = ns->params.nsid;
+ int i;
- if (nvme_ns_setup(ns, errp)) {
+ if (nvme_ns_setup(n, ns, errp)) {
return;
}
- if (ns->subsys) {
- if (nvme_subsys_register_ns(ns, errp)) {
+ if (!nsid) {
+ for (i = 1; i <= NVME_MAX_NAMESPACES; i++) {
+ if (nvme_ns(n, i) || nvme_subsys_ns(subsys, i)) {
+ continue;
+ }
+
+ nsid = ns->params.nsid = i;
+ break;
+ }
+
+ if (!nsid) {
+ error_setg(errp, "no free namespace id");
return;
}
} else {
- if (nvme_register_namespace(n, ns, errp)) {
+ if (nvme_ns(n, nsid) || nvme_subsys_ns(subsys, nsid)) {
+ error_setg(errp, "namespace id '%d' already allocated", nsid);
return;
}
}
+
+ if (subsys) {
+ subsys->namespaces[nsid] = ns;
+
+ if (ns->params.detached) {
+ return;
+ }
+
+ if (ns->params.shared) {
+ for (i = 0; i < ARRAY_SIZE(subsys->ctrls); i++) {
+ NvmeCtrl *ctrl = subsys->ctrls[i];
+
+ if (ctrl) {
+ nvme_attach_ns(ctrl, ns);
+ }
+ }
+
+ return;
+ }
+ }
+
+ nvme_attach_ns(n, ns);
}
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_BOOL("shared", NvmeNamespace, params.shared, false),
DEFINE_PROP_UINT32("nsid", NvmeNamespace, params.nsid, 0),
DEFINE_PROP_UUID("uuid", NvmeNamespace, params.uuid),
DEFINE_PROP_UINT16("ms", NvmeNamespace, params.ms, 0),
diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h
index 82340c4b25..fb0a41f912 100644
--- a/hw/block/nvme-ns.h
+++ b/hw/block/nvme-ns.h
@@ -29,6 +29,7 @@ typedef struct NvmeZone {
typedef struct NvmeNamespaceParams {
bool detached;
+ bool shared;
uint32_t nsid;
QemuUUID uuid;
@@ -60,8 +61,8 @@ typedef struct NvmeNamespace {
const uint32_t *iocs;
uint8_t csi;
uint16_t status;
+ int attached;
- NvmeSubsystem *subsys;
QTAILQ_ENTRY(NvmeNamespace) entry;
NvmeIdNsZoned *id_ns_zoned;
@@ -99,11 +100,6 @@ static inline uint32_t nvme_nsid(NvmeNamespace *ns)
return 0;
}
-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;
@@ -225,7 +221,7 @@ static inline void nvme_aor_dec_active(NvmeNamespace *ns)
}
void nvme_ns_init_format(NvmeNamespace *ns);
-int nvme_ns_setup(NvmeNamespace *ns, Error **errp);
+int nvme_ns_setup(NvmeCtrl *n, NvmeNamespace *ns, Error **errp);
void nvme_ns_drain(NvmeNamespace *ns);
void nvme_ns_shutdown(NvmeNamespace *ns);
void nvme_ns_cleanup(NvmeNamespace *ns);
diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
index 9fadef8cec..283a97b79d 100644
--- a/hw/block/nvme-subsys.c
+++ b/hw/block/nvme-subsys.c
@@ -43,34 +43,6 @@ 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;
- uint32_t nsid = nvme_nsid(ns);
- int i;
-
- assert(nsid && nsid <= NVME_SUBSYS_MAX_NAMESPACES);
-
- if (subsys->namespaces[nsid]) {
- error_setg(errp, "namespace %d already registerd to subsy %s",
- nvme_nsid(ns), subsys->parent_obj.id);
- return -1;
- }
-
- subsys->namespaces[nsid] = 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)
{
const char *nqn = subsys->params.nqn ?
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
index aafa04b848..24132edd00 100644
--- a/hw/block/nvme-subsys.h
+++ b/hw/block/nvme-subsys.h
@@ -14,7 +14,7 @@
OBJECT_CHECK(NvmeSubsystem, (obj), TYPE_NVME_SUBSYS)
#define NVME_SUBSYS_MAX_CTRLS 32
-#define NVME_SUBSYS_MAX_NAMESPACES 256
+#define NVME_MAX_NAMESPACES 256
typedef struct NvmeCtrl NvmeCtrl;
typedef struct NvmeNamespace NvmeNamespace;
@@ -24,7 +24,7 @@ typedef struct NvmeSubsystem {
NvmeCtrl *ctrls[NVME_SUBSYS_MAX_CTRLS];
/* Allocated namespaces for this subsystem */
- NvmeNamespace *namespaces[NVME_SUBSYS_MAX_NAMESPACES + 1];
+ NvmeNamespace *namespaces[NVME_MAX_NAMESPACES + 1];
struct {
char *nqn;
@@ -32,7 +32,6 @@ typedef struct NvmeSubsystem {
} NvmeSubsystem;
int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp);
-int nvme_subsys_register_ns(NvmeNamespace *ns, Error **errp);
static inline NvmeCtrl *nvme_subsys_ctrl(NvmeSubsystem *subsys,
uint32_t cntlid)
@@ -54,7 +53,7 @@ static inline NvmeNamespace *nvme_subsys_ns(NvmeSubsystem *subsys,
return NULL;
}
- assert(nsid && nsid <= NVME_SUBSYS_MAX_NAMESPACES);
+ assert(nsid && nsid <= NVME_MAX_NAMESPACES);
return subsys->namespaces[nsid];
}
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 0e5ba27c46..82b3d453f5 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -93,10 +93,13 @@
*
* nvme namespace device parameters
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- * - `subsys`
- * If given, the namespace will be attached to all controllers in the
- * subsystem. Otherwise, `bus` must be given to attach this namespace to a
- * specific controller as a non-shared namespace.
+ * - `shared`
+ * When the parent nvme device (as defined explicitly by the 'bus' parameter
+ * or implicitly by the most recently defined NvmeBus) is linked to an
+ * nvme-subsys device, the namespace will be attached to all controllers in
+ * the subsystem. If set to 'off' (the default), the namespace will remain a
+ * private namespace and may only be attached to a single controller at a
+ * time.
*
* - `detached`
* This parameter is only valid together with the `subsys` parameter. If left
@@ -4242,7 +4245,7 @@ static uint16_t nvme_identify_ns_attached_list(NvmeCtrl *n, NvmeRequest *req)
continue;
}
- if (!nvme_ns_is_attached(ctrl, ns)) {
+ if (!nvme_ns(ctrl, c->nsid)) {
continue;
}
@@ -4899,6 +4902,10 @@ static uint16_t nvme_ns_attachment(NvmeCtrl *n, NvmeRequest *req)
trace_pci_nvme_ns_attachment(nvme_cid(req), dw10 & 0xf);
+ if (!nvme_nsid_valid(n, nsid)) {
+ return NVME_INVALID_NSID | NVME_DNR;
+ }
+
ns = nvme_subsys_ns(n->subsys, nsid);
if (!ns) {
return NVME_INVALID_FIELD | NVME_DNR;
@@ -4920,18 +4927,23 @@ static uint16_t nvme_ns_attachment(NvmeCtrl *n, NvmeRequest *req)
}
if (attach) {
- if (nvme_ns_is_attached(ctrl, ns)) {
+ if (nvme_ns(ctrl, nsid)) {
return NVME_NS_ALREADY_ATTACHED | NVME_DNR;
}
- nvme_ns_attach(ctrl, ns);
+ if (ns->attached && !ns->params.shared) {
+ return NVME_NS_PRIVATE | NVME_DNR;
+ }
+
+ nvme_attach_ns(ctrl, ns);
__nvme_select_ns_iocs(ctrl, ns);
} else {
- if (!nvme_ns_is_attached(ctrl, ns)) {
+ if (!nvme_ns(ctrl, nsid)) {
return NVME_NS_NOT_ATTACHED | NVME_DNR;
}
- nvme_ns_detach(ctrl, ns);
+ ctrl->namespaces[nsid - 1] = NULL;
+ ns->attached--;
nvme_update_dmrsl(ctrl);
}
@@ -5822,6 +5834,12 @@ static void nvme_check_constraints(NvmeCtrl *n, Error **errp)
params->max_ioqpairs = params->num_queues - 1;
}
+ if (n->namespace.blkconf.blk && n->subsys) {
+ error_setg(errp, "subsystem support is unavailable with legacy "
+ "namespace ('drive' property)");
+ return;
+ }
+
if (params->max_ioqpairs < 1 ||
params->max_ioqpairs > NVME_MAX_IOQPAIRS) {
error_setg(errp, "max_ioqpairs must be between 1 and %d",
@@ -5882,75 +5900,6 @@ 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);
-
- if (nsid > NVME_MAX_NAMESPACES) {
- error_setg(errp, "invalid namespace id (must be between 0 and %d)",
- NVME_MAX_NAMESPACES);
- return -1;
- }
-
- if (!nsid) {
- for (int i = 1; i <= n->num_namespaces; i++) {
- if (!nvme_ns(n, i)) {
- nsid = ns->params.nsid = i;
- break;
- }
- }
-
- if (!nsid) {
- error_setg(errp, "no free namespace id");
- return -1;
- }
- } else {
- if (n->namespaces[nsid - 1]) {
- error_setg(errp, "namespace id '%d' is already in use", nsid);
- return -1;
- }
- }
-
- trace_pci_nvme_register_namespace(nsid);
-
- /*
- * If subsys is not given, namespae is always attached to the controller
- * because there's no subsystem to manage namespace allocation.
- */
- if (!n->subsys) {
- if (ns->params.detached) {
- error_setg(errp,
- "detached needs nvme-subsys specified nvme or nvme-ns");
- return -1;
- }
-
- return nvme_attach_namespace(n, ns, errp);
- } else {
- if (!ns->params.detached) {
- return nvme_attach_namespace(n, ns, errp);
- }
- }
-
- n->dmrsl = MIN_NON_ZERO(n->dmrsl,
- BDRV_REQUEST_MAX_BYTES / nvme_l2b(ns, 1));
-
- return 0;
-}
-
static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
{
uint64_t cmb_size = n->params.cmb_size_mb * MiB;
@@ -6180,6 +6129,18 @@ static int nvme_init_subsys(NvmeCtrl *n, Error **errp)
return 0;
}
+void nvme_attach_ns(NvmeCtrl *n, NvmeNamespace *ns)
+{
+ uint32_t nsid = ns->params.nsid;
+ assert(nsid && nsid <= NVME_MAX_NAMESPACES);
+
+ n->namespaces[nsid - 1] = ns;
+ ns->attached++;
+
+ n->dmrsl = MIN_NON_ZERO(n->dmrsl,
+ BDRV_REQUEST_MAX_BYTES / nvme_l2b(ns, 1));
+}
+
static void nvme_realize(PCIDevice *pci_dev, Error **errp)
{
NvmeCtrl *n = NVME(pci_dev);
@@ -6211,13 +6172,11 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
ns = &n->namespace;
ns->params.nsid = 1;
- if (nvme_ns_setup(ns, errp)) {
+ if (nvme_ns_setup(n, ns, errp)) {
return;
}
- if (nvme_register_namespace(n, ns, errp)) {
- return;
- }
+ nvme_attach_ns(n, ns);
}
}
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 8d1806cc94..5d05ec368f 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -6,17 +6,9 @@
#include "nvme-subsys.h"
#include "nvme-ns.h"
-#define NVME_MAX_NAMESPACES 256
-
#define NVME_DEFAULT_ZONE_SIZE (128 * MiB)
#define NVME_DEFAULT_MAX_ZA_SIZE (128 * KiB)
-/*
- * Subsystem namespace list for allocated namespaces should be larger than
- * attached namespace list in a controller.
- */
-QEMU_BUILD_BUG_ON(NVME_MAX_NAMESPACES > NVME_SUBSYS_MAX_NAMESPACES);
-
typedef struct NvmeParams {
char *serial;
uint32_t num_queues; /* deprecated since 5.1 */
@@ -234,35 +226,6 @@ 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)
-{
- int nsid;
-
- for (nsid = 1; nsid <= n->num_namespaces; nsid++) {
- if (nvme_ns(n, nsid) == ns) {
- return true;
- }
- }
-
- return false;
-}
-
-static inline void nvme_ns_attach(NvmeCtrl *n, NvmeNamespace *ns)
-{
- uint32_t nsid = nvme_nsid(ns);
- assert(nsid && nsid <= NVME_MAX_NAMESPACES);
-
- n->namespaces[nsid - 1] = ns;
-}
-
-static inline void nvme_ns_detach(NvmeCtrl *n, NvmeNamespace *ns)
-{
- uint32_t nsid = nvme_nsid(ns);
- assert(nsid && nsid <= NVME_MAX_NAMESPACES);
-
- n->namespaces[nsid - 1] = NULL;
-}
-
static inline NvmeCQueue *nvme_cq(NvmeRequest *req)
{
NvmeSQueue *sq = req->sq;
@@ -291,7 +254,7 @@ typedef enum NvmeTxDirection {
NVME_TX_DIRECTION_FROM_DEVICE = 1,
} NvmeTxDirection;
-int nvme_register_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp);
+void nvme_attach_ns(NvmeCtrl *n, NvmeNamespace *ns);
uint16_t nvme_bounce_data(NvmeCtrl *n, uint8_t *ptr, uint32_t len,
NvmeTxDirection dir, NvmeRequest *req);
uint16_t nvme_bounce_mdata(NvmeCtrl *n, uint8_t *ptr, uint32_t len,
diff --git a/hw/block/trace-events b/hw/block/trace-events
index 22da06986d..fa12e3a67a 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -51,7 +51,6 @@ hd_geometry_guess(void *blk, uint32_t cyls, uint32_t heads, uint32_t secs, int t
# nvme.c
# nvme traces for successful events
-pci_nvme_register_namespace(uint32_t nsid) "nsid %"PRIu32""
pci_nvme_irq_msix(uint32_t vector) "raising MSI-X IRQ vector %u"
pci_nvme_irq_pin(void) "pulsing IRQ pin"
pci_nvme_irq_masked(void) "IRQ is masked"
diff --git a/include/block/nvme.h b/include/block/nvme.h
index b0a4e42916..4ac926fbc6 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -847,6 +847,7 @@ enum NvmeStatusCodes {
NVME_FEAT_NOT_NS_SPEC = 0x010f,
NVME_FW_REQ_SUSYSTEM_RESET = 0x0110,
NVME_NS_ALREADY_ATTACHED = 0x0118,
+ NVME_NS_PRIVATE = 0x0119,
NVME_NS_NOT_ATTACHED = 0x011A,
NVME_NS_CTRL_LIST_INVALID = 0x011C,
NVME_CONFLICTING_ATTRS = 0x0180,
From 102ce606fbfa72ce0f22d0320ae9bb8068091dde Mon Sep 17 00:00:00 2001
From: Klaus Jensen <k.jensen@samsung.com>
Date: Mon, 5 Apr 2021 15:32:18 +0200
Subject: [PATCH 07/10] hw/block/nvme: add missing copyright headers
Add missing license/copyright headers to the nvme-dif.{c,h} files.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Keith Busch <kbusch@kernel.org>
---
hw/block/nvme-dif.c | 10 ++++++++++
hw/block/nvme-dif.h | 10 ++++++++++
2 files changed, 20 insertions(+)
diff --git a/hw/block/nvme-dif.c b/hw/block/nvme-dif.c
index e6f04faafb..81b0a4cb13 100644
--- a/hw/block/nvme-dif.c
+++ b/hw/block/nvme-dif.c
@@ -1,3 +1,13 @@
+/*
+ * QEMU NVM Express End-to-End Data Protection support
+ *
+ * Copyright (c) 2021 Samsung Electronics Co., Ltd.
+ *
+ * Authors:
+ * Klaus Jensen <k.jensen@samsung.com>
+ * Gollu Appalanaidu <anaidu.gollu@samsung.com>
+ */
+
#include "qemu/osdep.h"
#include "hw/block/block.h"
#include "sysemu/dma.h"
diff --git a/hw/block/nvme-dif.h b/hw/block/nvme-dif.h
index 5a8e37c852..524faffbd7 100644
--- a/hw/block/nvme-dif.h
+++ b/hw/block/nvme-dif.h
@@ -1,3 +1,13 @@
+/*
+ * QEMU NVM Express End-to-End Data Protection support
+ *
+ * Copyright (c) 2021 Samsung Electronics Co., Ltd.
+ *
+ * Authors:
+ * Klaus Jensen <k.jensen@samsung.com>
+ * Gollu Appalanaidu <anaidu.gollu@samsung.com>
+ */
+
#ifndef HW_NVME_DIF_H
#define HW_NVME_DIF_H
From 8eb5c8069a5ccb8dadf35765b6f9cca10fb98b84 Mon Sep 17 00:00:00 2001
From: Klaus Jensen <k.jensen@samsung.com>
Date: Wed, 7 Apr 2021 06:54:34 +0200
Subject: [PATCH 08/10] hw/block/nvme: fix ns attachment out-of-bounds read
nvme_ns_attachment() does not verify the contents of the host-supplied
16 bit "Number of Identifiers" field in the command payload.
Make sure the value is capped at 2047 and fix the out-of-bounds read.
Fixes: 645ce1a70cb6 ("hw/block/nvme: support namespace attachment command")
Cc: Minwoo Im <minwoo.im.dev@gmail.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
hw/block/nvme.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 82b3d453f5..6b1f056a0e 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -4920,6 +4920,7 @@ static uint16_t nvme_ns_attachment(NvmeCtrl *n, NvmeRequest *req)
return NVME_NS_CTRL_LIST_INVALID | NVME_DNR;
}
+ *nr_ids = MIN(*nr_ids, NVME_CONTROLLER_LIST_SIZE - 1);
for (i = 0; i < *nr_ids; i++) {
ctrl = nvme_subsys_ctrl(n->subsys, ids[i]);
if (!ctrl) {
From ec20329748d02728b823443436fe26eadb04f8cc Mon Sep 17 00:00:00 2001
From: Klaus Jensen <k.jensen@samsung.com>
Date: Wed, 7 Apr 2021 07:07:43 +0200
Subject: [PATCH 09/10] hw/block/nvme: fix assert crash in nvme_subsys_ns
nvme_subsys_ns() is used in contexts where the namespace identifier is
taken from an untrusted source. Commit 3921756dee6d ("hw/block/nvme:
assert namespaces array indices") tried to guard against this by
introducing an assert on the namespace identifier.
This is wrong since it is perfectly valid to call the function with an
invalid namespace identifier and like nvme_ns(), nvme_subsys_ns() should
simply return NULL.
Fixes: 3921756dee6d ("hw/block/nvme: assert namespaces array indices")
Fixes: 94d8d6d16781 ("hw/block/nvme: support allocated namespace type")
Cc: Minwoo Im <minwoo.im.dev@gmail.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
hw/block/nvme-subsys.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
index 24132edd00..1cbcad9be2 100644
--- a/hw/block/nvme-subsys.h
+++ b/hw/block/nvme-subsys.h
@@ -49,12 +49,10 @@ static inline NvmeCtrl *nvme_subsys_ctrl(NvmeSubsystem *subsys,
static inline NvmeNamespace *nvme_subsys_ns(NvmeSubsystem *subsys,
uint32_t nsid)
{
- if (!subsys) {
+ if (!subsys || !nsid || nsid > NVME_MAX_NAMESPACES) {
return NULL;
}
- assert(nsid && nsid <= NVME_MAX_NAMESPACES);
-
return subsys->namespaces[nsid];
}
From 7645f21f409b67eb9aad9feef6283c2e186e3703 Mon Sep 17 00:00:00 2001
From: Klaus Jensen <k.jensen@samsung.com>
Date: Wed, 7 Apr 2021 07:16:14 +0200
Subject: [PATCH 10/10] hw/block/nvme: fix out-of-bounds read in
nvme_subsys_ctrl
nvme_subsys_ctrl() is used in contexts where the given controller
identifier is from an untrusted source. Like its friends nvme_ns() and
nvme_subsys_ns(), nvme_subsys_ctrl() should just return NULL if an
invalid identifier is given.
Fixes: 645ce1a70cb6 ("hw/block/nvme: support namespace attachment command")
Cc: Minwoo Im <minwoo.im.dev@gmail.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
hw/block/nvme-subsys.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
index 1cbcad9be2..7d7ef5f7f1 100644
--- a/hw/block/nvme-subsys.h
+++ b/hw/block/nvme-subsys.h
@@ -36,7 +36,7 @@ int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp);
static inline NvmeCtrl *nvme_subsys_ctrl(NvmeSubsystem *subsys,
uint32_t cntlid)
{
- if (!subsys) {
+ if (!subsys || cntlid >= NVME_SUBSYS_MAX_CTRLS) {
return NULL;
}