vfio: Implement VFIO_DEVICE_FEATURE_DMA_BUF and use it in virtio-gpu

[RFC 4/6] virtio-gpu: Don't rely on res->blob to identify blob resources

Posted by Vivek Kasireddy 4 days, 11 hours ago

The res->blob pointer is only valid for blobs that have their
backing storage in memfd. Therefore, we cannot use it to determine
if a resource is a blob or not. Instead, we could use res->blob_size
to make this determination as it is non-zero for blob resources
regardless of where their backing storage is located.

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>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 0a1a625b0e..2f9133c3b6 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -57,7 +57,7 @@ void virtio_gpu_update_cursor_data(VirtIOGPU *g,
     }
 
     if (res->blob_size) {
-        if (res->blob_size < (s->current_cursor->width *
+        if (!res->blob || res->blob_size < (s->current_cursor->width *
                               s->current_cursor->height * 4)) {
             return;
         }
@@ -144,7 +144,7 @@ virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
     }
 
     if (require_backing) {
-        if (!res->iov || (!res->image && !res->blob)) {
+        if (!res->iov || (!res->image && !res->blob_size)) {
             qemu_log_mask(LOG_GUEST_ERROR, "%s: no backing storage %d\n",
                           caller, resource_id);
             if (error) {
@@ -444,7 +444,7 @@ static void virtio_gpu_transfer_to_host_2d(VirtIOGPU *g,
 
     res = virtio_gpu_find_check_resource(g, t2d.resource_id, true,
                                          __func__, &cmd->error);
-    if (!res || res->blob) {
+    if (!res || res->blob_size) {
         return;
     }
 
@@ -507,7 +507,7 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
         return;
     }
 
-    if (res->blob) {
+    if (res->blob_size) {
         for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
             scanout = &g->parent_obj.scanout[i];
             if (scanout->resource_id == res->resource_id &&
@@ -538,7 +538,7 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
         }
     }
 
-    if (!res->blob &&
+    if (!res->blob_size &&
         (rf.r.x > res->width ||
         rf.r.y > res->height ||
         rf.r.width > res->width ||
@@ -634,7 +634,7 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
 
     g->parent_obj.enable = 1;
 
-    if (res->blob) {
+    if (res->blob_size) {
         if (console_has_gl(scanout->con)) {
             if (!virtio_gpu_update_dmabuf(g, scanout_id, res, fb, r)) {
                 virtio_gpu_update_scanout(g, scanout_id, res, fb, r);
@@ -645,13 +645,16 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
             return true;
         }
 
+        if (!res->blob) {
+            return false;
+        }
         data = res->blob;
     } else {
         data = (uint8_t *)pixman_image_get_data(res->image);
     }
 
     /* create a surface for this scanout */
-    if ((res->blob && !console_has_gl(scanout->con)) ||
+    if ((res->blob_size && !console_has_gl(scanout->con)) ||
         !scanout->ds ||
         surface_data(scanout->ds) != data + fb->offset ||
         scanout->width != r->width ||
@@ -899,7 +902,7 @@ void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
     g_free(res->addrs);
     res->addrs = NULL;
 
-    if (res->blob) {
+    if (res->blob_size) {
         virtio_gpu_fini_udmabuf(res);
     }
 }
-- 
2.50.1

Re: [RFC 4/6] virtio-gpu: Don't rely on res->blob to identify blob resources

Posted by Akihiko Odaki 1 day, 13 hours ago

On 2025/09/03 7:42, Vivek Kasireddy wrote:
> The res->blob pointer is only valid for blobs that have their
> backing storage in memfd. Therefore, we cannot use it to determine
> if a resource is a blob or not. Instead, we could use res->blob_size
> to make this determination as it is non-zero for blob resources
> regardless of where their backing storage is located.

I guess this change needs to be applied before "[RFC 3/6] 
virtio-gpu-udmabuf: Create dmabuf for blobs associated with VFIO 
devices"; without this patch, the "create dmabuf" patch will probably 
create an invalid blob.

> 
> 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>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   hw/display/virtio-gpu.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 0a1a625b0e..2f9133c3b6 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -57,7 +57,7 @@ void virtio_gpu_update_cursor_data(VirtIOGPU *g,
>       }
>   
>       if (res->blob_size) {
> -        if (res->blob_size < (s->current_cursor->width *
> +        if (!res->blob || res->blob_size < (s->current_cursor->width *

I doubt that rejecting a valid blob due to an implementation concern 
(whether the backing storage is in memfd) is tolerated in the specification.

>                                 s->current_cursor->height * 4)) {
>               return;
>           }
> @@ -144,7 +144,7 @@ virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
>       }
>   
>       if (require_backing) {
> -        if (!res->iov || (!res->image && !res->blob)) {
> +        if (!res->iov || (!res->image && !res->blob_size)) {
>               qemu_log_mask(LOG_GUEST_ERROR, "%s: no backing storage %d\n",
>                             caller, resource_id);
>               if (error) {
> @@ -444,7 +444,7 @@ static void virtio_gpu_transfer_to_host_2d(VirtIOGPU *g,
>   
>       res = virtio_gpu_find_check_resource(g, t2d.resource_id, true,
>                                            __func__, &cmd->error);
> -    if (!res || res->blob) {
> +    if (!res || res->blob_size) {
>           return;
>       }
>   
> @@ -507,7 +507,7 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
>           return;
>       }
>   
> -    if (res->blob) {
> +    if (res->blob_size) {
>           for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
>               scanout = &g->parent_obj.scanout[i];
>               if (scanout->resource_id == res->resource_id &&
> @@ -538,7 +538,7 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
>           }
>       }
>   
> -    if (!res->blob &&
> +    if (!res->blob_size &&
>           (rf.r.x > res->width ||
>           rf.r.y > res->height ||
>           rf.r.width > res->width ||
> @@ -634,7 +634,7 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
>   
>       g->parent_obj.enable = 1;
>   
> -    if (res->blob) {
> +    if (res->blob_size) {
>           if (console_has_gl(scanout->con)) {
>               if (!virtio_gpu_update_dmabuf(g, scanout_id, res, fb, r)) {
>                   virtio_gpu_update_scanout(g, scanout_id, res, fb, r);
> @@ -645,13 +645,16 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
>               return true;
>           }
>   
> +        if (!res->blob) {
> +            return false;
> +        }
>           data = res->blob;
>       } else {
>           data = (uint8_t *)pixman_image_get_data(res->image);
>       }
>   
>       /* create a surface for this scanout */
> -    if ((res->blob && !console_has_gl(scanout->con)) ||
> +    if ((res->blob_size && !console_has_gl(scanout->con)) ||
>           !scanout->ds ||
>           surface_data(scanout->ds) != data + fb->offset ||
>           scanout->width != r->width ||
> @@ -899,7 +902,7 @@ void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
>       g_free(res->addrs);
>       res->addrs = NULL;
>   
> -    if (res->blob) {
> +    if (res->blob_size) {
>           virtio_gpu_fini_udmabuf(res);
>       }
>   }

[RFC 1/6] linux-headers: Update vfio.h to include VFIO_DEVICE_FEATURE_DMA_BUF
[RFC 2/6] vfio: Add support for VFIO_DEVICE_FEATURE_DMA_BUF
[RFC 3/6] virtio-gpu-udmabuf: Create dmabuf for blobs associated with VFIO devices
[RFC 4/6] virtio-gpu: Don't rely on res->blob to identify blob resources
[RFC 5/6] virtio-gpu: Recreate the resource's dmabuf if new backing is attached
[RFC 6/6] virtio-gpu: Find the host addr given gpa associated with a ram device