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.
Thanks, Vivek > > Regards, > Akihiko Odaki
