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

Jason Ekstrand
2017-10-25T14:33:46Z

On October 25, 2017 06:05:16 Joonas Lahtinen
<joonas.lahtinen@linux.intel.com> wrote:

> 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").

We probably don't need *all* of them initialized. For instance, the
initial values of the ALU registers or the indirect draw parameter
registers will probably never matter. However, if you want to just
initialized them all, that's fine.

> 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.

Once this is all sorted out, it would be good if we had a property we could
query that tells us we have golden contexts so that we can start "getting
fancy" again.

> 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.

Dumb question, but would it be less workaround pain if you just extended
the context initialization batch to just set more registers?

--Jason

> 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