[PATCH v2 2/3] nvme: Add ocp to the subsys

Joel Granados posted 3 patches 3 years, 2 months ago
Maintainers: Keith Busch <kbusch@kernel.org>, Klaus Jensen <its@irrelevant.dk>, Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
There is a newer version of this series
[PATCH v2 2/3] nvme: Add ocp to the subsys
Posted by Joel Granados 3 years, 2 months ago
The Open Compute Project defines a Datacenter NVMe SSD Spec that sits on
top of the NVMe spec. Additional commands and NVMe behaviors specific for
the Datacenter. This is a preparation patch that introduces an argument to
activate OCP in nvme.

Signed-off-by: Joel Granados <j.granados@samsung.com>
---
 hw/nvme/nvme.h   | 1 +
 hw/nvme/subsys.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 79f5c281c2..aa99c0c57c 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -56,6 +56,7 @@ typedef struct NvmeSubsystem {
 
     struct {
         char *nqn;
+        bool ocp;
     } params;
 } NvmeSubsystem;
 
diff --git a/hw/nvme/subsys.c b/hw/nvme/subsys.c
index 9d2643678b..ecca28449c 100644
--- a/hw/nvme/subsys.c
+++ b/hw/nvme/subsys.c
@@ -129,8 +129,8 @@ static void nvme_subsys_realize(DeviceState *dev, Error **errp)
 
 static Property nvme_subsystem_props[] = {
     DEFINE_PROP_STRING("nqn", NvmeSubsystem, params.nqn),
-    DEFINE_PROP_END_OF_LIST(),
-};
+    DEFINE_PROP_BOOL("ocp", NvmeSubsystem, params.ocp, false),
+    DEFINE_PROP_END_OF_LIST(), };
 
 static void nvme_subsys_class_init(ObjectClass *oc, void *data)
 {
-- 
2.30.2
Re: [PATCH v2 2/3] nvme: Add ocp to the subsys
Posted by Klaus Jensen 3 years, 2 months ago
On Nov 14 14:50, Joel Granados wrote:
> The Open Compute Project defines a Datacenter NVMe SSD Spec that sits on
> top of the NVMe spec. Additional commands and NVMe behaviors specific for
> the Datacenter. This is a preparation patch that introduces an argument to
> activate OCP in nvme.
> 
> Signed-off-by: Joel Granados <j.granados@samsung.com>
> ---
>  hw/nvme/nvme.h   | 1 +
>  hw/nvme/subsys.c | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
> index 79f5c281c2..aa99c0c57c 100644
> --- a/hw/nvme/nvme.h
> +++ b/hw/nvme/nvme.h
> @@ -56,6 +56,7 @@ typedef struct NvmeSubsystem {
>  
>      struct {
>          char *nqn;
> +        bool ocp;
>      } params;
>  } NvmeSubsystem;
>  
> diff --git a/hw/nvme/subsys.c b/hw/nvme/subsys.c
> index 9d2643678b..ecca28449c 100644
> --- a/hw/nvme/subsys.c
> +++ b/hw/nvme/subsys.c
> @@ -129,8 +129,8 @@ static void nvme_subsys_realize(DeviceState *dev, Error **errp)
>  
>  static Property nvme_subsystem_props[] = {
>      DEFINE_PROP_STRING("nqn", NvmeSubsystem, params.nqn),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> +    DEFINE_PROP_BOOL("ocp", NvmeSubsystem, params.ocp, false),

It is the controller that implements the OCP specification, not the
namespace or the subsystem. The parameter should be on the controller
device.

We discussed that the Get Log Page was subsystem scoped and not
namespace scoped, but that is unrelated to this.

> +    DEFINE_PROP_END_OF_LIST(), };
>  
>  static void nvme_subsys_class_init(ObjectClass *oc, void *data)
>  {
> -- 
> 2.30.2
> 
> 
Re: [PATCH v2 2/3] nvme: Add ocp to the subsys
Posted by Joel Granados 3 years, 2 months ago
On Tue, Nov 15, 2022 at 12:11:50PM +0100, Klaus Jensen wrote:
> On Nov 14 14:50, Joel Granados wrote:
> > The Open Compute Project defines a Datacenter NVMe SSD Spec that sits on
> > top of the NVMe spec. Additional commands and NVMe behaviors specific for
> > the Datacenter. This is a preparation patch that introduces an argument to
> > activate OCP in nvme.
> > 
> > Signed-off-by: Joel Granados <j.granados@samsung.com>
> > ---
> >  hw/nvme/nvme.h   | 1 +
> >  hw/nvme/subsys.c | 4 ++--
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
> > index 79f5c281c2..aa99c0c57c 100644
> > --- a/hw/nvme/nvme.h
> > +++ b/hw/nvme/nvme.h
> > @@ -56,6 +56,7 @@ typedef struct NvmeSubsystem {
> >  
> >      struct {
> >          char *nqn;
> > +        bool ocp;
> >      } params;
> >  } NvmeSubsystem;
> >  
> > diff --git a/hw/nvme/subsys.c b/hw/nvme/subsys.c
> > index 9d2643678b..ecca28449c 100644
> > --- a/hw/nvme/subsys.c
> > +++ b/hw/nvme/subsys.c
> > @@ -129,8 +129,8 @@ static void nvme_subsys_realize(DeviceState *dev, Error **errp)
> >  
> >  static Property nvme_subsystem_props[] = {
> >      DEFINE_PROP_STRING("nqn", NvmeSubsystem, params.nqn),
> > -    DEFINE_PROP_END_OF_LIST(),
> > -};
> > +    DEFINE_PROP_BOOL("ocp", NvmeSubsystem, params.ocp, false),
> 
> It is the controller that implements the OCP specification, not the
> namespace or the subsystem. The parameter should be on the controller
> device.
Makes sense. I'll put the option in hw/nvme/ctrl.c

> 
> We discussed that the Get Log Page was subsystem scoped and not
> namespace scoped, but that is unrelated to this.
Yep, this was the confusion. Thx for clarifying.

> 
> > +    DEFINE_PROP_END_OF_LIST(), };
> >  
> >  static void nvme_subsys_class_init(ObjectClass *oc, void *data)
> >  {
> > -- 
> > 2.30.2
> > 
> >