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. Thanks, Vivek > > > 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); > > } > > }
