The following changes since commit 88afdc92b644120e0182c8567e1b1d236e471b12:

Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2021-09-05 15:48:42 +0100)

are available in the Git repository at:

https://gitlab.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 9bd2788f49c331b02372cc257b11e4c984d39708:

block/nvme: Only report VFIO error on failed retry (2021-09-07 09:08:24 +0100)

Pull request

Userspace NVMe driver patches.

Philippe Mathieu-Daudé (11):

block/nvme: Use safer trace format string

util/vfio-helpers: Let qemu_vfio_verify_mappings() use error_report()

util/vfio-helpers: Replace qemu_mutex_lock() calls with

QEMU_LOCK_GUARD

util/vfio-helpers: Remove unreachable code in qemu_vfio_dma_map()

block/nvme: Have nvme_create_queue_pair() report errors consistently

util/vfio-helpers: Pass Error handle to qemu_vfio_dma_map()

util/vfio-helpers: Extract qemu_vfio_water_mark_reached()

util/vfio-helpers: Use error_setg in qemu_vfio_find_[fixed/temp]_iova

util/vfio-helpers: Simplify qemu_vfio_dma_map() returning directly

util/vfio-helpers: Let qemu_vfio_do_mapping() propagate Error

block/nvme: Only report VFIO error on failed retry

include/qemu/vfio-helpers.h | 2 +-

block/nvme.c | 29 +++++++----

util/vfio-helpers.c | 99 ++++++++++++++++++++-----------------

block/trace-events | 2 +-

4 files changed, 76 insertions(+), 56 deletions(-)

2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Fix when building with -Wshorten-64-to-32:

warning: implicit conversion loses integer precision: 'unsigned long' to 'int' [-Wshorten-64-to-32]

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20210902070025.197072-2-philmd@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

block/trace-events | 2 +-

diff --git a/block/trace-events b/block/trace-events

--- a/block/trace-events

+++ b/block/trace-events

@@ -XXX,XX +XXX,XX @@ nvme_dsm(void *s, uint64_t offset, uint64_t bytes) "s %p offset 0x%"PRIx64" byte

nvme_dsm_done(void *s, uint64_t offset, uint64_t bytes, int ret) "s %p offset 0x%"PRIx64" bytes %"PRId64" ret %d"

nvme_dma_map_flush(void *s) "s %p"

nvme_free_req_queue_wait(void *s, unsigned q_index) "s %p q #%u"

-nvme_create_queue_pair(unsigned q_index, void *q, unsigned size, void *aio_context, int fd) "index %u q %p size %u aioctx %p fd %d"

+nvme_create_queue_pair(unsigned q_index, void *q, size_t size, void *aio_context, int fd) "index %u q %p size %zu aioctx %p fd %d"

nvme_free_queue_pair(unsigned q_index, void *q) "index %u q %p"

nvme_cmd_map_qiov(void *s, void *cmd, void *req, void *qiov, int entries) "s %p cmd %p req %p qiov %p entries %d"

nvme_cmd_map_qiov_pages(void *s, int i, uint64_t page) "s %p page[%d] 0x%"PRIx64

2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Instead of displaying the error on stderr, use error_report()

which also report to the monitor.

Reviewed-by: Fam Zheng <fam@euphon.net>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20210902070025.197072-3-philmd@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

util/vfio-helpers.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c

--- a/util/vfio-helpers.c

+++ b/util/vfio-helpers.c

@@ -XXX,XX +XXX,XX @@ static bool qemu_vfio_verify_mappings(QEMUVFIOState *s)

if (QEMU_VFIO_DEBUG) {

for (i = 0; i < s->nr_mappings - 1; ++i) {

if (!(s->mappings[i].host < s->mappings[i + 1].host)) {

- fprintf(stderr, "item %d not sorted!\n", i);

+ error_report("item %d not sorted!", i);

qemu_vfio_dump_mappings(s);

return false;

}

if (!(s->mappings[i].host + s->mappings[i].size <=

s->mappings[i + 1].host)) {

- fprintf(stderr, "item %d overlap with next!\n", i);

+ error_report("item %d overlap with next!", i);

qemu_vfio_dump_mappings(s);

return false;

}

2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Simplify qemu_vfio_dma_[un]map() handlers by replacing a pair of

qemu_mutex_lock/qemu_mutex_unlock calls by the WITH_QEMU_LOCK_GUARD

macro.

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20210902070025.197072-4-philmd@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

util/vfio-helpers.c | 9 +++------

1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c

--- a/util/vfio-helpers.c

+++ b/util/vfio-helpers.c

@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,

assert(QEMU_PTR_IS_ALIGNED(host, qemu_real_host_page_size));

assert(QEMU_IS_ALIGNED(size, qemu_real_host_page_size));

trace_qemu_vfio_dma_map(s, host, size, temporary, iova);

- qemu_mutex_lock(&s->lock);

+ QEMU_LOCK_GUARD(&s->lock);

mapping = qemu_vfio_find_mapping(s, host, &index);

if (mapping) {

iova0 = mapping->iova + ((uint8_t *)host - (uint8_t *)mapping->host);

@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,

*iova = iova0;

}

out:

- qemu_mutex_unlock(&s->lock);

return ret;

@@ -XXX,XX +XXX,XX @@ void qemu_vfio_dma_unmap(QEMUVFIOState *s, void *host)

}

trace_qemu_vfio_dma_unmap(s, host);

- qemu_mutex_lock(&s->lock);

+ QEMU_LOCK_GUARD(&s->lock);

m = qemu_vfio_find_mapping(s, host, &index);

if (!m) {

- goto out;

}

qemu_vfio_undo_mapping(s, m, NULL);

-out:

- qemu_mutex_unlock(&s->lock);

static void qemu_vfio_reset(QEMUVFIOState *s)

2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

qemu_vfio_add_mapping() returns a pointer to an indexed entry

in pre-allocated QEMUVFIOState::mappings[], thus can not be NULL.

Remove the pointless check.

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20210902070025.197072-5-philmd@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

util/vfio-helpers.c | 4 ----

1 file changed, 4 deletions(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c

--- a/util/vfio-helpers.c

+++ b/util/vfio-helpers.c

@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,

}

mapping = qemu_vfio_add_mapping(s, host, size, index + 1, iova0);

- if (!mapping) {

- ret = -ENOMEM;

- goto out;

- }

assert(qemu_vfio_verify_mappings(s));

ret = qemu_vfio_do_mapping(s, host, size, iova0);

if (ret) {

2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

nvme_create_queue_pair() does not return a boolean value (indicating

eventual error) but a pointer, and is inconsistent in how it fills the

error handler. To fulfill callers expectations, always set an error

message on failure.

Reported-by: Auger Eric <eric.auger@redhat.com>

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20210902070025.197072-6-philmd@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

block/nvme.c | 3 +++

1 file changed, 3 insertions(+)

diff --git a/block/nvme.c b/block/nvme.c

--- a/block/nvme.c

+++ b/block/nvme.c

@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,

q = g_try_new0(NVMeQueuePair, 1);

if (!q) {

+ error_setg(errp, "Cannot allocate queue pair");

return NULL;

trace_nvme_create_queue_pair(idx, q, size, aio_context,

@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,

qemu_real_host_page_size);

q->prp_list_pages = qemu_try_memalign(qemu_real_host_page_size, bytes);

if (!q->prp_list_pages) {

+ error_setg(errp, "Cannot allocate PRP page list");

goto fail;

memset(q->prp_list_pages, 0, bytes);

@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,

r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages, bytes,

false, &prp_list_iova);

if (r) {

+ error_setg_errno(errp, -r, "Cannot map buffer for DMA");

goto fail;

q->free_req_head = -1;

2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Currently qemu_vfio_dma_map() displays errors on stderr.

When using management interface, this information is simply

lost. Pass qemu_vfio_dma_map() an Error** handle so it can

propagate the error to callers.

Reviewed-by: Fam Zheng <fam@euphon.net>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20210902070025.197072-7-philmd@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

include/qemu/vfio-helpers.h | 2 +-

block/nvme.c | 22 +++++++++++-----------

util/vfio-helpers.c | 10 ++++++----

3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/include/qemu/vfio-helpers.h b/include/qemu/vfio-helpers.h

--- a/include/qemu/vfio-helpers.h

+++ b/include/qemu/vfio-helpers.h

@@ -XXX,XX +XXX,XX @@ typedef struct QEMUVFIOState QEMUVFIOState;

QEMUVFIOState *qemu_vfio_open_pci(const char *device, Error **errp);

void qemu_vfio_close(QEMUVFIOState *s);

int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,

- bool temporary, uint64_t *iova_list);

+ bool temporary, uint64_t *iova_list, Error **errp);

int qemu_vfio_dma_reset_temporary(QEMUVFIOState *s);

void qemu_vfio_dma_unmap(QEMUVFIOState *s, void *host);

void *qemu_vfio_pci_map_bar(QEMUVFIOState *s, int index,

diff --git a/block/nvme.c b/block/nvme.c

index XXXXXXX..XXXXXXX 100644

--- a/block/nvme.c

+++ b/block/nvme.c

@@ -XXX,XX +XXX,XX @@ static bool nvme_init_queue(BDRVNVMeState *s, NVMeQueue *q,

return false;

}

memset(q->queue, 0, bytes);

- r = qemu_vfio_dma_map(s->vfio, q->queue, bytes, false, &q->iova);

+ r = qemu_vfio_dma_map(s->vfio, q->queue, bytes, false, &q->iova, errp);

if (r) {

- error_setg(errp, "Cannot map queue");

- return false;

+ error_prepend(errp, "Cannot map queue: ");

}

- return true;

+ return r == 0;

}

static void nvme_free_queue_pair(NVMeQueuePair *q)

@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,

qemu_co_queue_init(&q->free_req_queue);

q->completion_bh = aio_bh_new(aio_context, nvme_process_completion_bh, q);

r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages, bytes,

- false, &prp_list_iova);

+ false, &prp_list_iova, errp);

if (r) {

- error_setg_errno(errp, -r, "Cannot map buffer for DMA");

+ error_prepend(errp, "Cannot map buffer for DMA: ");

goto fail;

}

q->free_req_head = -1;

@@ -XXX,XX +XXX,XX @@ static bool nvme_identify(BlockDriverState *bs, int namespace, Error **errp)

error_setg(errp, "Cannot allocate buffer for identify response");

goto out;

}

- r = qemu_vfio_dma_map(s->vfio, id, id_size, true, &iova);

+ r = qemu_vfio_dma_map(s->vfio, id, id_size, true, &iova, errp);

if (r) {

- error_setg(errp, "Cannot map buffer for DMA");

+ error_prepend(errp, "Cannot map buffer for DMA: ");

goto out;

}

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_cmd_map_qiov(BlockDriverState *bs, NvmeCmd *cmd,

try_map:

r = qemu_vfio_dma_map(s->vfio,

qiov->iov[i].iov_base,

- len, true, &iova);

+ len, true, &iova, NULL);

if (r == -ENOSPC) {

/*

* In addition to the -ENOMEM error, the VFIO_IOMMU_MAP_DMA

@@ -XXX,XX +XXX,XX @@ static void nvme_aio_unplug(BlockDriverState *bs)

static void nvme_register_buf(BlockDriverState *bs, void *host, size_t size)

int ret;

+ Error *local_err = NULL;

BDRVNVMeState *s = bs->opaque;

- ret = qemu_vfio_dma_map(s->vfio, host, size, false, NULL);

+ ret = qemu_vfio_dma_map(s->vfio, host, size, false, NULL, &local_err);

if (ret) {

/* FIXME: we may run out of IOVA addresses after repeated

* bdrv_register_buf/bdrv_unregister_buf, because nvme_vfio_dma_unmap

* doesn't reclaim addresses for fixed mappings. */

- error_report("nvme_register_buf failed: %s", strerror(-ret));

+ error_reportf_err(local_err, "nvme_register_buf failed: ");

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c

index XXXXXXX..XXXXXXX 100644

--- a/util/vfio-helpers.c

+++ b/util/vfio-helpers.c

@@ -XXX,XX +XXX,XX @@ static void qemu_vfio_ram_block_added(RAMBlockNotifier *n, void *host,

size_t size, size_t max_size)

QEMUVFIOState *s = container_of(n, QEMUVFIOState, ram_notifier);

+ Error *local_err = NULL;

int ret;

trace_qemu_vfio_ram_block_added(s, host, max_size);

- ret = qemu_vfio_dma_map(s, host, max_size, false, NULL);

+ ret = qemu_vfio_dma_map(s, host, max_size, false, NULL, &local_err);

if (ret) {

- error_report("qemu_vfio_dma_map(%p, %zu) failed: %s", host, max_size,

- strerror(-ret));

+ error_reportf_err(local_err,

+ "qemu_vfio_dma_map(%p, %zu) failed: ",

+ host, max_size);

@@ -XXX,XX +XXX,XX @@ qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)

* mapping status within this area is not allowed).

*/

int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,

- bool temporary, uint64_t *iova)

+ bool temporary, uint64_t *iova, Error **errp)

int ret = 0;

int index;

2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Extract qemu_vfio_water_mark_reached() for readability,

and have it provide an error hint it its Error* handle.

Suggested-by: Klaus Jensen <k.jensen@samsung.com>

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20210902070025.197072-8-philmd@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

util/vfio-helpers.c | 17 ++++++++++++++++-

1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c

--- a/util/vfio-helpers.c

+++ b/util/vfio-helpers.c

@@ -XXX,XX +XXX,XX @@ qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)

return -ENOMEM;

+/**

+ * qemu_vfio_water_mark_reached:

+ *

+ * Returns %true if high watermark has been reached, %false otherwise.

+ */

+static bool qemu_vfio_water_mark_reached(QEMUVFIOState *s, size_t size,

+ Error **errp)

+ if (s->high_water_mark - s->low_water_mark + 1 < size) {

+ error_setg(errp, "iova exhausted (water mark reached)");

+ return true;

+ return false;

/* Map [host, host + size) area into a contiguous IOVA address space, and store

* the result in @iova if not NULL. The caller need to make sure the area is

* aligned to page size, and mustn't overlap with existing mapping areas (split

@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,

if (mapping) {

iova0 = mapping->iova + ((uint8_t *)host - (uint8_t *)mapping->host);

} else {

- if (s->high_water_mark - s->low_water_mark + 1 < size) {

+ if (qemu_vfio_water_mark_reached(s, size, errp)) {

ret = -ENOMEM;

goto out;