From: Klaus Jensen <k.jensen@samsung.com> Hi, The following changes since commit ac6721b88df944ade0048822b2b74210f543d656: Merge tag 'vhost-user-rtc-pr-1' of https://gitlab.com/epilys/qemu into staging (2026-05-16 17:37:33 -0400) are available in the Git repository at: https://gitlab.com/birkelund/qemu.git tags/pull-nvme-20260518 for you to fetch changes up to 2293d8b4bd88d3f29730cfd608935f77247919b6: hw/nvme: fix admin cq msix setup (2026-05-18 13:47:25 +0200) ---------------------------------------------------------------- nvme queue ---------------------------------------------------------------- Daniel P. Berrangé (4): include/block: define constants for NVME string fields hw/nvme: report error for oversized 'serial' parameter hw/nvme: add user controlled 'model' property hw/nvme: add user controlled 'firmware-version' property Klaus Jensen (1): hw/nvme: fix admin cq msix setup docs/system/devices/nvme.rst | 10 ++++++++++ hw/nvme/ctrl.c | 35 ++++++++++++++++++++++++++++++----- hw/nvme/nvme.h | 2 ++ include/block/nvme.h | 10 +++++++--- 4 files changed, 49 insertions(+), 8 deletions(-) From: Daniel P. Berrangé <berrange@redhat.com> The version, model and serial fields accept fixed length strings. Add constants to enable user supplied strings to be validated. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- include/block/nvme.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/include/block/nvme.h b/include/block/nvme.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/nvme.h +++ b/include/block/nvme.h @@ -XXX,XX +XXX,XX @@ enum NvmeIdCns { NVME_ID_CNS_CS_IND_NS_ALLOCATED = 0x1f, }; +#define NVME_ID_CTRL_SN_MAX_LEN 20 +#define NVME_ID_CTRL_MN_MAX_LEN 40 +#define NVME_ID_CTRL_FR_MAX_LEN 8 + typedef struct QEMU_PACKED NvmeIdCtrl { uint16_t vid; uint16_t ssvid; - uint8_t sn[20]; - uint8_t mn[40]; - uint8_t fr[8]; + uint8_t sn[NVME_ID_CTRL_SN_MAX_LEN]; + uint8_t mn[NVME_ID_CTRL_MN_MAX_LEN]; + uint8_t fr[NVME_ID_CTRL_FR_MAX_LEN]; uint8_t rab; uint8_t ieee[3]; uint8_t cmic; -- 2.53.0 From: Daniel P. Berrangé <berrange@redhat.com> The 'serial' accepted by the NVME device is at most 20 characters long. An over-sized user supplied value should be reported rather than silently truncated. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- hw/nvme/ctrl.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index XXXXXXX..XXXXXXX 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -XXX,XX +XXX,XX @@ static bool nvme_check_params(NvmeCtrl *n, Error **errp) error_setg(errp, "serial property not set"); return false; } + if (strlen(params->serial) > NVME_ID_CTRL_SN_MAX_LEN) { + error_setg(errp, "'serial' parameter '%s' can be at most '%d' characters", + params->serial, NVME_ID_CTRL_SN_MAX_LEN); + return false; + } if (params->mqes < 1) { error_setg(errp, "mqes property cannot be less than 1"); -- 2.53.0 From: Daniel P. Berrangé <berrange@redhat.com> This enables overriding the built in default "QEMU NVMe Ctrl" string with a user specified string. The value can be at most 40 characters in length. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- docs/system/devices/nvme.rst | 5 +++++ hw/nvme/ctrl.c | 14 ++++++++++++-- hw/nvme/nvme.h | 1 + 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/docs/system/devices/nvme.rst b/docs/system/devices/nvme.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/devices/nvme.rst +++ b/docs/system/devices/nvme.rst @@ -XXX,XX +XXX,XX @@ parameters. the SMART / Health information extended log become available in the controller. We emulate version 5 of this log page. +``model`` (default: ``QEMU NVMe Ctrl``) + Override the default reported model, which can be used when needing + to more closely impersonate a particular device type. The model name + can be a maximum of 40 characters in length. + Additional Namespaces --------------------- diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index XXXXXXX..XXXXXXX 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -XXX,XX +XXX,XX @@ * atomic.dn=<on|off[optional]>, \ * atomic.awun<N[optional]>, \ * atomic.awupf<N[optional]>, \ - * subsys=<subsys_id> + * subsys=<subsys_id>, \ + * model=<model-str> * -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\ * zoned=<true|false[optional]>, \ * subsys=<subsys_id>,shared=<true|false[optional]>, \ @@ -XXX,XX +XXX,XX @@ static bool nvme_check_params(NvmeCtrl *n, Error **errp) return false; } + if (params->model && + strlen(params->model) > NVME_ID_CTRL_MN_MAX_LEN) { + error_setg(errp, "'model' parameter '%s' can be at most '%d' characters", + params->model, NVME_ID_CTRL_MN_MAX_LEN); + return false; + } + if (params->mqes < 1) { error_setg(errp, "mqes property cannot be less than 1"); return false; @@ -XXX,XX +XXX,XX @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev) id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID)); id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID)); - strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' '); + strpadcpy((char *)id->mn, sizeof(id->mn), + n->params.model ? n->params.model : "QEMU NVMe Ctrl", ' '); strpadcpy((char *)id->fr, sizeof(id->fr), QEMU_VERSION, ' '); strpadcpy((char *)id->sn, sizeof(id->sn), n->params.serial, ' '); @@ -XXX,XX +XXX,XX @@ static const Property nvme_props[] = { DEFINE_PROP_LINK("subsys", NvmeCtrl, subsys, TYPE_NVME_SUBSYS, NvmeSubsystem *), DEFINE_PROP_STRING("serial", NvmeCtrl, params.serial), + DEFINE_PROP_STRING("model", NvmeCtrl, params.model), DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, params.cmb_size_mb, 0), DEFINE_PROP_UINT32("num_queues", NvmeCtrl, params.num_queues, 0), DEFINE_PROP_UINT32("max_ioqpairs", NvmeCtrl, params.max_ioqpairs, 64), diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h index XXXXXXX..XXXXXXX 100644 --- a/hw/nvme/nvme.h +++ b/hw/nvme/nvme.h @@ -XXX,XX +XXX,XX @@ typedef struct NvmeCQueue { typedef struct NvmeParams { char *serial; + char *model; uint32_t num_queues; /* deprecated since 5.1 */ uint32_t max_ioqpairs; uint16_t msix_qsize; -- 2.53.0 From: Daniel P. Berrangé <berrange@redhat.com> This enables overriding the built in default QEMU project version string with a user specified string. The value can be at most 8 characters in length. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- docs/system/devices/nvme.rst | 5 +++++ hw/nvme/ctrl.c | 14 ++++++++++++-- hw/nvme/nvme.h | 1 + 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/docs/system/devices/nvme.rst b/docs/system/devices/nvme.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/devices/nvme.rst +++ b/docs/system/devices/nvme.rst @@ -XXX,XX +XXX,XX @@ parameters. to more closely impersonate a particular device type. The model name can be a maximum of 40 characters in length. +``firmware-version`` (default: current QEMU version number) + Override the default reported firmware version, which can be used when + needing to more closely impersonate a particular device type. The version + can be a maximum of 8 characters in length. + Additional Namespaces --------------------- diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index XXXXXXX..XXXXXXX 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -XXX,XX +XXX,XX @@ * atomic.awun<N[optional]>, \ * atomic.awupf<N[optional]>, \ * subsys=<subsys_id>, \ - * model=<model-str> + * model=<model-str>, \ + * firmware-version=<version-str> * -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\ * zoned=<true|false[optional]>, \ * subsys=<subsys_id>,shared=<true|false[optional]>, \ @@ -XXX,XX +XXX,XX @@ static bool nvme_check_params(NvmeCtrl *n, Error **errp) return false; } + if (params->firmware_version && + strlen(params->firmware_version) > NVME_ID_CTRL_FR_MAX_LEN) { + error_setg(errp, "'firmware-version' parameter '%s' can be at most '%d' characters", + params->firmware_version, NVME_ID_CTRL_FR_MAX_LEN); + return false; + } + if (params->mqes < 1) { error_setg(errp, "mqes property cannot be less than 1"); return false; @@ -XXX,XX +XXX,XX @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev) id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID)); strpadcpy((char *)id->mn, sizeof(id->mn), n->params.model ? n->params.model : "QEMU NVMe Ctrl", ' '); - strpadcpy((char *)id->fr, sizeof(id->fr), QEMU_VERSION, ' '); + strpadcpy((char *)id->fr, sizeof(id->fr), + n->params.firmware_version ? n->params.firmware_version : QEMU_VERSION, ' '); strpadcpy((char *)id->sn, sizeof(id->sn), n->params.serial, ' '); id->cntlid = cpu_to_le16(n->cntlid); @@ -XXX,XX +XXX,XX @@ static const Property nvme_props[] = { NvmeSubsystem *), DEFINE_PROP_STRING("serial", NvmeCtrl, params.serial), DEFINE_PROP_STRING("model", NvmeCtrl, params.model), + DEFINE_PROP_STRING("firmware-version", NvmeCtrl, params.firmware_version), DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, params.cmb_size_mb, 0), DEFINE_PROP_UINT32("num_queues", NvmeCtrl, params.num_queues, 0), DEFINE_PROP_UINT32("max_ioqpairs", NvmeCtrl, params.max_ioqpairs, 64), diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h index XXXXXXX..XXXXXXX 100644 --- a/hw/nvme/nvme.h +++ b/hw/nvme/nvme.h @@ -XXX,XX +XXX,XX @@ typedef struct NvmeCQueue { typedef struct NvmeParams { char *serial; char *model; + char *firmware_version; uint32_t num_queues; /* deprecated since 5.1 */ uint32_t max_ioqpairs; uint16_t msix_qsize; -- 2.53.0 From: Klaus Jensen <k.jensen@samsung.com> If MSI-X is not enabled when the admin completion queue is created, msix_vector_use() is not called. But, if MSI-X is subsequently enabled, msix_notify() will fail to fire the interrupt because the use count for the vector remains at 0. msix_vector_use/unuse should be called if MSI-X is *present*, not *enabled*. Fix this. Cc: qemu-stable@nongnu.org Reported-by: Andreas Hindborg <a.hindborg@samsung.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- hw/nvme/ctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index XXXXXXX..XXXXXXX 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -XXX,XX +XXX,XX @@ static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n) event_notifier_set_handler(&cq->notifier, NULL); event_notifier_cleanup(&cq->notifier); } - if (msix_enabled(pci) && cq->irq_enabled) { + if (msix_present(pci) && cq->irq_enabled) { msix_vector_unuse(pci, cq->vector); } if (cq->cqid) { @@ -XXX,XX +XXX,XX @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n, uint64_t dma_addr, { PCIDevice *pci = PCI_DEVICE(n); - if (msix_enabled(pci) && irq_enabled) { + if (msix_present(pci) && irq_enabled) { msix_vector_use(pci, vector); } -- 2.53.0 From: Klaus Jensen <k.jensen@samsung.com> Hi, The following changes since commit b428fe036233cbd15d37e3c027ab6ca4d3661a80: Merge tag 'pull-target-arm-20260731' of https://gitlab.com/pm215/qemu into staging (2026-07-31 16:19:04 -0400) are available in the Git repository at: https://gitlab.com/birkelund/qemu.git tags/pull-nvme-20260803 for you to fetch changes up to 7a34f7b8794b29cd1bd4dfa45f7d8e8daba7cff5: hw/nvme: fix leak on copy ranges (2026-08-03 15:35:20 -0700) ---------------------------------------------------------------- nvme queue ---------------------------------------------------------------- Klaus Jensen (1): hw/nvme: fix leak on copy ranges Minwoo Im (3): hw/nvme: drop AER requests without aiocb in nvme_del_sq() hw/nvme: factor out nvme_sq_cancel_inflight() hw/nvme: cancel inflight requests on controller reset hw/nvme/ctrl.c | 42 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-) From: Minwoo Im <minwoo.im.dev@gmail.com> nvme_del_sq() asserted r->aiocb was always set when canceling a queue's inflight requests. A pending Async Event Request has no aiocb (nvme_aer() parks it without issuing any block I/O), so deleting a queue with an outstanding AER trips the assert instead of just dropping the request. Cc: qemu-stable@nongnu.org Signed-off-by: Minwoo Im <minwoo.im@samsung.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- hw/nvme/ctrl.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index XXXXXXX..XXXXXXX 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -XXX,XX +XXX,XX @@ static uint16_t nvme_del_sq(NvmeCtrl *n, NvmeRequest *req) sq = n->sq[qid]; while (!QTAILQ_EMPTY(&sq->out_req_list)) { r = QTAILQ_FIRST(&sq->out_req_list); - assert(r->aiocb); r->status = NVME_CMD_ABORT_SQ_DEL; - blk_aio_cancel(r->aiocb); - } - assert(QTAILQ_EMPTY(&sq->out_req_list)); + if (r->aiocb) { + blk_aio_cancel(r->aiocb); + } else { + QTAILQ_REMOVE(&sq->out_req_list, r, entry); + } + } if (!nvme_check_cqid(n, sq->cqid)) { cq = n->cq[sq->cqid]; -- 2.53.0 From: Minwoo Im <minwoo.im.dev@gmail.com> Factor the cancel-and-wait loop used by nvme_del_sq() into nvme_sq_cancel_inflight(), so it can be reused to drain queues on controller reset. Cc: qemu-stable@nongnu.org Signed-off-by: Minwoo Im <minwoo.im@samsung.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- hw/nvme/ctrl.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index XXXXXXX..XXXXXXX 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -XXX,XX +XXX,XX @@ static int nvme_init_sq_ioeventfd(NvmeSQueue *sq) return 0; } +/* + * A pending Async Event Request has no aiocb (nvme_aer() parks it without + * issuing any block I/O), so there is nothing to cancel; just drop it. + */ +static void nvme_sq_cancel_inflight(NvmeSQueue *sq, uint16_t status) +{ + NvmeRequest *r; + + while (!QTAILQ_EMPTY(&sq->out_req_list)) { + r = QTAILQ_FIRST(&sq->out_req_list); + r->status = status; + + if (r->aiocb) { + blk_aio_cancel(r->aiocb); + } else { + QTAILQ_REMOVE(&sq->out_req_list, r, entry); + } + } +} + static void nvme_free_sq(NvmeSQueue *sq, NvmeCtrl *n) { uint16_t offset = sq->sqid << 3; @@ -XXX,XX +XXX,XX @@ static uint16_t nvme_del_sq(NvmeCtrl *n, NvmeRequest *req) trace_pci_nvme_del_sq(qid); sq = n->sq[qid]; - while (!QTAILQ_EMPTY(&sq->out_req_list)) { - r = QTAILQ_FIRST(&sq->out_req_list); - r->status = NVME_CMD_ABORT_SQ_DEL; - - if (r->aiocb) { - blk_aio_cancel(r->aiocb); - } else { - QTAILQ_REMOVE(&sq->out_req_list, r, entry); - } - } + nvme_sq_cancel_inflight(sq, NVME_CMD_ABORT_SQ_DEL); if (!nvme_check_cqid(n, sq->cqid)) { cq = n->cq[sq->cqid]; -- 2.53.0 From: Minwoo Im <minwoo.im.dev@gmail.com> nvme_ctrl_reset() freed every SQ/CQ right after nvme_ns_drain(), which only waits out requests on a per-namespace BlockBackend. That is safe as long as the guest first tore down I/O queues gracefully (Delete I/O SQ/CQ), since nvme_del_sq() already cancels and waits for anything left on a queue before freeing it. A reset that happens without that graceful sequence first (e.g. an abrupt/asynchronous controller reset) can still have commands inflight on blk_aio_*. Freeing sq/cq before those complete leaves their completion callbacks (nvme_rw_cb() and friends) to run against already-freed NvmeRequest/NvmeSQueue/NvmeCQueue memory via nvme_enqueue_req_completion(), causing a use-after-free/segfault. Run nvme_sq_cancel_inflight() over every queue in nvme_ctrl_reset() before the free loops, so no in-flight blk_aio_* callback can fire after sq/cq memory is freed. Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3398 Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3883 Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4068 Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4072 Signed-off-by: Minwoo Im <minwoo.im@samsung.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- hw/nvme/ctrl.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index XXXXXXX..XXXXXXX 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -XXX,XX +XXX,XX @@ static void nvme_ctrl_reset(NvmeCtrl *n, NvmeResetType rst) nvme_ns_drain(ns); } + /* + * Cancel and wait out every inflight command on every queue first. A + * reset is not required to be preceded by the guest's graceful + * Delete I/O SQ/CQ sequence, so sq/cq must not be freed below while a + * blk_aio_* completion for them could still be in flight. + */ + for (i = 0; i < n->num_queues; i++) { + if (n->sq[i] != NULL) { + nvme_sq_cancel_inflight(n->sq[i], NVME_CMD_ABORT_SQ_DEL); + } + } + for (i = 0; i < n->num_queues; i++) { if (n->sq[i] != NULL) { nvme_free_sq(n->sq[i], n); -- 2.53.0 From: Klaus Jensen <k.jensen@samsung.com> The buffer holding the ranges for the copy command is not correctly deallocated. Fix this. Cc: qemu-stable@nongnu.org Link: https://gitlab.com/qemu-project/qemu/-/work_items/4072 Fixes: 796d20681d9b ("hw/nvme: reimplement the copy command to allow aio cancellation") Reviewed-by: Jesper Wendel Devantier <foss@defmacro.it> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> --- hw/nvme/ctrl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index XXXXXXX..XXXXXXX 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -XXX,XX +XXX,XX @@ static void nvme_copy_done(NvmeCopyAIOCB *iocb) qemu_iovec_destroy(&iocb->iov); g_free(iocb->bounce); + g_free(iocb->ranges); if (iocb->ret < 0) { block_acct_failed(stats, &iocb->acct.read); -- 2.53.0