On Thu, Apr 8, 2021 at 2:27 AM Gerd Hoffmann <kraxel@redhat.com> 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
>
>