[virglrenderer-devel] coherent memory access for virgl

[Gurchetan Singh]

On Fri, Oct 5, 2018 at 1:30 AM Gerd Hoffmann <kraxel at redhat.com> wrote:
>
>   Hi,
>
> > > Or will the guest have to first ask the host create a
> > > resource, then query how big it is, finally map the thing into the guest
> > > address space?
> >
> > That's right -- though the "compressed size" and the "linear size" may
> > be different.
> >
> > The proposed mechanism on the host for mapping is gbm_bo_map(..),
> > which does de-tiling and returns a map-time stride.  We could emulate
> > those semantics.  The size given to mmap on the guest would be
> > (box_height * map_stride).
>
> Ah, that clarifies things alot.  Should have looked into the mesa source
> code earlier ...
>
> So, the guest's TRANSFER_TO_HOST request will be processed by
> virglrenderer this way:
>
>   (1) gbm_bo_map(GBM_BO_TRANSFER_WRITE)
>   (2) copy data from guest memory to mapping
>   (3) gbm_bo_unmap()
>
> Depending on the format and possibly other factors (1) might or might
> not map the bo directly.  In case it is not mapped directly
> gbm_bo_unmap() will update the bo and convert (tile/compress) the data
> if needed.
>
> In case of coherent buffers the bo will be mapped directly and the unmap
> call is not needed to make the changes visible to the gpu.
>
> Correct?

Yes, though gbm doesn't have a way to signal coherency.  Maybe we
could add gbm_bo_is_coherent(bo) on the host.

> So, yes, we could create a gbm_bo_map/unmap like interface at virtio
> protocol level, so the guest would ...
>
>    (1) MAP
>    (2) write to mapping
>    (3) UNMAP

Yes.

> .. instead of ...
>
>    (1) ATTACH_BACKING
>    (2) TRANSFER_TO_HOST
>    (3) DETACH_BACKING
>
> I think the guest doesn't need to know which modifiers are used on the
> host side then, because the host-side gbm_bo_map/gbm_bo_unmap calls will
> tile/detile/compress/uncompress so it'll be transparent to the guest.

Depends on what guest userspace does -- if
gbm_bo_create_with_modifiers is called and the wayland guest proxy
needs modifiers, then we'll need to know modifiers.

> Alternatively we could map the tiled/compressed bo as-is into the guest,
> then have gbm_bo_map/gbm_bo_unmap calls in the guest handle the
> tile/detile/compress/uncompress.  Is it possible in the first place to
> map the raw bo on all hardware?  Would that allow to skip the roundtrip
> to the host for map/unmap?

The gbm backend on the guest on is virgl.

https://cgit.freedesktop.org/mesa/mesa/tree/src/gbm/backends/dri/gbm_dri.c#n1268
https://cgit.freedesktop.org/mesa/mesa/tree/include/GL/internal/dri_interface.h#n1588
https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/state_trackers/dri/dri2.c#n1651

On the host, it'll be the host DRI backend.  Unless we can somehow get
the host logic in the guest, we can't avoid the roundtrip for
non-coherent buffers.

> > It's possible the protocol doesn't need to be amended.  We call
> > TRANSFER_FROM_HOST before we map the buffer, which potentially allows
> > us a way to calculate the correct mmap() size:
> >
> > https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/virgl/virgl_texture.c#n192
> >
> > But we comment out the stride in TRANSFER_FROM_HOST / TRANSFER_TO_HOST:
> >
> > https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c#n288
> >
> > In theory, since it's never been used, we can define the stride to be
> > an output of the ioctl.
>
> That might work at ioctl level, but not at virtio protocol level.
>
> > We can add even more flags to DRM_VIRTGPU_RESOURCE_INFO (i.e,
> > TRANSFER_STRIDE_DIFFERENT) since for most host buffers map_stride ==
> > compressed_stride, so we can avoid vm-exits associated with
> > TRANSFER_FROM_HOST / TRANSFER_TO_HOST when only need to mmap().  Or we
> > can extend the protocol.
>
> Hmm, that assumes we have the guest's gbm_bo_map/gbm_bo_unmap handle
> tile/detile/compress/uncompress, correct?

We will.  The flow is:

1) Guest queries guest EGL, gets modifier.  wayland guest proxy
somehow gets modifier from host KMS.
2) gbm_bo_create_with_modifiers -- we'll be using the virgl DRI
interface, which will call essentially VIRTGPU_RESOURCE_CREATE2 (which
passes down modifiers).
3) gbm_bo_create_with_modifiers on the host.
4) guest gbm buffer is imported to guest 3D driver, which is backed by
a virglrenderer resource (which has been imported in host GL)
5) Texturing/Rendering ensues.
6) If the guest needs to read the contents of the buffer, we can do a
gbm_bo_map on the host and put this into the PCI bar.
7) wayland guest proxy sends buffer to display, host proxy actually displays

>
> > > Or qemu can pass the udmabuf handle to virglrenderer.
> > >
> > > The later has the advantage that virglrenderer can possibly do more with
> > > the dmabuf than simply mmap()ing it (specifically let the gpu driver
> > > import it).  The former has the advantage that it works without
> > > virglrenderer changes.
> >
> > Remember, we can't import multi-level textures into GL.
> >
> > For single-level linear textures, there's the issue of stride /
> > alignment -- we can't assume bytes-per-pixel * width will work.  I've
> > seen EGL_EXT_image_dma_buf_import fail when every plane isn't 64-byte
> > aligned, for example.
>
> Well, virglrenderer can still fallback to mmap(dmabuf) in case the
> direct import doesn't work for whatever reason.

If the direct import works, then that's a win.  Then we will have two
classes of EGL import in virglrenderer:

a) host-optimized
b) guest memory pooled by udmabuf

Since I assume host memory can be scarce, (b) will be a good fallback.
I assume Vulkan/GL will use the same PCI bar, so that's where the
memory management kicks in.  Maybe having an ioctl telling guest
userspace "you have %u host memory remaining" is good idea, so it can
alternate between VIRTGPU_RESOURCE_CREATE2 / VIRTGPU_RESOURCE_CREATE.

>
> > But for KMS, I can imagine scenarios where it can change.  For my
> > setup, guest kms also advertises only one primary plane (XR24 AR24
> > BX24 BA24 RX24 RA24 XB24 AB24) all the time.  If the virtio-kms driver
> > can change what it advertises, and userspace queries KMS based on
> > resize-events, maybe it can work.
>
> I don't think this is possible.  The list of supported formats is a
> fixed property of the plane, and I think userspace will never try to
> re-read that either.

That's true, other DRM drivers set this on boot.  There'll be two
methods of displaying things on the host then:

1) VIRTIO_GPU_CMD_SET_SCANOUT
2) wayland proxing




> > We could in theory even support overlays inside the guest...
>
> Yes, we could.
>
> cheers,
>   Gerd
>