On 2025/11/13 12:14, Kasireddy, Vivek wrote:
Hi Akihiko,
Subject: Re: [PATCH v2 09/10] virtio-gpu-dmabuf: Introduce
qemu_iovec_same_memory_regions()
On 2025/11/12 13:24, Kasireddy, Vivek wrote:
Hi Akihiko,
Subject: Re: [PATCH v2 09/10] virtio-gpu-dmabuf: Introduce
qemu_iovec_same_memory_regions()
On 2025/11/09 14:33, Vivek Kasireddy wrote:
Add a helper to check whether the addresses in an iovec array
belong to the same memory region or not. This is useful to verify
before trying to create a dmabuf from an iovec array.
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/virtio-gpu-dmabuf.c | 29
+++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
dmabuf.c
index c34d4c85bc..80143034d4 100644
--- a/hw/display/virtio-gpu-dmabuf.c
+++ b/hw/display/virtio-gpu-dmabuf.c
@@ -27,6 +27,31 @@
#include "standard-headers/linux/udmabuf.h"
#include "standard-headers/drm/drm_fourcc.h"
+static bool qemu_iovec_same_memory_regions(const struct iovec *iov,
int iov_cnt)
+{
+ RAMBlock *rb, *curr_rb;
+ ram_addr_t offset;
+ int i;
+
+ rb = qemu_ram_block_from_host(iov[0].iov_base, false, &offset);
+ if (!rb) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Could not find ramblock/memory region\n",
__func__);
+ return false;
+ }
+
+ for (i = 1; i < iov_cnt; i++) {
+ curr_rb = qemu_ram_block_from_host(iov[i].iov_base, false,
&offset);
+ if (curr_rb != rb) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: memory regions not same for iov entries\n",
+ __func__);
+ return false;
+ }
+ }
+ return true;
+}
+
static void virtio_gpu_create_udmabuf(struct
virtio_gpu_simple_resource
*res)
{
struct udmabuf_create_list *list;
@@ -137,6 +162,10 @@ void virtio_gpu_init_dmabuf(struct
virtio_gpu_simple_resource *res)
res->iov[0].iov_len < 4096) {
pdata = res->iov[0].iov_base;
} else {
+ if (!qemu_iovec_same_memory_regions(res->iov, res->iov_cnt)) {
+ return;
+ }
+
This check is unnecessary. Perhaps rejecting iov with different memory
regions may be fine if that simplifies the code, but this actually adds
some code.
I think we can keep this sanity check but I don't really mind dropping this
patch given that buffers with mixed memory regions are not encountered
in practical situations. Or, I guess I could move the if (curr_rb != rb) check
to virtio_gpu_create_udmabuf() and vfio_device_create_dmabuf_fd()
like you suggested previously.
I won't call it a "sanity check"; it is "unlikely" to have different
memory regions, but it is still not "wrong" and is sane.
I'd say "very unlikely". The only scenario I can imagine where this might
happen is if a buffer is partially migrated (from one memory region to another).
And, if we do come across such a buffer, it has to be rejected because
there is no way we can create a dmabuf (via vfio or udmabuf) for such a
buffer/resource.
The check virtio_gpu_create_udmabuf() already has and the check I
suggested for vfio_device_create_dmabuf_fd() (checking if all memory
regions belong to one VFIO device) should be sufficient to reject all
cases that DMA-BUF cannot be created.
Regards,
Akihiko Odaki
The VFIO code path still needs to check if the memory regions belong to
one VFIO device. Trying to create one DMA-BUF using multiple VFIO
devices is wrong.
Ok, I'll add this check in vfio_device_create_dmabuf_fd() to avoid iterating
over all the entries separately.
Thanks,
Vivek
Regards,
Akihiko Odaki
