* [PATCH 0/6] hw/block/nvme: zoned misc fixes
@ 2021-01-11 12:32 Klaus Jensen
2021-01-11 12:32 ` [PATCH 1/6] hw/block/nvme: fix shutdown/reset logic Klaus Jensen
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Klaus Jensen @ 2021-01-11 12:32 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Dmitry Fomichev, Klaus Jensen, Max Reitz,
Klaus Jensen, Keith Busch
From: Klaus Jensen <k.jensen@samsung.com>
These are some follow-up patches to the just merged zoned series.
The biggest addition here is asynchronous zeroing of zones on reset.
Klaus Jensen (6):
hw/block/nvme: fix shutdown/reset logic
hw/block/nvme: merge implicitly/explicitly opened processing masks
hw/block/nvme: enum style fix
hw/block/nvme: zero out zones on reset
hw/block/nvme: add missing string representations for commands
hw/block/nvme: remove unnecessary check for append
hw/block/nvme-ns.h | 4 +-
hw/block/nvme.h | 4 +
include/block/nvme.h | 4 +-
hw/block/nvme.c | 200 +++++++++++++++++++++++++++---------------
hw/block/trace-events | 1 +
5 files changed, 140 insertions(+), 73 deletions(-)
--
2.30.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/6] hw/block/nvme: fix shutdown/reset logic 2021-01-11 12:32 [PATCH 0/6] hw/block/nvme: zoned misc fixes Klaus Jensen @ 2021-01-11 12:32 ` Klaus Jensen 2021-01-14 23:49 ` Keith Busch 2021-01-11 12:32 ` [PATCH 2/6] hw/block/nvme: merge implicitly/explicitly opened processing masks Klaus Jensen ` (5 subsequent siblings) 6 siblings, 1 reply; 11+ messages in thread From: Klaus Jensen @ 2021-01-11 12:32 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, qemu-block, Dmitry Fomichev, Klaus Jensen, Max Reitz, Klaus Jensen, Keith Busch From: Klaus Jensen <k.jensen@samsung.com> A shutdown is only about flushing stuff. It is the host that should delete any queues, so do not perform a reset here. Also, on shutdown, make sure that the PMR is flushed if in use. Fixes: 368f4e752cf9 ("hw/block/nvme: Process controller reset and shutdown differently") Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- hw/block/nvme.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 0854ee307227..cba509e90537 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -3413,7 +3413,7 @@ static void nvme_process_sq(void *opaque) } } -static void nvme_clear_ctrl(NvmeCtrl *n) +static void nvme_ctrl_reset(NvmeCtrl *n) { NvmeNamespace *ns; int i; @@ -3447,11 +3447,7 @@ static void nvme_clear_ctrl(NvmeCtrl *n) n->aer_queued = 0; n->outstanding_aers = 0; n->qs_created = false; -} -static void nvme_ctrl_reset(NvmeCtrl *n) -{ - nvme_clear_ctrl(n); n->bar.cc = 0; } @@ -3460,7 +3456,9 @@ static void nvme_ctrl_shutdown(NvmeCtrl *n) NvmeNamespace *ns; int i; - nvme_clear_ctrl(n); + if (n->pmrdev) { + memory_region_msync(&n->pmrdev->mr, 0, n->pmrdev->size); + } for (i = 1; i <= n->num_namespaces; i++) { ns = nvme_ns(n, i); @@ -4303,7 +4301,7 @@ static void nvme_exit(PCIDevice *pci_dev) NvmeNamespace *ns; int i; - nvme_ctrl_shutdown(n); + nvme_ctrl_reset(n); for (i = 1; i <= n->num_namespaces; i++) { ns = nvme_ns(n, i); -- 2.30.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/6] hw/block/nvme: fix shutdown/reset logic 2021-01-11 12:32 ` [PATCH 1/6] hw/block/nvme: fix shutdown/reset logic Klaus Jensen @ 2021-01-14 23:49 ` Keith Busch 0 siblings, 0 replies; 11+ messages in thread From: Keith Busch @ 2021-01-14 23:49 UTC (permalink / raw) To: Klaus Jensen Cc: Kevin Wolf, qemu-block, Dmitry Fomichev, Klaus Jensen, qemu-devel, Max Reitz On Mon, Jan 11, 2021 at 01:32:18PM +0100, Klaus Jensen wrote: > From: Klaus Jensen <k.jensen@samsung.com> > > A shutdown is only about flushing stuff. It is the host that should > delete any queues, so do not perform a reset here. > > Also, on shutdown, make sure that the PMR is flushed if in use. > > Fixes: 368f4e752cf9 ("hw/block/nvme: Process controller reset and shutdown differently") > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> Yes, and exiting a controller shutdown state requires a controller reset, so a functioning host shouldn't require the controller behave the way it's currently done. Reviewed-by: Keith Busch <kbusch@kernel.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/6] hw/block/nvme: merge implicitly/explicitly opened processing masks 2021-01-11 12:32 [PATCH 0/6] hw/block/nvme: zoned misc fixes Klaus Jensen 2021-01-11 12:32 ` [PATCH 1/6] hw/block/nvme: fix shutdown/reset logic Klaus Jensen @ 2021-01-11 12:32 ` Klaus Jensen 2021-01-11 12:32 ` [PATCH 3/6] hw/block/nvme: enum style fix Klaus Jensen ` (4 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Klaus Jensen @ 2021-01-11 12:32 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, qemu-block, Dmitry Fomichev, Klaus Jensen, Max Reitz, Klaus Jensen, Keith Busch From: Klaus Jensen <k.jensen@samsung.com> Implicitly and explicitly opended zones are always bulk processed together, so merge the two processing masks. Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- hw/block/nvme.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index cba509e90537..8b25c509c6b5 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -1740,11 +1740,10 @@ typedef uint16_t (*op_handler_t)(NvmeNamespace *, NvmeZone *, enum NvmeZoneProcessingMask { NVME_PROC_CURRENT_ZONE = 0, - NVME_PROC_IMP_OPEN_ZONES = 1 << 0, - NVME_PROC_EXP_OPEN_ZONES = 1 << 1, - NVME_PROC_CLOSED_ZONES = 1 << 2, - NVME_PROC_READ_ONLY_ZONES = 1 << 3, - NVME_PROC_FULL_ZONES = 1 << 4, + NVME_PROC_OPENED_ZONES = 1 << 0, + NVME_PROC_CLOSED_ZONES = 1 << 1, + NVME_PROC_READ_ONLY_ZONES = 1 << 2, + NVME_PROC_FULL_ZONES = 1 << 3, }; static uint16_t nvme_open_zone(NvmeNamespace *ns, NvmeZone *zone, @@ -1885,10 +1884,8 @@ static uint16_t nvme_bulk_proc_zone(NvmeNamespace *ns, NvmeZone *zone, switch (zs) { case NVME_ZONE_STATE_IMPLICITLY_OPEN: - proc_zone = proc_mask & NVME_PROC_IMP_OPEN_ZONES; - break; case NVME_ZONE_STATE_EXPLICITLY_OPEN: - proc_zone = proc_mask & NVME_PROC_EXP_OPEN_ZONES; + proc_zone = proc_mask & NVME_PROC_OPENED_ZONES; break; case NVME_ZONE_STATE_CLOSED: proc_zone = proc_mask & NVME_PROC_CLOSED_ZONES; @@ -1929,15 +1926,14 @@ static uint16_t nvme_do_zone_op(NvmeNamespace *ns, NvmeZone *zone, } } } - if (proc_mask & NVME_PROC_IMP_OPEN_ZONES) { + if (proc_mask & NVME_PROC_OPENED_ZONES) { QTAILQ_FOREACH_SAFE(zone, &ns->imp_open_zones, entry, next) { status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr); if (status != NVME_SUCCESS) { goto out; } } - } - if (proc_mask & NVME_PROC_EXP_OPEN_ZONES) { + QTAILQ_FOREACH_SAFE(zone, &ns->exp_open_zones, entry, next) { status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr); if (status != NVME_SUCCESS) { @@ -2012,7 +2008,7 @@ static uint16_t nvme_zone_mgmt_send(NvmeCtrl *n, NvmeRequest *req) case NVME_ZONE_ACTION_CLOSE: if (all) { - proc_mask = NVME_PROC_IMP_OPEN_ZONES | NVME_PROC_EXP_OPEN_ZONES; + proc_mask = NVME_PROC_OPENED_ZONES; } trace_pci_nvme_close_zone(slba, zone_idx, all); status = nvme_do_zone_op(ns, zone, proc_mask, nvme_close_zone); @@ -2020,8 +2016,7 @@ static uint16_t nvme_zone_mgmt_send(NvmeCtrl *n, NvmeRequest *req) case NVME_ZONE_ACTION_FINISH: if (all) { - proc_mask = NVME_PROC_IMP_OPEN_ZONES | NVME_PROC_EXP_OPEN_ZONES | - NVME_PROC_CLOSED_ZONES; + proc_mask = NVME_PROC_OPENED_ZONES | NVME_PROC_CLOSED_ZONES; } trace_pci_nvme_finish_zone(slba, zone_idx, all); status = nvme_do_zone_op(ns, zone, proc_mask, nvme_finish_zone); @@ -2029,8 +2024,8 @@ static uint16_t nvme_zone_mgmt_send(NvmeCtrl *n, NvmeRequest *req) case NVME_ZONE_ACTION_RESET: if (all) { - proc_mask = NVME_PROC_IMP_OPEN_ZONES | NVME_PROC_EXP_OPEN_ZONES | - NVME_PROC_CLOSED_ZONES | NVME_PROC_FULL_ZONES; + proc_mask = NVME_PROC_OPENED_ZONES | NVME_PROC_CLOSED_ZONES | + NVME_PROC_FULL_ZONES; } trace_pci_nvme_reset_zone(slba, zone_idx, all); status = nvme_do_zone_op(ns, zone, proc_mask, nvme_reset_zone); -- 2.30.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/6] hw/block/nvme: enum style fix 2021-01-11 12:32 [PATCH 0/6] hw/block/nvme: zoned misc fixes Klaus Jensen 2021-01-11 12:32 ` [PATCH 1/6] hw/block/nvme: fix shutdown/reset logic Klaus Jensen 2021-01-11 12:32 ` [PATCH 2/6] hw/block/nvme: merge implicitly/explicitly opened processing masks Klaus Jensen @ 2021-01-11 12:32 ` Klaus Jensen 2021-01-11 12:32 ` [PATCH 4/6] hw/block/nvme: zero out zones on reset Klaus Jensen ` (3 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Klaus Jensen @ 2021-01-11 12:32 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, qemu-block, Dmitry Fomichev, Klaus Jensen, Max Reitz, Klaus Jensen, Keith Busch From: Klaus Jensen <k.jensen@samsung.com> Align with existing style and use a typedef for header-file enums. Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- hw/block/nvme-ns.h | 4 ++-- include/block/nvme.h | 4 ++-- hw/block/nvme.c | 19 +++++++++---------- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h index f8f3c28c360b..a0baa5f6d44c 100644 --- a/hw/block/nvme-ns.h +++ b/hw/block/nvme-ns.h @@ -102,12 +102,12 @@ static inline size_t nvme_l2b(NvmeNamespace *ns, uint64_t lba) typedef struct NvmeCtrl NvmeCtrl; -static inline enum NvmeZoneState nvme_get_zone_state(NvmeZone *zone) +static inline NvmeZoneState nvme_get_zone_state(NvmeZone *zone) { return zone->d.zs >> 4; } -static inline void nvme_set_zone_state(NvmeZone *zone, enum NvmeZoneState state) +static inline void nvme_set_zone_state(NvmeZone *zone, NvmeZoneState state) { zone->d.zs = state << 4; } diff --git a/include/block/nvme.h b/include/block/nvme.h index 9494246f1f59..45b2678db1f0 100644 --- a/include/block/nvme.h +++ b/include/block/nvme.h @@ -1212,7 +1212,7 @@ typedef struct QEMU_PACKED NvmeZoneDescr { uint8_t rsvd32[32]; } NvmeZoneDescr; -enum NvmeZoneState { +typedef enum NvmeZoneState { NVME_ZONE_STATE_RESERVED = 0x00, NVME_ZONE_STATE_EMPTY = 0x01, NVME_ZONE_STATE_IMPLICITLY_OPEN = 0x02, @@ -1221,7 +1221,7 @@ enum NvmeZoneState { NVME_ZONE_STATE_READ_ONLY = 0x0D, NVME_ZONE_STATE_FULL = 0x0E, NVME_ZONE_STATE_OFFLINE = 0x0F, -}; +} NvmeZoneState; static inline void _nvme_check_size(void) { diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 8b25c509c6b5..7c2ec17ad7d9 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -198,7 +198,7 @@ static uint16_t nvme_sqid(NvmeRequest *req) } static void nvme_assign_zone_state(NvmeNamespace *ns, NvmeZone *zone, - enum NvmeZoneState state) + NvmeZoneState state) { if (QTAILQ_IN_USE(zone, entry)) { switch (nvme_get_zone_state(zone)) { @@ -1735,8 +1735,7 @@ static uint16_t nvme_get_mgmt_zone_slba_idx(NvmeNamespace *ns, NvmeCmd *c, return NVME_SUCCESS; } -typedef uint16_t (*op_handler_t)(NvmeNamespace *, NvmeZone *, - enum NvmeZoneState); +typedef uint16_t (*op_handler_t)(NvmeNamespace *, NvmeZone *, NvmeZoneState); enum NvmeZoneProcessingMask { NVME_PROC_CURRENT_ZONE = 0, @@ -1747,7 +1746,7 @@ enum NvmeZoneProcessingMask { }; static uint16_t nvme_open_zone(NvmeNamespace *ns, NvmeZone *zone, - enum NvmeZoneState state) + NvmeZoneState state) { uint16_t status; @@ -1780,7 +1779,7 @@ static uint16_t nvme_open_zone(NvmeNamespace *ns, NvmeZone *zone, } static uint16_t nvme_close_zone(NvmeNamespace *ns, NvmeZone *zone, - enum NvmeZoneState state) + NvmeZoneState state) { switch (state) { case NVME_ZONE_STATE_EXPLICITLY_OPEN: @@ -1796,7 +1795,7 @@ static uint16_t nvme_close_zone(NvmeNamespace *ns, NvmeZone *zone, } static uint16_t nvme_finish_zone(NvmeNamespace *ns, NvmeZone *zone, - enum NvmeZoneState state) + NvmeZoneState state) { switch (state) { case NVME_ZONE_STATE_EXPLICITLY_OPEN: @@ -1819,7 +1818,7 @@ static uint16_t nvme_finish_zone(NvmeNamespace *ns, NvmeZone *zone, } static uint16_t nvme_reset_zone(NvmeNamespace *ns, NvmeZone *zone, - enum NvmeZoneState state) + NvmeZoneState state) { switch (state) { case NVME_ZONE_STATE_EXPLICITLY_OPEN: @@ -1842,7 +1841,7 @@ static uint16_t nvme_reset_zone(NvmeNamespace *ns, NvmeZone *zone, } static uint16_t nvme_offline_zone(NvmeNamespace *ns, NvmeZone *zone, - enum NvmeZoneState state) + NvmeZoneState state) { switch (state) { case NVME_ZONE_STATE_READ_ONLY: @@ -1879,7 +1878,7 @@ static uint16_t nvme_bulk_proc_zone(NvmeNamespace *ns, NvmeZone *zone, op_handler_t op_hndlr) { uint16_t status = NVME_SUCCESS; - enum NvmeZoneState zs = nvme_get_zone_state(zone); + NvmeZoneState zs = nvme_get_zone_state(zone); bool proc_zone; switch (zs) { @@ -2077,7 +2076,7 @@ static uint16_t nvme_zone_mgmt_send(NvmeCtrl *n, NvmeRequest *req) static bool nvme_zone_matches_filter(uint32_t zafs, NvmeZone *zl) { - enum NvmeZoneState zs = nvme_get_zone_state(zl); + NvmeZoneState zs = nvme_get_zone_state(zl); switch (zafs) { case NVME_ZONE_REPORT_ALL: -- 2.30.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/6] hw/block/nvme: zero out zones on reset 2021-01-11 12:32 [PATCH 0/6] hw/block/nvme: zoned misc fixes Klaus Jensen ` (2 preceding siblings ...) 2021-01-11 12:32 ` [PATCH 3/6] hw/block/nvme: enum style fix Klaus Jensen @ 2021-01-11 12:32 ` Klaus Jensen 2021-01-18 3:35 ` Dmitry Fomichev 2021-01-11 12:32 ` [PATCH 5/6] hw/block/nvme: add missing string representations for commands Klaus Jensen ` (2 subsequent siblings) 6 siblings, 1 reply; 11+ messages in thread From: Klaus Jensen @ 2021-01-11 12:32 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, qemu-block, Dmitry Fomichev, Klaus Jensen, Max Reitz, Klaus Jensen, Keith Busch From: Klaus Jensen <k.jensen@samsung.com> The zoned command set specification states that "All logical blocks in a zone *shall* be marked as deallocated when [the zone is reset]". Since the device guarantees 0x00 to be read from deallocated blocks we have to issue a pwrite_zeroes since we cannot be sure that a discard will do anything. But typically, this will be achieved with an efficient unmap/discard operation. Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- hw/block/nvme.c | 150 +++++++++++++++++++++++++++++++----------- hw/block/trace-events | 1 + 2 files changed, 113 insertions(+), 38 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 7c2ec17ad7d9..b3658595fe1b 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -1371,6 +1371,53 @@ static void nvme_aio_discard_cb(void *opaque, int ret) nvme_enqueue_req_completion(nvme_cq(req), req); } +struct nvme_zone_reset_ctx { + NvmeRequest *req; + NvmeZone *zone; +}; + +static void nvme_aio_zone_reset_cb(void *opaque, int ret) +{ + struct nvme_zone_reset_ctx *ctx = opaque; + NvmeRequest *req = ctx->req; + NvmeNamespace *ns = req->ns; + NvmeZone *zone = ctx->zone; + uintptr_t *resets = (uintptr_t *)&req->opaque; + + g_free(ctx); + + trace_pci_nvme_aio_zone_reset_cb(nvme_cid(req), zone->d.zslba); + + if (!ret) { + switch (nvme_get_zone_state(zone)) { + case NVME_ZONE_STATE_EXPLICITLY_OPEN: + case NVME_ZONE_STATE_IMPLICITLY_OPEN: + nvme_aor_dec_open(ns); + /* fall through */ + case NVME_ZONE_STATE_CLOSED: + nvme_aor_dec_active(ns); + /* fall through */ + case NVME_ZONE_STATE_FULL: + zone->w_ptr = zone->d.zslba; + zone->d.wp = zone->w_ptr; + nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_EMPTY); + /* fall through */ + default: + break; + } + } else { + nvme_aio_err(req, ret); + } + + (*resets)--; + + if (*resets) { + return; + } + + nvme_enqueue_req_completion(nvme_cq(req), req); +} + struct nvme_compare_ctx { QEMUIOVector iov; uint8_t *bounce; @@ -1735,7 +1782,8 @@ static uint16_t nvme_get_mgmt_zone_slba_idx(NvmeNamespace *ns, NvmeCmd *c, return NVME_SUCCESS; } -typedef uint16_t (*op_handler_t)(NvmeNamespace *, NvmeZone *, NvmeZoneState); +typedef uint16_t (*op_handler_t)(NvmeNamespace *, NvmeZone *, NvmeZoneState, + NvmeRequest *); enum NvmeZoneProcessingMask { NVME_PROC_CURRENT_ZONE = 0, @@ -1746,7 +1794,7 @@ enum NvmeZoneProcessingMask { }; static uint16_t nvme_open_zone(NvmeNamespace *ns, NvmeZone *zone, - NvmeZoneState state) + NvmeZoneState state, NvmeRequest *req) { uint16_t status; @@ -1779,7 +1827,7 @@ static uint16_t nvme_open_zone(NvmeNamespace *ns, NvmeZone *zone, } static uint16_t nvme_close_zone(NvmeNamespace *ns, NvmeZone *zone, - NvmeZoneState state) + NvmeZoneState state, NvmeRequest *req) { switch (state) { case NVME_ZONE_STATE_EXPLICITLY_OPEN: @@ -1795,7 +1843,7 @@ static uint16_t nvme_close_zone(NvmeNamespace *ns, NvmeZone *zone, } static uint16_t nvme_finish_zone(NvmeNamespace *ns, NvmeZone *zone, - NvmeZoneState state) + NvmeZoneState state, NvmeRequest *req) { switch (state) { case NVME_ZONE_STATE_EXPLICITLY_OPEN: @@ -1818,30 +1866,42 @@ static uint16_t nvme_finish_zone(NvmeNamespace *ns, NvmeZone *zone, } static uint16_t nvme_reset_zone(NvmeNamespace *ns, NvmeZone *zone, - NvmeZoneState state) + NvmeZoneState state, NvmeRequest *req) { + uintptr_t *resets = (uintptr_t *)&req->opaque; + struct nvme_zone_reset_ctx *ctx; + switch (state) { - case NVME_ZONE_STATE_EXPLICITLY_OPEN: - case NVME_ZONE_STATE_IMPLICITLY_OPEN: - nvme_aor_dec_open(ns); - /* fall through */ - case NVME_ZONE_STATE_CLOSED: - nvme_aor_dec_active(ns); - /* fall through */ - case NVME_ZONE_STATE_FULL: - zone->w_ptr = zone->d.zslba; - zone->d.wp = zone->w_ptr; - nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_EMPTY); - /* fall through */ case NVME_ZONE_STATE_EMPTY: return NVME_SUCCESS; + case NVME_ZONE_STATE_EXPLICITLY_OPEN: + case NVME_ZONE_STATE_IMPLICITLY_OPEN: + case NVME_ZONE_STATE_CLOSED: + case NVME_ZONE_STATE_FULL: + break; default: return NVME_ZONE_INVAL_TRANSITION; } + + /* + * The zone reset aio callback needs to know the zone that is being reset + * in order to transition the zone on completion. + */ + ctx = g_new(struct nvme_zone_reset_ctx, 1); + ctx->req = req; + ctx->zone = zone; + + (*resets)++; + + blk_aio_pwrite_zeroes(ns->blkconf.blk, nvme_l2b(ns, zone->d.zslba), + nvme_l2b(ns, ns->zone_size), BDRV_REQ_MAY_UNMAP, + nvme_aio_zone_reset_cb, ctx); + + return NVME_NO_COMPLETE; } static uint16_t nvme_offline_zone(NvmeNamespace *ns, NvmeZone *zone, - NvmeZoneState state) + NvmeZoneState state, NvmeRequest *req) { switch (state) { case NVME_ZONE_STATE_READ_ONLY: @@ -1875,7 +1935,7 @@ static uint16_t nvme_set_zd_ext(NvmeNamespace *ns, NvmeZone *zone) static uint16_t nvme_bulk_proc_zone(NvmeNamespace *ns, NvmeZone *zone, enum NvmeZoneProcessingMask proc_mask, - op_handler_t op_hndlr) + op_handler_t op_hndlr, NvmeRequest *req) { uint16_t status = NVME_SUCCESS; NvmeZoneState zs = nvme_get_zone_state(zone); @@ -1900,7 +1960,7 @@ static uint16_t nvme_bulk_proc_zone(NvmeNamespace *ns, NvmeZone *zone, } if (proc_zone) { - status = op_hndlr(ns, zone, zs); + status = op_hndlr(ns, zone, zs, req); } return status; @@ -1908,42 +1968,46 @@ static uint16_t nvme_bulk_proc_zone(NvmeNamespace *ns, NvmeZone *zone, static uint16_t nvme_do_zone_op(NvmeNamespace *ns, NvmeZone *zone, enum NvmeZoneProcessingMask proc_mask, - op_handler_t op_hndlr) + op_handler_t op_hndlr, NvmeRequest *req) { NvmeZone *next; uint16_t status = NVME_SUCCESS; int i; if (!proc_mask) { - status = op_hndlr(ns, zone, nvme_get_zone_state(zone)); + status = op_hndlr(ns, zone, nvme_get_zone_state(zone), req); } else { if (proc_mask & NVME_PROC_CLOSED_ZONES) { QTAILQ_FOREACH_SAFE(zone, &ns->closed_zones, entry, next) { - status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr); - if (status != NVME_SUCCESS) { + status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr, + req); + if (status && status != NVME_NO_COMPLETE) { goto out; } } } if (proc_mask & NVME_PROC_OPENED_ZONES) { QTAILQ_FOREACH_SAFE(zone, &ns->imp_open_zones, entry, next) { - status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr); - if (status != NVME_SUCCESS) { + status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr, + req); + if (status && status != NVME_NO_COMPLETE) { goto out; } } QTAILQ_FOREACH_SAFE(zone, &ns->exp_open_zones, entry, next) { - status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr); - if (status != NVME_SUCCESS) { + status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr, + req); + if (status && status != NVME_NO_COMPLETE) { goto out; } } } if (proc_mask & NVME_PROC_FULL_ZONES) { QTAILQ_FOREACH_SAFE(zone, &ns->full_zones, entry, next) { - status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr); - if (status != NVME_SUCCESS) { + status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr, + req); + if (status && status != NVME_NO_COMPLETE) { goto out; } } @@ -1951,8 +2015,9 @@ static uint16_t nvme_do_zone_op(NvmeNamespace *ns, NvmeZone *zone, if (proc_mask & NVME_PROC_READ_ONLY_ZONES) { for (i = 0; i < ns->num_zones; i++, zone++) { - status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr); - if (status != NVME_SUCCESS) { + status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr, + req); + if (status && status != NVME_NO_COMPLETE) { goto out; } } @@ -1968,6 +2033,7 @@ static uint16_t nvme_zone_mgmt_send(NvmeCtrl *n, NvmeRequest *req) NvmeCmd *cmd = (NvmeCmd *)&req->cmd; NvmeNamespace *ns = req->ns; NvmeZone *zone; + uintptr_t *resets; uint8_t *zd_ext; uint32_t dw13 = le32_to_cpu(cmd->cdw13); uint64_t slba = 0; @@ -2002,7 +2068,7 @@ static uint16_t nvme_zone_mgmt_send(NvmeCtrl *n, NvmeRequest *req) proc_mask = NVME_PROC_CLOSED_ZONES; } trace_pci_nvme_open_zone(slba, zone_idx, all); - status = nvme_do_zone_op(ns, zone, proc_mask, nvme_open_zone); + status = nvme_do_zone_op(ns, zone, proc_mask, nvme_open_zone, req); break; case NVME_ZONE_ACTION_CLOSE: @@ -2010,7 +2076,7 @@ static uint16_t nvme_zone_mgmt_send(NvmeCtrl *n, NvmeRequest *req) proc_mask = NVME_PROC_OPENED_ZONES; } trace_pci_nvme_close_zone(slba, zone_idx, all); - status = nvme_do_zone_op(ns, zone, proc_mask, nvme_close_zone); + status = nvme_do_zone_op(ns, zone, proc_mask, nvme_close_zone, req); break; case NVME_ZONE_ACTION_FINISH: @@ -2018,24 +2084,32 @@ static uint16_t nvme_zone_mgmt_send(NvmeCtrl *n, NvmeRequest *req) proc_mask = NVME_PROC_OPENED_ZONES | NVME_PROC_CLOSED_ZONES; } trace_pci_nvme_finish_zone(slba, zone_idx, all); - status = nvme_do_zone_op(ns, zone, proc_mask, nvme_finish_zone); + status = nvme_do_zone_op(ns, zone, proc_mask, nvme_finish_zone, req); break; case NVME_ZONE_ACTION_RESET: + resets = (uintptr_t *)&req->opaque; + if (all) { proc_mask = NVME_PROC_OPENED_ZONES | NVME_PROC_CLOSED_ZONES | NVME_PROC_FULL_ZONES; } trace_pci_nvme_reset_zone(slba, zone_idx, all); - status = nvme_do_zone_op(ns, zone, proc_mask, nvme_reset_zone); - break; + + *resets = 1; + + status = nvme_do_zone_op(ns, zone, proc_mask, nvme_reset_zone, req); + + (*resets)--; + + return *resets ? NVME_NO_COMPLETE : req->status; case NVME_ZONE_ACTION_OFFLINE: if (all) { proc_mask = NVME_PROC_READ_ONLY_ZONES; } trace_pci_nvme_offline_zone(slba, zone_idx, all); - status = nvme_do_zone_op(ns, zone, proc_mask, nvme_offline_zone); + status = nvme_do_zone_op(ns, zone, proc_mask, nvme_offline_zone, req); break; case NVME_ZONE_ACTION_SET_ZD_EXT: diff --git a/hw/block/trace-events b/hw/block/trace-events index deaacdae5097..78d76b0a71c1 100644 --- a/hw/block/trace-events +++ b/hw/block/trace-events @@ -49,6 +49,7 @@ pci_nvme_dsm_deallocate(uint16_t cid, uint32_t nsid, uint64_t slba, uint32_t nlb pci_nvme_compare(uint16_t cid, uint32_t nsid, uint64_t slba, uint32_t nlb) "cid %"PRIu16" nsid %"PRIu32" slba 0x%"PRIx64" nlb %"PRIu32"" pci_nvme_compare_cb(uint16_t cid) "cid %"PRIu16"" pci_nvme_aio_discard_cb(uint16_t cid) "cid %"PRIu16"" +pci_nvme_aio_zone_reset_cb(uint16_t cid, uint64_t zslba) "cid %"PRIu16" zslba 0x%"PRIx64"" pci_nvme_create_sq(uint64_t addr, uint16_t sqid, uint16_t cqid, uint16_t qsize, uint16_t qflags) "create submission queue, addr=0x%"PRIx64", sqid=%"PRIu16", cqid=%"PRIu16", qsize=%"PRIu16", qflags=%"PRIu16"" pci_nvme_create_cq(uint64_t addr, uint16_t cqid, uint16_t vector, uint16_t size, uint16_t qflags, int ien) "create completion queue, addr=0x%"PRIx64", cqid=%"PRIu16", vector=%"PRIu16", qsize=%"PRIu16", qflags=%"PRIu16", ien=%d" pci_nvme_del_sq(uint16_t qid) "deleting submission queue sqid=%"PRIu16"" -- 2.30.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 4/6] hw/block/nvme: zero out zones on reset 2021-01-11 12:32 ` [PATCH 4/6] hw/block/nvme: zero out zones on reset Klaus Jensen @ 2021-01-18 3:35 ` Dmitry Fomichev 0 siblings, 0 replies; 11+ messages in thread From: Dmitry Fomichev @ 2021-01-18 3:35 UTC (permalink / raw) To: its, qemu-devel; +Cc: kbusch, kwolf, k.jensen, qemu-block, mreitz On Mon, 2021-01-11 at 13:32 +0100, Klaus Jensen wrote: > From: Klaus Jensen <k.jensen@samsung.com> > > The zoned command set specification states that "All logical blocks in a > zone *shall* be marked as deallocated when [the zone is reset]". Since > the device guarantees 0x00 to be read from deallocated blocks we have to > issue a pwrite_zeroes since we cannot be sure that a discard will do > anything. But typically, this will be achieved with an efficient > unmap/discard operation. > > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > --- > hw/block/nvme.c | 150 +++++++++++++++++++++++++++++++----------- > hw/block/trace-events | 1 + > 2 files changed, 113 insertions(+), 38 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 7c2ec17ad7d9..b3658595fe1b 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -1371,6 +1371,53 @@ static void nvme_aio_discard_cb(void *opaque, int ret) > nvme_enqueue_req_completion(nvme_cq(req), req); > } > > > > > +struct nvme_zone_reset_ctx { > + NvmeRequest *req; > + NvmeZone *zone; > +}; > + > +static void nvme_aio_zone_reset_cb(void *opaque, int ret) > +{ > + struct nvme_zone_reset_ctx *ctx = opaque; > + NvmeRequest *req = ctx->req; > + NvmeNamespace *ns = req->ns; > + NvmeZone *zone = ctx->zone; > + uintptr_t *resets = (uintptr_t *)&req->opaque; > + > + g_free(ctx); > + > + trace_pci_nvme_aio_zone_reset_cb(nvme_cid(req), zone->d.zslba); > + > + if (!ret) { > + switch (nvme_get_zone_state(zone)) { > + case NVME_ZONE_STATE_EXPLICITLY_OPEN: > + case NVME_ZONE_STATE_IMPLICITLY_OPEN: > + nvme_aor_dec_open(ns); > + /* fall through */ > + case NVME_ZONE_STATE_CLOSED: > + nvme_aor_dec_active(ns); > + /* fall through */ > + case NVME_ZONE_STATE_FULL: > + zone->w_ptr = zone->d.zslba; > + zone->d.wp = zone->w_ptr; > + nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_EMPTY); > + /* fall through */ > + default: > + break; > + } > + } else { > + nvme_aio_err(req, ret); > + } > + > + (*resets)--; > + > + if (*resets) { > + return; > + } > + > + nvme_enqueue_req_completion(nvme_cq(req), req); A tiny nit - the above could be simplified to if (!*resets) { nvme_enqueue_req_completion(nvme_cq(req), req); } Overall, this patch looks good to me. > +} > + > struct nvme_compare_ctx { > QEMUIOVector iov; > uint8_t *bounce; > @@ -1735,7 +1782,8 @@ static uint16_t nvme_get_mgmt_zone_slba_idx(NvmeNamespace *ns, NvmeCmd *c, > return NVME_SUCCESS; > } > > > > > -typedef uint16_t (*op_handler_t)(NvmeNamespace *, NvmeZone *, NvmeZoneState); > +typedef uint16_t (*op_handler_t)(NvmeNamespace *, NvmeZone *, NvmeZoneState, > + NvmeRequest *); > > > > > enum NvmeZoneProcessingMask { > NVME_PROC_CURRENT_ZONE = 0, > @@ -1746,7 +1794,7 @@ enum NvmeZoneProcessingMask { > }; > > > > > static uint16_t nvme_open_zone(NvmeNamespace *ns, NvmeZone *zone, > - NvmeZoneState state) > + NvmeZoneState state, NvmeRequest *req) > { > uint16_t status; > > > > > @@ -1779,7 +1827,7 @@ static uint16_t nvme_open_zone(NvmeNamespace *ns, NvmeZone *zone, > } > > > > > static uint16_t nvme_close_zone(NvmeNamespace *ns, NvmeZone *zone, > - NvmeZoneState state) > + NvmeZoneState state, NvmeRequest *req) > { > switch (state) { > case NVME_ZONE_STATE_EXPLICITLY_OPEN: > @@ -1795,7 +1843,7 @@ static uint16_t nvme_close_zone(NvmeNamespace *ns, NvmeZone *zone, > } > > > > > static uint16_t nvme_finish_zone(NvmeNamespace *ns, NvmeZone *zone, > - NvmeZoneState state) > + NvmeZoneState state, NvmeRequest *req) > { > switch (state) { > case NVME_ZONE_STATE_EXPLICITLY_OPEN: > @@ -1818,30 +1866,42 @@ static uint16_t nvme_finish_zone(NvmeNamespace *ns, NvmeZone *zone, > } > > > > > static uint16_t nvme_reset_zone(NvmeNamespace *ns, NvmeZone *zone, > - NvmeZoneState state) > + NvmeZoneState state, NvmeRequest *req) > { > + uintptr_t *resets = (uintptr_t *)&req->opaque; > + struct nvme_zone_reset_ctx *ctx; > + > switch (state) { > - case NVME_ZONE_STATE_EXPLICITLY_OPEN: > - case NVME_ZONE_STATE_IMPLICITLY_OPEN: > - nvme_aor_dec_open(ns); > - /* fall through */ > - case NVME_ZONE_STATE_CLOSED: > - nvme_aor_dec_active(ns); > - /* fall through */ > - case NVME_ZONE_STATE_FULL: > - zone->w_ptr = zone->d.zslba; > - zone->d.wp = zone->w_ptr; > - nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_EMPTY); > - /* fall through */ > case NVME_ZONE_STATE_EMPTY: > return NVME_SUCCESS; > + case NVME_ZONE_STATE_EXPLICITLY_OPEN: > + case NVME_ZONE_STATE_IMPLICITLY_OPEN: > + case NVME_ZONE_STATE_CLOSED: > + case NVME_ZONE_STATE_FULL: > + break; > default: > return NVME_ZONE_INVAL_TRANSITION; > } > + > + /* > + * The zone reset aio callback needs to know the zone that is being reset > + * in order to transition the zone on completion. > + */ > + ctx = g_new(struct nvme_zone_reset_ctx, 1); > + ctx->req = req; > + ctx->zone = zone; > + > + (*resets)++; > + > + blk_aio_pwrite_zeroes(ns->blkconf.blk, nvme_l2b(ns, zone->d.zslba), > + nvme_l2b(ns, ns->zone_size), BDRV_REQ_MAY_UNMAP, > + nvme_aio_zone_reset_cb, ctx); > + > + return NVME_NO_COMPLETE; > } > > > > > static uint16_t nvme_offline_zone(NvmeNamespace *ns, NvmeZone *zone, > - NvmeZoneState state) > + NvmeZoneState state, NvmeRequest *req) > { > switch (state) { > case NVME_ZONE_STATE_READ_ONLY: > @@ -1875,7 +1935,7 @@ static uint16_t nvme_set_zd_ext(NvmeNamespace *ns, NvmeZone *zone) > > > > > static uint16_t nvme_bulk_proc_zone(NvmeNamespace *ns, NvmeZone *zone, > enum NvmeZoneProcessingMask proc_mask, > - op_handler_t op_hndlr) > + op_handler_t op_hndlr, NvmeRequest *req) > { > uint16_t status = NVME_SUCCESS; > NvmeZoneState zs = nvme_get_zone_state(zone); > @@ -1900,7 +1960,7 @@ static uint16_t nvme_bulk_proc_zone(NvmeNamespace *ns, NvmeZone *zone, > } > > > > > if (proc_zone) { > - status = op_hndlr(ns, zone, zs); > + status = op_hndlr(ns, zone, zs, req); > } > > > > > return status; > @@ -1908,42 +1968,46 @@ static uint16_t nvme_bulk_proc_zone(NvmeNamespace *ns, NvmeZone *zone, > > > > > static uint16_t nvme_do_zone_op(NvmeNamespace *ns, NvmeZone *zone, > enum NvmeZoneProcessingMask proc_mask, > - op_handler_t op_hndlr) > + op_handler_t op_hndlr, NvmeRequest *req) > { > NvmeZone *next; > uint16_t status = NVME_SUCCESS; > int i; > > > > > if (!proc_mask) { > - status = op_hndlr(ns, zone, nvme_get_zone_state(zone)); > + status = op_hndlr(ns, zone, nvme_get_zone_state(zone), req); > } else { > if (proc_mask & NVME_PROC_CLOSED_ZONES) { > QTAILQ_FOREACH_SAFE(zone, &ns->closed_zones, entry, next) { > - status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr); > - if (status != NVME_SUCCESS) { > + status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr, > + req); > + if (status && status != NVME_NO_COMPLETE) { > goto out; > } > } > } > if (proc_mask & NVME_PROC_OPENED_ZONES) { > QTAILQ_FOREACH_SAFE(zone, &ns->imp_open_zones, entry, next) { > - status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr); > - if (status != NVME_SUCCESS) { > + status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr, > + req); > + if (status && status != NVME_NO_COMPLETE) { > goto out; > } > } > > > > > QTAILQ_FOREACH_SAFE(zone, &ns->exp_open_zones, entry, next) { > - status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr); > - if (status != NVME_SUCCESS) { > + status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr, > + req); > + if (status && status != NVME_NO_COMPLETE) { > goto out; > } > } > } > if (proc_mask & NVME_PROC_FULL_ZONES) { > QTAILQ_FOREACH_SAFE(zone, &ns->full_zones, entry, next) { > - status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr); > - if (status != NVME_SUCCESS) { > + status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr, > + req); > + if (status && status != NVME_NO_COMPLETE) { > goto out; > } > } > @@ -1951,8 +2015,9 @@ static uint16_t nvme_do_zone_op(NvmeNamespace *ns, NvmeZone *zone, > > > > > if (proc_mask & NVME_PROC_READ_ONLY_ZONES) { > for (i = 0; i < ns->num_zones; i++, zone++) { > - status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr); > - if (status != NVME_SUCCESS) { > + status = nvme_bulk_proc_zone(ns, zone, proc_mask, op_hndlr, > + req); > + if (status && status != NVME_NO_COMPLETE) { > goto out; > } > } > @@ -1968,6 +2033,7 @@ static uint16_t nvme_zone_mgmt_send(NvmeCtrl *n, NvmeRequest *req) > NvmeCmd *cmd = (NvmeCmd *)&req->cmd; > NvmeNamespace *ns = req->ns; > NvmeZone *zone; > + uintptr_t *resets; > uint8_t *zd_ext; > uint32_t dw13 = le32_to_cpu(cmd->cdw13); > uint64_t slba = 0; > @@ -2002,7 +2068,7 @@ static uint16_t nvme_zone_mgmt_send(NvmeCtrl *n, NvmeRequest *req) > proc_mask = NVME_PROC_CLOSED_ZONES; > } > trace_pci_nvme_open_zone(slba, zone_idx, all); > - status = nvme_do_zone_op(ns, zone, proc_mask, nvme_open_zone); > + status = nvme_do_zone_op(ns, zone, proc_mask, nvme_open_zone, req); > break; > > > > > case NVME_ZONE_ACTION_CLOSE: > @@ -2010,7 +2076,7 @@ static uint16_t nvme_zone_mgmt_send(NvmeCtrl *n, NvmeRequest *req) > proc_mask = NVME_PROC_OPENED_ZONES; > } > trace_pci_nvme_close_zone(slba, zone_idx, all); > - status = nvme_do_zone_op(ns, zone, proc_mask, nvme_close_zone); > + status = nvme_do_zone_op(ns, zone, proc_mask, nvme_close_zone, req); > break; > > > > > case NVME_ZONE_ACTION_FINISH: > @@ -2018,24 +2084,32 @@ static uint16_t nvme_zone_mgmt_send(NvmeCtrl *n, NvmeRequest *req) > proc_mask = NVME_PROC_OPENED_ZONES | NVME_PROC_CLOSED_ZONES; > } > trace_pci_nvme_finish_zone(slba, zone_idx, all); > - status = nvme_do_zone_op(ns, zone, proc_mask, nvme_finish_zone); > + status = nvme_do_zone_op(ns, zone, proc_mask, nvme_finish_zone, req); > break; > > > > > case NVME_ZONE_ACTION_RESET: > + resets = (uintptr_t *)&req->opaque; > + > if (all) { > proc_mask = NVME_PROC_OPENED_ZONES | NVME_PROC_CLOSED_ZONES | > NVME_PROC_FULL_ZONES; > } > trace_pci_nvme_reset_zone(slba, zone_idx, all); > - status = nvme_do_zone_op(ns, zone, proc_mask, nvme_reset_zone); > - break; > + > + *resets = 1; > + > + status = nvme_do_zone_op(ns, zone, proc_mask, nvme_reset_zone, req); > + > + (*resets)--; > + > + return *resets ? NVME_NO_COMPLETE : req->status; > > > > > case NVME_ZONE_ACTION_OFFLINE: > if (all) { > proc_mask = NVME_PROC_READ_ONLY_ZONES; > } > trace_pci_nvme_offline_zone(slba, zone_idx, all); > - status = nvme_do_zone_op(ns, zone, proc_mask, nvme_offline_zone); > + status = nvme_do_zone_op(ns, zone, proc_mask, nvme_offline_zone, req); > break; > > > > > case NVME_ZONE_ACTION_SET_ZD_EXT: > diff --git a/hw/block/trace-events b/hw/block/trace-events > index deaacdae5097..78d76b0a71c1 100644 > --- a/hw/block/trace-events > +++ b/hw/block/trace-events > @@ -49,6 +49,7 @@ pci_nvme_dsm_deallocate(uint16_t cid, uint32_t nsid, uint64_t slba, uint32_t nlb > pci_nvme_compare(uint16_t cid, uint32_t nsid, uint64_t slba, uint32_t nlb) "cid %"PRIu16" nsid %"PRIu32" slba 0x%"PRIx64" nlb %"PRIu32"" > pci_nvme_compare_cb(uint16_t cid) "cid %"PRIu16"" > pci_nvme_aio_discard_cb(uint16_t cid) "cid %"PRIu16"" > +pci_nvme_aio_zone_reset_cb(uint16_t cid, uint64_t zslba) "cid %"PRIu16" zslba 0x%"PRIx64"" > pci_nvme_create_sq(uint64_t addr, uint16_t sqid, uint16_t cqid, uint16_t qsize, uint16_t qflags) "create submission queue, addr=0x%"PRIx64", sqid=%"PRIu16", cqid=%"PRIu16", qsize=%"PRIu16", qflags=%"PRIu16"" > pci_nvme_create_cq(uint64_t addr, uint16_t cqid, uint16_t vector, uint16_t size, uint16_t qflags, int ien) "create completion queue, addr=0x%"PRIx64", cqid=%"PRIu16", vector=%"PRIu16", qsize=%"PRIu16", qflags=%"PRIu16", ien=%d" > pci_nvme_del_sq(uint16_t qid) "deleting submission queue sqid=%"PRIu16"" ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5/6] hw/block/nvme: add missing string representations for commands 2021-01-11 12:32 [PATCH 0/6] hw/block/nvme: zoned misc fixes Klaus Jensen ` (3 preceding siblings ...) 2021-01-11 12:32 ` [PATCH 4/6] hw/block/nvme: zero out zones on reset Klaus Jensen @ 2021-01-11 12:32 ` Klaus Jensen 2021-01-11 12:32 ` [PATCH 6/6] hw/block/nvme: remove unnecessary check for append Klaus Jensen 2021-01-18 3:35 ` [PATCH 0/6] hw/block/nvme: zoned misc fixes Dmitry Fomichev 6 siblings, 0 replies; 11+ messages in thread From: Klaus Jensen @ 2021-01-11 12:32 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, qemu-block, Dmitry Fomichev, Klaus Jensen, Max Reitz, Klaus Jensen, Keith Busch From: Klaus Jensen <k.jensen@samsung.com> Add missing string representations for a couple of new commands. Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- hw/block/nvme.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/block/nvme.h b/hw/block/nvme.h index b7fbcca39d9f..65540b650e1d 100644 --- a/hw/block/nvme.h +++ b/hw/block/nvme.h @@ -64,8 +64,12 @@ static inline const char *nvme_io_opc_str(uint8_t opc) case NVME_CMD_FLUSH: return "NVME_NVM_CMD_FLUSH"; case NVME_CMD_WRITE: return "NVME_NVM_CMD_WRITE"; case NVME_CMD_READ: return "NVME_NVM_CMD_READ"; + case NVME_CMD_COMPARE: return "NVME_NVM_CMD_COMPARE"; case NVME_CMD_WRITE_ZEROES: return "NVME_NVM_CMD_WRITE_ZEROES"; case NVME_CMD_DSM: return "NVME_NVM_CMD_DSM"; + case NVME_CMD_ZONE_MGMT_SEND: return "NVME_ZONED_CMD_MGMT_SEND"; + case NVME_CMD_ZONE_MGMT_RECV: return "NVME_ZONED_CMD_MGMT_RECV"; + case NVME_CMD_ZONE_APPEND: return "NVME_ZONED_CMD_ZONE_APPEND"; default: return "NVME_NVM_CMD_UNKNOWN"; } } -- 2.30.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 6/6] hw/block/nvme: remove unnecessary check for append 2021-01-11 12:32 [PATCH 0/6] hw/block/nvme: zoned misc fixes Klaus Jensen ` (4 preceding siblings ...) 2021-01-11 12:32 ` [PATCH 5/6] hw/block/nvme: add missing string representations for commands Klaus Jensen @ 2021-01-11 12:32 ` Klaus Jensen 2021-01-18 3:35 ` [PATCH 0/6] hw/block/nvme: zoned misc fixes Dmitry Fomichev 6 siblings, 0 replies; 11+ messages in thread From: Klaus Jensen @ 2021-01-11 12:32 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, qemu-block, Dmitry Fomichev, Klaus Jensen, Max Reitz, Klaus Jensen, Keith Busch From: Klaus Jensen <k.jensen@samsung.com> nvme_io_cmd already checks if the namespace supports the Zone Append command, so the removed check is dead code. Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- hw/block/nvme.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index b3658595fe1b..1f175c7f8256 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -1707,10 +1707,6 @@ static uint16_t nvme_do_write(NvmeCtrl *n, NvmeRequest *req, bool append, } res->slba = nvme_advance_zone_wp(ns, zone, nlb); - } else if (append) { - trace_pci_nvme_err_invalid_opc(rw->opcode); - status = NVME_INVALID_OPCODE; - goto invalid; } data_offset = nvme_l2b(ns, slba); -- 2.30.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/6] hw/block/nvme: zoned misc fixes 2021-01-11 12:32 [PATCH 0/6] hw/block/nvme: zoned misc fixes Klaus Jensen ` (5 preceding siblings ...) 2021-01-11 12:32 ` [PATCH 6/6] hw/block/nvme: remove unnecessary check for append Klaus Jensen @ 2021-01-18 3:35 ` Dmitry Fomichev 2021-01-18 5:58 ` Klaus Jensen 6 siblings, 1 reply; 11+ messages in thread From: Dmitry Fomichev @ 2021-01-18 3:35 UTC (permalink / raw) To: its, qemu-devel; +Cc: kbusch, kwolf, k.jensen, qemu-block, mreitz On Mon, 2021-01-11 at 13:32 +0100, Klaus Jensen wrote: > From: Klaus Jensen <k.jensen@samsung.com> Klaus, This series looks good to me. I've made a comment in "zero out zones on reset" patch, but it is only cosmetic in nature. I am going to send a patch with another small fix in ZNS code. Best regards, Dmitry Tested-by: Dmitry Fomichev <dmitry.fomichev@wdc.com> Reviewed-by: Dmitry Fomichev <dmitry.fomichev@wdc.com> > > These are some follow-up patches to the just merged zoned series. > > The biggest addition here is asynchronous zeroing of zones on reset. > > Klaus Jensen (6): > hw/block/nvme: fix shutdown/reset logic > hw/block/nvme: merge implicitly/explicitly opened processing masks > hw/block/nvme: enum style fix > hw/block/nvme: zero out zones on reset > hw/block/nvme: add missing string representations for commands > hw/block/nvme: remove unnecessary check for append > > hw/block/nvme-ns.h | 4 +- > hw/block/nvme.h | 4 + > include/block/nvme.h | 4 +- > hw/block/nvme.c | 200 +++++++++++++++++++++++++++--------------- > hw/block/trace-events | 1 + > 5 files changed, 140 insertions(+), 73 deletions(-) > > -- > 2.30.0 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/6] hw/block/nvme: zoned misc fixes 2021-01-18 3:35 ` [PATCH 0/6] hw/block/nvme: zoned misc fixes Dmitry Fomichev @ 2021-01-18 5:58 ` Klaus Jensen 0 siblings, 0 replies; 11+ messages in thread From: Klaus Jensen @ 2021-01-18 5:58 UTC (permalink / raw) To: Dmitry Fomichev; +Cc: kwolf, qemu-block, k.jensen, qemu-devel, mreitz, kbusch [-- Attachment #1: Type: text/plain, Size: 577 bytes --] On Jan 18 03:35, Dmitry Fomichev wrote: > On Mon, 2021-01-11 at 13:32 +0100, Klaus Jensen wrote: > > From: Klaus Jensen <k.jensen@samsung.com> > > Klaus, > > This series looks good to me. I've made a comment in "zero out zones on reset" > patch, but it is only cosmetic in nature. I am going to send a patch > with another small fix in ZNS code. > > Best regards, > Dmitry > > Tested-by: Dmitry Fomichev <dmitry.fomichev@wdc.com> > Reviewed-by: Dmitry Fomichev <dmitry.fomichev@wdc.com> > Thanks for the review AND testing! :) Applying to nvme-next! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-01-18 6:07 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-11 12:32 [PATCH 0/6] hw/block/nvme: zoned misc fixes Klaus Jensen 2021-01-11 12:32 ` [PATCH 1/6] hw/block/nvme: fix shutdown/reset logic Klaus Jensen 2021-01-14 23:49 ` Keith Busch 2021-01-11 12:32 ` [PATCH 2/6] hw/block/nvme: merge implicitly/explicitly opened processing masks Klaus Jensen 2021-01-11 12:32 ` [PATCH 3/6] hw/block/nvme: enum style fix Klaus Jensen 2021-01-11 12:32 ` [PATCH 4/6] hw/block/nvme: zero out zones on reset Klaus Jensen 2021-01-18 3:35 ` Dmitry Fomichev 2021-01-11 12:32 ` [PATCH 5/6] hw/block/nvme: add missing string representations for commands Klaus Jensen 2021-01-11 12:32 ` [PATCH 6/6] hw/block/nvme: remove unnecessary check for append Klaus Jensen 2021-01-18 3:35 ` [PATCH 0/6] hw/block/nvme: zoned misc fixes Dmitry Fomichev 2021-01-18 5:58 ` Klaus Jensen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).