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

[Kristian Høgsberg]

On Fri, Oct 23, 2015 at 2:28 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> 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

Thanks,  that all sounds good.

Kristian
>
> --
> Chris Wilson, Intel Open Source Technology Centre