Re: [Mesa-dev] [PATCH] i965: Revert absolute mode for constant buffer pointers.

From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
To: Kenneth Graunke <kenneth@whitecape.org>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org,
	"Kristian Høgsberg" <hoegsberg@gmail.com>,
	mesa-dev <mesa-dev@lists.freedesktop.org>
Subject: Re: [Mesa-dev] [PATCH] i965: Revert absolute mode for constant buffer pointers.
Date: Wed, 25 Oct 2017 16:05:09 +0300	[thread overview]
Message-ID: <1508936709.13538.20.camel@linux.intel.com> (raw)
In-Reply-To: <1608938.uWnjWL9i7Y@kirito>

On Mon, 2017-10-23 at 16:19 -0700, Kenneth Graunke wrote:
> On Monday, October 23, 2017 3:53:15 PM PDT Rodrigo Vivi wrote:
> > On Mon, Oct 23, 2017 at 10:32:43PM +0000, Jordan Justen wrote:
> > > On 2017-10-19 16:30:44, Kristian Høgsberg wrote:
> > > > On Thu, Oct 19, 2017 at 4:18 PM, Kenneth Graunke <kenneth@whitecape.org> wrote:
> > > > > The kernel doesn't initialize the value of the INSTPM or CS_DEBUG_MODE2
> > > > > registers at context initialization time.  Instead, they're inherited
> > > > > from whatever happened to be running on the GPU prior to first run of a
> > > > > new context.  So, when we started setting these, other contexts in the
> > > > > system started inheriting our values.  Since this controls whether
> > > > > 3DSTATE_CONSTANT_* takes a pointer or an offset, getting the wrong
> > > > > setting is fatal for almost any process which isn't expecting this.
> > > > > 
> > > > > Unfortunately, VA-API and Beignet don't initialize this (nor does older
> > > > > Mesa), so they will die horribly if we start doing this.  UXA and SNA
> > > > > don't use any push constants, so they are unaffected.
> > > > > 
> > > > > The kernel developers are apparently uninterested in making the proto-
> > > > > context initialize these registers to guarantee deterministic behavior.
> > > > 
> > > > Could somebody from the kernel team elaborate here? This is obviously
> > > > broken and undermines the security and containerization that hw
> > > > contexts are supposed to provide. I'm really curious what the thinking
> > > > is here...
> > > > 
> > > > Kristian
> > > 
> > > Cc intel-gfx, maintainers
> > 
> > Is this the null-state/golden-context related discussions?
> > 
> > I assume we are ok for older platforms, but the problem would be only for
> > CNL+ where we are not adding the null context initialization yet.
> > Am I getting it right?
> 
> No, this problem exists on earlier platforms as well.  We saw the issue
> on Broadwell and Kabylake.

+ Daniel, Chris

There indeed seems to be quite a lot of missing registers from the i915
driver where the context is initialized. (Psst. You can read that as:
"all the 33 non-privileged registers we could quickly list, are
missing"). You can see for yourself at execlists_init_reg_state() in
"intel_lrc.c". So currently you can expect issues if some userspace
sets fancy register values that the rest of the userspaces are not
setting.

We'll be providing a rework on the context register state
initialization code to fix the issue. There's quite some detail to it,
considering the golden render context, W/As and all. In the meanwhile,
revert would be the only option for Mesa.

Chris wrote a nice I-G-T test to replicate the scenario:

https://patchwork.freedesktop.org/patch/184573/

What was also observed is that messing with some of the non-privileged
register will not only take the GPU with them, but the whole machine.
But that's not exactly a surprise.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx