| Message ID | 20180518142657.29064-1-lionel.g.landwerlin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 18/05/18 15:26, Lionel Landwerlin wrote: > On Gen8+ this register is not priviledged and we want to use it in > Mesa to implement a feature required by GPA called Null Hardware. The > idea is to have the command parser turn 3DPRIMITIVE/GPGPU_WALKER into > NOOPs. > > This patch just whitelists the bits that we need and that are > currently not used by the kernel. > One thing I don't really know is whether is should be considered an issue with the current command parser and therefore be backported as a fix. It would certainly make things easier because we can't really detect the behavior from userspace. - Lionel
Quoting Lionel Landwerlin (2018-05-18 15:29:21) > On 18/05/18 15:26, Lionel Landwerlin wrote: > > On Gen8+ this register is not priviledged and we want to use it in > > Mesa to implement a feature required by GPA called Null Hardware. The > > idea is to have the command parser turn 3DPRIMITIVE/GPGPU_WALKER into > > NOOPs. > > > > This patch just whitelists the bits that we need and that are > > currently not used by the kernel. > > > > One thing I don't really know is whether is should be considered an > issue with the current command parser and therefore be backported as a fix. > It would certainly make things easier because we can't really detect the > behavior from userspace. You can always bump the cmdparser version. The cmdparser doesn't no-op, it just rejects if a command is outside the whitelist, right? It would be detectable... The key question is whether INSTPM is context saved on gen7. A test in gem_exec_parse to confirm we allow INSTPM in the command parser and that one context does not leak to another would be useful. -Chris
On 18/05/18 15:40, Chris Wilson wrote: > Quoting Lionel Landwerlin (2018-05-18 15:29:21) >> On 18/05/18 15:26, Lionel Landwerlin wrote: >>> On Gen8+ this register is not priviledged and we want to use it in >>> Mesa to implement a feature required by GPA called Null Hardware. The >>> idea is to have the command parser turn 3DPRIMITIVE/GPGPU_WALKER into >>> NOOPs. >>> >>> This patch just whitelists the bits that we need and that are >>> currently not used by the kernel. >>> >> One thing I don't really know is whether is should be considered an >> issue with the current command parser and therefore be backported as a fix. >> It would certainly make things easier because we can't really detect the >> behavior from userspace. > You can always bump the cmdparser version. The cmdparser doesn't no-op, > it just rejects if a command is outside the whitelist, right? It would > be detectable... Ah right! > > The key question is whether INSTPM is context saved on gen7. Documentation says it is. I'm not sure I trust it though :( > A test in > gem_exec_parse to confirm we allow INSTPM in the command parser and that > one context does not leak to another would be useful. Sure. > -Chris >
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index 95478db9998b..1db6447460eb 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -534,6 +534,14 @@ struct drm_i915_reg_descriptor { { .addr = _reg ## _UDW(idx) } static const struct drm_i915_reg_descriptor gen7_render_regs[] = { + REG32(INSTPM, + .mask = ~((INSTPM_3D_STATE_INSTRUCTION_DISABLE | + INSTPM_3D_RENDERING_INSTRUCTION_DISABLE | + INSTPM_3D_MEDIA_INSTRUCTION_DISABLE) << 16 | + (INSTPM_3D_STATE_INSTRUCTION_DISABLE | + INSTPM_3D_RENDERING_INSTRUCTION_DISABLE | + INSTPM_3D_MEDIA_INSTRUCTION_DISABLE)), + .value = 0), REG64(GPGPU_THREADS_DISPATCHED), REG64(HS_INVOCATION_COUNT), REG64(DS_INVOCATION_COUNT), diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 196a0eb79272..2db9b6a177d9 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2531,6 +2531,9 @@ enum i915_power_well_id { #define INSTPM_FORCE_ORDERING (1<<7) /* GEN6+ */ #define INSTPM_TLB_INVALIDATE (1<<9) #define INSTPM_SYNC_FLUSH (1<<5) +#define INSTPM_3D_MEDIA_INSTRUCTION_DISABLE (1<<3) /* GEN6+ */ +#define INSTPM_3D_RENDERING_INSTRUCTION_DISABLE (1<<2) /* GEN6+ */ +#define INSTPM_3D_STATE_INSTRUCTION_DISABLE (1<<1) /* GEN6+ */ #define ACTHD _MMIO(0x20c8) #define MEM_MODE _MMIO(0x20cc) #define MEM_DISPLAY_B_TRICKLE_FEED_DISABLE (1<<3) /* 830 only */
On Gen8+ this register is not priviledged and we want to use it in Mesa to implement a feature required by GPA called Null Hardware. The idea is to have the command parser turn 3DPRIMITIVE/GPGPU_WALKER into NOOPs. This patch just whitelists the bits that we need and that are currently not used by the kernel. Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/i915_cmd_parser.c | 8 ++++++++ drivers/gpu/drm/i915/i915_reg.h | 3 +++ 2 files changed, 11 insertions(+)