On 2025/09/15 15:33, Kasireddy, Vivek wrote:
Hi Akihiko,
Subject: Re: [RFC 4/6] virtio-gpu: Don't rely on res->blob to identify blob
resources
On 2025/09/13 11:48, Kasireddy, Vivek wrote:
Hi Akihiko,
Subject: Re: [RFC 4/6] virtio-gpu: Don't rely on res->blob to identify blob
resources
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.
Ok, makes sense. I'll move it earlier in the patch series.
Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
Cc: Alex Bennée <alex.ben...@linaro.org>
Cc: Akihiko Odaki <od...@rsg.ci.i.u-tokyo.ac.jp>
Cc: Dmitry Osipenko <dmitry.osipe...@collabora.com>
Signed-off-by: Vivek Kasireddy <vivek.kasire...@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.
Are you suggesting that the whole if (res->blob_size < (s->current_cursor-
width *...
check needs to be removed? I think it is just a sanity check to ensure that the
blob
size is big enough for cursor.
I referred to !res->blob, the new condition. It rejects a valid blob
that is backed by VFIO.
The problem is that for blobs backed by VFIO, res->blob can be NULL but this
function
(virtio_gpu_update_cursor_data) is relying on res->blob always being valid.
That's why
the new condition !res->blob is needed here to check the validity of res->blob.
I understand the host-side implementation difficulty to support this
operation for VFIO, but the guest may not be prepared for the failure of
the operation so we shouldn't simply reject it.
By the way, perhaps it may be possible to provide res->blob for VFIO.
Since "[RFC 3/6] virtio-gpu-udmabuf: Create dmabuf for blobs associated
with VFIO devices" checks memory_region_is_ram_device(), the VFIO
backend providing the blob should support mmap().
Regards,
Akihiko Odaki
