Re: [PATCH v2 08/18] hw/block/nvme: Make Zoned NS Command Set definitions

From: Klaus Jensen <its@irrelevant.dk>
To: Dmitry Fomichev <dmitry.fomichev@wdc.com>
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Niklas Cassel" <niklas.cassel@wdc.com>,
	"Damien Le Moal" <damien.lemoal@wdc.com>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org,
	"Keith Busch" <kbusch@kernel.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Matias Bjorling" <matias.bjorling@wdc.com>
Subject: Re: [PATCH v2 08/18] hw/block/nvme: Make Zoned NS Command Set definitions
Date: Tue, 30 Jun 2020 14:08:01 +0200	[thread overview]
Message-ID: <20200630120801.hvy736qj2y2ogiiq@apples.localdomain> (raw)
In-Reply-To: <20200630114448.pspldgtlxrzjxps3@apples.localdomain>

On Jun 30 13:44, Klaus Jensen wrote:
> On Jun 18 06:34, Dmitry Fomichev wrote:
> > Define values and structures that are needed to support Zoned
> > Namespace Command Set (NVMe TP 4053) in PCI NVMe controller emulator.
> > 
> > All new protocol definitions are located in include/block/nvme.h
> > and everything added that is specific to this implementation is kept
> > in hw/block/nvme.h.
> > 
> > In order to improve scalability, all open, closed and full zones
> > are organized in separate linked lists. Consequently, almost all
> > zone operations don't require scanning of the entire zone array
> > (which potentially can be quite large) - it is only necessary to
> > enumerate one or more zone lists. Zone lists are designed to be
> > position-independent as they can be persisted to the backing file
> > as a part of zone metadata. NvmeZoneList struct defined in this patch
> > serves as a head of every zone list.
> > 
> > NvmeZone structure encapsulates NvmeZoneDescriptor defined in Zoned
> > Command Set specification and adds a few more fields that are
> > internal to this implementation.
> > 
> > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
> > Signed-off-by: Ajay Joshi <ajay.joshi@wdc.com>
> > Signed-off-by: Matias Bjorling <matias.bjorling@wdc.com>
> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> > Signed-off-by: Alexey Bogoslavsky <alexey.bogoslavsky@wdc.com>
> > Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> > ---
> >  hw/block/nvme.h      | 130 +++++++++++++++++++++++++++++++++++++++++++
> >  include/block/nvme.h | 119 ++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 248 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> > index 0d29f75475..2c932b5e29 100644
> > --- a/hw/block/nvme.h
> > +++ b/hw/block/nvme.h
> > @@ -121,6 +165,86 @@ static inline uint64_t nvme_ns_nlbas(NvmeCtrl *n, NvmeNamespace *ns)
> >      return n->ns_size >> nvme_ns_lbads(ns);
> >  }
> >  
> > +static inline uint8_t nvme_get_zone_state(NvmeZone *zone)
> > +{
> > +    return zone->d.zs >> 4;
> > +}
> > +
> > +static inline void nvme_set_zone_state(NvmeZone *zone, enum NvmeZoneState state)
> > +{
> > +    zone->d.zs = state << 4;
> > +}
> > +
> > +static inline uint64_t nvme_zone_rd_boundary(NvmeCtrl *n, NvmeZone *zone)
> > +{
> > +    return zone->d.zslba + n->params.zone_size;
> > +}
> > +
> > +static inline uint64_t nvme_zone_wr_boundary(NvmeZone *zone)
> > +{
> > +    return zone->d.zslba + zone->d.zcap;
> > +}
> 
> Everything working on zone->d needs leXX_to_cpu() conversions.

Disregard this. I see from the following patches that you keep zone->d
in cpu endianess and convert on zone management receive.

Sorry!