[Intel-gfx] [PATCH] drm/i915: Deny wrapping an userptr into a framebuffer

[Chris Wilson]

On Fri, Oct 23, 2015 at 10:04:24AM +0200, Daniel Vetter wrote:
> On Thu, Oct 22, 2015 at 04:23:09PM -0700, Kristian Høgsberg wrote:
> > On Tue, Oct 13, 2015 at 6:22 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > > Pinning a userptr onto the hardware raises interesting questions about
> > > the lifetime of such a surface as the framebuffer extends that life
> > > beyond the client's address space. That is the hardware will need to
> > > keep scanning out from the backing storage even after the client wants
> > > to remap its address space. As the hardware pins the backing storage,
> > > the userptr becomes invalid and this raises a WARN when the clients
> > > tries to unmap its address space. The situation can be even more
> > > complicated when the buffer is passed between processes, between a
> > > client and display server, where the lifetime and hardware access is
> > > even more confusing. Deny it.
> > 
> > Can we allow this for unsynchronized userptrs?
> 
> I'd like to not add more complexity to a root-only feature.

I've considered dropping the root-only restriction. As we've spent more
time analysing what exactly happens if we miss the mmu-notification and
we've decided that it can't grant access to other pages, it just causes
the information on the GPU and on the CPU to become unsynchronized. In
some situations that can be problematic (such as when the surface is
pinned by the hardware and we cannot keep the contract of maintaining
sync with the client address range), but normally the error is just
consistent with failing to the SET_DOMAIN api correctly. On that scale
of things, it is not as large a shotgun as I first feared and we could
ease the restriction and allow it for all. (I still would say that
unsync should only be used for objects being allocated and under full
control by the driver, importing client memory should be extremely
cautious).

We still have the requirement that surfaces exported between processes
use mmu-notifiers in order to revoke the exported surface when the
original mm is torndown, so it is not as simple to just allow fb on some
userptr and not others. (As we may still end up in the situation where
we need to revoke the pinned fb and fail miserably.) But that may just
be overthinking the issue, and letting the pages from one mm be pinned
onto the hw by another process and persist after the first is freed is
not an issue either.

So yes, following the same chain of logic, we could allow unsync fb, but
first we need to relax a few other restrictions en route and then we can
just reject creating fb on userptr with mmu-notifiers attached.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre