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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On October 25, 2017 06:05:16 Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux