On 2025/09/16 3:06, Kasireddy, Vivek wrote:
Subject: Re: [RFC 4/6] virtio-gpu: Don't rely on res->blob to identify blob
resources
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 <[email protected]>
Cc: Alex Bennée <[email protected]>
Cc: Akihiko Odaki <[email protected]>
Cc: Dmitry Osipenko <[email protected]>
Signed-off-by: Vivek Kasireddy <[email protected]>
---
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.
I think the worst case scenario here is Guest VM thinks its cursor is being
drawn using the image it provided but nothing gets drawn. I guess we need
to separately figure out if there are any alternate solutions to address this
issue (such as rendering the cursor on the Host side).
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().
The problem is that for dmabuf implementations in the kernel, providing
mmap() support is optional. And, the current vfio-pci implementation (that
provides dmabuf feature) does not support it as there was some pushback.
Can you provide a reference of a relevant discussion if any?
So, relying on res->blob always being valid is not going to work regardless.
It is still possible to mmap() using the standard VFIO device API even
if we cannot mmap() via DMA-BUF.
Regards,
Akihiko Odaki
