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

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

[Jordan Justen]

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

> 
> > Apparently, the "Default Value" of registers in the documentation is a
> > total lie, and cannot be relied upon by userspace.  So, we need to find
> > a new solution.  One would be to patch VA-API and Beignet to initialize
> > these, then get every distributor to ship those in tandem with the newer
> > Mesa version.  I'm unclear when va-intel-driver releases might happen.
> > Another option would be to hack Mesa to restore the register back to the
> > historical default (relative mode) at the end of each batch.  This is
> > also gross, as it has non-zero cost, and is also relying on userspace to
> > be "polite" to other broken userspace.  A large part of the motivation
> > for contexts was to provide proper process isolation, so we should not
> > have to do this.  But, we're already doing it in some cases, because
> > our kernel fixes to enforce isolation were reverted.
> >
> > In the meantime, I guess we'll just revert this patch and abandon using
> > the feature.  It will lead to fewer pushed UBOs on Broadwell+, which may
> > lead to lower performance, but I don't have any data on the impact.
> >
> > Cc: "17.2" <mesa-stable@lists.freedesktop.org>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102774
> > ---
> >  src/mesa/drivers/dri/i965/brw_state_upload.c | 24 ------------------------
> >  src/mesa/drivers/dri/i965/intel_screen.c     |  2 +-
> >  2 files changed, 1 insertion(+), 25 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c
> > index 16f44d03bbe..23e4ebda259 100644
> > --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
> > +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
> > @@ -101,30 +101,6 @@ brw_upload_initial_gpu_state(struct brw_context *brw)
> >        OUT_BATCH(0);
> >        ADVANCE_BATCH();
> >     }
> > -
> > -   /* Set the "CONSTANT_BUFFER Address Offset Disable" bit, so
> > -    * 3DSTATE_CONSTANT_XS buffer 0 is an absolute address.
> > -    *
> > -    * On Gen6-7.5, we use an execbuf parameter to do this for us.
> > -    * However, the kernel ignores that when execlists are in use.
> > -    * Fortunately, we can just write the registers from userspace
> > -    * on Gen8+, and they're context saved/restored.
> > -    */
> > -   if (devinfo->gen >= 9) {
> > -      BEGIN_BATCH(3);
> > -      OUT_BATCH(MI_LOAD_REGISTER_IMM | (3 - 2));
> > -      OUT_BATCH(CS_DEBUG_MODE2);
> > -      OUT_BATCH(REG_MASK(CSDBG2_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE) |
> > -                CSDBG2_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE);
> > -      ADVANCE_BATCH();
> > -   } else if (devinfo->gen == 8) {
> > -      BEGIN_BATCH(3);
> > -      OUT_BATCH(MI_LOAD_REGISTER_IMM | (3 - 2));
> > -      OUT_BATCH(INSTPM);
> > -      OUT_BATCH(REG_MASK(INSTPM_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE) |
> > -                INSTPM_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE);
> > -      ADVANCE_BATCH();
> > -   }
> >  }
> >
> >  static inline const struct brw_tracked_state *
> > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> > index ea04a72e860..776c2898d5b 100644
> > --- a/src/mesa/drivers/dri/i965/intel_screen.c
> > +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> > @@ -2510,7 +2510,7 @@ __DRIconfig **intelInitScreen2(__DRIscreen *dri_screen)
> >     screen->compiler = brw_compiler_create(screen, devinfo);
> >     screen->compiler->shader_debug_log = shader_debug_log_mesa;
> >     screen->compiler->shader_perf_log = shader_perf_log_mesa;
> > -   screen->compiler->constant_buffer_0_is_relative = devinfo->gen < 8;
> > +   screen->compiler->constant_buffer_0_is_relative = true;
> >     screen->compiler->supports_pull_constants = true;
> >
> >     screen->has_exec_fence =
> > --
> > 2.14.2
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Rodrigo Vivi]

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?

> 
> > 
> > > Apparently, the "Default Value" of registers in the documentation is a
> > > total lie, and cannot be relied upon by userspace.  So, we need to find
> > > a new solution.  One would be to patch VA-API and Beignet to initialize
> > > these, then get every distributor to ship those in tandem with the newer
> > > Mesa version.  I'm unclear when va-intel-driver releases might happen.
> > > Another option would be to hack Mesa to restore the register back to the
> > > historical default (relative mode) at the end of each batch.  This is
> > > also gross, as it has non-zero cost, and is also relying on userspace to
> > > be "polite" to other broken userspace.  A large part of the motivation
> > > for contexts was to provide proper process isolation, so we should not
> > > have to do this.  But, we're already doing it in some cases, because
> > > our kernel fixes to enforce isolation were reverted.
> > >
> > > In the meantime, I guess we'll just revert this patch and abandon using
> > > the feature.  It will lead to fewer pushed UBOs on Broadwell+, which may
> > > lead to lower performance, but I don't have any data on the impact.
> > >
> > > Cc: "17.2" <mesa-stable@lists.freedesktop.org>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102774
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_state_upload.c | 24 ------------------------
> > >  src/mesa/drivers/dri/i965/intel_screen.c     |  2 +-
> > >  2 files changed, 1 insertion(+), 25 deletions(-)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c
> > > index 16f44d03bbe..23e4ebda259 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
> > > @@ -101,30 +101,6 @@ brw_upload_initial_gpu_state(struct brw_context *brw)
> > >        OUT_BATCH(0);
> > >        ADVANCE_BATCH();
> > >     }
> > > -
> > > -   /* Set the "CONSTANT_BUFFER Address Offset Disable" bit, so
> > > -    * 3DSTATE_CONSTANT_XS buffer 0 is an absolute address.
> > > -    *
> > > -    * On Gen6-7.5, we use an execbuf parameter to do this for us.
> > > -    * However, the kernel ignores that when execlists are in use.
> > > -    * Fortunately, we can just write the registers from userspace
> > > -    * on Gen8+, and they're context saved/restored.
> > > -    */
> > > -   if (devinfo->gen >= 9) {
> > > -      BEGIN_BATCH(3);
> > > -      OUT_BATCH(MI_LOAD_REGISTER_IMM | (3 - 2));
> > > -      OUT_BATCH(CS_DEBUG_MODE2);
> > > -      OUT_BATCH(REG_MASK(CSDBG2_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE) |
> > > -                CSDBG2_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE);
> > > -      ADVANCE_BATCH();
> > > -   } else if (devinfo->gen == 8) {
> > > -      BEGIN_BATCH(3);
> > > -      OUT_BATCH(MI_LOAD_REGISTER_IMM | (3 - 2));
> > > -      OUT_BATCH(INSTPM);
> > > -      OUT_BATCH(REG_MASK(INSTPM_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE) |
> > > -                INSTPM_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE);
> > > -      ADVANCE_BATCH();
> > > -   }
> > >  }
> > >
> > >  static inline const struct brw_tracked_state *
> > > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> > > index ea04a72e860..776c2898d5b 100644
> > > --- a/src/mesa/drivers/dri/i965/intel_screen.c
> > > +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> > > @@ -2510,7 +2510,7 @@ __DRIconfig **intelInitScreen2(__DRIscreen *dri_screen)
> > >     screen->compiler = brw_compiler_create(screen, devinfo);
> > >     screen->compiler->shader_debug_log = shader_debug_log_mesa;
> > >     screen->compiler->shader_perf_log = shader_perf_log_mesa;
> > > -   screen->compiler->constant_buffer_0_is_relative = devinfo->gen < 8;
> > > +   screen->compiler->constant_buffer_0_is_relative = true;
> > >     screen->compiler->supports_pull_constants = true;
> > >
> > >     screen->has_exec_fence =
> > > --
> > > 2.14.2
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Kenneth Graunke]

[-- Attachment #1.1: Type: text/plain, Size: 1881 bytes --]

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.

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

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

[Joonas Lahtinen]

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

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

[Jason Ekstrand]

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


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

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

[Kenneth Graunke]

[-- Attachment #1.1: Type: text/plain, Size: 4154 bytes --]

On Wednesday, October 25, 2017 7:33:41 AM PDT Jason Ekstrand wrote:
> On October 25, 2017 06:05:16 Joonas Lahtinen wrote:
[snip]
> > 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.

I agree - I think we can cut down the list substantially, if you like.
Here's my breakdown of Skylake's non-privileged register list:

Cache_Mode_0               0x7000
Cache_Mode_1               0x7004
GT_MODE                    0x7008
L3_Config                  0x7034
TD_CTL                     0xE400
TD_CTL2                    0xE404
L3SQCREG4                  0xB118
NOPID                      0x2094
INSTPM                     0x20C0

   Should be initialized by the kernel.  Several of these can severely
   break unsuspecting userspace, and we'd like to be able to rely on a
   default value.

IA_VERTICES_COUNT          0x2310
IA_PRIMITIVES_COUNT        0x2318
VS_INVOCATION_COUNT        0x2320
HS_INVOCATION_COUNT        0x2300
DS_INVOCATION_COUNT        0x2308
GS_INVOCATION_COUNT        0x2328
GS_PRIMITIVES_COUNT        0x2330
SO_NUM_PRIMS_WRITTEN0      0x5200
SO_NUM_PRIMS_WRITTEN1      0x5208
SO_NUM_PRIMS_WRITTEN2      0x5210
SO_NUM_PRIMS_WRITTEN3      0x5218
SO_PRIM_STORAGE_NEEDED0    0x5240
SO_PRIM_STORAGE_NEEDED1    0x5248
SO_PRIM_STORAGE_NEEDED2    0x5250
SO_PRIM_STORAGE_NEEDED3    0x5258
CL_INVOCATION_COUNT        0x2338
CL_PRIMITIVES_COUNT        0x2340
PS_INVOCATION_COUNT_0      0x22C8
PS_DEPTH_COUNT_0           0x22D8
PS_INVOCATION_COUNT_1      0x22F0
PS_DEPTH_COUNT_1           0x22F8
PS_INVOCATION_COUNT_2      0x2448
PS_DEPTH_COUNT_2           0x2450
GPGPU_THREADS_DISPATCHED   0x2290

   The kernel can skip these if you like.  Statistics registers just count
   things, and userspace always calculates (end counter - start counter)
   deltas, so the initial value doesn't really matter.

SO_WRITE_OFFSET0           0x5280
SO_WRITE_OFFSET1           0x5284
SO_WRITE_OFFSET2           0x5288
SO_WRITE_OFFSET3           0x528C
GPUGPU_DISPATCHDIMX        0x2500
GPUGPU_DISPATCHDIMY        0x2504
GPUGPU_DISPATCHDIMZ        0x2508
MI_PREDICATE_SRC0          0x2400
MI_PREDICATE_SRC0          0x2404
MI_PREDICATE_SRC1          0x2408
MI_PREDICATE_SRC1          0x240C
MI_PREDICATE_DATA          0x2410
MI_PREDICATE_DATA          0x2414
MI_PREDICATE_RESULT        0x2418
MI_PREDICATE_RESULT_1      0x241C
MI_PREDICATE_RESULT_2      0x23BC
3DPRIM_END_OFFSET          0x2420
3DPRIM_START_VERTEX        0x2430
3DPRIM_VERTEX_COUNT        0x2434
3DPRIM_INSTANCE_COUNT      0x2438
3DPRIM_START_INSTANCE      0x243C
3DPRIM_BASE_VERTEX         0x2440

   The kernel can skip these if you like, IMO.  These registers are only
   used when enabling an optional feature - stream out (SO_WRITE_*),
   indirect compute dispatch (GPGPU_*), predicated draws (MI_PREDICATE_*),
   indirect draws (3DPRIM_*).  Userspace has to explicitly opt in to each
   of these features by enabling a flag, so there isn't a cross-context
   contamination problem.  If userspace opts in to these features, it can
   be responsible for programming the registers correctly.

CS_GPR (1-16)              0x2600

   The kernel can skip these if you like.  They're temporary storage when
   using the MI_MATH instruction.  Example usage: load values into CS_GPR1
   and CS_GPR2, add them, store the result in CS_GPR3.  Store to memory.

   Nobody should be doing math on register values without setting them.
   That's clearly a userspace bug.

BB_OFFSET                  0x2158
OA_CTX_CONTROL             0x2360
OACTXID                    0x2364
OA CONTROL                 0x2B00
PERF_CNT_1_DW0             0x91b8
PERF_CNT_1_DW1             0x91bc
PERF_CNT_2_DW0             0x91c0
PERF_CNT_2_DW1             0x91c4

   I don't know about these.

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [Intel-gfx] [PATCH] i965: Revert absolute mode for constant buffer pointers.

[Jason Ekstrand]

[-- Attachment #1.1: Type: text/plain, Size: 4554 bytes --]

On Wed, Oct 25, 2017 at 10:31 AM, Kenneth Graunke <kenneth@whitecape.org>
wrote:

> On Wednesday, October 25, 2017 7:33:41 AM PDT Jason Ekstrand wrote:
> > On October 25, 2017 06:05:16 Joonas Lahtinen wrote:
> [snip]
> > > 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.
>
> I agree - I think we can cut down the list substantially, if you like.
> Here's my breakdown of Skylake's non-privileged register list:
>
> Cache_Mode_0               0x7000
> Cache_Mode_1               0x7004
> GT_MODE                    0x7008
> L3_Config                  0x7034
> TD_CTL                     0xE400
> TD_CTL2                    0xE404
> L3SQCREG4                  0xB118
> NOPID                      0x2094
> INSTPM                     0x20C0
>
>    Should be initialized by the kernel.  Several of these can severely
>    break unsuspecting userspace, and we'd like to be able to rely on a
>    default value.
>
> IA_VERTICES_COUNT          0x2310
> IA_PRIMITIVES_COUNT        0x2318
> VS_INVOCATION_COUNT        0x2320
> HS_INVOCATION_COUNT        0x2300
> DS_INVOCATION_COUNT        0x2308
> GS_INVOCATION_COUNT        0x2328
> GS_PRIMITIVES_COUNT        0x2330
> SO_NUM_PRIMS_WRITTEN0      0x5200
> SO_NUM_PRIMS_WRITTEN1      0x5208
> SO_NUM_PRIMS_WRITTEN2      0x5210
> SO_NUM_PRIMS_WRITTEN3      0x5218
> SO_PRIM_STORAGE_NEEDED0    0x5240
> SO_PRIM_STORAGE_NEEDED1    0x5248
> SO_PRIM_STORAGE_NEEDED2    0x5250
> SO_PRIM_STORAGE_NEEDED3    0x5258
> CL_INVOCATION_COUNT        0x2338
> CL_PRIMITIVES_COUNT        0x2340
> PS_INVOCATION_COUNT_0      0x22C8
> PS_DEPTH_COUNT_0           0x22D8
> PS_INVOCATION_COUNT_1      0x22F0
> PS_DEPTH_COUNT_1           0x22F8
> PS_INVOCATION_COUNT_2      0x2448
> PS_DEPTH_COUNT_2           0x2450
> GPGPU_THREADS_DISPATCHED   0x2290
>
>    The kernel can skip these if you like.  Statistics registers just count
>    things, and userspace always calculates (end counter - start counter)
>    deltas, so the initial value doesn't really matter.
>
> SO_WRITE_OFFSET0           0x5280
> SO_WRITE_OFFSET1           0x5284
> SO_WRITE_OFFSET2           0x5288
> SO_WRITE_OFFSET3           0x528C
> GPUGPU_DISPATCHDIMX        0x2500
> GPUGPU_DISPATCHDIMY        0x2504
> GPUGPU_DISPATCHDIMZ        0x2508
> MI_PREDICATE_SRC0          0x2400
> MI_PREDICATE_SRC0          0x2404
> MI_PREDICATE_SRC1          0x2408
> MI_PREDICATE_SRC1          0x240C
> MI_PREDICATE_DATA          0x2410
> MI_PREDICATE_DATA          0x2414
> MI_PREDICATE_RESULT        0x2418
> MI_PREDICATE_RESULT_1      0x241C
> MI_PREDICATE_RESULT_2      0x23BC
> 3DPRIM_END_OFFSET          0x2420
> 3DPRIM_START_VERTEX        0x2430
> 3DPRIM_VERTEX_COUNT        0x2434
> 3DPRIM_INSTANCE_COUNT      0x2438
> 3DPRIM_START_INSTANCE      0x243C
> 3DPRIM_BASE_VERTEX         0x2440
>
>    The kernel can skip these if you like, IMO.  These registers are only
>    used when enabling an optional feature - stream out (SO_WRITE_*),
>    indirect compute dispatch (GPGPU_*), predicated draws (MI_PREDICATE_*),
>    indirect draws (3DPRIM_*).  Userspace has to explicitly opt in to each
>    of these features by enabling a flag, so there isn't a cross-context
>    contamination problem.  If userspace opts in to these features, it can
>    be responsible for programming the registers correctly.
>
> CS_GPR (1-16)              0x2600
>
>    The kernel can skip these if you like.  They're temporary storage when
>    using the MI_MATH instruction.  Example usage: load values into CS_GPR1
>    and CS_GPR2, add them, store the result in CS_GPR3.  Store to memory.
>
>    Nobody should be doing math on register values without setting them.
>    That's clearly a userspace bug.
>
> BB_OFFSET                  0x2158
>

This is used for indirect BATCH_BUFFER_START which is a thing on SKL+ I
believe (I didn't look at the docs).


> OA_CTX_CONTROL             0x2360
> OACTXID                    0x2364
> OA CONTROL                 0x2B00
> PERF_CNT_1_DW0             0x91b8
> PERF_CNT_1_DW1             0x91bc
> PERF_CNT_2_DW0             0x91c0
> PERF_CNT_2_DW1             0x91c4
>
>    I don't know about these.
>

[-- Attachment #1.2: Type: text/html, Size: 5846 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Intel-gfx] [PATCH] i965: Revert absolute mode for constant buffer pointers.

[Joonas Lahtinen]

Hi,

After some brainstorming, we decided to go big and grab all the
hardware provided defaults for all registers. That's for the sake of
not having to decide which registers are important.

This will also allow us not to be concerned of any context registers
possibly containing 'secure' information to be leaked, which we
currently don't expect from any known userspace. But you never know
what somebody could be doing with unused-for-them registers :)

The fix is not quite stable material, containing changes to the
hardware initialization ordering and being rather invasive in nature.

We're currently discussing the uAPI to expose to userspace, please stay
tuned. It'll likely be a bitmask of engine classes, which will shortly
be introduced from the PMU work.

Regards, Joonas

On Wed, 2017-10-25 at 10:53 -0700, Jason Ekstrand wrote:
> On Wed, Oct 25, 2017 at 10:31 AM, Kenneth Graunke <kenneth@whitecape.org> wrote:
> > On Wednesday, October 25, 2017 7:33:41 AM PDT Jason Ekstrand wrote:
> > > On October 25, 2017 06:05:16 Joonas Lahtinen wrote:
> > [snip]
> > > > 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.
> > 
> > I agree - I think we can cut down the list substantially, if you like.
> > Here's my breakdown of Skylake's non-privileged register list:
> > 
> > Cache_Mode_0               0x7000
> > Cache_Mode_1               0x7004
> > GT_MODE                    0x7008
> > L3_Config                  0x7034
> > TD_CTL                     0xE400
> > TD_CTL2                    0xE404
> > L3SQCREG4                  0xB118
> > NOPID                      0x2094
> > INSTPM                     0x20C0
> > 
> >    Should be initialized by the kernel.  Several of these can severely
> >    break unsuspecting userspace, and we'd like to be able to rely on a
> >    default value.
> > 
> > IA_VERTICES_COUNT          0x2310
> > IA_PRIMITIVES_COUNT        0x2318
> > VS_INVOCATION_COUNT        0x2320
> > HS_INVOCATION_COUNT        0x2300
> > DS_INVOCATION_COUNT        0x2308
> > GS_INVOCATION_COUNT        0x2328
> > GS_PRIMITIVES_COUNT        0x2330
> > SO_NUM_PRIMS_WRITTEN0      0x5200
> > SO_NUM_PRIMS_WRITTEN1      0x5208
> > SO_NUM_PRIMS_WRITTEN2      0x5210
> > SO_NUM_PRIMS_WRITTEN3      0x5218
> > SO_PRIM_STORAGE_NEEDED0    0x5240
> > SO_PRIM_STORAGE_NEEDED1    0x5248
> > SO_PRIM_STORAGE_NEEDED2    0x5250
> > SO_PRIM_STORAGE_NEEDED3    0x5258
> > CL_INVOCATION_COUNT        0x2338
> > CL_PRIMITIVES_COUNT        0x2340
> > PS_INVOCATION_COUNT_0      0x22C8
> > PS_DEPTH_COUNT_0           0x22D8
> > PS_INVOCATION_COUNT_1      0x22F0
> > PS_DEPTH_COUNT_1           0x22F8
> > PS_INVOCATION_COUNT_2      0x2448
> > PS_DEPTH_COUNT_2           0x2450
> > GPGPU_THREADS_DISPATCHED   0x2290
> > 
> >    The kernel can skip these if you like.  Statistics registers just count
> >    things, and userspace always calculates (end counter - start counter)
> >    deltas, so the initial value doesn't really matter.
> > 
> > SO_WRITE_OFFSET0           0x5280
> > SO_WRITE_OFFSET1           0x5284
> > SO_WRITE_OFFSET2           0x5288
> > SO_WRITE_OFFSET3           0x528C
> > GPUGPU_DISPATCHDIMX        0x2500
> > GPUGPU_DISPATCHDIMY        0x2504
> > GPUGPU_DISPATCHDIMZ        0x2508
> > MI_PREDICATE_SRC0          0x2400
> > MI_PREDICATE_SRC0          0x2404
> > MI_PREDICATE_SRC1          0x2408
> > MI_PREDICATE_SRC1          0x240C
> > MI_PREDICATE_DATA          0x2410
> > MI_PREDICATE_DATA          0x2414
> > MI_PREDICATE_RESULT        0x2418
> > MI_PREDICATE_RESULT_1      0x241C
> > MI_PREDICATE_RESULT_2      0x23BC
> > 3DPRIM_END_OFFSET          0x2420
> > 3DPRIM_START_VERTEX        0x2430
> > 3DPRIM_VERTEX_COUNT        0x2434
> > 3DPRIM_INSTANCE_COUNT      0x2438
> > 3DPRIM_START_INSTANCE      0x243C
> > 3DPRIM_BASE_VERTEX         0x2440
> > 
> >    The kernel can skip these if you like, IMO.  These registers are only
> >    used when enabling an optional feature - stream out (SO_WRITE_*),
> >    indirect compute dispatch (GPGPU_*), predicated draws (MI_PREDICATE_*),
> >    indirect draws (3DPRIM_*).  Userspace has to explicitly opt in to each
> >    of these features by enabling a flag, so there isn't a cross-context
> >    contamination problem.  If userspace opts in to these features, it can
> >    be responsible for programming the registers correctly.
> > 
> > CS_GPR (1-16)              0x2600
> > 
> >    The kernel can skip these if you like.  They're temporary storage when
> >    using the MI_MATH instruction.  Example usage: load values into CS_GPR1
> >    and CS_GPR2, add them, store the result in CS_GPR3.  Store to memory.
> > 
> >    Nobody should be doing math on register values without setting them.
> >    That's clearly a userspace bug.
> > 
> > BB_OFFSET                  0x2158
> 
> This is used for indirect BATCH_BUFFER_START which is a thing on SKL+ I believe (I didn't look at the docs).
>  
> > OA_CTX_CONTROL             0x2360
> > OACTXID                    0x2364
> > OA CONTROL                 0x2B00
> > PERF_CNT_1_DW0             0x91b8
> > PERF_CNT_1_DW1             0x91bc
> > PERF_CNT_2_DW0             0x91c0
> > PERF_CNT_2_DW1             0x91c4
> > 
> >    I don't know about these.
> 
> 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev