Re: [PATCH v3 01/15] hw/block/nvme: Define 64 bit cqe.result

[Klaus Jensen]

On Sep 15 20:44, Dmitry Fomichev wrote:
> > -----Original Message-----
> > From: Klaus Jensen <its@irrelevant.dk>
> > Sent: Tuesday, September 15, 2020 3:56 PM
> > To: Dmitry Fomichev <Dmitry.Fomichev@wdc.com>
> > Cc: Fam Zheng <fam@euphon.net>; Kevin Wolf <kwolf@redhat.com>;
> > Damien Le Moal <Damien.LeMoal@wdc.com>; qemu-block@nongnu.org;
> > Niklas Cassel <Niklas.Cassel@wdc.com>; Klaus Jensen
> > <k.jensen@samsung.com>; qemu-devel@nongnu.org; Alistair Francis
> > <Alistair.Francis@wdc.com>; Keith Busch <kbusch@kernel.org>; Philippe
> > Mathieu-Daudé <philmd@redhat.com>; Matias Bjorling
> > <Matias.Bjorling@wdc.com>
> > Subject: Re: [PATCH v3 01/15] hw/block/nvme: Define 64 bit cqe.result
> > 
> > On Sep 15 18:56, Dmitry Fomichev wrote:
> > > > -----Original Message-----
> > > > From: Klaus Jensen <its@irrelevant.dk>
> > > > Sent: Tuesday, September 15, 2020 3:37 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>; Niklas Cassel
> > > > <Niklas.Cassel@wdc.com>; Damien Le Moal
> > <Damien.LeMoal@wdc.com>;
> > > > qemu-block@nongnu.org; qemu-devel@nongnu.org; Alistair Francis
> > > > <Alistair.Francis@wdc.com>; Matias Bjorling <Matias.Bjorling@wdc.com>
> > > > Subject: Re: [PATCH v3 01/15] hw/block/nvme: Define 64 bit cqe.result
> > > >
> > > > On Sep 14 07:14, Dmitry Fomichev wrote:
> > > > > From: Ajay Joshi <ajay.joshi@wdc.com>
> > > > >
> > > > > A new write command, Zone Append, is added as a part of Zoned
> > > > > Namespace Command Set. Upon successful completion of this
> > command,
> > > > > the controller returns the start LBA of the performed write operation
> > > > > in cqe.result field. Therefore, the maximum size of this variable
> > > > > needs to be changed from 32 to 64 bit, consuming the reserved 32 bit
> > > > > field that follows the result in CQE struct. Since the existing
> > > > > commands are expected to return a 32 bit LE value, two separate
> > > > > variables, result32 and result64, are now kept in a union.
> > > > >
> > > > > Signed-off-by: Ajay Joshi <ajay.joshi@wdc.com>
> > > > > Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> > > > > Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
> > > >
> > > > I know that I R-b'ed this, but can this be moved to the namespace types
> > > > patch, since that is the TP that changes this.
> > >
> > > You probably meant the ZNS patch since result64 is first used there to
> > return
> > > ZA starting data LBA. Sure, I can move this stuff to that patch.
> > >
> > 
> > No, I actually did mean the NST patch since TP 4056 is the TP that
> > "unreserves" dw1 in the CQE.
> 
> It is not necessary to change it in NST patch since result64 field is not used
> in that patch. But if you insist, I can move it to NST patch :)

You are right of course, but since it is a change to the "spec" related
data structures that go into include/block/nvme.h, I think it belongs in
"hw/block/nvme: Introduce the Namespace Types definitions".

signature.asc
Description: PGP signature