[Intel-gfx] [PATCH] drm/i915/vlv: Added write-enable pte bit support

[Daniel Vetter]

Aside: Your quoting is a bit strange, it looks like part of Chris' mail is
indented differently than other paragraphs from the same mail, which then
in turn match the indent for my reply. And the indent-levels are wrong, my
replay has >> but Chris's has only one >. Rather confusing.

I suggest you switch to a saner mail client ;-)

On Fri, Feb 07, 2014 at 05:44:00AM +0000, Goel, Akash wrote:
> > Don't cause us to rewrite the PTE for the batch buffer between each
> > execbuffer (ro for the batch, rw next time it gets used as a texture).
> > In fact, do not change ro without user intervention.
> > gt_old_ro is unused
> Sorry we didn't consider this scenario, that a batch buffer could be
> subsequently used as a different type of buffer also, like as a texture
> buffer. But is the remap going to have a significant overhead.
> 
> >> Use byt_pte_encode() instead of hacking the result of ppgtt->pte_encode().
> >> Consider expanding i915_cache_level so that it included the concept of PROT_READ | PROT_WRITE.
> Thanks, actually we initially thought of doing it in a same way. Can we
> overload the i915_cache_level enum with the RO flag, while calling
> 'insert_entries' for VALLEYVIEW platform. 
> 
> >> Also, what's the exact use-case for this here? And if we need to expose this to userspace, then it needs a testcase.
> >> -Daniel
> Since we have this RO bit available for our disposal on BYT platform, we
> thought we can avail it in order to have an extra protection on the
> buffers.
> We initially used it only for Ring & Batch buffers as we know, without
> User's input, that they are supposed to be read-only from GPU side.
> For extending this for other buffers, we need a Libdrm level change. 
> Or can we use the Read/Write domains information in relocation entries
> to decide this internally on the Driver side, but that will require some
> change in the exec-buffer path.

I think a start would be to roll the r/o bit out just for ringbuffers.
With that we can then implement the byt_pte_encode stuff and related
infrastructure.

Once we have that we can use it in more places internally, e.g. the
command scanner could copy scanned batches to r/o gem buffers. With that
use-case we'll never have a conflict which requires switching to r/w
access again since the batch buffer scanner gem bos will be on a separate
list.

Once we've knocked down the easy security improvements this allows we can
look at any benefits for userspace-allocated gem buffers. But since the
gpu doesn't fault it's not useful as a debug feature. And for better
isolation we really want ppgtt (which would resolve all these issues even
without r/o bits).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch