Re: [PATCH] hw/nvme: add param to control auto zone transitioning to zone state closed

[Klaus Jensen <its@irrelevant.dk>]

[-- Attachment #1: Type: text/plain, Size: 3037 bytes --]

On May 28 11:05, Niklas Cassel wrote:
>From: Niklas Cassel <niklas.cassel@wdc.com>
>
>In the Zoned Namespace Command Set Specification, chapter
>2.5.1 Managing resources
>
>"The controller may transition zones in the ZSIO:Implicitly Opened state
>to the ZSC:Closed state for resource management purposes."
>
>The word may in this sentence means that automatically transitioning
>an implicitly opened zone to closed is completely optional.
>
>Add a new parameter so that the user can control if this automatic
>transitioning should be performed or not.
>
>Being able to control this can help with verifying that e.g. a user-space
>program behaves properly even without this optional ZNS feature.
>
>The default value is set to true, in order to not change the existing
>behavior.
>
>Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
>---
> hw/nvme/ctrl.c | 9 ++++++++-
> hw/nvme/ns.c   | 2 ++
> hw/nvme/nvme.h | 1 +
> 3 files changed, 11 insertions(+), 1 deletion(-)
>
>diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
>index 40a7efcea9..d00f0297a5 100644
>--- a/hw/nvme/ctrl.c
>+++ b/hw/nvme/ctrl.c
>@@ -141,6 +141,11 @@
>  *
>  *     zoned.cross_read=<enable RAZB, default: false>
>  *         Setting this property to true enables Read Across Zone Boundaries.
>+ *
>+ *     zoned.auto_transition=<enable auto resource management, default: true>
>+ *         Indicates if zones in zone state implicitly opened can be
>+ *         automatically transitioned to zone state closed for resource
>+ *         management purposes.
>  */
>
> #include "qemu/osdep.h"
>@@ -1699,7 +1704,9 @@ static uint16_t nvme_zrm_open_flags(NvmeNamespace *ns, NvmeZone *zone,
>         /* fallthrough */
>
>     case NVME_ZONE_STATE_CLOSED:
>-        nvme_zrm_auto_transition_zone(ns);
>+        if (ns->params.auto_transition_zones) {
>+            nvme_zrm_auto_transition_zone(ns);
>+        }
>         status = nvme_aor_check(ns, act, 1);
>         if (status) {
>             return status;
>diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
>index 3fec9c6273..31dee43d30 100644
>--- a/hw/nvme/ns.c
>+++ b/hw/nvme/ns.c
>@@ -531,6 +531,8 @@ static Property nvme_ns_props[] = {
>                        params.max_open_zones, 0),
>     DEFINE_PROP_UINT32("zoned.descr_ext_size", NvmeNamespace,
>                        params.zd_extension_size, 0),
>+    DEFINE_PROP_BOOL("zoned.auto_transition", NvmeNamespace,
>+                     params.auto_transition_zones, true),
>     DEFINE_PROP_END_OF_LIST(),
> };
>
>diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
>index 81a35cda14..bd86054db2 100644
>--- a/hw/nvme/nvme.h
>+++ b/hw/nvme/nvme.h
>@@ -100,6 +100,7 @@ typedef struct NvmeNamespaceParams {
>     uint32_t max_active_zones;
>     uint32_t max_open_zones;
>     uint32_t zd_extension_size;
>+    bool     auto_transition_zones;
> } NvmeNamespaceParams;
>
> typedef struct NvmeNamespace {
>-- 
>2.31.1
>

Looks good Niklas!

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]