* [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
* [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
* [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 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
* 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
* 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).