From 5ef1f4ec6fd8cbf0b312acb32be8166ab8b1b24d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 2 Sep 2021 09:00:15 +0200 Subject: [PATCH 01/11] block/nvme: Use safer trace format string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Philippe Mathieu-Daudé Message-id: 20210902070025.197072-2-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- block/trace-events | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/trace-events b/block/trace-events index b3d2b1e62c..f4f1267c8c 100644 --- a/block/trace-events +++ b/block/trace-events @@ -156,7 +156,7 @@ 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 From cb49dfce586367cda313fe9ce92e095f534e8178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 2 Sep 2021 09:00:16 +0200 Subject: [PATCH 02/11] util/vfio-helpers: Let qemu_vfio_verify_mappings() use error_report() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of displaying the error on stderr, use error_report() which also report to the monitor. Reviewed-by: Fam Zheng Reviewed-by: Stefan Hajnoczi Reviewed-by: Klaus Jensen Signed-off-by: Philippe Mathieu-Daudé Message-id: 20210902070025.197072-3-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- util/vfio-helpers.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c index 911115b86e..1d14913629 100644 --- a/util/vfio-helpers.c +++ b/util/vfio-helpers.c @@ -660,13 +660,13 @@ 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; } From a990858b0cc0a7fcbf6e0ee102603e6adbbbbaef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 2 Sep 2021 09:00:17 +0200 Subject: [PATCH 03/11] util/vfio-helpers: Replace qemu_mutex_lock() calls with QEMU_LOCK_GUARD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Philippe Mathieu-Daudé Message-id: 20210902070025.197072-4-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- util/vfio-helpers.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c index 1d14913629..d956866b4e 100644 --- a/util/vfio-helpers.c +++ b/util/vfio-helpers.c @@ -735,7 +735,7 @@ 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); @@ -778,7 +778,6 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size, *iova = iova0; } out: - qemu_mutex_unlock(&s->lock); return ret; } @@ -813,14 +812,12 @@ 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; + return; } qemu_vfio_undo_mapping(s, m, NULL); -out: - qemu_mutex_unlock(&s->lock); } static void qemu_vfio_reset(QEMUVFIOState *s) From 3f4c0affcfbc7c1da0b117bee0cd5be6f52fa3e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 2 Sep 2021 09:00:18 +0200 Subject: [PATCH 04/11] util/vfio-helpers: Remove unreachable code in qemu_vfio_dma_map() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Philippe Mathieu-Daudé Message-id: 20210902070025.197072-5-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- util/vfio-helpers.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c index d956866b4e..e7909222cf 100644 --- a/util/vfio-helpers.c +++ b/util/vfio-helpers.c @@ -751,10 +751,6 @@ 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) { From 526c37c19df31a651b3f9de7b5f2f5100c25017b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 2 Sep 2021 09:00:19 +0200 Subject: [PATCH 05/11] block/nvme: Have nvme_create_queue_pair() report errors consistently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Klaus Jensen Signed-off-by: Philippe Mathieu-Daudé Message-id: 20210902070025.197072-6-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- block/nvme.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/nvme.c b/block/nvme.c index e8dbbc2317..0786c501e4 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -220,6 +220,7 @@ 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, @@ -228,6 +229,7 @@ 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); @@ -239,6 +241,7 @@ 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; From 521b97cd4e5f4a636f56515057084b1a86552b0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 2 Sep 2021 09:00:20 +0200 Subject: [PATCH 06/11] util/vfio-helpers: Pass Error handle to qemu_vfio_dma_map() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Stefan Hajnoczi Reviewed-by: Klaus Jensen Signed-off-by: Philippe Mathieu-Daudé Message-id: 20210902070025.197072-7-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- block/nvme.c | 22 +++++++++++----------- include/qemu/vfio-helpers.h | 2 +- util/vfio-helpers.c | 10 ++++++---- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index 0786c501e4..80546b0bab 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -176,12 +176,11 @@ 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) @@ -239,9 +238,9 @@ 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; @@ -534,9 +533,9 @@ 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; } @@ -1032,7 +1031,7 @@ 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 @@ -1524,14 +1523,15 @@ 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/include/qemu/vfio-helpers.h b/include/qemu/vfio-helpers.h index 4491c8e1a6..bde9495b25 100644 --- a/include/qemu/vfio-helpers.h +++ b/include/qemu/vfio-helpers.h @@ -18,7 +18,7 @@ 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/util/vfio-helpers.c b/util/vfio-helpers.c index e7909222cf..77cdec845d 100644 --- a/util/vfio-helpers.c +++ b/util/vfio-helpers.c @@ -463,13 +463,15 @@ 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); } } @@ -725,7 +727,7 @@ 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; From 71e3038c1500e2d6075b306f26d565f982287756 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 2 Sep 2021 09:00:21 +0200 Subject: [PATCH 07/11] util/vfio-helpers: Extract qemu_vfio_water_mark_reached() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract qemu_vfio_water_mark_reached() for readability, and have it provide an error hint it its Error* handle. Suggested-by: Klaus Jensen Reviewed-by: Klaus Jensen Signed-off-by: Philippe Mathieu-Daudé Message-id: 20210902070025.197072-8-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- util/vfio-helpers.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c index 77cdec845d..306b5a83e4 100644 --- a/util/vfio-helpers.c +++ b/util/vfio-helpers.c @@ -721,6 +721,21 @@ 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 @@ -742,7 +757,7 @@ 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; } From 453095e98dc2cefba81dae800614f136f3c1c341 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 2 Sep 2021 09:00:22 +0200 Subject: [PATCH 08/11] util/vfio-helpers: Use error_setg in qemu_vfio_find_[fixed/temp]_iova MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both qemu_vfio_find_fixed_iova() and qemu_vfio_find_temp_iova() return an errno which is unused (or overwritten). Have them propagate eventual errors to callers, returning a boolean (which is what the Error API recommends, see commit e3fe3988d78 "error: Document Error API usage rules" for rationale). Suggested-by: Klaus Jensen Reviewed-by: Klaus Jensen Signed-off-by: Philippe Mathieu-Daudé Message-id: 20210902070025.197072-9-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- util/vfio-helpers.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c index 306b5a83e4..b93a3d3578 100644 --- a/util/vfio-helpers.c +++ b/util/vfio-helpers.c @@ -677,8 +677,8 @@ static bool qemu_vfio_verify_mappings(QEMUVFIOState *s) return true; } -static int -qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova) +static bool qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, + uint64_t *iova, Error **errp) { int i; @@ -693,14 +693,16 @@ qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova) s->usable_iova_ranges[i].end - s->low_water_mark + 1 == 0) { *iova = s->low_water_mark; s->low_water_mark += size; - return 0; + return true; } } - return -ENOMEM; + error_setg(errp, "fixed iova range not found"); + + return false; } -static int -qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova) +static bool qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, + uint64_t *iova, Error **errp) { int i; @@ -715,10 +717,12 @@ qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova) s->high_water_mark - s->usable_iova_ranges[i].start + 1 == 0) { *iova = s->high_water_mark - size; s->high_water_mark = *iova; - return 0; + return true; } } - return -ENOMEM; + error_setg(errp, "temporary iova range not found"); + + return false; } /** @@ -762,7 +766,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size, goto out; } if (!temporary) { - if (qemu_vfio_find_fixed_iova(s, size, &iova0)) { + if (!qemu_vfio_find_fixed_iova(s, size, &iova0, errp)) { ret = -ENOMEM; goto out; } @@ -776,7 +780,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size, } qemu_vfio_dump_mappings(s); } else { - if (qemu_vfio_find_temp_iova(s, size, &iova0)) { + if (!qemu_vfio_find_temp_iova(s, size, &iova0, errp)) { ret = -ENOMEM; goto out; } From 5a4f1626e307024765f7e5b45f5884e34a27d968 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 2 Sep 2021 09:00:23 +0200 Subject: [PATCH 09/11] util/vfio-helpers: Simplify qemu_vfio_dma_map() returning directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To simplify qemu_vfio_dma_map(): - reduce 'ret' (returned value) scope by returning errno directly, - remove the goto 'out' label. Reviewed-by: Klaus Jensen Signed-off-by: Philippe Mathieu-Daudé Message-id: 20210902070025.197072-10-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- util/vfio-helpers.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c index b93a3d3578..66148bd381 100644 --- a/util/vfio-helpers.c +++ b/util/vfio-helpers.c @@ -748,7 +748,6 @@ static bool qemu_vfio_water_mark_reached(QEMUVFIOState *s, size_t size, int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size, bool temporary, uint64_t *iova, Error **errp) { - int ret = 0; int index; IOVAMapping *mapping; uint64_t iova0; @@ -761,32 +760,31 @@ 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 { + int ret; + if (qemu_vfio_water_mark_reached(s, size, errp)) { - ret = -ENOMEM; - goto out; + return -ENOMEM; } if (!temporary) { if (!qemu_vfio_find_fixed_iova(s, size, &iova0, errp)) { - ret = -ENOMEM; - goto out; + return -ENOMEM; } mapping = qemu_vfio_add_mapping(s, host, size, index + 1, iova0); assert(qemu_vfio_verify_mappings(s)); ret = qemu_vfio_do_mapping(s, host, size, iova0); - if (ret) { + if (ret < 0) { qemu_vfio_undo_mapping(s, mapping, NULL); - goto out; + return ret; } qemu_vfio_dump_mappings(s); } else { if (!qemu_vfio_find_temp_iova(s, size, &iova0, errp)) { - ret = -ENOMEM; - goto out; + return -ENOMEM; } ret = qemu_vfio_do_mapping(s, host, size, iova0); - if (ret) { - goto out; + if (ret < 0) { + return ret; } } } @@ -794,8 +792,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size, if (iova) { *iova = iova0; } -out: - return ret; + return 0; } /* Reset the high watermark and free all "temporary" mappings. */ From f38b376d421dcb35027e6d9bf852b4091dc87d83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 2 Sep 2021 09:00:24 +0200 Subject: [PATCH 10/11] util/vfio-helpers: Let qemu_vfio_do_mapping() propagate Error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass qemu_vfio_do_mapping() an Error* argument so it can propagate any error to callers. Replace error_report() which only report to the monitor by the more generic error_setg_errno(). Reviewed-by: Fam Zheng Reviewed-by: Stefan Hajnoczi Reviewed-by: Klaus Jensen Signed-off-by: Philippe Mathieu-Daudé Message-id: 20210902070025.197072-11-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- util/vfio-helpers.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c index 66148bd381..00a80431a0 100644 --- a/util/vfio-helpers.c +++ b/util/vfio-helpers.c @@ -610,7 +610,7 @@ static IOVAMapping *qemu_vfio_add_mapping(QEMUVFIOState *s, /* Do the DMA mapping with VFIO. */ static int qemu_vfio_do_mapping(QEMUVFIOState *s, void *host, size_t size, - uint64_t iova) + uint64_t iova, Error **errp) { struct vfio_iommu_type1_dma_map dma_map = { .argsz = sizeof(dma_map), @@ -622,7 +622,7 @@ static int qemu_vfio_do_mapping(QEMUVFIOState *s, void *host, size_t size, trace_qemu_vfio_do_mapping(s, host, iova, size); if (ioctl(s->container, VFIO_IOMMU_MAP_DMA, &dma_map)) { - error_report("VFIO_MAP_DMA failed: %s", strerror(errno)); + error_setg_errno(errp, errno, "VFIO_MAP_DMA failed"); return -errno; } return 0; @@ -772,7 +772,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size, mapping = qemu_vfio_add_mapping(s, host, size, index + 1, iova0); assert(qemu_vfio_verify_mappings(s)); - ret = qemu_vfio_do_mapping(s, host, size, iova0); + ret = qemu_vfio_do_mapping(s, host, size, iova0, errp); if (ret < 0) { qemu_vfio_undo_mapping(s, mapping, NULL); return ret; @@ -782,7 +782,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size, if (!qemu_vfio_find_temp_iova(s, size, &iova0, errp)) { return -ENOMEM; } - ret = qemu_vfio_do_mapping(s, host, size, iova0); + ret = qemu_vfio_do_mapping(s, host, size, iova0, errp); if (ret < 0) { return ret; } From 9bd2788f49c331b02372cc257b11e4c984d39708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 2 Sep 2021 09:00:25 +0200 Subject: [PATCH 11/11] block/nvme: Only report VFIO error on failed retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We expect the first qemu_vfio_dma_map() to fail (indicating DMA mappings exhaustion, see commit 15a730e7a3a). Do not report the first failure as error, since we are going to flush the mappings and retry. This removes spurious error message displayed on the monitor: (qemu) c (qemu) qemu-kvm: VFIO_MAP_DMA failed: No space left on device (qemu) info status VM status: running Reported-by: Tingting Mao Reviewed-by: Klaus Jensen Signed-off-by: Philippe Mathieu-Daudé Message-id: 20210902070025.197072-12-philmd@redhat.com Signed-off-by: Stefan Hajnoczi --- block/nvme.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/block/nvme.c b/block/nvme.c index 80546b0bab..abfe305baf 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -1019,6 +1019,7 @@ static coroutine_fn int nvme_cmd_map_qiov(BlockDriverState *bs, NvmeCmd *cmd, uint64_t *pagelist = req->prp_list_page; int i, j, r; int entries = 0; + Error *local_err = NULL, **errp = NULL; assert(qiov->size); assert(QEMU_IS_ALIGNED(qiov->size, s->page_size)); @@ -1031,7 +1032,7 @@ 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, NULL); + len, true, &iova, errp); if (r == -ENOSPC) { /* * In addition to the -ENOMEM error, the VFIO_IOMMU_MAP_DMA @@ -1066,6 +1067,8 @@ try_map: goto fail; } } + errp = &local_err; + goto try_map; } if (r) { @@ -1109,6 +1112,9 @@ fail: * because they are already mapped before calling this function; for * temporary mappings, a later nvme_cmd_(un)map_qiov will reclaim by * calling qemu_vfio_dma_reset_temporary when necessary. */ + if (local_err) { + error_reportf_err(local_err, "Cannot map buffer for DMA: "); + } return r; }