[PATCH] drm/virtio: Create Dumb BOs as guest Blobs (v2)

[Gerd Hoffmann]

  Hi,

> > 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.

Oh.  My memory was failing me then.  We should *really* clarify the spec
for BLOB_MEM_GUEST.

So shared mappings are allowed for all BLOB_MEM_GUEST resources, right?

> > 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

Yes.

> 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.

Well, we have two different cases:

  (1) No udmabuf available.  qemu will have a host-side shadow then and
      the workflow will be largely identical to the non-blob resource
      workflow.

  (2) With udmabuf support.  qemu can create udmabufs for the resources,
      mmap() the dmabuf to get a linear mapping, create a pixman buffer
      backed by that dmabuf (no copying needed then).  Depending on
      capabilities pass either the pixman image (gl=off) or the dmabuf
      handle (gl=on) to the UI code to actually show the guest display.

The guest doesn't need to know any of this, it'll just send transfer and
flush commands.  In case (1) qemu must process the transfer commands and
for case (2) qemu can simply ignore them.

> 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.

This is what happens today (using non-blob resources).

> 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

Again:  Why do we actually need that?  Is there any benefit other than
the guest knowing it doesn't need to send transfer commands?

I see the whole udmabuf thing as a host-side performance optimization
and I think this should be fully transparent to the guest as the host
can easily just ignore the transfer commands.  Given we batch commands
the extra commands don't lead to extra context switches, so there
shouldn't be much overhead.

If we really want make the guest aware of the hosts udmabuf state I
think this should be designed the other way around:  Add some way for
the host to tell the guest transfer commands are not needed for a
specific BLOB_MEM_GUEST resource.

take care,
  Gerd