On Thu, Apr 8, 2021 at 2:27 AM Gerd Hoffmann <kraxel@xxxxxxxxxx> wrote:
> > +
> > + if (vgdev->has_resource_blob) {
> > + params.blob_mem = VIRTGPU_BLOB_MEM_GUEST;
> > + params.blob_flags = VIRTGPU_BLOB_FLAG_USE_SHAREABLE;
> >
>
> This creates some log spam with crosvm + virgl_3d + vanilla linux, since
> transfers don't work for guest blobs. Two options:
>
> a) Add vgdev->has_virgl_3d check and don't create a guest blob in that case.
> b) The interactions between TRANSFER_TO_HOST_2D and VIRTGPU_BLOB_MEM_GUEST
> are a bit under-defined in the spec.
Indeed.
> Though since you don't have a host
> side resource, you can safely skip the transfer and crosvm can be modified
> to do the right thing in case of RESOURCE_FLUSH.
IIRC the VIRTGPU_BLOB_FLAG_USE_SHAREABLE flag means that the host *can*
create a shared mapping (i.e. the host seeing guest-side changes without
explicit transfer doesn't cause problems for the guest). It doesn not
mean the host *must* create a shared mapping (note that there is no
negotiation whenever the host supports shared mappings or not).
VIRTGPU_BLOB_FLAG_USE_SHAREABLE means guest userspace intends to share the blob resource with another virtgpu driver instance via drmPrimeHandleToFd. It's a rough analogue to VkExportMemoryAllocateInfoKHR or PIPE_BIND_USE_SHARED.
The dumb case is a bit interesting because there is no userspace to provide that information. Though I think even VIRTGPU_BLOB_FLAG_USE_MAPPABLE is fine, since for my vanilla Linux setup, I'm seeing the guest blob is mapped only and drmPrimeHandleToFd(..) isn't called on it. We can also modify the virtio-gpu spec to say "blob_flags may be undefined/zero for BLOB_MEM_GUEST when 3D mode is not on".
Though all options work for me. The implicit dumb blob udmabuf case for me is more about advancing blob development rather than being super rigorous.
So the transfer calls are still needed, and the host can decide to
shortcut them in case it can create a shared mapping. In case there is
no shared mapping (say due to missing udmabuf support) the host can
fallback to copying.
Transfers are a bit under-defined for BLOB_MEM_GUEST. Even without udmabuf on the host, there is no host side resource for guest-only blobs? Before blob resources, the dumb flow was:
1) update guest side resource
2) TRANSFER_TO_HOST_2D to copy guest side contents to host side private resource [Pixman??]
3) RESOURCE_FLUSH to copy the host-side contents to the framebuffer and page-flip
At least for crosvm, this is possible:
1) update guest side resource
2) RESOURCE_FLUSH to copy the guest-side contents to the framebuffer and pageflip
With implicit udmabuf, it may be possible to do this:
1) update guest side resource
2) RESOURCE_FLUSH to page-flip
So I think crosvm should be fixed to not consider transfer commands for
VIRTGPU_BLOB_MEM_GUEST resources an error.
It's a simple change to make and we can definitely do it, if TRANSFER_2D is helpful for the QEMU case. I haven't looked at the QEMU side patches.
> It makes a ton of sense to have a explicit udmabuf-like flag
> ("BLOB_FLAG_CREATE_GUEST_HANDLE" or "BLOB_FLAG_HANDLE_FROM_GUEST" -- want
> to host OS agnostic -- any other ideas?), especially with 3d mode.
Why? Can't this be simply an host implementation detail which the guest
doesn't need to worry about?
For 3D mode, it's desirable to create an {EGL image}/{VkDeviceMemory} from guest memory for certain zero-copy use cases. If no explicit guarantee exists for the paravirtualized user-space that there will be a host side OS-specific handle associated with guest memory, then guest user space must fall-back to old-style transfers.
For the PCI-passthrough + guest blob case, the end goal is to share it with the host compositor. If there is no guarantee the guest memory can be converted to an OS-handle (to share with the host compositor), then I think the guest user space should fallback to another technique involving memcpy() to share the memory.
So essentially, thinking for two new protocol additions:
F_CREATE_GUEST_HANDLE (or F_HANDLE_FROM_GUEST) --> means an OS-specific udmabuf-like mechanism exists on the host.
BLOB_FLAG_CREATE_GUEST_HANDLE (or BLOB_FLAG_HANDLE_FROM_GUEST)--> tells host userspace "you must create a udmabuf" [or OS-specific equivalent] upon success
Though much testing/work remains (both with the PCI passthough case + virgl3d case), could be a good chance to float the nomenclature by everyone. Happy to collaborate further with Tina/Vivek on making such a thing happen.
take care,
Gerd
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
