| Message ID | 20200930220414.562527-1-kbusch@kernel.org |
|---|---|
| Headers | show |
| Series | nvme qemu cleanups and fixes | expand |
> -----Original Message----- > From: Keith Busch <kbusch@kernel.org> > Sent: Wednesday, September 30, 2020 6:04 PM > To: qemu-block@nongnu.org; qemu-devel@nongnu.org; Klaus Jensen > <k.jensen@samsung.com> > Cc: Niklas Cassel <Niklas.Cassel@wdc.com>; Dmitry Fomichev > <Dmitry.Fomichev@wdc.com>; Kevin Wolf <kwolf@redhat.com>; Philippe > Mathieu-Daudé <philmd@redhat.com>; Keith Busch <kbusch@kernel.org> > Subject: [PATCH 6/9] hw/block/nvme: reject io commands if only admin > command set selected > > From: Klaus Jensen <k.jensen@samsung.com> > > If the host sets CC.CSS to 111b, all commands submitted to I/O queues > should be completed with status Invalid Command Opcode. > > Note that this is technically a v1.4 feature, but it does not hurt to > implement before we finally bump the reported version implemented. > > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > Signed-off-by: Keith Busch <kbusch@kernel.org> Reviewed-by: Dmitry Fomichev <dmitry.fomichev@wdc.com> > --- > hw/block/nvme.c | 4 ++++ > include/block/nvme.h | 5 +++++ > 2 files changed, 9 insertions(+) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index ec7363ea40..80730e1c03 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -1026,6 +1026,10 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, > NvmeRequest *req) > trace_pci_nvme_io_cmd(nvme_cid(req), nsid, nvme_sqid(req), > req->cmd.opcode, nvme_io_opc_str(req->cmd.opcode)); > > + if (NVME_CC_CSS(n->bar.cc) == NVME_CC_CSS_ADMIN_ONLY) { > + return NVME_INVALID_OPCODE | NVME_DNR; > + } > + > if (!nvme_nsid_valid(n, nsid)) { > return NVME_INVALID_NSID | NVME_DNR; > } > diff --git a/include/block/nvme.h b/include/block/nvme.h > index 521533fd2a..6de2d5aa75 100644 > --- a/include/block/nvme.h > +++ b/include/block/nvme.h > @@ -115,6 +115,11 @@ enum NvmeCcMask { > #define NVME_CC_IOSQES(cc) ((cc >> CC_IOSQES_SHIFT) & > CC_IOSQES_MASK) > #define NVME_CC_IOCQES(cc) ((cc >> CC_IOCQES_SHIFT) & > CC_IOCQES_MASK) > > +enum NvmeCcCss { > + NVME_CC_CSS_NVM = 0x0, > + NVME_CC_CSS_ADMIN_ONLY = 0x7, > +}; > + > enum NvmeCstsShift { > CSTS_RDY_SHIFT = 0, > CSTS_CFS_SHIFT = 1, > -- > 2.24.1
> -----Original Message----- > From: Keith Busch <kbusch@kernel.org> > Sent: Wednesday, September 30, 2020 6:04 PM > To: qemu-block@nongnu.org; qemu-devel@nongnu.org; Klaus Jensen > <k.jensen@samsung.com> > Cc: Niklas Cassel <Niklas.Cassel@wdc.com>; Dmitry Fomichev > <Dmitry.Fomichev@wdc.com>; Kevin Wolf <kwolf@redhat.com>; Philippe > Mathieu-Daudé <philmd@redhat.com>; Keith Busch <kbusch@kernel.org> > Subject: [PATCH 5/9] hw/block/nvme: support for admin-only command set > > Signed-off-by: Keith Busch <kbusch@kernel.org> > --- > hw/block/nvme.c | 1 + > include/block/nvme.h | 3 ++- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 6c582e6874..ec7363ea40 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -2755,6 +2755,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice > *pci_dev) > NVME_CAP_SET_CQR(n->bar.cap, 1); > NVME_CAP_SET_TO(n->bar.cap, 0xf); > NVME_CAP_SET_CSS(n->bar.cap, NVME_CAP_CSS_NVM); > + NVME_CAP_SET_CSS(n->bar.cap, NVME_CAP_CSS_ADMIN_ONLY); This could be - NVME_CAP_SET_CSS(n->bar.cap, NVME_CAP_CSS_NVM); + NVME_CAP_SET_CSS(n->bar.cap, (NVME_CAP_CSS_NVM | NVME_CAP_CSS_ADMIN_ONLY)); Unfortunately, parentheses are needed above because NVME_CAP_SET_CSS macro and other similar macros use "val" instead of (val). A possible cleanup topic... > NVME_CAP_SET_MPSMAX(n->bar.cap, 4); > > n->bar.vs = NVME_SPEC_VER; > diff --git a/include/block/nvme.h b/include/block/nvme.h > index bc20a2ba5e..521533fd2a 100644 > --- a/include/block/nvme.h > +++ b/include/block/nvme.h > @@ -83,7 +83,8 @@ enum NvmeCapMask { > << CAP_PMR_SHIFT) > > enum NvmeCapCss { > - NVME_CAP_CSS_NVM = 1 << 0, > + NVME_CAP_CSS_NVM = 1 << 0, > + NVME_CAP_CSS_ADMIN_ONLY = 1 << 7, > }; > > enum NvmeCcShift { > -- > 2.24.1
On Sep 30 15:04, Keith Busch wrote: > The code switches on the opcode to invoke a function specific to that > opcode. There's no point in consolidating back to a common function that > just switches on that same opcode without any actual common code. > Restore the opcode specific behavior without going back through another > level of switches. > > Signed-off-by: Keith Busch <kbusch@kernel.org> Reviewed-by: Klaus Jensen <k.jensen@samsung.com> Point taken. I could've sweared I had a better reason for this. > --- > hw/block/nvme.c | 91 ++++++++++++++++--------------------------------- > 1 file changed, 29 insertions(+), 62 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index da8344f196..db52ea0db9 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -927,68 +927,12 @@ static void nvme_rw_cb(void *opaque, int ret) > nvme_enqueue_req_completion(nvme_cq(req), req); > } > > -static uint16_t nvme_do_aio(BlockBackend *blk, int64_t offset, size_t len, > - NvmeRequest *req) > -{ > - BlockAcctCookie *acct = &req->acct; > - BlockAcctStats *stats = blk_get_stats(blk); > - > - bool is_write = false; > - > - trace_pci_nvme_do_aio(nvme_cid(req), req->cmd.opcode, > - nvme_io_opc_str(req->cmd.opcode), blk_name(blk), > - offset, len); > - > - switch (req->cmd.opcode) { > - case NVME_CMD_FLUSH: > - block_acct_start(stats, acct, 0, BLOCK_ACCT_FLUSH); > - req->aiocb = blk_aio_flush(blk, nvme_rw_cb, req); > - break; > - > - case NVME_CMD_WRITE_ZEROES: > - block_acct_start(stats, acct, len, BLOCK_ACCT_WRITE); > - req->aiocb = blk_aio_pwrite_zeroes(blk, offset, len, > - BDRV_REQ_MAY_UNMAP, nvme_rw_cb, > - req); > - break; > - > - case NVME_CMD_WRITE: > - is_write = true; > - > - /* fallthrough */ > - > - case NVME_CMD_READ: > - block_acct_start(stats, acct, len, > - is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ); > - > - if (req->qsg.sg) { > - if (is_write) { > - req->aiocb = dma_blk_write(blk, &req->qsg, offset, > - BDRV_SECTOR_SIZE, nvme_rw_cb, req); > - } else { > - req->aiocb = dma_blk_read(blk, &req->qsg, offset, > - BDRV_SECTOR_SIZE, nvme_rw_cb, req); > - } > - } else { > - if (is_write) { > - req->aiocb = blk_aio_pwritev(blk, offset, &req->iov, 0, > - nvme_rw_cb, req); > - } else { > - req->aiocb = blk_aio_preadv(blk, offset, &req->iov, 0, > - nvme_rw_cb, req); > - } > - } > - > - break; > - } > - > - return NVME_NO_COMPLETE; > -} > - > static uint16_t nvme_flush(NvmeCtrl *n, NvmeRequest *req) > { > - NvmeNamespace *ns = req->ns; > - return nvme_do_aio(ns->blkconf.blk, 0, 0, req); > + block_acct_start(blk_get_stats(n->conf.blk), &req->acct, 0, > + BLOCK_ACCT_FLUSH); > + req->aiocb = blk_aio_flush(n->conf.blk, nvme_rw_cb, req); > + return NVME_NO_COMPLETE; > } > > static uint16_t nvme_write_zeroes(NvmeCtrl *n, NvmeRequest *req) > @@ -1009,7 +953,11 @@ static uint16_t nvme_write_zeroes(NvmeCtrl *n, NvmeRequest *req) > return status; > } > > - return nvme_do_aio(ns->blkconf.blk, offset, count, req); > + block_acct_start(blk_get_stats(n->conf.blk), &req->acct, 0, > + BLOCK_ACCT_WRITE); > + req->aiocb = blk_aio_pwrite_zeroes(n->conf.blk, offset, count, > + BDRV_REQ_MAY_UNMAP, nvme_rw_cb, req); > + return NVME_NO_COMPLETE; > } > > static uint16_t nvme_rw(NvmeCtrl *n, NvmeRequest *req) > @@ -1023,6 +971,7 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeRequest *req) > uint64_t data_offset = nvme_l2b(ns, slba); > enum BlockAcctType acct = req->cmd.opcode == NVME_CMD_WRITE ? > BLOCK_ACCT_WRITE : BLOCK_ACCT_READ; > + BlockBackend *blk = ns->blkconf.blk; > uint16_t status; > > trace_pci_nvme_rw(nvme_cid(req), nvme_io_opc_str(rw->opcode), > @@ -1045,7 +994,25 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeRequest *req) > goto invalid; > } > > - return nvme_do_aio(ns->blkconf.blk, data_offset, data_size, req); > + block_acct_start(blk_get_stats(blk), &req->acct, data_size, acct); > + if (req->qsg.sg) { > + if (acct == BLOCK_ACCT_WRITE) { > + req->aiocb = dma_blk_write(blk, &req->qsg, data_offset, > + BDRV_SECTOR_SIZE, nvme_rw_cb, req); > + } else { > + req->aiocb = dma_blk_read(blk, &req->qsg, data_offset, > + BDRV_SECTOR_SIZE, nvme_rw_cb, req); > + } > + } else { > + if (acct == BLOCK_ACCT_WRITE) { > + req->aiocb = blk_aio_pwritev(blk, data_offset, &req->iov, 0, > + nvme_rw_cb, req); > + } else { > + req->aiocb = blk_aio_preadv(blk, data_offset, &req->iov, 0, > + nvme_rw_cb, req); > + } > + } > + return NVME_NO_COMPLETE; > > invalid: > block_acct_invalid(blk_get_stats(ns->blkconf.blk), acct); > -- > 2.24.1 > >
On Sep 30 15:04, Keith Busch wrote: > Signed-off-by: Keith Busch <kbusch@kernel.org> > --- Reviewed-by: Klaus Jensen <k.jensen@samsung.com> > hw/block/nvme.c | 1 + > include/block/nvme.h | 3 ++- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 6c582e6874..ec7363ea40 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -2755,6 +2755,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev) > NVME_CAP_SET_CQR(n->bar.cap, 1); > NVME_CAP_SET_TO(n->bar.cap, 0xf); > NVME_CAP_SET_CSS(n->bar.cap, NVME_CAP_CSS_NVM); > + NVME_CAP_SET_CSS(n->bar.cap, NVME_CAP_CSS_ADMIN_ONLY); > NVME_CAP_SET_MPSMAX(n->bar.cap, 4); > > n->bar.vs = NVME_SPEC_VER; > diff --git a/include/block/nvme.h b/include/block/nvme.h > index bc20a2ba5e..521533fd2a 100644 > --- a/include/block/nvme.h > +++ b/include/block/nvme.h > @@ -83,7 +83,8 @@ enum NvmeCapMask { > << CAP_PMR_SHIFT) > > enum NvmeCapCss { > - NVME_CAP_CSS_NVM = 1 << 0, > + NVME_CAP_CSS_NVM = 1 << 0, > + NVME_CAP_CSS_ADMIN_ONLY = 1 << 7, > }; > > enum NvmeCcShift { > -- > 2.24.1 > >
On Oct 1 06:05, Klaus Jensen wrote: > On Sep 30 15:04, Keith Busch wrote: > > The code switches on the opcode to invoke a function specific to that > > opcode. There's no point in consolidating back to a common function that > > just switches on that same opcode without any actual common code. > > Restore the opcode specific behavior without going back through another > > level of switches. > > > > Signed-off-by: Keith Busch <kbusch@kernel.org> > > Reviewed-by: Klaus Jensen <k.jensen@samsung.com> > > Point taken. I could've sweared I had a better reason for this. > Can you also remove the nvme_do_aio trace event?
On Thu, Oct 01, 2020 at 10:48:03AM +0200, Klaus Jensen wrote: > On Oct 1 06:05, Klaus Jensen wrote: > > On Sep 30 15:04, Keith Busch wrote: > > > The code switches on the opcode to invoke a function specific to that > > > opcode. There's no point in consolidating back to a common function that > > > just switches on that same opcode without any actual common code. > > > Restore the opcode specific behavior without going back through another > > > level of switches. > > > > > > Signed-off-by: Keith Busch <kbusch@kernel.org> > > > > Reviewed-by: Klaus Jensen <k.jensen@samsung.com> > > > > Point taken. I could've sweared I had a better reason for this. > > > > Can you also remove the nvme_do_aio trace event? Ah, thanks for pointing that out. I'll fix up the trace. I think you may have a case where this might make sense still in the making? If so we can reevaluate when it's ready.
On Oct 1 06:05, Klaus Jensen wrote: > On Sep 30 15:04, Keith Busch wrote: > > The code switches on the opcode to invoke a function specific to that > > opcode. There's no point in consolidating back to a common function that > > just switches on that same opcode without any actual common code. > > Restore the opcode specific behavior without going back through another > > level of switches. > > > > Signed-off-by: Keith Busch <kbusch@kernel.org> > > Reviewed-by: Klaus Jensen <k.jensen@samsung.com> > > Point taken. I could've sweared I had a better reason for this. > > > --- > > hw/block/nvme.c | 91 ++++++++++++++++--------------------------------- > > 1 file changed, 29 insertions(+), 62 deletions(-) > > > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > > index da8344f196..db52ea0db9 100644 > > --- a/hw/block/nvme.c > > +++ b/hw/block/nvme.c > > @@ -927,68 +927,12 @@ static void nvme_rw_cb(void *opaque, int ret) > > nvme_enqueue_req_completion(nvme_cq(req), req); > > } > > > > -static uint16_t nvme_do_aio(BlockBackend *blk, int64_t offset, size_t len, > > - NvmeRequest *req) > > -{ > > - BlockAcctCookie *acct = &req->acct; > > - BlockAcctStats *stats = blk_get_stats(blk); > > - > > - bool is_write = false; > > - > > - trace_pci_nvme_do_aio(nvme_cid(req), req->cmd.opcode, > > - nvme_io_opc_str(req->cmd.opcode), blk_name(blk), > > - offset, len); > > - > > - switch (req->cmd.opcode) { > > - case NVME_CMD_FLUSH: > > - block_acct_start(stats, acct, 0, BLOCK_ACCT_FLUSH); > > - req->aiocb = blk_aio_flush(blk, nvme_rw_cb, req); > > - break; > > - > > - case NVME_CMD_WRITE_ZEROES: > > - block_acct_start(stats, acct, len, BLOCK_ACCT_WRITE); > > - req->aiocb = blk_aio_pwrite_zeroes(blk, offset, len, > > - BDRV_REQ_MAY_UNMAP, nvme_rw_cb, > > - req); > > - break; > > - > > - case NVME_CMD_WRITE: > > - is_write = true; > > - > > - /* fallthrough */ > > - > > - case NVME_CMD_READ: > > - block_acct_start(stats, acct, len, > > - is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ); > > - > > - if (req->qsg.sg) { > > - if (is_write) { > > - req->aiocb = dma_blk_write(blk, &req->qsg, offset, > > - BDRV_SECTOR_SIZE, nvme_rw_cb, req); > > - } else { > > - req->aiocb = dma_blk_read(blk, &req->qsg, offset, > > - BDRV_SECTOR_SIZE, nvme_rw_cb, req); > > - } > > - } else { > > - if (is_write) { > > - req->aiocb = blk_aio_pwritev(blk, offset, &req->iov, 0, > > - nvme_rw_cb, req); > > - } else { > > - req->aiocb = blk_aio_preadv(blk, offset, &req->iov, 0, > > - nvme_rw_cb, req); > > - } > > - } > > - > > - break; > > - } > > - > > - return NVME_NO_COMPLETE; > > -} > > - > > static uint16_t nvme_flush(NvmeCtrl *n, NvmeRequest *req) > > { > > - NvmeNamespace *ns = req->ns; > > - return nvme_do_aio(ns->blkconf.blk, 0, 0, req); > > + block_acct_start(blk_get_stats(n->conf.blk), &req->acct, 0, Uh, ouch! This and the rest needs to be changed to ns->blkconf.blk and not n->conf.blk. > > + BLOCK_ACCT_FLUSH); > > + req->aiocb = blk_aio_flush(n->conf.blk, nvme_rw_cb, req); > > + return NVME_NO_COMPLETE; > > } > > > > static uint16_t nvme_write_zeroes(NvmeCtrl *n, NvmeRequest *req) > > @@ -1009,7 +953,11 @@ static uint16_t nvme_write_zeroes(NvmeCtrl *n, NvmeRequest *req) > > return status; > > } > > > > - return nvme_do_aio(ns->blkconf.blk, offset, count, req); > > + block_acct_start(blk_get_stats(n->conf.blk), &req->acct, 0, > > + BLOCK_ACCT_WRITE); > > + req->aiocb = blk_aio_pwrite_zeroes(n->conf.blk, offset, count, > > + BDRV_REQ_MAY_UNMAP, nvme_rw_cb, req); > > + return NVME_NO_COMPLETE; > > } > > > > static uint16_t nvme_rw(NvmeCtrl *n, NvmeRequest *req) > > @@ -1023,6 +971,7 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeRequest *req) > > uint64_t data_offset = nvme_l2b(ns, slba); > > enum BlockAcctType acct = req->cmd.opcode == NVME_CMD_WRITE ? > > BLOCK_ACCT_WRITE : BLOCK_ACCT_READ; > > + BlockBackend *blk = ns->blkconf.blk; > > uint16_t status; > > > > trace_pci_nvme_rw(nvme_cid(req), nvme_io_opc_str(rw->opcode), > > @@ -1045,7 +994,25 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeRequest *req) > > goto invalid; > > } > > > > - return nvme_do_aio(ns->blkconf.blk, data_offset, data_size, req); > > + block_acct_start(blk_get_stats(blk), &req->acct, data_size, acct); > > + if (req->qsg.sg) { > > + if (acct == BLOCK_ACCT_WRITE) { > > + req->aiocb = dma_blk_write(blk, &req->qsg, data_offset, > > + BDRV_SECTOR_SIZE, nvme_rw_cb, req); > > + } else { > > + req->aiocb = dma_blk_read(blk, &req->qsg, data_offset, > > + BDRV_SECTOR_SIZE, nvme_rw_cb, req); > > + } > > + } else { > > + if (acct == BLOCK_ACCT_WRITE) { > > + req->aiocb = blk_aio_pwritev(blk, data_offset, &req->iov, 0, > > + nvme_rw_cb, req); > > + } else { > > + req->aiocb = blk_aio_preadv(blk, data_offset, &req->iov, 0, > > + nvme_rw_cb, req); > > + } > > + } > > + return NVME_NO_COMPLETE; > > > > invalid: > > block_acct_invalid(blk_get_stats(ns->blkconf.blk), acct); > > -- > > 2.24.1 > > > > > > -- > One of us - No more doubt, silence or taboo about mental illness.
On Sep 30 15:04, Keith Busch wrote: > After going through the zns enabling, I notice the controller enabling > is not correct. Then I just continued maked more stuff. The series, I > think, contains some of the less controversial patches from the two > conflicting zns series, preceeded by some cleanups and fixes from me. > > If this is all fine, I took the liberty of porting the zns enabling to > it and made a public branch for consideration here: > > http://git.infradead.org/qemu-nvme.git/shortlog/refs/heads/kb-zns > I rebased my patches on top of Keith's fixups as well. And in compliance with the concensus, I removed persistence. https://irrelevant.dk/g/pci-nvme.git/log/?h=zns
On Sep 30 15:04, Keith Busch wrote: > After going through the zns enabling, I notice the controller enabling > is not correct. Then I just continued maked more stuff. The series, I > think, contains some of the less controversial patches from the two > conflicting zns series, preceeded by some cleanups and fixes from me. > > If this is all fine, I took the liberty of porting the zns enabling to > it and made a public branch for consideration here: > > http://git.infradead.org/qemu-nvme.git/shortlog/refs/heads/kb-zns > > Dmitry Fomichev (1): > hw/block/nvme: report actual LBA data shift in LBAF > > Keith Busch (5): > hw/block/nvme: remove pointless rw indirection > hw/block/nvme: fix log page offset check > hw/block/nvme: support per-namespace smart log > hw/block/nvme: validate command set selected > hw/block/nvme: support for admin-only command set > > Klaus Jensen (3): > hw/block/nvme: reject io commands if only admin command set selected > hw/block/nvme: add nsid to get/setfeat trace events > hw/block/nvme: add trace event for requests with non-zero status code > > hw/block/nvme-ns.c | 5 ++ > hw/block/nvme.c | 194 ++++++++++++++++++++---------------------- > hw/block/trace-events | 6 +- > include/block/nvme.h | 11 +++ > 4 files changed, 114 insertions(+), 102 deletions(-) > > -- > 2.24.1 > > These fixes all look good to me apart from the odd fixes that has been mentioned in the reviews. Since soft freeze is only two weeks away (Oct 27th), it would be nice to get this staged on nvme-next so we can get a pull sent off to Peter.
On Tue, Oct 13, 2020 at 11:04:01AM +0200, Klaus Jensen wrote: > On Sep 30 15:04, Keith Busch wrote: > > After going through the zns enabling, I notice the controller enabling > > is not correct. Then I just continued maked more stuff. The series, I > > think, contains some of the less controversial patches from the two > > conflicting zns series, preceeded by some cleanups and fixes from me. > > > > If this is all fine, I took the liberty of porting the zns enabling to > > it and made a public branch for consideration here: > > > > http://git.infradead.org/qemu-nvme.git/shortlog/refs/heads/kb-zns > > > > Dmitry Fomichev (1): > > hw/block/nvme: report actual LBA data shift in LBAF > > > > Keith Busch (5): > > hw/block/nvme: remove pointless rw indirection > > hw/block/nvme: fix log page offset check > > hw/block/nvme: support per-namespace smart log > > hw/block/nvme: validate command set selected > > hw/block/nvme: support for admin-only command set > > > > Klaus Jensen (3): > > hw/block/nvme: reject io commands if only admin command set selected > > hw/block/nvme: add nsid to get/setfeat trace events > > hw/block/nvme: add trace event for requests with non-zero status code > > > > hw/block/nvme-ns.c | 5 ++ > > hw/block/nvme.c | 194 ++++++++++++++++++++---------------------- > > hw/block/trace-events | 6 +- > > include/block/nvme.h | 11 +++ > > 4 files changed, 114 insertions(+), 102 deletions(-) > > > > -- > > 2.24.1 > > > > > > These fixes all look good to me apart from the odd fixes that has been > mentioned in the reviews. Since soft freeze is only two weeks away (Oct > 27th), it would be nice to get this staged on nvme-next so we can get a > pull sent off to Peter. I've fixed up the comments mentioned and added the received reviews. Since it was pretty trivial fixups and passes my basic santify tests, I went ahead and pushed to nvme-next.
On Oct 13 10:48, Keith Busch wrote: > On Tue, Oct 13, 2020 at 11:04:01AM +0200, Klaus Jensen wrote: > > On Sep 30 15:04, Keith Busch wrote: > > > After going through the zns enabling, I notice the controller enabling > > > is not correct. Then I just continued maked more stuff. The series, I > > > think, contains some of the less controversial patches from the two > > > conflicting zns series, preceeded by some cleanups and fixes from me. > > > > > > If this is all fine, I took the liberty of porting the zns enabling to > > > it and made a public branch for consideration here: > > > > > > http://git.infradead.org/qemu-nvme.git/shortlog/refs/heads/kb-zns > > > > > > Dmitry Fomichev (1): > > > hw/block/nvme: report actual LBA data shift in LBAF > > > > > > Keith Busch (5): > > > hw/block/nvme: remove pointless rw indirection > > > hw/block/nvme: fix log page offset check > > > hw/block/nvme: support per-namespace smart log > > > hw/block/nvme: validate command set selected > > > hw/block/nvme: support for admin-only command set > > > > > > Klaus Jensen (3): > > > hw/block/nvme: reject io commands if only admin command set selected > > > hw/block/nvme: add nsid to get/setfeat trace events > > > hw/block/nvme: add trace event for requests with non-zero status code > > > > > > hw/block/nvme-ns.c | 5 ++ > > > hw/block/nvme.c | 194 ++++++++++++++++++++---------------------- > > > hw/block/trace-events | 6 +- > > > include/block/nvme.h | 11 +++ > > > 4 files changed, 114 insertions(+), 102 deletions(-) > > > > > > -- > > > 2.24.1 > > > > > > > > > > These fixes all look good to me apart from the odd fixes that has been > > mentioned in the reviews. Since soft freeze is only two weeks away (Oct > > 27th), it would be nice to get this staged on nvme-next so we can get a > > pull sent off to Peter. > > I've fixed up the comments mentioned and added the received reviews. > Since it was pretty trivial fixups and passes my basic santify tests, I > went ahead and pushed to nvme-next. > Looks good, thanks!