Re: [Beignet] Preventing zero GPU virtual address allocation

Daniel Vetter
2015-03-19T14:57:08Z

On Thu, Mar 19, 2015 at 12:09:53PM +0200, David Weinehall wrote:
> On Thu, Mar 19, 2015 at 03:22:42AM +0000, Song, Ruiling wrote:
> >
> > > Yeah, MAP_FIXED sounds a bit more ambitious and though I think it would
> > > work for OCL 2.0 pointer sharing, it's a little different than we were planning.
> > > To summarize, we have three possible approaches, each with its own
> > > problems:
> > > 1) simple patch to avoid binding at address 0 in PPGTT:
> > > does impact the ABI (though generally not in a harmful way), and
> > > may not be possible with aliasing PPGTT with e.g. framebuffers
> > > bound at offset 0
> > > 2) exposing PIN_BIAS to userspace
> > > Would allow userspace to avoid pinning any buffers at offset 0 at
> > > execbuf time, but still has the problem with previously bound buffers
> > > and aliasing PPGTT
> > > 3) MAP_FIXED interface
> > > Flexible approach allowing userspace to manage its own virtual
> > > memory, but still has the same issues with aliasing PPGTT, and with
> > > shared contexts, which would have to negotiate between libraries
> > > how to
> > > handle the zero page
> > >
> > > For (1) and (2) the kernel pieces are really already in place, the main thing we
> > > need is a new flag to userspace to indicate behavior. I'd prefer (1) with a
> > > context creation flag to indicate "don't bind at 0".
> > > Execbuf would try to honor this, and userspace could check if any buffers
> > > ended up at 0 in the aliasing PPGTT case by checking the resulting offsets
> > > following the call. I expect in most cases this would be fine.
> > >
> > > It should be pretty easy to extend Ruiling's patch to use a context flag to
> > > determine the behavior; is that something you can do? Any objections to
> > > this approach?
> >
> > I am ok with adding a context flag to indicate "don't bind at 0". Any objections from others?
> > The patch is not from me, it is from David. I am not familiar with KMD. David, could you help on this patch?
>
> Yup, assuming, of course, that such an approach is acceptable.

Yeah my big concern was with not making this opt-in like the old patch or
adding an interface which does a lot more than what we need right now
(Chris' patch). Just a bitflag to ask for this seems best and is fine with
me.

And for the implementation I think we should reuse the PIN_BIAS logic
since that'll work in all places where it's possible. One open from my
side is how we should handle failures to move buffers (in case they ended
up at 0 somehow) - we can either silently fail or return an error to
userspace.

Note that this is only possible if you render to an elg image from ocl,
and if that egl image is a pinned frontbuffer and if we don't have full
ppgtt support. I don't know what the spec requires us to do here, or
whether we should care at all.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation