[virglrenderer-devel] coherent memory access for virgl

[Elie Tournier]

On Thu, Oct 04, 2018 at 10:57:29AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > > Yes, it stands for ARM frame buffer compression and it's widely used
> > > > on that architecture.
> > >
> > > Speaking of compression:  How does buffer size calculation and buffer
> > > allocation work in case compression is used?
> > 
> > There's a header + compressed data or an auxiliary buffer.  Here are
> > some examples:
> > 
> > https://chromium.googlesource.com/chromiumos/platform/minigbm/+/master/rockchip.c#33
> 
> Hmm, so you need some format-specific knowledge to calculate the buffer
> size.  How does the allocation workflow look like?  Userspace (libgbm)
> calculates the size, then asks the kernel create a bo with that size?
> 
> Related: how is the stride calculated?
> 
> I'm asking because I want get the workflow right for hostmapped
> resource allocation.  Right now we have no modifiers, so it is
> simple to calculate the size needed for a given resource.  Also given we
> have no shared mapping between host and guest and copy around the data,
> so any stride mismatches can be accounted for when copying.  Both will
> not be true any more when we add hostmap and modifier support ...
> 
> So the fundamental question is:  Can the guest calculate the size needed
> in advance?  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?
> 
> Related:  Who should decide which format modifiers should be used?
> guest or host?
> 
> > https://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/drivers/dri/i965/intel_screen.c#n735
> 
> What is in the auxiliary buffer?  The compression metadata (i.e. simliar
> to what afbc has in the header)?
> 
> How they are passed around?  Is the auxiliary buffer just another bo?
> 
> > > How does mesa handle modifiers?  Does it use the drm modifiers directly?
> > > Or has mesa/gallium its own naming scheme?
> > >
> > > Right now the format specification used by virtio is based on gallium
> > > formats ...
> > 
> > The modifiers are from DRM, and drm_fourcc.h is considered the source
> > of truth (I guess that's where some of the problems with v4l2
> > integration arise).
> 
> Ok.
> 
> > Anyways, here are the conclusions that I can discern from this discussion:
> > 
> > 1) Host mapped memory should be fine, as long it's optional for the VMM
> 
> Yes, we need a virtio feature flag for this.  First, for backward
> compatibilty reasons.  Second, because the VMM might want to make this
> configurable per VM.

I fully agree with that.
> 
> > 2) Texture uploads can be improved.  BTW, what's the benefit of
> > udmabuf over an iov with one element for say a 5-level texture?
> 
> Well, udmabuf can be used to get a linear mapping of a scattered
> resource.  Now qemu can map that udmabuf, the pass on the one-elem-iov
> to virglrenderer.  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.

So at Collabora, we are also looking at reducing copies during texture uploads.
We were thinking about the second conclusion: "qemu can pass the udmabuf handle to virglrenderer"
and I'd like to recapitulate what has been discussed here on this regard.
My understanding is that we'd require that the VMM allocates all the memory of the machine in a single memfd region.
Then, when a client in the guest requests a texture upload, Mesa would create a virtio-gpu resource
to wrap the buffer that the client passed with the texture data.
virtio-gpu would pass the physical addresses backing that buffer to the virtio queue message
for creating a buffer or attaching memory to it.
When the VMM processes the message, it would create a dmabuf via udmabuf out of those physical addresses.
That dmabuf can then be mapped and the address can be directly passed to read_transfer_data in virglrenderer.

Did I get that right? If we agree with a plan, we will be happy to help with the implementation or the code review.

Elie
> 
> > 3) Coherent memory ioctl needed for Vulkan/gl4.5
> 
> Yes.
> 
> > 4) The new ioctl should take into account format modifiers
> 
> Well, it isn't just the ioctl.  We also have to hash out the virtio
> protocol extension (even though it'll probably look simliar to the
> ioctl interface), and the virglrenderer api extension.
> 
> > 5) The DRM_VIRTGPU_RESOURCE_INFO needs to be fixed and expanded for
> > (1), (3) and (4)
> 
> Not just that one, we need a new/improved resource create ioctl too.
> And while being at it we might want split the thing, into an ioctl to
> create buffers (aka gem objects), and into one (or more) ioctls to
> specifiy what is in there (format, width + height, modifiers, ...).
> 
> > 6) There's no clear path forward on how to pipe host display
> > information to the guest, such that the guest can know which modifier
> > is the best.  Hopefully, the wayland proxying Collabora is working on
> > can clear this up..
> 
> Ah, so you think the guest should pick the modifier then?
> 
> Guess we could use a capset to provide a list (ordered by preference)
> to the guest.
> 
> 
> cheers,
>   Gerd
> 
> _______________________________________________
> virglrenderer-devel mailing list
> virglrenderer-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel