On Fri, May 28, 2021 at 01:22:38PM +0200, Klaus Jensen wrote:
> On May 28 11:05, Niklas Cassel wrote:
> > From: Niklas Cassel <niklas.cas...@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.cas...@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.jen...@samsung.com>
In reality, it is the controller that does the auto transitioning.
In theory, one namespace could be attached to two different controllers,
and I guess, in that case, it depends on if the controller that we used
when doing the write supports auto transitioning or not, that determines
if a zone will be auto transitioned or not.
If we were to change this to be a parameter of the controller instead
of a parameter of the namespace, we would require to refactor a lot of
code in the regular write path. As we currently don't have any NvmeRequest
object in nvme_zrm_open_flags().
Thoughts?
Kind regards,
Niklas
