[virglrenderer-devel] multiprocess model and GL

[Gurchetan Singh]

On Wed, Feb 5, 2020 at 10:35 PM Gerd Hoffmann <kraxel at redhat.com> wrote:
>
>   Hi,
>
> > > > Multiple sources are perfectly valid.  The virglrenderer protocol is a
> > > > superset of all capset defined protocols.  capset 1 and capset 2
> > > > (virgl_hw.h) only apply to virgl (context type 0).
> > > >
> > > > capset 3 version 1 can apply to context type 0
> > > > capset 3 version 2 can apply to context type 1
>
> Hmm?  I don't think version is meant to be used like this ...

Yeah, I think we structure this in various ways.

>
> > The questions in my mind with memory-v4 where:
> >
> > 1) VIRTGPU_RESOURCE_FLAG_ALLOC_EXECBUFFER vs.
> > VIRTGPU_RESOURCE_FLAG_ALLOC_{3D, ARGS} -- since in Virgl, execbuffer
> > is used for (pipe context) rendering commands commands and not (pipe
> > screen) allocations, I'd tend to support the latter.  Do you
> > agree/disagree?
>
> Latest memory-v4 has just EXECBUFFER (and DUMB, for virtio only, it's
> pointless at ioctl level).
>
> Didn't we agree on using execbuffer for all allocations, so the VMM just
> passes on the execbuffer and virglrenderer handles the details?

I agree with passing opaque 3d creation arguments to virglrenderer.  I
wouldn't call it execbuffer, since currently resource creation and
execbuffer are different in virglrenderer.

In particular, I think the ioctl should go to
virgl_renderer_resource_create_blob(...) only and not take a detour at
virgl_renderer_submit_cmd.  I think Chia's latest version is the same
in that regard.

For object id based flows, I would favor:

3d_resource_create_cmd[0] = header
3d_resource_create_cmd[1] = object_id.

That way, the fact the resource was created by a prior
DRM_VIRTGPU_EXECBUFFER ioctl can also be an implementation detail.
For allocation based flows, I would favor:

3d_resource_create_cmd[0] = header
3d_resource_create_cmd[1] = gltarget
..

VMM/kernel wouldn't know, such it's a bit like execbuffer.  Here's the
latest version:

https://gitlab.freedesktop.org/virgl/drm-misc-next/commit/5bf2a0f9938cbb8b616c224bda6bb4c1bfa02900

>
> > 2) SHARED_ALLOW vs SHARED_REQUIRE --  SHARED_REQUIRE is the way to go.
> > Maybe just HOSTVMM_3D since VMM creates the dma-buf?
>
> It's named just SHARED in latest v4, but yes the plan is that the host
> can't choose but must use a shared mapping.
>
> > 3) {GET_HOST_BACKING, RELEASE_HOST_BACKING} vs. {ATTACH_HOST_BACKING,
> > DETACH_HOST_BACKING} vs. {RESOURCE_MAP, RESOURCE_UNMAP} -- something
> > like the first two seem nice, since they mirror what we have for guest
> > memory and we want to do want to batch hypercalls.  Thoughts?
>
> Well, the plan is to *not* use ATTACH_BACKING for blob resources.
> What is bad with MAP / UNMAP?

No strong opinions on the subject.  MAP/UNMAP should be fine.

>
> > 2) How to define protocols -- I think the answer here is the capabilities set.
>
> Agree.
>
> > > This will be the core resource creation command going forward. It can
> > > create resources from context-local objects.  Once virgl_protocol.h is
> > > extended to support local allocations, it can be used with
> > > virgl_protocol.h to replace the v1 resource creation command.
> >
> > As someone (the only one?) who's looked into the virglrenderer
> > implementation, I think it's easier to extend the current model rather
> > than requiring object IDs for Virgl.  It's just the way the code is
> > structured, and it's structured that way on based the Gallium model.
>
> I don't think you have to put everything upside down just to use object
> ids as temporary cookie in virglrenderer.  Just use u64 for the id, then
> you can use ...
>
>     vrend_resource->id = (context_id << 32) | object_id
>
> ... when creating the object via execbuffer.  When creating the resource
> you can lookup the object using context_id+object_id and replace the id
> with the (global) resource_id.

With the flow described above, we don't have to do this trick.

>
> cheers,
>   Gerd
>