On 2026/02/19 16:23, Kasireddy, Vivek wrote:
Hi Akihiko,
Subject: Re: [PATCH v6 11/11] virtio-gpu-dmabuf: Create dmabuf for
blobs associated with VFIO devices
In addition to memfd, a blob resource can also have its backing
storage in a VFIO device region. Since, there is no effective way
to determine where the backing storage is located, we first try to
create a dmabuf assuming it is in memfd. If that fails, we try to
create a dmabuf assuming it is in VFIO device region.
So, we first call virtio_gpu_create_udmabuf() to check if the blob's
backing storage is located in a memfd or not. If it is not, we invoke
the vfio_device_create_dmabuf() API which identifies the right VFIO
device and eventually creates a dmabuf fd.
Note that in virtio_gpu_remap_dmabuf(), we first try to test if
the VFIO dmabuf exporter supports mmap or not. If it doesn't, we
fall back to the vfio_device_mmap_dmabuf() API to get a mapping
created for the dmabuf.
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]>
Signed-off-by: Vivek Kasireddy <[email protected]>
---
hw/display/Kconfig | 5 ++++
hw/display/virtio-gpu-dmabuf.c | 50
++++++++++++++++++++++++++++------
2 files changed, 47 insertions(+), 8 deletions(-)
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index 1e95ab28ef..0d090f25f5 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -106,6 +106,11 @@ config VIRTIO_VGA
depends on VIRTIO_PCI
select VGA
+config VIRTIO_GPU_VFIO_BLOB
+ bool
+ default y
+ depends on VFIO
+
config VHOST_USER_GPU
bool
default y
diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
dmabuf.c
index d9b2ecaf31..54d010b995 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,6 +28,27 @@
#include "standard-headers/linux/udmabuf.h"
#include "standard-headers/drm/drm_fourcc.h"
+static bool virtio_gpu_create_vfio_dmabuf(struct
virtio_gpu_simple_resource *r,
+ Error **errp)
+{
+ bool ret = false;
+#if defined(CONFIG_VIRTIO_GPU_VFIO_BLOB)
+ Error *local_err = NULL;
+ int fd;
+
+ if (!vfio_device_create_dmabuf(r->iov, r->iov_cnt, &fd,
&local_err))
{
+ error_report_err(*errp);
+ *errp = NULL;
+ error_propagate(errp, local_err);
+ return false;
+ }
+
+ ret = true;
+ r->dmabuf_fd = fd;
+#endif
+ return ret;
+}
+
static bool virtio_gpu_create_udmabuf(struct
virtio_gpu_simple_resource *res,
Error **errp)
{
@@ -73,16 +95,28 @@ static bool
virtio_gpu_create_udmabuf(struct
virtio_gpu_simple_resource *res,
static bool virtio_gpu_remap_dmabuf(struct
virtio_gpu_simple_resource *res,
Error **errp)
{
+ Error *local_err = NULL;
+ bool ret = true;
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");
- res->remapped = NULL;
- return false;
+ map = NULL;
+ ret = false;
+#if defined(CONFIG_VIRTIO_GPU_VFIO_BLOB)
+ if (vfio_device_mmap_dmabuf(res->iov, res->iov_cnt, &map,
+ res->blob_size, &local_err)) {
+ ret = true;
+ } else {
+ error_report_err(*errp);
+ *errp = NULL;
+ }
+#endif
+ error_propagate(errp, local_err);
}
res->remapped = map;
- return true;
+ return ret;
}
static void virtio_gpu_destroy_dmabuf(struct
virtio_gpu_simple_resource *res)
@@ -145,8 +179,10 @@ void virtio_gpu_init_dmabuf(struct
virtio_gpu_simple_resource *res)
pdata = res->iov[0].iov_base;
} else {
if (!virtio_gpu_create_udmabuf(res, &local_err)) {
virtio_gpu_create_udmabuf() will log "Could not find valid ramblock"
even for VFIO, which shouldn't happen.
AFAICS, it won't log a failure here because rb->fd is valid in the VFIO
case
especially after 8cfaf22668 ("Create dmabuf for PCI BAR per region").
The failure would occur in UDMABUF_CREATE_LIST IOCTL most likely
because rb->fd is not memfd in this case.
It is still nice to ensure that the log will never be emitted in the
VFIO case without relying on the internal behavior of the VFIO device
code.
Ok, got it.
This logic should look like as follows:
virtio_gpu_create_udmabuf(res);
if (it turned out the memory is incompatible with udmabuf) {
virtio_gpu_create_vfio_dmabuf(res);
if (it turned out the memory is not backed by VFIO) {
qemu_log_mask(LOG_GUEST_ERROR, "incompatible\n");
return;
I think this is mostly how I have it right now except that I am using
error_report_err() instead of qemu_log_mask(). Do you want me
to only use qemu_log_mask() when we cannot create a dmabuf via
udmabuf and vfio?
More precisely, use LOG_GUEST_ERROR if and only if creating a DMA-
BUF
failed:
- in *both* virtio_gpu_create_udmabuf() and
virtio_gpu_create_vfio_dmabuf()
Ok, but should we also set the 'Error *' when this failure occurs?
- and it is due to a guest's fault.
I think the only case where we can consider Guest is at fault is invalid rb/addr
(i.e, qemu_ram_block_from_host() returning NULL). Can we limit it to just this
particular case because in all other failure cases (for example,
UDMABUF_CREATE_LIST
IOCTL failure), I don't see any obvious way to deduce whether Guest or Host
is at fault?
We should assume that the guest is at fault. In general, when a feature
is missing and the guest tries to use it (e.g., by accessing MMIO
registers present only with the feature), there is a possibility that
the user misconfigured the VM. But we just emit LOG_GUEST_ERROR for such
cases.
As you can see, I am using a mix of error_report_err() and
qemu_log_mask()
after Cedric suggested to improve error handling by using 'Error *'.
On a slightly different note, do you (or Cedric/anyone) know how to
properly include CONFIG_DEVICES header to fix the following error:
../hw/display/virtio-gpu-dmabuf.c: In function
'virtio_gpu_create_vfio_dmabuf':
../hw/display/virtio-gpu-dmabuf.c:35:13: error: attempt to use
poisoned 'CONFIG_VIRTIO_GPU_VFIO_BLOB'
35 | #if defined(CONFIG_VIRTIO_GPU_VFIO_BLOB)
Looks like I am missing some key detail as I have tried different things
but
have not been able to fix this compilation error.
I think you are supposed to add stubs. See hw/vfio/iommufd-stubs.c for
example.
I have tried adding stubs following the example of:
cca621c782 ("intel_iommu_accel: Check for compatibility with IOMMUFD backed device
when x-flts=on")
but it doesn't help. I still get the following errors:
You are looking at the wrong example. It uses target-specific macros
because intel_iommu_accel is specific to i386; virtio-gpu isn't. Refer
to hw/vfio/iommufd-stubs.c, which I mentioned. For the user-side
example, refer to migration/cpr.c.
In file included from ../hw/display/virtio-gpu-dmabuf.c:21:
/media/myvdi/build/git-libs-64/cedric_qemu/qemu/hw/display/virtio-gpu-vfio-dmabuf.h:14:10:
error: '#include' expects '"FILENAME"' or '<FILENAME>'
14 | #include CONFIG_DEVICES
| ^~~~~~~~~~~~~~
CONFIG_DEVICES is a target-specific macro. Directly include the relevant
header as migration/cpr.c includes hw/vfio/vfio-device.h.
/media/myvdi/build/git-libs-64/cedric_qemu/qemu/hw/display/virtio-gpu-vfio-dmabuf.h:16:8:
error: attempt to use poisoned 'CONFIG_VIRTIO_GPU_VFIO_BLOB'
16 | #ifdef CONFIG_VIRTIO_GPU_VFIO_BLOB
| ^
In file included from
/media/myvdi/build/git-libs-64/cedric_qemu/qemu/include/exec/poison.h:7,
from
/media/myvdi/build/git-libs-64/cedric_qemu/qemu/include/qemu/osdep.h:38,
from ../hw/display/virtio-gpu-dmabuf.c:14:
./config-poison.h:233:20: note: poisoned here
233 | #pragma GCC poison CONFIG_VIRTIO_GPU_VFIO_BLOB
CONFIG_VIRTIO_GPU_VFIO_BLOB should be removed. It should always use the
necessary symbols as migration/cpr.c always uses vmstate_cpr_vfio_devices.
Regards,
Akihiko Odaki
