On 5/13/24 12:18, Akihiko Odaki wrote:
>> static void virgl_cmd_resource_unref(VirtIOGPU *g,
>> - struct virtio_gpu_ctrl_command
>> *cmd)
>> + struct virtio_gpu_ctrl_command
>> *cmd,
>> + bool *cmd_suspended)
>
> This parameter is not used as it returns an error if the resource is
> still mapped.
Missed to remove it by accident
> It may be better to actually implement unmapping instead of returning an
> error for consistency with the iov operation. Apparently crosvm also
> unmaps blobs with VIRTIO_GPU_CMD_RESOURCE_UNREF.
Then I'll add back `async_unmap_in_progress` because resource can be
both mapped/unmapped on unref, and we'll need flag to know whether async
unmapping has been finished to do the final unmapping of the resource.
...
>> + QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
>> +
>> + virgl_args.res_handle = cblob.resource_id;
>> + virgl_args.ctx_id = cblob.hdr.ctx_id;
>> + virgl_args.blob_mem = cblob.blob_mem;
>> + virgl_args.blob_id = cblob.blob_id;
>> + virgl_args.blob_flags = cblob.blob_flags;
>> + virgl_args.size = cblob.size;
>> + virgl_args.iovecs = res->base.iov;
>> + virgl_args.num_iovs = res->base.iov_cnt;
>> +
>> + ret = virgl_renderer_resource_create_blob(&virgl_args);
>> + if (ret) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: virgl blob create error:
>> %s\n",
>> + __func__, strerror(-ret));
>> + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>
> reslist keeps the stale res even if an error happens.
Good catch
--
Best regards,
Dmitry
