On 10/7/26 22:09, Vivek Kasireddy wrote:
Make the error handling more robust in virtio_gpu_init_udmabuf()
by introducing 'Error **' parameter to capture errors and using
an enum from VFIO to categorize different errors. This allows for
better error reporting and handling of errors from
virtio_gpu_create_udmabuf() and virtio_gpu_remap_dmabuf().
Furtheremore, the main reason for using the VFIO enum is to handle
the special INVALID_IOV case which can happen in two scenarios:
1) When the Blob resource's memory is backed by a VFIO region. This
would cause INVALID_IOV error when we try to create a dmabuf via
udmabuf driver but we can recover from this error by having the
dmabuf be created by the VFIO driver instead.
2) When the Blob resource's memory is not backed by memfd or VFIO
region. In this case, since the error can be attributed to the
guest, we should use qemu_log_mask() instead of error_report_err()
and emit a message that can be used to debug the guest error.
Note that, to diagnose errors, a common pattern is to return -errno
to enable the caller to implement specific error handling. However, in
this case, since we want to distinguish the INVALID_IOV error from other
errors, using the VFIO enum allows us to do so and is very convenient.
Cc: Marc-André Lureau <[email protected]>
Cc: Alex Bennée <[email protected]>
Cc: Akihiko Odaki <[email protected]>
Cc: Dmitry Osipenko <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Cédric Le Goater <[email protected]>
Reviewed-by: Akihiko Odaki <[email protected]>
Signed-off-by: Vivek Kasireddy <[email protected]>
---
hw/display/virtio-gpu-dmabuf.c | 69 ++++++++++++++++++++++------------
1 file changed, 46 insertions(+), 23 deletions(-)
diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
index 6f05f367d0..c9de8e9ba3 100644
--- a/hw/display/virtio-gpu-dmabuf.c
+++ b/hw/display/virtio-gpu-dmabuf.c
@@ -18,6 +18,7 @@
#include "ui/console.h"
#include "hw/virtio/virtio-gpu.h"
#include "hw/virtio/virtio-gpu-pixman.h"
+#include "hw/vfio/vfio-device.h"
#include "trace.h"
#include "system/ramblock.h"
#include "system/hostmem.h"
@@ -27,28 +28,32 @@
#include "standard-headers/linux/udmabuf.h"
#include "standard-headers/drm/drm_fourcc.h"
-static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
+static int virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res,
+ Error **errp)
{
g_autofree struct udmabuf_create_list *list = NULL;
RAMBlock *rb;
ram_addr_t offset;
- int udmabuf, i;
+ int udmabuf, i, fd;
udmabuf = udmabuf_fd();
if (udmabuf < 0) {
- return;
+ error_setg(errp, "udmabuf device not available or enabled");
return -ENODEV
+ return VFIO_DMABUF_CREATE_ERR_UNSPEC;
}
list = g_try_malloc0(sizeof(struct udmabuf_create_list) +
sizeof(struct udmabuf_create_item) * res->iov_cnt);
if (!list) {
- return;
+ error_setg(errp, "failed to allocate udmabuf create list");
return -ENOMEM;
+ return VFIO_DMABUF_CREATE_ERR_UNSPEC;
}
for (i = 0; i < res->iov_cnt; i++) {
rb = qemu_ram_block_from_host(res->iov[i].iov_base, false, &offset);
if (!rb || rb->fd < 0) {
- return;
+ error_setg(errp, "IOV memory address incompatible with udmabuf ");
+ return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
return -EINVAL;
}
list->list[i].memfd = rb->fd;
@@ -59,22 +64,28 @@ static void virtio_gpu_create_udmabuf(struct
virtio_gpu_simple_resource *res)
list->count = res->iov_cnt;
list->flags = UDMABUF_FLAGS_CLOEXEC;
- res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
- if (res->dmabuf_fd < 0) {
- warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
- strerror(errno));
+ fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
+ if (fd < 0) {
int saved_errno = errno;
+ error_setg_errno(errp, errno, "UDMABUF_CREATE_LIST: ioctl failed");
+ if (errno == EINVAL || errno == EBADFD) {
+ return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
return -EINVAL;
+ }
+ return VFIO_DMABUF_CREATE_ERR_UNSPEC;
return -saved_errno;
}
+ return fd;
}
-static void virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res)
+static void *virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res,
+ Error **errp)
{
- res->remapped = mmap(NULL, res->blob_size, PROT_READ,
- MAP_SHARED, res->dmabuf_fd, 0);
- if (res->remapped == MAP_FAILED) {
- warn_report("%s: dmabuf mmap failed: %s", __func__,
- strerror(errno));
- res->remapped = NULL;
+ void *map;
+
+ map = mmap(NULL, res->blob_size, PROT_READ, MAP_SHARED, res->dmabuf_fd, 0);
+ if (map == MAP_FAILED) {
+ error_setg_errno(errp, errno, "dmabuf mmap failed");
+ return NULL;
}
+ return map;
}
void virtio_gpu_fini_dmabuf(struct virtio_gpu_simple_resource *res)
@@ -130,6 +141,7 @@ bool virtio_gpu_have_udmabuf(void)
bool virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
{
+ Error *local_err = NULL;
void *pdata = NULL;
res->dmabuf_fd = -1;
@@ -137,17 +149,28 @@ bool virtio_gpu_init_dmabuf(struct
virtio_gpu_simple_resource *res)
res->iov[0].iov_len < 4096) {
pdata = res->iov[0].iov_base;
} else if (res->blob_size) {
- virtio_gpu_create_udmabuf(res);
+ res->dmabuf_fd = virtio_gpu_create_udmabuf(res, &local_err);
+ if (res->dmabuf_fd == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
if (res->dmabuf_fd == -EINVAL) {
+ error_free_or_abort(&local_err);
+
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "Cannot create dmabuf: incompatible memory\n");
+ return false;
+ } else if (res->dmabuf_fd >= 0) {
+ pdata = virtio_gpu_remap_dmabuf(res, &local_err);
+ if (!pdata) {
+ virtio_gpu_fini_dmabuf(res);
+ }
+ } else {
+ res->dmabuf_fd = -1;
+ }
+
if (res->dmabuf_fd < 0) {
- return false;
- }
- virtio_gpu_remap_dmabuf(res);
- if (!res->remapped) {
- virtio_gpu_fini_dmabuf(res);
+ error_report_err(local_err);
return false;
}
res->share_handle = res->dmabuf_fd;
- pdata = res->remapped;
+ res->remapped = pdata;
}
res->blob = pdata;
