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

[Gurchetan Singh]

On Fri, Apr 9, 2021 at 12:48 AM Gerd Hoffmann <kraxel at redhat.com> wrote:

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

The guest iovecs are always shared with the host, so they may be copied
to/from directly depending on the operation.  In the case of RESOURCE_FLUSH
+ BLOB_MEM_GUEST, it could be a copy from the guest iovecs to the host
framebuffer [host framebuffer != host shadow memory].


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

I think this is the key difference.  With BLOB_MEM_GUEST, crosvm can only
have a guest side iovecs and no host-side shadow memory.  With
BLOB_MEM_GUEST_HOST3D, host-side shadow memory will exist.

I guess it boils down the Pixman dependency.  crosvm sits right on top of
display APIs (X, wayland) rather than having intermediary layers.  Adding a
new Pixman API takes time too.

There's a bunch of options:

1) Don't use BLOB_MEM_GUEST dumb buffers in 3D mode.
2) virglrenderer or crosvm modified to implicitly ignore
TRANSFER_TO_HOST_2D for BLOB_MEM_GUEST when in 3D mode.
3) It's probably possible to create an implicit udmabuf
for RESOURCE_CREATE_2D resources and ignore the transfer there too.  The
benefit of this is TRANSFER_TO_HOST_2D makes a ton of sense for non-blob
resources.  No kernel side change needed here, just QEMU.
4) modify QEMU display integration

I would choose (1) since it solves the log spam problem and it advances
blob support in QEMU.  Though I leave the decision to QEMU devs.


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


So the use case I'm most interested in (and Vivek/Tina?) is
tiled/compressed udmabufs, so they may be eventually shared with the host
compositor via the DRM modifier API.

Transfers to linear udmabufs make sense.  Maybe transfers to
tiled/compressed udmabufs shouldn't even be attempted.

It's a complicated case with many ambiguities, especially with PCI
passthrough involved.  Explicit tiled/compressed udmabufs are just an idea,
will have to think more about it / have some proof of concept [with virgl
and PCI passthrough], before making any concrete proposals.  Will keep your
idea of just ignoring transfers on the host in mind.


> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20210412/0c72eb21/attachment.htm>