[PATCH v2 09/10] virtio-gpu-dmabuf: Introduce qemu_iovec_same_memory_regions()

Vivek Kasireddy posted 10 patches 5 days, 16 hours ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, "Michael S. Tsirkin" <mst@redhat.com>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>, Cornelia Huck <cohuck@redhat.com>
[PATCH v2 09/10] virtio-gpu-dmabuf: Introduce qemu_iovec_same_memory_regions()
Posted by Vivek Kasireddy 5 days, 16 hours ago
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 <marcandre.lureau@redhat.com>
Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 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;
+        }
+
         virtio_gpu_create_udmabuf(res);
         if (res->dmabuf_fd < 0) {
             return;
-- 
2.50.1


Re: [PATCH v2 09/10] virtio-gpu-dmabuf: Introduce qemu_iovec_same_memory_regions()
Posted by Akihiko Odaki 4 days, 17 hours ago
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 <marcandre.lureau@redhat.com>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   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.

>           virtio_gpu_create_udmabuf(res);
>           if (res->dmabuf_fd < 0) {
>               return;

Regards,
Akihiko Odaki

RE: [PATCH v2 09/10] virtio-gpu-dmabuf: Introduce qemu_iovec_same_memory_regions()
Posted by Kasireddy, Vivek 2 days, 17 hours ago
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 <marcandre.lureau@redhat.com>
> > Cc: Alex Bennée <alex.bennee@linaro.org>
> > Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> > Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Cédric Le Goater <clg@redhat.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   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.

Thanks,
Vivek

> 
> >           virtio_gpu_create_udmabuf(res);
> >           if (res->dmabuf_fd < 0) {
> >               return;
> 
> Regards,
> Akihiko Odaki
Re: [PATCH v2 09/10] virtio-gpu-dmabuf: Introduce qemu_iovec_same_memory_regions()
Posted by Akihiko Odaki 2 days, 14 hours ago
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 <marcandre.lureau@redhat.com>
>>> Cc: Alex Bennée <alex.bennee@linaro.org>
>>> Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>> Cc: Alex Williamson <alex.williamson@redhat.com>
>>> Cc: Cédric Le Goater <clg@redhat.com>
>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>> ---
>>>    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.

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.

Regards,
Akihiko Odaki