> -----Original Message----- > From: Klaus Jensen <i...@irrelevant.dk> > Sent: Friday, September 18, 2020 5:20 PM > To: Dmitry Fomichev <dmitry.fomic...@wdc.com> > Cc: Keith Busch <kbu...@kernel.org>; Klaus Jensen > <k.jen...@samsung.com>; Kevin Wolf <kw...@redhat.com>; Philippe > Mathieu-Daudé <phi...@redhat.com>; Maxim Levitsky > <mlevi...@redhat.com>; Fam Zheng <f...@euphon.net>; Niklas Cassel > <niklas.cas...@wdc.com>; Damien Le Moal <damien.lem...@wdc.com>; > qemu-bl...@nongnu.org; qemu-devel@nongnu.org; Alistair Francis > <alistair.fran...@wdc.com>; Matias Bjorling <matias.bjorl...@wdc.com> > Subject: Re: [PATCH v3 10/15] hw/block/nvme: Support Zoned Namespace > Command Set > > On Sep 14 07:14, 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. > > > > 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. > > > > The code to support for Zone Descriptor Extensions is not included in > > this commit and ZDES 0 is always reported. A later commit in this > > series will add ZDE support. > > > > This commit doesn't yet include checks for active and open zone > > limits. It is assumed that there are no limits on either active or > > open zones. > > > > Signed-off-by: Niklas Cassel <niklas.cas...@wdc.com> > > Signed-off-by: Hans Holmberg <hans.holmb...@wdc.com> > > Signed-off-by: Ajay Joshi <ajay.jo...@wdc.com> > > Signed-off-by: Chaitanya Kulkarni <chaitanya.kulka...@wdc.com> > > Signed-off-by: Matias Bjorling <matias.bjorl...@wdc.com> > > Signed-off-by: Aravind Ramesh <aravind.ram...@wdc.com> > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawas...@wdc.com> > > Signed-off-by: Adam Manzanares <adam.manzana...@wdc.com> > > Signed-off-by: Dmitry Fomichev <dmitry.fomic...@wdc.com> > > --- > > hw/block/nvme.c | 968 > ++++++++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 939 insertions(+), 29 deletions(-) > > > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > > index c740cbcf1f..0bf7471815 100644 > > --- a/hw/block/nvme.c > > +++ b/hw/block/nvme.c > > @@ -709,11 +965,77 @@ static uint16_t nvme_rw(NvmeCtrl *n, > NvmeRequest *req) > > return status; > > } > > > > + if (n->params.zoned) { > > + zone_idx = nvme_zone_idx(n, slba); > > + assert(zone_idx < n->num_zones); > > + zone = &ns->zone_array[zone_idx]; > > + > > + if (is_write) { > > + status = nvme_check_zone_write(zone, slba, nlb); > > + if (status != NVME_SUCCESS) { > > + trace_pci_nvme_err_zone_write_not_ok(slba, nlb, status); > > + return status | NVME_DNR; > > + } > > + > > + assert(nvme_wp_is_valid(zone)); > > + if (append) { > > + if (unlikely(slba != zone->d.zslba)) { > > + trace_pci_nvme_err_append_not_at_start(slba, zone- > >d.zslba); > > + return NVME_ZONE_INVALID_WRITE | NVME_DNR; > > + } > > + if (data_size > (n->page_size << n->zasl)) { > > + trace_pci_nvme_err_append_too_large(slba, nlb, > > n->zasl); > > + return NVME_INVALID_FIELD | NVME_DNR; > > + } > > + slba = zone->d.wp; > > Your append is broken. > > Since you moved the write pointer update to post aio completion (which I > totally agree with), you are now assigning the same SLBA to all appends > that are in queue when nvme_process_sq is invoked.
Yes, these newer changes to update the WP in aio callback turned out to be buggy. Will fix this in the next version.