RE: [PATCH v6 05/11] hw/block/nvme: Support Zoned Namespace Command Set

[Dmitry Fomichev <Dmitry.Fomichev@wdc.com>]

> -----Original Message-----
> From: Niklas Cassel <Niklas.Cassel@wdc.com>
> Sent: Wednesday, October 14, 2020 7:59 AM
> To: Dmitry Fomichev <Dmitry.Fomichev@wdc.com>
> Cc: Keith Busch <kbusch@kernel.org>; Klaus Jensen
> <k.jensen@samsung.com>; Kevin Wolf <kwolf@redhat.com>; Philippe
> Mathieu-Daudé <philmd@redhat.com>; Maxim Levitsky
> <mlevitsk@redhat.com>; Fam Zheng <fam@euphon.net>; Alistair Francis
> <Alistair.Francis@wdc.com>; Matias Bjorling <Matias.Bjorling@wdc.com>;
> Damien Le Moal <Damien.LeMoal@wdc.com>; qemu-block@nongnu.org;
> qemu-devel@nongnu.org
> Subject: Re: [PATCH v6 05/11] hw/block/nvme: Support Zoned Namespace
> Command Set
> 
> On Wed, Oct 14, 2020 at 06:42:06AM +0900, Dmitry Fomichev wrote:
> > The emulation code has been changed to advertise NVM Command Set
> when
> > "zoned" device property is not set (default) and Zoned Namespace
> > Command Set otherwise.
> >
> > Define values and structures that are needed to support Zoned
> > Namespace Command Set (NVMe TP 4053) in PCI NVMe controller
> emulator.
> > Define trace events where needed in newly introduced code.
> >
> > 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.
> >
> > Handlers for three new NVMe commands introduced in Zoned Namespace
> > Command Set specification are added, namely for Zone Management
> > Receive, Zone Management Send and Zone Append.
> >
> > Device initialization code has been extended to create a proper
> > configuration for zoned operation using device properties.
> >
> > Read/Write command handler is modified to only allow writes at the
> > write pointer if the namespace is zoned. For Zone Append command,
> > writes implicitly happen at the write pointer and the starting write
> > pointer value is returned as the result of the command. Write Zeroes
> > handler is modified to add zoned checks that are identical to those
> > done as a part of Write flow.
> >
> > Subsequent commits in this series add ZDE support and checks for
> > active and open zone limits.
> >
> > 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: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> > Signed-off-by: Matias Bjorling <matias.bjorling@wdc.com>
> > Signed-off-by: Aravind Ramesh <aravind.ramesh@wdc.com>
> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> > Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
> > Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> 
> (snip)
> 
> > @@ -2260,6 +3155,11 @@ static void nvme_select_ns_iocs(NvmeCtrl *n)
> >                  ns->iocs = nvme_cse_iocs_nvm;
> >              }
> >              break;
> > +        case NVME_CSI_ZONED:
> > +            if (NVME_CC_CSS(n->bar.cc) == NVME_CC_CSS_CSI) {
> > +                ns->iocs = nvme_cse_iocs_zoned;
> > +            }
> > +            break;
> >          }
> >      }
> >  }
> 
> Who knows how this whole command set mess is supposed to work,
> since e.g. the Key Value Command Set assigns opcodes for new commands
> (Delete, Exist) with a opcode values (0x10,0x14) smaller than the
> current highest opcode value (0x15) in the NVM Command Set,
> while those opcodes (0x10,0x14) are reserved in the NVM Command Set.
> 
> At least for Zoned Command Set, they defined the new commands
> (Zone Mgmt Send, Zone Mgmt Recv) to opcode values (0x79,0x7a)
> that are higher than the current highest opcode value in the
> NVM Command Set.
> 
> So since we know that the Zoned Command Set is a strict superset of
> the NVM Command Set, I guess it might be nice to do something like:
> 
> case NVME_CSI_ZONED:
>     if (NVME_CC_CSS(n->bar.cc) == NVME_CC_CSS_CSI) {
>         ns->iocs = nvme_cse_iocs_zoned;
>     } else if (NVME_CC_CSS(n->bar.cc) == NVME_CC_CSS_NVM) {
>         ns->iocs = nvme_cse_iocs_nvm;
>     }
>     break;
> 
> 
> Since I assume that the spec people intended reads/writes
> to a ZNS namespace to still be possible when CC_CSS == NVM,
> but who knows?

Yes, I think it should be this way, thanks. Now it is matched with what
CSE log reports in this case.

> 
> 
> Kind regards,
> Niklas