Re: [PATCH] hw/nvme: add param to control auto zone transitioning to zone state closed
On May 31 21:39, Klaus Jensen wrote:
On May 31 15:42, Niklas Cassel wrote:
On Fri, May 28, 2021 at 01:22:38PM +0200, Klaus Jensen wrote:
On May 28 11:05, Niklas Cassel wrote:
From: Niklas Cassel
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
---
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=
* Setting this property to true enables Read Across Zone Boundaries.
+ *
+ * zoned.auto_transition=
+ * 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
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?
I think you are right. This should be controller-specific behavior. I
took the liberty of moving the parameter; the refactor is minimal.
From: Niklas Cassel
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
[k.jensen: moved parameter to controller]
Signed-off-by: Klaus Jensen
---
hw/nvme/nvme.h | 1 +
hw/nvme/ctrl.c | 32 ++--
2 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 81a35cda142b..93a7e0e5380e 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -382,6 +382,7 @@ typedef struct NvmeParams {
uint8_t vsl;
bool use_intel_id;
uint8_t zasl;
+bool auto_transition_zones;
bool legacy_cmb;
} NvmeParams;
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 40a7efcea914..8dd9cb2ccbf3 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -34,6 +34,7 @@
*
Re: [PATCH] hw/nvme: add param to control auto zone transitioning to zone state closed
On Jun 7 09:58, Niklas Cassel wrote:
On Mon, Jun 07, 2021 at 11:54:02AM +0200, Klaus Jensen wrote:
On Jun 1 07:30, Niklas Cassel wrote:
> On Mon, May 31, 2021 at 09:39:20PM +0200, Klaus Jensen wrote:
> > On May 31 15:42, Niklas Cassel wrote:
> > > On Fri, May 28, 2021 at 01:22:38PM +0200, Klaus Jensen wrote:
> > > > On May 28 11:05, Niklas Cassel wrote:
> > > > > From: Niklas Cassel
> > > > >
> > > > > 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
> > > > > ---
> > > > > 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=
> > > > > * Setting this property to true enables Read Across Zone
Boundaries.
> > > > > + *
> > > > > + * zoned.auto_transition=
> > > > > + * 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
> > >
> > > 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?
> > >
> >
> > I think you are right. This should be controller-specific behavior. I took
> > the liberty of moving the parameter; the refactor is minimal.
> >
> >
> > From: Niklas Cassel
> >
> > 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
Re: [PATCH] hw/nvme: add param to control auto zone transitioning to zone state closed
On Mon, Jun 07, 2021 at 11:54:02AM +0200, Klaus Jensen wrote:
> On Jun 1 07:30, Niklas Cassel wrote:
> > On Mon, May 31, 2021 at 09:39:20PM +0200, Klaus Jensen wrote:
> > > On May 31 15:42, Niklas Cassel wrote:
> > > > On Fri, May 28, 2021 at 01:22:38PM +0200, Klaus Jensen wrote:
> > > > > On May 28 11:05, Niklas Cassel wrote:
> > > > > > From: Niklas Cassel
> > > > > >
> > > > > > 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
> > > > > > ---
> > > > > > 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=
> > > > > > * Setting this property to true enables Read Across Zone
> > > > > > Boundaries.
> > > > > > + *
> > > > > > + * zoned.auto_transition= > > > > > 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
> > > >
> > > > 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?
> > > >
> > >
> > > I think you are right. This should be controller-specific behavior. I took
> > > the liberty of moving the parameter; the refactor is minimal.
> > >
> >
Re: [PATCH] hw/nvme: add param to control auto zone transitioning to zone state closed
On Jun 1 07:30, Niklas Cassel wrote:
On Mon, May 31, 2021 at 09:39:20PM +0200, Klaus Jensen wrote:
On May 31 15:42, Niklas Cassel wrote:
> On Fri, May 28, 2021 at 01:22:38PM +0200, Klaus Jensen wrote:
> > On May 28 11:05, Niklas Cassel wrote:
> > > From: Niklas Cassel
> > >
> > > 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
> > > ---
> > > 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=
> > > * Setting this property to true enables Read Across Zone
Boundaries.
> > > + *
> > > + * zoned.auto_transition=
> > > + * 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
>
> 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?
>
I think you are right. This should be controller-specific behavior. I took
the liberty of moving the parameter; the refactor is minimal.
From: Niklas Cassel
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
[k.jensen: moved parameter to controller]
Signed-off-by: Klaus Jensen
---
hw/nvme/nvme.h | 1 +
Re: [PATCH] hw/nvme: add param to control auto zone transitioning to zone state closed
On Mon, May 31, 2021 at 09:39:20PM +0200, Klaus Jensen wrote:
> On May 31 15:42, Niklas Cassel wrote:
> > On Fri, May 28, 2021 at 01:22:38PM +0200, Klaus Jensen wrote:
> > > On May 28 11:05, Niklas Cassel wrote:
> > > > From: Niklas Cassel
> > > >
> > > > 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
> > > > ---
> > > > 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=
> > > > * Setting this property to true enables Read Across Zone
> > > > Boundaries.
> > > > + *
> > > > + * zoned.auto_transition= > > > 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
> >
> > 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?
> >
>
> I think you are right. This should be controller-specific behavior. I took
> the liberty of moving the parameter; the refactor is minimal.
>
>
> From: Niklas Cassel
>
> 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
Re: [PATCH] hw/nvme: add param to control auto zone transitioning to zone state closed
On May 31 15:42, Niklas Cassel wrote:
On Fri, May 28, 2021 at 01:22:38PM +0200, Klaus Jensen wrote:
On May 28 11:05, Niklas Cassel wrote:
> From: Niklas Cassel
>
> 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
> ---
> 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=
> * Setting this property to true enables Read Across Zone Boundaries.
> + *
> + * zoned.auto_transition=
> + * 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
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?
I think you are right. This should be controller-specific behavior. I
took the liberty of moving the parameter; the refactor is minimal.
From: Niklas Cassel
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
[k.jensen: moved parameter to controller]
Signed-off-by: Klaus Jensen
---
hw/nvme/nvme.h | 1 +
hw/nvme/ctrl.c | 32 ++--
2 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 81a35cda142b..93a7e0e5380e 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -382,6 +382,7 @@ typedef struct NvmeParams {
uint8_t vsl;
bool use_intel_id;
uint8_t zasl;
+bool auto_transition_zones;
bool legacy_cmb;
} NvmeParams;
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index
Re: [PATCH] hw/nvme: add param to control auto zone transitioning to zone state closed
On Fri, May 28, 2021 at 01:22:38PM +0200, Klaus Jensen wrote:
> On May 28 11:05, Niklas Cassel wrote:
> > From: Niklas Cassel
> >
> > 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
> > ---
> > 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=
> > * Setting this property to true enables Read Across Zone
> > Boundaries.
> > + *
> > + * zoned.auto_transition= > 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
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
Re: [PATCH] hw/nvme: add param to control auto zone transitioning to zone state closed
On May 28 11:05, Niklas Cassel wrote:
From: Niklas Cassel
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
---
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=
* Setting this property to true enables Read Across Zone Boundaries.
+ *
+ * zoned.auto_transition=
+ * 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
signature.asc
Description: PGP signature
[PATCH] hw/nvme: add param to control auto zone transitioning to zone state closed
From: Niklas Cassel
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
---
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=
* Setting this property to true enables Read Across Zone Boundaries.
+ *
+ * zoned.auto_transition=
+ * 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
