On 2024/05/16 2:15, Dmitry Osipenko wrote:
On 5/15/24 20:04, Akihiko Odaki wrote:
VIRTIO_GPU_CMD_RESOURCE_UNREF should also call
virtio_gpu_virgl_async_unmap_resource_blob(). I guess that's the
original intention of having a function for this instead of inlining the
content of this function to virgl_cmd_resource_unmap_blob().
Correct, previous patchset versions unmapped resource on unref.
In v11 I dropped unmapping from unref to avoid adding additional
`async_unmap_in_progress` flag because normally map/unmap will be
balanced by guest anyways.
The virtio-gpu spec doesn't tell that resource have to be implicitly
unmapped on unref. In a case of Linux guest, it actually will be a bug
to unref a mapped resource because guest will continue to map and use
the destroyed resource.
Additional `async_unmap_in_progress` flag should not be necessary as
explained earlier.
It is a valid design not to issue UNMAP_BLOB before UNREF if the
automatically performs the unmapping operation. A guest needs to ensure
the blob is not mapped in a guest userspace virtual address space, but
it does not require issuing UNMAP_BLOB, which is to unmap the blob from
the guest physical address space.
In case of Linux, virtio_gpu_vram_free() calls virtio_gpu_cmd_unmap() to
issue UNMAP_BLOB before UNREF, which is actually not necessary. Linux
still needs to ensure that the blob is not mapped in a guest userspace
virtual address space, but that is done before virtio_gpu_vram_free()
gets called, and virtio_gpu_cmd_unmap() has nothing to do with that.
It is still a good practice for a guest to issue UNMAP_BLOB in such a
case because the spec does not say the VMM will automatically unmap the
blob for UNREF, and that's what Linux does. From the VMM perspective,
it's better to perform the unmapping operation for UNREF because the
spec does not say the guest always issue UNMAP_BLOB before UNREF.