[virglrenderer-devel] multiprocess model and GL

[Chia-I Wu]

On Fri, Jan 31, 2020 at 4:11 PM Gurchetan Singh
<gurchetansingh at chromium.org> wrote:
>
> On Fri, Jan 31, 2020 at 1:16 AM Gerd Hoffmann <kraxel at redhat.com> wrote:
> >
> >   Hi,
> >
> > > Userspace should decide which allocator to use or use EXECBUFFER,
> > > regardless of context.  It sets the context and therefore should know
> > > what's accurate virtualization for the situation.
> >
> > Ok, so a separate generic-cpu context for the gbm allocator wouldn't
> > work.
> >
> > > > execbuffer interface (but, yes, effectively not much of a difference to
> > > > an args blob).
> > >
> > > Perhaps a dumb question: why is the "execbuffer" protocol/"args"
> > > protcol distinction important?  Using SUBMIT_CMD as a transport could
> > > work, but I envisage the resource creation/sharing protocol as
> > > different from the current execbuffer protocol (i.e, different
> > > header):
> >
> > Hmm?  Why?  What makes the resource creation fundamentally different
> > from other commands for the virglrenderer?
>
> Because they're defined in different headers and used by different ioctls:
>
> execbuffer: https://github.com/freedesktop/virglrenderer/blob/master/src/virgl_protocol.h
> resouce create v1:
> https://github.com/freedesktop/virglrenderer/blob/master/src/virgl_hw.h
>
> (note the formats and binding types -- virglrenderer wouldn't work without it)
>
> So I wouldn't call the resource create v2 protocol as execbuffer.
I will say virgl_hw.h is a part of the virgl_protocol.h.  Together,
they define the virgl protocol.  If you look at the various _FORMAT
fields in virgl_protocol.h, they refers to the formats defined in
virgl_hw.h.

I think a more fair way to put this is that some of the virgl protocol
details are leaked by drm_virtgpu_resource_create.  IMO, a reasonable
way to plug the leak is to make sure the virgl protocol details are
contained in EXECBUFFER in resource create v2.

>
> >
> > > https://github.com/freedesktop/virglrenderer/blob/master/src/virgl_protocol.h
> > >
> > > Note: I think the VK renderer implementation doesn't modify the
> > > virgl_protocol.h either, but uses EXECBUFFER.
> >
> > Other renderer, other execbuffer format, host can look at the context
> > type to figure which renderer should interpret this.
> >
> > > As Chia pointed out, (2) is unnecessary when the object id is in the
> > > variadic arguments.
> >
> > See other reply, we explicitly tried to avoid that, but yes it makes
> > sense to rethink that as it probably is simpler in the end.
>
> As I mentioned during the discussion about CTX_EXPORT_RESOURCE[1][2],
> it's fine to put the object_id as the opaque resource creation
> argument.  It's a userspace metadata cookie after all.  Since the
> kernel and VMM don't really need to know about it, I wouldn't put in
> the virtio/ioctl interface to maintain logical consistency.
>
> [1] https://gitlab.freedesktop.org/virgl/drm-misc-next/merge_requests/5#note_370009
> [2] https://gitlab.freedesktop.org/virgl/drm-misc-next/merge_requests/5#note_382121
>
> >
> > > Batching hypercalls is easier than batching ioctls.  How easy is it to
> > > batch 2-3 hypercalls from one kick to a submit a single command to
> > > virglrenderer?  Otherwise, it'll be like:
> > >
> > > virgl_renderer_submit_cmd (allocate and store away something)
> > > virgl_renderer_resource_init (get that something just to setup
> > > res_id->resource map)
> >
> > Why do you think multiple function calls should be avoided?
>
> It makes the userspace nicer.  For example, it's nice to have the
> guest SG backing and the metadata when creating a udmabuf.
>
> >
> > cheers,
> >   Gerd
> >