[PATCH 0/4] hw/nvme: tp4146 misc

[PATCH 0/4] hw/nvme: tp4146 misc

[Klaus Jensen]

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

A set of fixes and small quality-of-life improvements for the TP4146
("Flexible Data Placement") support.

Klaus Jensen (4):
  hw/nvme: fix verification of number of ruhis
  hw/nvme: verify uniqueness of reclaim unit handle identifiers
  hw/nvme: add placement handle list ranges
  docs: update hw/nvme documentation for TP4146

 docs/system/devices/nvme.rst | 37 +++++++++++++++++++++++-
 hw/nvme/ns.c                 | 55 ++++++++++++++++++++++++++++--------
 hw/nvme/subsys.c             |  6 ++--
 3 files changed, 84 insertions(+), 14 deletions(-)

-- 
2.40.0

[PATCH 1/4] hw/nvme: fix verification of number of ruhis

[Klaus Jensen]

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

Fix a off-by-one error when verifying the number of reclaim unit handle
identifiers specified in fdp.ruhs. To make the fix nicer, move the
verification of the fdp.nruh parameter to an earlier point.

Fixes: 73064edfb864 ("hw/nvme: flexible data placement emulation")
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ns.c     | 4 +---
 hw/nvme/subsys.c | 6 ++++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 547c0b154312..050fdaf50fcd 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -438,9 +438,7 @@ static bool nvme_ns_init_fdp(NvmeNamespace *ns, Error **errp)
 
     /* parse the placement handle identifiers */
     while ((token = qemu_strsep(&p, ";")) != NULL) {
-        ns->fdp.nphs += 1;
-        if (ns->fdp.nphs > NVME_FDP_MAXPIDS ||
-            ns->fdp.nphs == endgrp->fdp.nruh) {
+        if (ns->fdp.nphs++ == endgrp->fdp.nruh) {
             error_setg(errp, "too many placement handles");
             free(r);
             return false;
diff --git a/hw/nvme/subsys.c b/hw/nvme/subsys.c
index 24ddec860e45..d30bb8bfd5b4 100644
--- a/hw/nvme/subsys.c
+++ b/hw/nvme/subsys.c
@@ -158,8 +158,10 @@ static bool nvme_subsys_setup_fdp(NvmeSubsystem *subsys, Error **errp)
 
     endgrp->fdp.nrg = subsys->params.fdp.nrg;
 
-    if (!subsys->params.fdp.nruh) {
-        error_setg(errp, "fdp.nruh must be non-zero");
+    if (!subsys->params.fdp.nruh ||
+        subsys->params.fdp.nruh > NVME_FDP_MAXPIDS) {
+        error_setg(errp, "fdp.nruh must be non-zero and less than %u",
+                   NVME_FDP_MAXPIDS);
         return false;
     }
 
-- 
2.40.0

[PATCH 2/4] hw/nvme: verify uniqueness of reclaim unit handle identifiers

[Klaus Jensen]

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

Verify that a reclaim unit handle identifier is only specified once in
fdp.ruhs.

Fixes: 73064edfb864 ("hw/nvme: flexible data placement emulation")
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ns.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 050fdaf50fcd..c4ea2033bb1c 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -453,6 +453,17 @@ static bool nvme_ns_init_fdp(NvmeNamespace *ns, Error **errp)
 
     free(r);
 
+    /* verify that the ruhids are unique */
+    for (unsigned int i = 0; i < ns->fdp.nphs; i++) {
+        for (unsigned int j = i + 1; j < ns->fdp.nphs; j++) {
+            if (ruhids[i] == ruhids[j]) {
+                error_setg(errp, "duplicate reclaim unit handle identifier: %u",
+                           ruhids[i]);
+                return false;
+            }
+        }
+    }
+
     ph = ns->fdp.phs = g_new(uint16_t, ns->fdp.nphs);
 
     ruhid = ruhids;
-- 
2.40.0

[PATCH 3/4] hw/nvme: add placement handle list ranges

[Klaus Jensen]

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

Allow the placement handles to be specified as ranges, i.e.
`fdp.ruhs=1:3-5` will attempt to assign ruh 1, 3, 4 and 5 to the
namespace.

Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ns.c | 42 +++++++++++++++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index c4ea2033bb1c..44aba8f4d9cf 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -400,8 +400,9 @@ static bool nvme_ns_init_fdp(NvmeNamespace *ns, Error **errp)
     NvmeRuHandle *ruh;
     uint8_t lbafi = NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas);
     g_autofree unsigned int *ruhids = NULL;
-    unsigned int *ruhid;
-    char *r, *p, *token;
+    unsigned int n, m, *ruhid;
+    const char *endptr, *token;
+    char *r, *p;
     uint16_t *ph;
 
     if (!ns->params.fdp.ruhs) {
@@ -438,17 +439,40 @@ static bool nvme_ns_init_fdp(NvmeNamespace *ns, Error **errp)
 
     /* parse the placement handle identifiers */
     while ((token = qemu_strsep(&p, ";")) != NULL) {
-        if (ns->fdp.nphs++ == endgrp->fdp.nruh) {
-            error_setg(errp, "too many placement handles");
-            free(r);
-            return false;
-        }
-
-        if (qemu_strtoui(token, NULL, 0, ruhid++) < 0) {
+        if (qemu_strtoui(token, &endptr, 0, &n) < 0) {
             error_setg(errp, "cannot parse reclaim unit handle identifier");
             free(r);
             return false;
         }
+
+        m = n;
+
+        /* parse range */
+        if (*endptr == '-') {
+            token = endptr + 1;
+
+            if (qemu_strtoui(token, NULL, 0, &m) < 0) {
+                error_setg(errp, "cannot parse reclaim unit handle identifier");
+                free(r);
+                return false;
+            }
+
+            if (m < n) {
+                error_setg(errp, "invalid reclaim unit handle identifier range");
+                free(r);
+                return false;
+            }
+        }
+
+        for (; n <= m; n++) {
+            if (ns->fdp.nphs++ == endgrp->fdp.nruh) {
+                error_setg(errp, "too many placement handles");
+                free(r);
+                return false;
+            }
+
+            *ruhid++ = n;
+        }
     }
 
     free(r);
-- 
2.40.0

[PATCH 4/4] docs: update hw/nvme documentation for TP4146

[Klaus Jensen]

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

Update documentation for TP4146 ("Flexible Data Placement") emulation.

Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 docs/system/devices/nvme.rst | 37 +++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/docs/system/devices/nvme.rst b/docs/system/devices/nvme.rst
index 30f841ef6222..a8bb8d729cd2 100644
--- a/docs/system/devices/nvme.rst
+++ b/docs/system/devices/nvme.rst
@@ -212,6 +212,41 @@ The namespace may be configured with additional parameters
   the minimum memory page size (CAP.MPSMIN). The default value (``0``)
   has this property inherit the ``mdts`` value.
 
+Flexible Data Placement
+-----------------------
+
+The device may be configured to support TP4146 ("Flexible Data Placement") by
+configuring it (``fdp=on``) on the subsystem::
+
+    -device nvme-subsys,id=nvme-subsys-0,nqn=subsys0,fdp=on,fdp.nruh=16
+
+The subsystem emulates a single Endurance Group, on which Flexible Data
+Placement will be supported. Also note that the device emulation deviates
+slightly from the specification, by always enabling the "FDP Mode" feature on
+the controller if the subsystems is configured for Flexible Data Placement.
+
+Enabling Flexible Data Placement on the subsyste enables the following
+parameters:
+
+``fdp.nrg`` (default: ``1``)
+  Set the number of Reclaim Groups.
+
+``fdp.nruh`` (default: ``0``)
+  Set the number of Reclaim Unit Handles. This is a mandatory paramater and
+  must be non-zero.
+
+``fdp.runs`` (default: ``96M``)
+  Set the Reclaim Unit Nominal Size. Defaults to 96 MiB.
+
+Namespaces within this subsystem may requests Reclaim Unit Handles::
+
+    -device nvme-ns,drive=nvm-1,fdp.ruhs=RUHLIST
+
+The ``RUHLIST`` is a semicolon separated list (i.e. ``0;1;2;3``) and may
+include ranges (i.e. ``0;8-15``). If no reclaim unit handle list is specified,
+the controller will assign the controller-specified reclaim unit handle to
+placement handle identifier 0.
+
 Metadata
 --------
 
@@ -320,4 +355,4 @@ controller are:
 
 .. code-block:: console
 
-   echo 0000:01:00.1 > /sys/bus/pci/drivers/nvme/bind
\ No newline at end of file
+   echo 0000:01:00.1 > /sys/bus/pci/drivers/nvme/bind
-- 
2.40.0

Re: [PATCH 0/4] hw/nvme: tp4146 misc

[Jesper Devantier]

On Wed, May 24, 2023 at 01:19:00PM +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> A set of fixes and small quality-of-life improvements for the TP4146
> ("Flexible Data Placement") support.
> 
> Klaus Jensen (4):
>   hw/nvme: fix verification of number of ruhis
>   hw/nvme: verify uniqueness of reclaim unit handle identifiers
>   hw/nvme: add placement handle list ranges
>   docs: update hw/nvme documentation for TP4146
> 
>  docs/system/devices/nvme.rst | 37 +++++++++++++++++++++++-
>  hw/nvme/ns.c                 | 55 ++++++++++++++++++++++++++++--------
>  hw/nvme/subsys.c             |  6 ++--
>  3 files changed, 84 insertions(+), 14 deletions(-)
> 
> -- 
> 2.40.0
> 
> 

Looks good to me :)

Reviewed-by: Jesper Wendel Devantier <j.devantier@samsung.com>