* [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer
@ 2020-07-18 0:04 Umesh Nerlige Ramappa
2020-07-18 0:04 ` [Intel-gfx] [PATCH 1/4] drm/i915/perf: Ensure observation logic is not clock gated Umesh Nerlige Ramappa
` (6 more replies)
0 siblings, 7 replies; 31+ messages in thread
From: Umesh Nerlige Ramappa @ 2020-07-18 0:04 UTC (permalink / raw)
To: intel-gfx
This cover letter is included to trigger "Test-with" an IGT patch.
Tests - https://patchwork.freedesktop.org/patch/377905/?series=79617&rev=1
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Test-with: 20200717235842.68574-1-umesh.nerlige.ramappa@intel.com
Piotr Maciejewski (4):
drm/i915/perf: Ensure observation logic is not clock gated
drm/i915/perf: Whitelist OA report trigger registers
drm/i915/perf: Whitelist OA counter and buffer registers
drm/i915/perf: Map OA buffer to user space for gen12 performance query
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 +
drivers/gpu/drm/i915/gt/intel_workarounds.c | 60 +++++++
.../gpu/drm/i915/gt/selftest_workarounds.c | 15 +-
drivers/gpu/drm/i915/i915_perf.c | 167 +++++++++++++++++-
drivers/gpu/drm/i915/i915_perf_types.h | 5 +
drivers/gpu/drm/i915/i915_reg.h | 12 ++
include/uapi/drm/i915_drm.h | 32 ++++
8 files changed, 290 insertions(+), 5 deletions(-)
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread* [Intel-gfx] [PATCH 1/4] drm/i915/perf: Ensure observation logic is not clock gated 2020-07-18 0:04 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa @ 2020-07-18 0:04 ` Umesh Nerlige Ramappa 2020-07-18 19:09 ` Lionel Landwerlin 2020-07-18 0:04 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa ` (5 subsequent siblings) 6 siblings, 1 reply; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-18 0:04 UTC (permalink / raw) To: intel-gfx From: Piotr Maciejewski <piotr.maciejewski@intel.com> A clock gating switch can control if the performance monitoring and observation logic is enaled or not. Ensure that we enable the clocks. v2: Separate code from other patches (Lionel) Fixes: 00a7f0d7155c ("drm/i915/tgl: Add perf support on TGL") Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> --- drivers/gpu/drm/i915/i915_perf.c | 8 ++++++++ drivers/gpu/drm/i915/i915_reg.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index c6f6370283cf..88610d52f30b 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -2493,6 +2493,14 @@ gen12_enable_metric_set(struct i915_perf_stream *stream, (period_exponent << GEN12_OAG_OAGLBCTXCTRL_TIMER_PERIOD_SHIFT)) : 0); + /* + * Initialize Super Queue Internal Cnt Register + * Set PMON Enable in order to collect valid metrics. + */ + intel_uncore_write(uncore, GEN12_SQCNT1, + intel_uncore_read(uncore, GEN12_SQCNT1) | + GEN12_SQCNT1_PMON_ENABLE); + /* * Update all contexts prior writing the mux configurations as we need * to make sure all slices/subslices are ON before writing to NOA diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index b9607ac3620d..1638f1282541 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -696,6 +696,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define OABUFFER_SIZE_16M (7 << 3) #define GEN12_OA_TLB_INV_CR _MMIO(0xceec) +#define GEN12_SQCNT1 _MMIO(0x8718) +#define GEN12_SQCNT1_PMON_ENABLE (1 << 30) /* Gen12 OAR unit */ #define GEN12_OAR_OACONTROL _MMIO(0x2960) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915/perf: Ensure observation logic is not clock gated 2020-07-18 0:04 ` [Intel-gfx] [PATCH 1/4] drm/i915/perf: Ensure observation logic is not clock gated Umesh Nerlige Ramappa @ 2020-07-18 19:09 ` Lionel Landwerlin 0 siblings, 0 replies; 31+ messages in thread From: Lionel Landwerlin @ 2020-07-18 19:09 UTC (permalink / raw) To: Umesh Nerlige Ramappa, intel-gfx On 18/07/2020 03:04, Umesh Nerlige Ramappa wrote: > From: Piotr Maciejewski <piotr.maciejewski@intel.com> > > A clock gating switch can control if the performance monitoring and > observation logic is enaled or not. Ensure that we enable the clocks. > > v2: Separate code from other patches (Lionel) > > Fixes: 00a7f0d7155c ("drm/i915/tgl: Add perf support on TGL") > Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> > Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> > --- > drivers/gpu/drm/i915/i915_perf.c | 8 ++++++++ > drivers/gpu/drm/i915/i915_reg.h | 2 ++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index c6f6370283cf..88610d52f30b 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -2493,6 +2493,14 @@ gen12_enable_metric_set(struct i915_perf_stream *stream, > (period_exponent << GEN12_OAG_OAGLBCTXCTRL_TIMER_PERIOD_SHIFT)) > : 0); > > + /* > + * Initialize Super Queue Internal Cnt Register > + * Set PMON Enable in order to collect valid metrics. > + */ > + intel_uncore_write(uncore, GEN12_SQCNT1, > + intel_uncore_read(uncore, GEN12_SQCNT1) | > + GEN12_SQCNT1_PMON_ENABLE); Thanks for splitting this. We just need to also disable this when i915-perf is disabled, documentation says it saved power when turned off. -Lionel > + > /* > * Update all contexts prior writing the mux configurations as we need > * to make sure all slices/subslices are ON before writing to NOA > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index b9607ac3620d..1638f1282541 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -696,6 +696,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > #define OABUFFER_SIZE_16M (7 << 3) > > #define GEN12_OA_TLB_INV_CR _MMIO(0xceec) > +#define GEN12_SQCNT1 _MMIO(0x8718) > +#define GEN12_SQCNT1_PMON_ENABLE (1 << 30) > > /* Gen12 OAR unit */ > #define GEN12_OAR_OACONTROL _MMIO(0x2960) _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-18 0:04 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa 2020-07-18 0:04 ` [Intel-gfx] [PATCH 1/4] drm/i915/perf: Ensure observation logic is not clock gated Umesh Nerlige Ramappa @ 2020-07-18 0:04 ` Umesh Nerlige Ramappa 2020-07-18 19:11 ` Lionel Landwerlin 2020-07-18 0:04 ` [Intel-gfx] [PATCH 3/4] drm/i915/perf: Whitelist OA counter and buffer registers Umesh Nerlige Ramappa ` (4 subsequent siblings) 6 siblings, 1 reply; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-18 0:04 UTC (permalink / raw) To: intel-gfx From: Piotr Maciejewski <piotr.maciejewski@intel.com> OA reports can be triggered into the OA buffer by writing into the OAREPORTTRIG registers. Whitelist the registers to allow user to trigger reports. v2: - Move related change to this patch (Lionel) - Bump up perf revision (Lionel) Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 26 +++++++++++++++++++++ drivers/gpu/drm/i915/i915_perf.c | 11 ++++++--- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 5726cd0a37e0..582a2c8cd219 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -1365,6 +1365,20 @@ whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg) whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_ACCESS_RW); } +static void gen9_whitelist_build_performance_counters(struct i915_wa_list *w) +{ + /* OA buffer trigger report 2/6 used by performance query */ + whitelist_reg(w, OAREPORTTRIG2); + whitelist_reg(w, OAREPORTTRIG6); +} + +static void gen12_whitelist_build_performance_counters(struct i915_wa_list *w) +{ + /* OA buffer trigger report 2/6 used by performance query */ + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG2); + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG6); +} + static void gen9_whitelist_build(struct i915_wa_list *w) { /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ @@ -1378,6 +1392,9 @@ static void gen9_whitelist_build(struct i915_wa_list *w) /* WaSendPushConstantsFromMMIO:skl,bxt */ whitelist_reg(w, COMMON_SLICE_CHICKEN2); + + /* Performance counters support */ + gen9_whitelist_build_performance_counters(w); } static void skl_whitelist_build(struct intel_engine_cs *engine) @@ -1471,6 +1488,9 @@ static void cnl_whitelist_build(struct intel_engine_cs *engine) /* WaEnablePreemptionGranularityControlByUMD:cnl */ whitelist_reg(w, GEN8_CS_CHICKEN1); + + /* Performance counters support */ + gen9_whitelist_build_performance_counters(w); } static void icl_whitelist_build(struct intel_engine_cs *engine) @@ -1500,6 +1520,9 @@ static void icl_whitelist_build(struct intel_engine_cs *engine) whitelist_reg_ext(w, PS_INVOCATION_COUNT, RING_FORCE_TO_NONPRIV_ACCESS_RD | RING_FORCE_TO_NONPRIV_RANGE_4); + + /* Performance counters support */ + gen9_whitelist_build_performance_counters(w); break; case VIDEO_DECODE_CLASS: @@ -1550,6 +1573,9 @@ static void tgl_whitelist_build(struct intel_engine_cs *engine) /* Wa_1806527549:tgl */ whitelist_reg(w, HIZ_CHICKEN); + + /* Performance counters support */ + gen12_whitelist_build_performance_counters(w); break; default: whitelist_reg_ext(w, diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 88610d52f30b..1a72565d1928 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1448,7 +1448,8 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN8_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK); /* Mark that we need updated tail pointers to read from... */ @@ -1501,7 +1502,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN12_OAG_OATAILPTR, gtt_offset & GEN12_OAG_OATAILPTR_MASK); @@ -4440,8 +4442,11 @@ int i915_perf_ioctl_version(void) * * 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the * interval for the hrtimer used to check for OA data. + * + * 6: Whitelist OATRIGGER registers to allow user to trigger reports + * into the OA buffer. */ - return 5; + return 6; } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-18 0:04 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa @ 2020-07-18 19:11 ` Lionel Landwerlin 0 siblings, 0 replies; 31+ messages in thread From: Lionel Landwerlin @ 2020-07-18 19:11 UTC (permalink / raw) To: Umesh Nerlige Ramappa, intel-gfx On 18/07/2020 03:04, Umesh Nerlige Ramappa wrote: > From: Piotr Maciejewski <piotr.maciejewski@intel.com> > > OA reports can be triggered into the OA buffer by writing into the > OAREPORTTRIG registers. Whitelist the registers to allow user to trigger > reports. > > v2: > - Move related change to this patch (Lionel) > - Bump up perf revision (Lionel) > > Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> > Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 26 +++++++++++++++++++++ > drivers/gpu/drm/i915/i915_perf.c | 11 ++++++--- > 2 files changed, 34 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index 5726cd0a37e0..582a2c8cd219 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -1365,6 +1365,20 @@ whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg) > whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_ACCESS_RW); > } > > +static void gen9_whitelist_build_performance_counters(struct i915_wa_list *w) > +{ > + /* OA buffer trigger report 2/6 used by performance query */ > + whitelist_reg(w, OAREPORTTRIG2); > + whitelist_reg(w, OAREPORTTRIG6); > +} > + > +static void gen12_whitelist_build_performance_counters(struct i915_wa_list *w) > +{ > + /* OA buffer trigger report 2/6 used by performance query */ > + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG2); > + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG6); > +} > + > static void gen9_whitelist_build(struct i915_wa_list *w) > { > /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ > @@ -1378,6 +1392,9 @@ static void gen9_whitelist_build(struct i915_wa_list *w) > > /* WaSendPushConstantsFromMMIO:skl,bxt */ > whitelist_reg(w, COMMON_SLICE_CHICKEN2); > + > + /* Performance counters support */ > + gen9_whitelist_build_performance_counters(w); > } > > static void skl_whitelist_build(struct intel_engine_cs *engine) > @@ -1471,6 +1488,9 @@ static void cnl_whitelist_build(struct intel_engine_cs *engine) > > /* WaEnablePreemptionGranularityControlByUMD:cnl */ > whitelist_reg(w, GEN8_CS_CHICKEN1); > + > + /* Performance counters support */ > + gen9_whitelist_build_performance_counters(w); > } > > static void icl_whitelist_build(struct intel_engine_cs *engine) > @@ -1500,6 +1520,9 @@ static void icl_whitelist_build(struct intel_engine_cs *engine) > whitelist_reg_ext(w, PS_INVOCATION_COUNT, > RING_FORCE_TO_NONPRIV_ACCESS_RD | > RING_FORCE_TO_NONPRIV_RANGE_4); > + > + /* Performance counters support */ > + gen9_whitelist_build_performance_counters(w); > break; > > case VIDEO_DECODE_CLASS: > @@ -1550,6 +1573,9 @@ static void tgl_whitelist_build(struct intel_engine_cs *engine) > > /* Wa_1806527549:tgl */ > whitelist_reg(w, HIZ_CHICKEN); > + > + /* Performance counters support */ > + gen12_whitelist_build_performance_counters(w); > break; > default: > whitelist_reg_ext(w, > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index 88610d52f30b..1a72565d1928 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -1448,7 +1448,8 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream) > * bit." > */ > intel_uncore_write(uncore, GEN8_OABUFFER, gtt_offset | > - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); > + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | > + GEN7_OABUFFER_EDGE_TRIGGER); > intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK); > > /* Mark that we need updated tail pointers to read from... */ > @@ -1501,7 +1502,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream) > * bit." > */ > intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset | > - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); > + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | > + GEN7_OABUFFER_EDGE_TRIGGER); > intel_uncore_write(uncore, GEN12_OAG_OATAILPTR, > gtt_offset & GEN12_OAG_OATAILPTR_MASK); > > @@ -4440,8 +4442,11 @@ int i915_perf_ioctl_version(void) > * > * 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the > * interval for the hrtimer used to check for OA data. > + * > + * 6: Whitelist OATRIGGER registers to allow user to trigger reports > + * into the OA buffer. If you could just add a comment this only applies to gen8+. Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Thanks! > */ > - return 5; > + return 6; > } > > #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 3/4] drm/i915/perf: Whitelist OA counter and buffer registers 2020-07-18 0:04 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa 2020-07-18 0:04 ` [Intel-gfx] [PATCH 1/4] drm/i915/perf: Ensure observation logic is not clock gated Umesh Nerlige Ramappa 2020-07-18 0:04 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa @ 2020-07-18 0:04 ` Umesh Nerlige Ramappa 2020-07-18 0:04 ` [Intel-gfx] [PATCH 4/4] drm/i915/perf: Map OA buffer to user space for gen12 performance query Umesh Nerlige Ramappa ` (3 subsequent siblings) 6 siblings, 0 replies; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-18 0:04 UTC (permalink / raw) To: intel-gfx From: Piotr Maciejewski <piotr.maciejewski@intel.com> It is useful to have markers in the OA reports to identify triggered reports. Whitelist some OA counters that can be used as markers. A triggered report can be found faster if we can sample the HW tail and head registers when the report was triggered. Whitelist OA buffer specific registers. v2: - Bump up the perf revision (Lionel) - Use indexing for counters (Lionel) - Fix selftest for oa ticking register (Umesh) Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 34 +++++++++++++++++++ .../gpu/drm/i915/gt/selftest_workarounds.c | 15 +++++++- drivers/gpu/drm/i915/i915_perf.c | 5 ++- drivers/gpu/drm/i915/i915_reg.h | 10 ++++++ 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 582a2c8cd219..5dfa3177d216 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -1370,6 +1370,23 @@ static void gen9_whitelist_build_performance_counters(struct i915_wa_list *w) /* OA buffer trigger report 2/6 used by performance query */ whitelist_reg(w, OAREPORTTRIG2); whitelist_reg(w, OAREPORTTRIG6); + + /* Performance counters A18-20 used by tbs marker query */ + whitelist_reg_ext(w, OA_PERF_COUNTER_A(18), + RING_FORCE_TO_NONPRIV_ACCESS_RW | + RING_FORCE_TO_NONPRIV_RANGE_4); + + whitelist_reg(w, OA_PERF_COUNTER_A(20)); + whitelist_reg(w, OA_PERF_COUNTER_A_UPPER(20)); + + /* Read access to gpu ticks */ + whitelist_reg_ext(w, GEN8_GPU_TICKS, + RING_FORCE_TO_NONPRIV_ACCESS_RD); + + /* Read access to: oa status, head, tail, buffer settings */ + whitelist_reg_ext(w, GEN8_OASTATUS, + RING_FORCE_TO_NONPRIV_ACCESS_RD | + RING_FORCE_TO_NONPRIV_RANGE_4); } static void gen12_whitelist_build_performance_counters(struct i915_wa_list *w) @@ -1377,6 +1394,23 @@ static void gen12_whitelist_build_performance_counters(struct i915_wa_list *w) /* OA buffer trigger report 2/6 used by performance query */ whitelist_reg(w, GEN12_OAG_OAREPORTTRIG2); whitelist_reg(w, GEN12_OAG_OAREPORTTRIG6); + + /* Performance counters A18-20 used by tbs marker query */ + whitelist_reg_ext(w, GEN12_OAG_PERF_COUNTER_A(18), + RING_FORCE_TO_NONPRIV_ACCESS_RW | + RING_FORCE_TO_NONPRIV_RANGE_4); + + whitelist_reg(w, GEN12_OAG_PERF_COUNTER_A(20)); + whitelist_reg(w, GEN12_OAG_PERF_COUNTER_A_UPPER(20)); + + /* Read access to gpu ticks */ + whitelist_reg_ext(w, GEN12_OAG_GPU_TICKS, + RING_FORCE_TO_NONPRIV_ACCESS_RD); + + /* Read access to: oa status, head, tail, buffer settings */ + whitelist_reg_ext(w, GEN12_OAG_OASTATUS, + RING_FORCE_TO_NONPRIV_ACCESS_RD | + RING_FORCE_TO_NONPRIV_RANGE_4); } static void gen9_whitelist_build(struct i915_wa_list *w) diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c index febc9e6692ba..5e0e839928f9 100644 --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c @@ -431,6 +431,19 @@ static bool timestamp(const struct intel_engine_cs *engine, u32 reg) } } +static bool oa_gpu_ticks(u32 reg) +{ + reg = reg & ~RING_FORCE_TO_NONPRIV_ACCESS_MASK; + switch (reg) { + case 0x2910: + case 0xda90: + return true; + + default: + return false; + } +} + static bool ro_register(u32 reg) { if ((reg & RING_FORCE_TO_NONPRIV_ACCESS_MASK) == @@ -511,7 +524,7 @@ static int check_dirty_whitelist(struct intel_context *ce) if (wo_register(engine, reg)) continue; - if (timestamp(engine, reg)) + if (timestamp(engine, reg) || oa_gpu_ticks(reg)) continue; /* timestamps are expected to autoincrement */ ro_reg = ro_register(reg); diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 1a72565d1928..15f31c250ca9 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -4445,8 +4445,11 @@ int i915_perf_ioctl_version(void) * * 6: Whitelist OATRIGGER registers to allow user to trigger reports * into the OA buffer. + * + * 7: Whitelist OA buffer head/tail registers for user to identify the + * location of triggered reports into the OA buffer. */ - return 6; + return 7; } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 1638f1282541..eeb41de84a0f 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -675,6 +675,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define GEN7_OASTATUS2_HEAD_MASK 0xffffffc0 #define GEN7_OASTATUS2_MEM_SELECT_GGTT (1 << 0) /* 0: PPGTT, 1: GGTT */ +#define GEN8_GPU_TICKS _MMIO(0x2910) #define GEN8_OASTATUS _MMIO(0x2b08) #define GEN8_OASTATUS_OVERRUN_STATUS (1 << 3) #define GEN8_OASTATUS_COUNTER_OVERFLOW (1 << 2) @@ -733,6 +734,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define GEN12_OAG_OA_DEBUG_DISABLE_GO_1_0_REPORTS (1 << 2) #define GEN12_OAG_OA_DEBUG_DISABLE_CTX_SWITCH_REPORTS (1 << 1) +#define GEN12_OAG_GPU_TICKS _MMIO(0xda90) #define GEN12_OAG_OASTATUS _MMIO(0xdafc) #define GEN12_OAG_OASTATUS_COUNTER_OVERFLOW (1 << 2) #define GEN12_OAG_OASTATUS_BUFFER_OVERFLOW (1 << 1) @@ -974,6 +976,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define OAREPORTTRIG8_NOA_SELECT_6_SHIFT 24 #define OAREPORTTRIG8_NOA_SELECT_7_SHIFT 28 +/* Performance counters registers */ +#define OA_PERF_COUNTER_A(idx) _MMIO(0x2800 + 8 * (idx)) +#define OA_PERF_COUNTER_A_UPPER(idx) _MMIO(0x2800 + 8 * (idx) + 4) + +/* Gen12 Performance counters registers */ +#define GEN12_OAG_PERF_COUNTER_A(idx) _MMIO(0xD980 + 8 * (idx)) +#define GEN12_OAG_PERF_COUNTER_A_UPPER(idx) _MMIO(0xD980 + 8 * (idx) + 4) + /* Same layout as OASTARTTRIGX */ #define GEN12_OAG_OASTARTTRIG1 _MMIO(0xd900) #define GEN12_OAG_OASTARTTRIG2 _MMIO(0xd904) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 4/4] drm/i915/perf: Map OA buffer to user space for gen12 performance query 2020-07-18 0:04 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa ` (2 preceding siblings ...) 2020-07-18 0:04 ` [Intel-gfx] [PATCH 3/4] drm/i915/perf: Whitelist OA counter and buffer registers Umesh Nerlige Ramappa @ 2020-07-18 0:04 ` Umesh Nerlige Ramappa 2020-07-18 11:44 ` kernel test robot ` (2 more replies) 2020-07-18 0:29 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Allow privileged user to map the OA buffer (rev2) Patchwork ` (2 subsequent siblings) 6 siblings, 3 replies; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-18 0:04 UTC (permalink / raw) To: intel-gfx From: Piotr Maciejewski <piotr.maciejewski@intel.com> i915 used to support time based sampling mode which is good for overall system monitoring, but is not enough for query mode used to measure a single draw call or dispatch. Gen9-Gen11 are using current i915 perf implementation for query, but Gen12+ requires a new approach for query based on triggered reports within oa buffer. Triggering reports into the OA buffer is achieved by writing into a a trigger register. Optionally an unused counter/register is set with a marker value such that a triggered report can be identified in the OA buffer. Reports are usually triggered at the start and end of work that is measured. Since OA buffer is large and queries can be frequent, an efficient way to look for triggered reports is required. By knowing the current head and tail offsets into the OA buffer, it is easier to determine the locality of the reports of interest. Current perf OA interface does not expose head/tail information to the user and it filters out invalid reports before sending data to user. Also considering limited size of user buffer used during a query, creating a 1:1 copy of the OA buffer at the user space added undesired complexity. The solution was to map the OA buffer to user space provided (1) that it is accessed from a privileged user. (2) OA report filtering is not used. These 2 conditions would satisfy the safety criteria that the current perf interface addresses. To enable the query: - Add an ioctl to expose head and tail to the user - Add an ioctl to return size and offset of the OA buffer - Map the OA buffer to the user space v2: - Improve commit message (Chris) - Do not mmap based on gem object filp. Instead, use perf_fd and support mmap syscall (Chris) - Pass non-zero offset in mmap to enforce the right object is mapped (Chris) - Do not expose gpu_address (Chris) - Verify start and length of vma for page alignment (Lionel) - Move SQNTL config out (Lionel) Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> --- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 + drivers/gpu/drm/i915/i915_perf.c | 147 ++++++++++++++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 5 + include/uapi/drm/i915_drm.h | 32 +++++ 5 files changed, 186 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index b23368529a40..7c4b9b0c334b 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -204,7 +204,7 @@ compute_partial_view(const struct drm_i915_gem_object *obj, return view; } -static vm_fault_t i915_error_to_vmf_fault(int err) +vm_fault_t i915_error_to_vmf_fault(int err) { switch (err) { default: diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h index efee9e0d2508..1190a3a228ea 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h @@ -29,4 +29,6 @@ void i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj); void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj); +vm_fault_t i915_error_to_vmf_fault(int err); + #endif diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 15f31c250ca9..e77582761a64 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -192,10 +192,12 @@ */ #include <linux/anon_inodes.h> +#include <linux/mman.h> #include <linux/sizes.h> #include <linux/uuid.h> #include "gem/i915_gem_context.h" +#include "gem/i915_gem_mman.h" #include "gt/intel_engine_pm.h" #include "gt/intel_engine_user.h" #include "gt/intel_gt.h" @@ -434,6 +436,30 @@ static u32 gen7_oa_hw_tail_read(struct i915_perf_stream *stream) return oastatus1 & GEN7_OASTATUS1_TAIL_MASK; } +static u32 gen12_oa_hw_head_read(struct i915_perf_stream *stream) +{ + struct intel_uncore *uncore = stream->uncore; + + return intel_uncore_read(uncore, GEN12_OAG_OAHEADPTR) & + GEN12_OAG_OAHEADPTR_MASK; +} + +static u32 gen8_oa_hw_head_read(struct i915_perf_stream *stream) +{ + struct intel_uncore *uncore = stream->uncore; + + return intel_uncore_read(uncore, GEN8_OAHEADPTR) & + GEN8_OAHEADPTR_MASK; +} + +static u32 gen7_oa_hw_head_read(struct i915_perf_stream *stream) +{ + struct intel_uncore *uncore = stream->uncore; + u32 oastatus2 = intel_uncore_read(uncore, GEN7_OASTATUS2); + + return oastatus2 & GEN7_OASTATUS2_HEAD_MASK; +} + /** * oa_buffer_check_unlocked - check for data and update tail ptr state * @stream: i915 stream instance @@ -3209,6 +3235,59 @@ static long i915_perf_config_locked(struct i915_perf_stream *stream, return ret; } +/** + * i915_perf_oa_buffer_head_tail_locked - head and tail of the OA buffer + * @stream: i915 perf stream + * @arg: pointer to oa buffer head and tail filled by this function. + */ +static int i915_perf_oa_buffer_head_tail_locked(struct i915_perf_stream *stream, + unsigned long arg) +{ + struct drm_i915_perf_oa_buffer_head_tail ht; + void __user *output = (void __user *)arg; + u32 gtt_offset = i915_ggtt_offset(stream->oa_buffer.vma); + + if (!output) + return -EINVAL; + + memset(&ht, 0, sizeof(ht)); + + ht.head = stream->perf->ops.oa_hw_head_read(stream) - gtt_offset; + ht.tail = stream->perf->ops.oa_hw_tail_read(stream) - gtt_offset; + + if (copy_to_user(output, &ht, sizeof(ht))) + return -EFAULT; + + return 0; +} + +#define I915_PERF_OA_BUFFER_MMAP_OFFSET 1 + +/** + * i915_perf_oa_buffer_info_locked - size and offset of the OA buffer + * @stream: i915 perf stream + * @arg: pointer to oa buffer info filled by this function. + */ +static int i915_perf_oa_buffer_info_locked(struct i915_perf_stream *stream, + unsigned long arg) +{ + struct drm_i915_perf_oa_buffer_info info; + void __user *output = (void __user *)arg; + + if (!output) + return -EINVAL; + + memset(&info, 0, sizeof(info)); + + info.size = stream->oa_buffer.vma->size; + info.offset = I915_PERF_OA_BUFFER_MMAP_OFFSET * PAGE_SIZE; + + if (copy_to_user(output, &info, sizeof(info))) + return -EFAULT; + + return 0; +} + /** * i915_perf_ioctl - support ioctl() usage with i915 perf stream FDs * @stream: An i915 perf stream @@ -3234,6 +3313,10 @@ static long i915_perf_ioctl_locked(struct i915_perf_stream *stream, return 0; case I915_PERF_IOCTL_CONFIG: return i915_perf_config_locked(stream, arg); + case I915_PERF_IOCTL_GET_OA_BUFFER_INFO: + return i915_perf_oa_buffer_info_locked(stream, arg); + case I915_PERF_IOCTL_GET_OA_BUFFER_HEAD_TAIL: + return i915_perf_oa_buffer_head_tail_locked(stream, arg); } return -EINVAL; @@ -3315,6 +3398,60 @@ static int i915_perf_release(struct inode *inode, struct file *file) return 0; } +static vm_fault_t vm_fault_oa(struct vm_fault *vmf) +{ + struct vm_area_struct *vma = vmf->vma; + struct i915_perf_stream *stream = vma->vm_private_data; + struct drm_i915_gem_object *obj = stream->oa_buffer.vma->obj; + int err; + + err = i915_gem_object_pin_pages(obj); + if (err) + goto out; + + err = remap_io_sg(vma, + vma->vm_start, vma->vm_end - vma->vm_start, + obj->mm.pages->sgl, -1); + + i915_gem_object_unpin_pages(obj); + +out: + return i915_error_to_vmf_fault(err); +} + +static const struct vm_operations_struct vm_ops_oa = { + .fault = vm_fault_oa, +}; + +int i915_perf_mmap(struct file *file, struct vm_area_struct *vma) +{ + struct i915_perf_stream *stream = file->private_data; + int len; + + if (!IS_ALIGNED(vma->vm_start, PAGE_SIZE)) + return -EINVAL; + + if (vma->vm_end < vma->vm_start) + return -EINVAL; + + len = vma->vm_end - vma->vm_start; + if (!IS_ALIGNED(len, PAGE_SIZE) || len > OA_BUFFER_SIZE) + return -EINVAL; + + if (vma->vm_flags & VM_WRITE) + return -EINVAL; + + if (vma->vm_pgoff != I915_PERF_OA_BUFFER_MMAP_OFFSET) + return -EINVAL; + + vma->vm_flags &= ~(VM_MAYWRITE | VM_MAYEXEC | VM_MAYSHARE); + vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; + vma->vm_page_prot = vm_get_page_prot(vma->vm_flags); + vma->vm_private_data = stream; + vma->vm_ops = &vm_ops_oa; + + return 0; +} static const struct file_operations fops = { .owner = THIS_MODULE, @@ -3327,6 +3464,7 @@ static const struct file_operations fops = { * to handle 32bits compatibility. */ .compat_ioctl = i915_perf_ioctl, + .mmap = i915_perf_mmap, }; @@ -4255,6 +4393,7 @@ void i915_perf_init(struct drm_i915_private *i915) perf->ops.oa_disable = gen7_oa_disable; perf->ops.read = gen7_oa_read; perf->ops.oa_hw_tail_read = gen7_oa_hw_tail_read; + perf->ops.oa_hw_head_read = gen7_oa_hw_head_read; perf->oa_formats = hsw_oa_formats; } else if (HAS_LOGICAL_RING_CONTEXTS(i915)) { @@ -4286,6 +4425,7 @@ void i915_perf_init(struct drm_i915_private *i915) perf->ops.enable_metric_set = gen8_enable_metric_set; perf->ops.disable_metric_set = gen8_disable_metric_set; perf->ops.oa_hw_tail_read = gen8_oa_hw_tail_read; + perf->ops.oa_hw_head_read = gen8_oa_hw_head_read; if (IS_GEN(i915, 8)) { perf->ctx_oactxctrl_offset = 0x120; @@ -4313,6 +4453,7 @@ void i915_perf_init(struct drm_i915_private *i915) perf->ops.enable_metric_set = gen8_enable_metric_set; perf->ops.disable_metric_set = gen10_disable_metric_set; perf->ops.oa_hw_tail_read = gen8_oa_hw_tail_read; + perf->ops.oa_hw_head_read = gen8_oa_hw_head_read; if (IS_GEN(i915, 10)) { perf->ctx_oactxctrl_offset = 0x128; @@ -4337,6 +4478,7 @@ void i915_perf_init(struct drm_i915_private *i915) perf->ops.enable_metric_set = gen12_enable_metric_set; perf->ops.disable_metric_set = gen12_disable_metric_set; perf->ops.oa_hw_tail_read = gen12_oa_hw_tail_read; + perf->ops.oa_hw_head_read = gen12_oa_hw_head_read; perf->ctx_flexeu0_offset = 0; perf->ctx_oactxctrl_offset = 0x144; @@ -4448,8 +4590,11 @@ int i915_perf_ioctl_version(void) * * 7: Whitelist OA buffer head/tail registers for user to identify the * location of triggered reports into the OA buffer. + * + * 8: Added an option to map oa buffer at umd driver level and trigger + * oa reports within oa buffer from command buffer. */ - return 7; + return 8; } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h index a36a455ae336..579f34e6fdd8 100644 --- a/drivers/gpu/drm/i915/i915_perf_types.h +++ b/drivers/gpu/drm/i915/i915_perf_types.h @@ -377,6 +377,11 @@ struct i915_oa_ops { * generations. */ u32 (*oa_hw_tail_read)(struct i915_perf_stream *stream); + + /** + * @oa_hw_head_read: read the OA head pointer register + */ + u32 (*oa_hw_head_read)(struct i915_perf_stream *stream); }; struct i915_perf { diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 00546062e023..2042f6339182 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -2048,6 +2048,38 @@ struct drm_i915_perf_open_param { */ #define I915_PERF_IOCTL_CONFIG _IO('i', 0x2) +/** + * Returns OA buffer properties to be used with mmap. + * + * This ioctl is available in perf revision 8. + */ +#define I915_PERF_IOCTL_GET_OA_BUFFER_INFO _IO('i', 0x3) + +/** + * OA buffer size and offset. + */ +struct drm_i915_perf_oa_buffer_info { + __u32 size; + __u32 offset; + __u64 reserved[4]; +}; + +/** + * Returns current position of OA buffer head and tail. + * + * This ioctl is available in perf revision 8. + */ +#define I915_PERF_IOCTL_GET_OA_BUFFER_HEAD_TAIL _IO('i', 0x4) + +/** + * OA buffer head and tail. + */ +struct drm_i915_perf_oa_buffer_head_tail { + __u32 head; + __u32 tail; + __u64 reserved[4]; +}; + /** * Common to all i915 perf records */ -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915/perf: Map OA buffer to user space for gen12 performance query 2020-07-18 0:04 ` [Intel-gfx] [PATCH 4/4] drm/i915/perf: Map OA buffer to user space for gen12 performance query Umesh Nerlige Ramappa @ 2020-07-18 11:44 ` kernel test robot 2020-07-18 11:44 ` kernel test robot 2020-07-20 12:21 ` [Intel-gfx] [PATCH 4/4] drm/i915/perf: Map OA buffer to user space for gen12 performance query Chris Wilson 2 siblings, 0 replies; 31+ messages in thread From: kernel test robot @ 2020-07-18 11:44 UTC (permalink / raw) To: Umesh Nerlige Ramappa, intel-gfx; +Cc: kbuild-all [-- Attachment #1: Type: text/plain, Size: 1374 bytes --] Hi Umesh, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on drm-tip/drm-tip next-20200717] [cannot apply to linus/master v5.8-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Umesh-Nerlige-Ramappa/Allow-privileged-user-to-map-the-OA-buffer/20200718-080929 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: i386-randconfig-s002-20200717 (attached as .config) compiler: gcc-9 (Debian 9.3.0-14) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-49-g707c5017-dirty # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> sparse warnings: (new ones prefixed by >>) >> drivers/gpu/drm/i915/i915_perf.c:3426:5: sparse: sparse: symbol 'i915_perf_mmap' was not declared. Should it be static? Please review and possibly fold the followup patch. --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 30807 bytes --] [-- Attachment #3: Type: text/plain, Size: 160 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915/perf: Map OA buffer to user space for gen12 performance query @ 2020-07-18 11:44 ` kernel test robot 0 siblings, 0 replies; 31+ messages in thread From: kernel test robot @ 2020-07-18 11:44 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 1410 bytes --] Hi Umesh, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on drm-tip/drm-tip next-20200717] [cannot apply to linus/master v5.8-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Umesh-Nerlige-Ramappa/Allow-privileged-user-to-map-the-OA-buffer/20200718-080929 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: i386-randconfig-s002-20200717 (attached as .config) compiler: gcc-9 (Debian 9.3.0-14) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-49-g707c5017-dirty # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> sparse warnings: (new ones prefixed by >>) >> drivers/gpu/drm/i915/i915_perf.c:3426:5: sparse: sparse: symbol 'i915_perf_mmap' was not declared. Should it be static? Please review and possibly fold the followup patch. --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 30807 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [RFC PATCH] drm/i915/perf: i915_perf_mmap() can be static 2020-07-18 0:04 ` [Intel-gfx] [PATCH 4/4] drm/i915/perf: Map OA buffer to user space for gen12 performance query Umesh Nerlige Ramappa @ 2020-07-18 11:44 ` kernel test robot 2020-07-18 11:44 ` kernel test robot 2020-07-20 12:21 ` [Intel-gfx] [PATCH 4/4] drm/i915/perf: Map OA buffer to user space for gen12 performance query Chris Wilson 2 siblings, 0 replies; 31+ messages in thread From: kernel test robot @ 2020-07-18 11:44 UTC (permalink / raw) To: Umesh Nerlige Ramappa, intel-gfx; +Cc: kbuild-all Signed-off-by: kernel test robot <lkp@intel.com> --- i915_perf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index e77582761a642..90535e38b91b9 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -3423,7 +3423,7 @@ static const struct vm_operations_struct vm_ops_oa = { .fault = vm_fault_oa, }; -int i915_perf_mmap(struct file *file, struct vm_area_struct *vma) +static int i915_perf_mmap(struct file *file, struct vm_area_struct *vma) { struct i915_perf_stream *stream = file->private_data; int len; _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [RFC PATCH] drm/i915/perf: i915_perf_mmap() can be static @ 2020-07-18 11:44 ` kernel test robot 0 siblings, 0 replies; 31+ messages in thread From: kernel test robot @ 2020-07-18 11:44 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 661 bytes --] Signed-off-by: kernel test robot <lkp@intel.com> --- i915_perf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index e77582761a642..90535e38b91b9 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -3423,7 +3423,7 @@ static const struct vm_operations_struct vm_ops_oa = { .fault = vm_fault_oa, }; -int i915_perf_mmap(struct file *file, struct vm_area_struct *vma) +static int i915_perf_mmap(struct file *file, struct vm_area_struct *vma) { struct i915_perf_stream *stream = file->private_data; int len; ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915/perf: Map OA buffer to user space for gen12 performance query 2020-07-18 0:04 ` [Intel-gfx] [PATCH 4/4] drm/i915/perf: Map OA buffer to user space for gen12 performance query Umesh Nerlige Ramappa 2020-07-18 11:44 ` kernel test robot 2020-07-18 11:44 ` kernel test robot @ 2020-07-20 12:21 ` Chris Wilson 2 siblings, 0 replies; 31+ messages in thread From: Chris Wilson @ 2020-07-20 12:21 UTC (permalink / raw) To: Umesh Nerlige Ramappa, intel-gfx Quoting Umesh Nerlige Ramappa (2020-07-18 01:04:37) > +static vm_fault_t vm_fault_oa(struct vm_fault *vmf) > +{ > + struct vm_area_struct *vma = vmf->vma; > + struct i915_perf_stream *stream = vma->vm_private_data; > + struct drm_i915_gem_object *obj = stream->oa_buffer.vma->obj; > + int err; > + > + err = i915_gem_object_pin_pages(obj); > + if (err) > + goto out; > + > + err = remap_io_sg(vma, > + vma->vm_start, vma->vm_end - vma->vm_start, > + obj->mm.pages->sgl, -1); > + > + i915_gem_object_unpin_pages(obj); Ok, basics look good (will handle any vma for which we have valid DMA addresses, and since we control the construction of the vma we know we can limit it to objects that work via CPU PTE). There is just one small catch we have to be wary off -- the CPU pte are not holding a reference to the pages themselves, and so we need to refault if we have to evict. However! The oa_buffer is perma-pinned so the pages cannot just disappear, but the mmap itself may outlive the perf-fd. So we need some way to determine that the vm_fault_oa() is called on an old stream (e..g if (stream->closed) return VM_FAULT_SIGBUS;), and during stream close to call unmap_mmaping_range(perf_file->f_mapping, 0, -1); Since the mmap may live longer than expected, the vm_ops should on open() take a reference to the stream, and on close() release that reference. Now would be a good time to mmap and close the perf-fd in an igt and verify the mmap is no longer accessible after the close. (To add complications, you can fork after opening the mmap(perf-fd).) > +out: > + return i915_error_to_vmf_fault(err); > +} > + > +static const struct vm_operations_struct vm_ops_oa = { > + .fault = vm_fault_oa, > +}; > + > +int i915_perf_mmap(struct file *file, struct vm_area_struct *vma) > +{ > + struct i915_perf_stream *stream = file->private_data; > + int len; > + > + if (!IS_ALIGNED(vma->vm_start, PAGE_SIZE)) > + return -EINVAL; > + > + if (vma->vm_end < vma->vm_start) > + return -EINVAL; These strike me as being redundant. > + len = vma->vm_end - vma->vm_start; > + if (!IS_ALIGNED(len, PAGE_SIZE) || len > OA_BUFFER_SIZE) > + return -EINVAL; > + > + if (vma->vm_flags & VM_WRITE) > + return -EINVAL; > + > + if (vma->vm_pgoff != I915_PERF_OA_BUFFER_MMAP_OFFSET) > + return -EINVAL; I'd throw this is into a switch and do it earlier, so the len and writable checks can be based on the oa_vma. (Having a switch makes it clearer that this doesn't have to be a one-off.) -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Allow privileged user to map the OA buffer (rev2) 2020-07-18 0:04 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa ` (3 preceding siblings ...) 2020-07-18 0:04 ` [Intel-gfx] [PATCH 4/4] drm/i915/perf: Map OA buffer to user space for gen12 performance query Umesh Nerlige Ramappa @ 2020-07-18 0:29 ` Patchwork 2020-07-18 0:30 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork 2020-07-18 0:52 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork 6 siblings, 0 replies; 31+ messages in thread From: Patchwork @ 2020-07-18 0:29 UTC (permalink / raw) To: Umesh Nerlige Ramappa; +Cc: intel-gfx == Series Details == Series: Allow privileged user to map the OA buffer (rev2) URL : https://patchwork.freedesktop.org/series/79460/ State : warning == Summary == $ dim checkpatch origin/drm-tip 24740597e5f9 drm/i915/perf: Ensure observation logic is not clock gated 216ee54f6578 drm/i915/perf: Whitelist OA report trigger registers 829409416d4b drm/i915/perf: Whitelist OA counter and buffer registers dac9a1608e77 drm/i915/perf: Map OA buffer to user space for gen12 performance query -:47: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #47: - Pass non-zero offset in mmap to enforce the right object is mapped (Chris) total: 0 errors, 1 warnings, 0 checks, 281 lines checked _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Allow privileged user to map the OA buffer (rev2) 2020-07-18 0:04 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa ` (4 preceding siblings ...) 2020-07-18 0:29 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Allow privileged user to map the OA buffer (rev2) Patchwork @ 2020-07-18 0:30 ` Patchwork 2020-07-18 0:52 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork 6 siblings, 0 replies; 31+ messages in thread From: Patchwork @ 2020-07-18 0:30 UTC (permalink / raw) To: Umesh Nerlige Ramappa; +Cc: intel-gfx == Series Details == Series: Allow privileged user to map the OA buffer (rev2) URL : https://patchwork.freedesktop.org/series/79460/ State : warning == Summary == $ dim sparse --fast origin/drm-tip Sparse version: v0.6.0 Fast mode used, each commit won't be checked separately. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for Allow privileged user to map the OA buffer (rev2) 2020-07-18 0:04 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa ` (5 preceding siblings ...) 2020-07-18 0:30 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork @ 2020-07-18 0:52 ` Patchwork 6 siblings, 0 replies; 31+ messages in thread From: Patchwork @ 2020-07-18 0:52 UTC (permalink / raw) To: Umesh Nerlige Ramappa; +Cc: intel-gfx [-- Attachment #1.1: Type: text/plain, Size: 15235 bytes --] == Series Details == Series: Allow privileged user to map the OA buffer (rev2) URL : https://patchwork.freedesktop.org/series/79460/ State : failure == Summary == CI Bug Log - changes from CI_DRM_8761 -> Patchwork_18209 ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with Patchwork_18209 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_18209, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/index.html Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_18209: ### IGT changes ### #### Possible regressions #### * igt@i915_selftest@live@workarounds: - fi-tgl-u2: [PASS][1] -> [DMESG-FAIL][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-tgl-u2/igt@i915_selftest@live@workarounds.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-tgl-u2/igt@i915_selftest@live@workarounds.html - fi-skl-6700k2: [PASS][3] -> [DMESG-FAIL][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-skl-6700k2/igt@i915_selftest@live@workarounds.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-skl-6700k2/igt@i915_selftest@live@workarounds.html - fi-cml-s: [PASS][5] -> [DMESG-FAIL][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-cml-s/igt@i915_selftest@live@workarounds.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-cml-s/igt@i915_selftest@live@workarounds.html - fi-icl-y: [PASS][7] -> [DMESG-FAIL][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-icl-y/igt@i915_selftest@live@workarounds.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-icl-y/igt@i915_selftest@live@workarounds.html - fi-kbl-x1275: [PASS][9] -> [DMESG-FAIL][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-kbl-x1275/igt@i915_selftest@live@workarounds.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-kbl-x1275/igt@i915_selftest@live@workarounds.html - fi-cfl-guc: [PASS][11] -> [DMESG-FAIL][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-cfl-guc/igt@i915_selftest@live@workarounds.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-cfl-guc/igt@i915_selftest@live@workarounds.html - fi-tgl-y: [PASS][13] -> [DMESG-FAIL][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-tgl-y/igt@i915_selftest@live@workarounds.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-tgl-y/igt@i915_selftest@live@workarounds.html - fi-skl-guc: [PASS][15] -> [DMESG-FAIL][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-skl-guc/igt@i915_selftest@live@workarounds.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-skl-guc/igt@i915_selftest@live@workarounds.html - fi-skl-6600u: [PASS][17] -> [DMESG-FAIL][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-skl-6600u/igt@i915_selftest@live@workarounds.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-skl-6600u/igt@i915_selftest@live@workarounds.html - fi-kbl-8809g: [PASS][19] -> [DMESG-FAIL][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-kbl-8809g/igt@i915_selftest@live@workarounds.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-kbl-8809g/igt@i915_selftest@live@workarounds.html - fi-cfl-8700k: [PASS][21] -> [DMESG-FAIL][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-cfl-8700k/igt@i915_selftest@live@workarounds.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-cfl-8700k/igt@i915_selftest@live@workarounds.html - fi-kbl-r: [PASS][23] -> [DMESG-FAIL][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-kbl-r/igt@i915_selftest@live@workarounds.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-kbl-r/igt@i915_selftest@live@workarounds.html - fi-icl-u2: [PASS][25] -> [DMESG-FAIL][26] [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-icl-u2/igt@i915_selftest@live@workarounds.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-icl-u2/igt@i915_selftest@live@workarounds.html - fi-cfl-8109u: [PASS][27] -> [DMESG-FAIL][28] [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-cfl-8109u/igt@i915_selftest@live@workarounds.html [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-cfl-8109u/igt@i915_selftest@live@workarounds.html - fi-skl-lmem: [PASS][29] -> [DMESG-FAIL][30] [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-skl-lmem/igt@i915_selftest@live@workarounds.html [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-skl-lmem/igt@i915_selftest@live@workarounds.html - fi-kbl-7500u: [PASS][31] -> [DMESG-FAIL][32] [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-kbl-7500u/igt@i915_selftest@live@workarounds.html [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-kbl-7500u/igt@i915_selftest@live@workarounds.html - fi-kbl-guc: [PASS][33] -> [DMESG-FAIL][34] [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-kbl-guc/igt@i915_selftest@live@workarounds.html [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-kbl-guc/igt@i915_selftest@live@workarounds.html - fi-kbl-soraka: [PASS][35] -> [DMESG-FAIL][36] [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-kbl-soraka/igt@i915_selftest@live@workarounds.html [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-kbl-soraka/igt@i915_selftest@live@workarounds.html - fi-cml-u2: [PASS][37] -> [DMESG-FAIL][38] [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-cml-u2/igt@i915_selftest@live@workarounds.html [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-cml-u2/igt@i915_selftest@live@workarounds.html #### Suppressed #### The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@i915_selftest@live@workarounds: - {fi-tgl-dsi}: [PASS][39] -> [DMESG-FAIL][40] [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-tgl-dsi/igt@i915_selftest@live@workarounds.html [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-tgl-dsi/igt@i915_selftest@live@workarounds.html - {fi-ehl-1}: [PASS][41] -> [DMESG-FAIL][42] [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-ehl-1/igt@i915_selftest@live@workarounds.html [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-ehl-1/igt@i915_selftest@live@workarounds.html - {fi-kbl-7560u}: [PASS][43] -> [DMESG-FAIL][44] [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-kbl-7560u/igt@i915_selftest@live@workarounds.html [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-kbl-7560u/igt@i915_selftest@live@workarounds.html Known issues ------------ Here are the changes found in Patchwork_18209 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_flink_basic@double-flink: - fi-tgl-y: [PASS][45] -> [DMESG-WARN][46] ([i915#402]) [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-tgl-y/igt@gem_flink_basic@double-flink.html [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-tgl-y/igt@gem_flink_basic@double-flink.html * igt@i915_module_load@reload: - fi-tgl-u2: [PASS][47] -> [DMESG-WARN][48] ([i915#402]) [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-tgl-u2/igt@i915_module_load@reload.html [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-tgl-u2/igt@i915_module_load@reload.html - fi-bsw-n3050: [PASS][49] -> [DMESG-WARN][50] ([i915#1982]) [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-bsw-n3050/igt@i915_module_load@reload.html [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-bsw-n3050/igt@i915_module_load@reload.html * igt@i915_selftest@live@gem_contexts: - fi-tgl-u2: [PASS][51] -> [INCOMPLETE][52] ([i915#2045]) [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-tgl-u2/igt@i915_selftest@live@gem_contexts.html [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-tgl-u2/igt@i915_selftest@live@gem_contexts.html * igt@i915_selftest@live@workarounds: - fi-apl-guc: [PASS][53] -> [DMESG-FAIL][54] ([i915#1635]) [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-apl-guc/igt@i915_selftest@live@workarounds.html [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-apl-guc/igt@i915_selftest@live@workarounds.html - fi-bxt-dsi: [PASS][55] -> [DMESG-FAIL][56] ([i915#1635]) [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-bxt-dsi/igt@i915_selftest@live@workarounds.html [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-bxt-dsi/igt@i915_selftest@live@workarounds.html * igt@kms_busy@basic@flip: - fi-tgl-y: [PASS][57] -> [DMESG-WARN][58] ([i915#1982]) [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-tgl-y/igt@kms_busy@basic@flip.html [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-tgl-y/igt@kms_busy@basic@flip.html #### Possible fixes #### * igt@gem_render_linear_blits@basic: - fi-tgl-y: [DMESG-WARN][59] ([i915#402]) -> [PASS][60] +1 similar issue [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-tgl-y/igt@gem_render_linear_blits@basic.html [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-tgl-y/igt@gem_render_linear_blits@basic.html * igt@i915_pm_rpm@basic-pci-d3-state: - fi-bsw-kefka: [DMESG-WARN][61] ([i915#1982]) -> [PASS][62] [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html * igt@i915_pm_rpm@module-reload: - fi-kbl-guc: [FAIL][63] ([i915#138]) -> [PASS][64] [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html * igt@i915_selftest@live@execlists: - fi-cfl-8700k: [INCOMPLETE][65] ([i915#2089]) -> [PASS][66] [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-cfl-8700k/igt@i915_selftest@live@execlists.html [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-cfl-8700k/igt@i915_selftest@live@execlists.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy: - fi-icl-u2: [DMESG-WARN][67] ([i915#1982]) -> [PASS][68] +1 similar issue [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence: - fi-tgl-u2: [DMESG-WARN][69] ([i915#402]) -> [PASS][70] [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-tgl-u2/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-tgl-u2/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html #### Warnings #### * igt@gem_exec_suspend@basic-s3: - fi-kbl-x1275: [DMESG-WARN][71] ([i915#62] / [i915#92]) -> [DMESG-WARN][72] ([i915#1982] / [i915#62] / [i915#92] / [i915#95]) [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-kbl-x1275/igt@gem_exec_suspend@basic-s3.html [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-kbl-x1275/igt@gem_exec_suspend@basic-s3.html * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size: - fi-kbl-x1275: [DMESG-WARN][73] ([i915#62] / [i915#92]) -> [DMESG-WARN][74] ([i915#62] / [i915#92] / [i915#95]) +2 similar issues [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size.html [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size.html * igt@kms_force_connector_basic@force-edid: - fi-kbl-x1275: [DMESG-WARN][75] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][76] ([i915#62] / [i915#92]) +3 similar issues [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8761/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [i915#138]: https://gitlab.freedesktop.org/drm/intel/issues/138 [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635 [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#2045]: https://gitlab.freedesktop.org/drm/intel/issues/2045 [i915#2089]: https://gitlab.freedesktop.org/drm/intel/issues/2089 [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402 [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62 [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92 [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95 Participating hosts (47 -> 40) ------------------------------ Missing (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus Build changes ------------- * IGT: IGT_5739 -> IGTPW_4776 * Linux: CI_DRM_8761 -> Patchwork_18209 CI-20190529: 20190529 CI_DRM_8761: b665aabc40b8c7e86f10a74171d3d3fd71251781 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_4776: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4776/index.html IGT_5739: 9b964d7359db9799f2b5b905403dda668ae28c87 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_18209: dac9a1608e77996c41690a5972dfc0b8122130e1 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == dac9a1608e77 drm/i915/perf: Map OA buffer to user space for gen12 performance query 829409416d4b drm/i915/perf: Whitelist OA counter and buffer registers 216ee54f6578 drm/i915/perf: Whitelist OA report trigger registers 24740597e5f9 drm/i915/perf: Ensure observation logic is not clock gated == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18209/index.html [-- Attachment #1.2: Type: text/html, Size: 17937 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer @ 2020-07-21 2:00 Umesh Nerlige Ramappa 2020-07-21 2:00 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 0 siblings, 1 reply; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-21 2:00 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson This cover letter is included to trigger "Test-with" an IGT patch. Tests - https://patchwork.freedesktop.org/series/79695/ Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Test-with: 20200721015717.46279-1-umesh.nerlige.ramappa@intel.com Piotr Maciejewski (4): drm/i915/perf: Ensure observation logic is not clock gated drm/i915/perf: Whitelist OA report trigger registers drm/i915/perf: Whitelist OA counter and buffer registers drm/i915/perf: Map OA buffer to user space for gen12 performance query drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 + drivers/gpu/drm/i915/gt/intel_workarounds.c | 60 +++++ .../gpu/drm/i915/gt/selftest_workarounds.c | 25 +- drivers/gpu/drm/i915/i915_perf.c | 221 +++++++++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 17 ++ drivers/gpu/drm/i915/i915_reg.h | 12 + include/uapi/drm/i915_drm.h | 32 +++ 8 files changed, 364 insertions(+), 7 deletions(-) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-21 2:00 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa @ 2020-07-21 2:00 ` Umesh Nerlige Ramappa 0 siblings, 0 replies; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-21 2:00 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson From: Piotr Maciejewski <piotr.maciejewski@intel.com> OA reports can be triggered into the OA buffer by writing into the OAREPORTTRIG registers. Whitelist the registers to allow user to trigger reports. v2: - Move related change to this patch (Lionel) - Bump up perf revision (Lionel) v3: Pardon whitelisted registers for selftest (Umesh) v4: Document supported gens for the feature (Lionel) Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 26 +++++++++++++++++++ .../gpu/drm/i915/gt/selftest_workarounds.c | 4 +++ drivers/gpu/drm/i915/i915_perf.c | 11 +++++--- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 5726cd0a37e0..582a2c8cd219 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -1365,6 +1365,20 @@ whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg) whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_ACCESS_RW); } +static void gen9_whitelist_build_performance_counters(struct i915_wa_list *w) +{ + /* OA buffer trigger report 2/6 used by performance query */ + whitelist_reg(w, OAREPORTTRIG2); + whitelist_reg(w, OAREPORTTRIG6); +} + +static void gen12_whitelist_build_performance_counters(struct i915_wa_list *w) +{ + /* OA buffer trigger report 2/6 used by performance query */ + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG2); + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG6); +} + static void gen9_whitelist_build(struct i915_wa_list *w) { /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ @@ -1378,6 +1392,9 @@ static void gen9_whitelist_build(struct i915_wa_list *w) /* WaSendPushConstantsFromMMIO:skl,bxt */ whitelist_reg(w, COMMON_SLICE_CHICKEN2); + + /* Performance counters support */ + gen9_whitelist_build_performance_counters(w); } static void skl_whitelist_build(struct intel_engine_cs *engine) @@ -1471,6 +1488,9 @@ static void cnl_whitelist_build(struct intel_engine_cs *engine) /* WaEnablePreemptionGranularityControlByUMD:cnl */ whitelist_reg(w, GEN8_CS_CHICKEN1); + + /* Performance counters support */ + gen9_whitelist_build_performance_counters(w); } static void icl_whitelist_build(struct intel_engine_cs *engine) @@ -1500,6 +1520,9 @@ static void icl_whitelist_build(struct intel_engine_cs *engine) whitelist_reg_ext(w, PS_INVOCATION_COUNT, RING_FORCE_TO_NONPRIV_ACCESS_RD | RING_FORCE_TO_NONPRIV_RANGE_4); + + /* Performance counters support */ + gen9_whitelist_build_performance_counters(w); break; case VIDEO_DECODE_CLASS: @@ -1550,6 +1573,9 @@ static void tgl_whitelist_build(struct intel_engine_cs *engine) /* Wa_1806527549:tgl */ whitelist_reg(w, HIZ_CHICKEN); + + /* Performance counters support */ + gen12_whitelist_build_performance_counters(w); break; default: whitelist_reg_ext(w, diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c index febc9e6692ba..c7d8af9ee34a 100644 --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c @@ -934,6 +934,10 @@ static bool pardon_reg(struct drm_i915_private *i915, i915_reg_t reg) static const struct regmask pardon[] = { { GEN9_CTX_PREEMPT_REG, INTEL_GEN_MASK(9, 9) }, { GEN8_L3SQCREG4, INTEL_GEN_MASK(9, 9) }, + { OAREPORTTRIG2, INTEL_GEN_MASK(8, 11) }, + { OAREPORTTRIG6, INTEL_GEN_MASK(8, 11) }, + { GEN12_OAG_OAREPORTTRIG2, INTEL_GEN_MASK(12, 12) }, + { GEN12_OAG_OAREPORTTRIG6, INTEL_GEN_MASK(12, 12) }, }; return find_reg(i915, reg, pardon, ARRAY_SIZE(pardon)); diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index fe408c327d3c..30f6aeb819aa 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1448,7 +1448,8 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN8_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK); /* Mark that we need updated tail pointers to read from... */ @@ -1501,7 +1502,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN12_OAG_OATAILPTR, gtt_offset & GEN12_OAG_OATAILPTR_MASK); @@ -4445,8 +4447,11 @@ int i915_perf_ioctl_version(void) * * 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the * interval for the hrtimer used to check for OA data. + * + * 6: Whitelist OATRIGGER registers to allow user to trigger reports + * into the OA buffer. This applies only to gen8+. */ - return 5; + return 6; } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer @ 2020-07-22 5:55 Umesh Nerlige Ramappa 2020-07-22 5:55 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 0 siblings, 1 reply; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-22 5:55 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson This cover letter is included to trigger "Test-with" an IGT patch. Tests - https://patchwork.freedesktop.org/series/79745/ Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Test-with: 20200722053805.57139-1-umesh.nerlige.ramappa@intel.com Piotr Maciejewski (4): drm/i915/perf: Ensure observation logic is not clock gated drm/i915/perf: Whitelist OA report trigger registers drm/i915/perf: Whitelist OA counter and buffer registers drm/i915/perf: Map OA buffer to user space for gen12 performance query drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 + drivers/gpu/drm/i915/gt/intel_workarounds.c | 60 +++++ .../gpu/drm/i915/gt/selftest_workarounds.c | 38 ++- drivers/gpu/drm/i915/i915_perf.c | 239 +++++++++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 17 ++ drivers/gpu/drm/i915/i915_reg.h | 12 + include/uapi/drm/i915_drm.h | 32 +++ 8 files changed, 396 insertions(+), 6 deletions(-) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-22 5:55 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa @ 2020-07-22 5:55 ` Umesh Nerlige Ramappa 0 siblings, 0 replies; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-22 5:55 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson From: Piotr Maciejewski <piotr.maciejewski@intel.com> OA reports can be triggered into the OA buffer by writing into the OAREPORTTRIG registers. Whitelist the registers to allow user to trigger reports. v2: - Move related change to this patch (Lionel) - Bump up perf revision (Lionel) v3: Pardon whitelisted registers for selftest (Umesh) v4: Document supported gens for the feature (Lionel) Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 26 +++++++++++++++++++ .../gpu/drm/i915/gt/selftest_workarounds.c | 8 ++++++ drivers/gpu/drm/i915/i915_perf.c | 11 +++++--- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index cef1c122696f..a72ebfd115e5 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -1387,6 +1387,20 @@ whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg) whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_ACCESS_RW); } +static void gen9_whitelist_build_performance_counters(struct i915_wa_list *w) +{ + /* OA buffer trigger report 2/6 used by performance query */ + whitelist_reg(w, OAREPORTTRIG2); + whitelist_reg(w, OAREPORTTRIG6); +} + +static void gen12_whitelist_build_performance_counters(struct i915_wa_list *w) +{ + /* OA buffer trigger report 2/6 used by performance query */ + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG2); + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG6); +} + static void gen9_whitelist_build(struct i915_wa_list *w) { /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ @@ -1400,6 +1414,9 @@ static void gen9_whitelist_build(struct i915_wa_list *w) /* WaSendPushConstantsFromMMIO:skl,bxt */ whitelist_reg(w, COMMON_SLICE_CHICKEN2); + + /* Performance counters support */ + gen9_whitelist_build_performance_counters(w); } static void skl_whitelist_build(struct intel_engine_cs *engine) @@ -1493,6 +1510,9 @@ static void cnl_whitelist_build(struct intel_engine_cs *engine) /* WaEnablePreemptionGranularityControlByUMD:cnl */ whitelist_reg(w, GEN8_CS_CHICKEN1); + + /* Performance counters support */ + gen9_whitelist_build_performance_counters(w); } static void icl_whitelist_build(struct intel_engine_cs *engine) @@ -1522,6 +1542,9 @@ static void icl_whitelist_build(struct intel_engine_cs *engine) whitelist_reg_ext(w, PS_INVOCATION_COUNT, RING_FORCE_TO_NONPRIV_ACCESS_RD | RING_FORCE_TO_NONPRIV_RANGE_4); + + /* Performance counters support */ + gen9_whitelist_build_performance_counters(w); break; case VIDEO_DECODE_CLASS: @@ -1572,6 +1595,9 @@ static void tgl_whitelist_build(struct intel_engine_cs *engine) /* Wa_1806527549:tgl */ whitelist_reg(w, HIZ_CHICKEN); + + /* Performance counters support */ + gen12_whitelist_build_performance_counters(w); break; default: whitelist_reg_ext(w, diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c index febc9e6692ba..3b1d3dbcd477 100644 --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c @@ -934,6 +934,10 @@ static bool pardon_reg(struct drm_i915_private *i915, i915_reg_t reg) static const struct regmask pardon[] = { { GEN9_CTX_PREEMPT_REG, INTEL_GEN_MASK(9, 9) }, { GEN8_L3SQCREG4, INTEL_GEN_MASK(9, 9) }, + { OAREPORTTRIG2, INTEL_GEN_MASK(8, 11) }, + { OAREPORTTRIG6, INTEL_GEN_MASK(8, 11) }, + { GEN12_OAG_OAREPORTTRIG2, INTEL_GEN_MASK(12, 12) }, + { GEN12_OAG_OAREPORTTRIG6, INTEL_GEN_MASK(12, 12) }, }; return find_reg(i915, reg, pardon, ARRAY_SIZE(pardon)); @@ -956,6 +960,10 @@ static bool writeonly_reg(struct drm_i915_private *i915, i915_reg_t reg) /* Some registers do not seem to behave and our writes unreadable */ static const struct regmask wo[] = { { GEN9_SLICE_COMMON_ECO_CHICKEN1, INTEL_GEN_MASK(9, 9) }, + { OAREPORTTRIG2, INTEL_GEN_MASK(8, 11) }, + { OAREPORTTRIG6, INTEL_GEN_MASK(8, 11) }, + { GEN12_OAG_OAREPORTTRIG2, INTEL_GEN_MASK(12, 12) }, + { GEN12_OAG_OAREPORTTRIG6, INTEL_GEN_MASK(12, 12) }, }; return find_reg(i915, reg, wo, ARRAY_SIZE(wo)); diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index fe408c327d3c..30f6aeb819aa 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1448,7 +1448,8 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN8_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK); /* Mark that we need updated tail pointers to read from... */ @@ -1501,7 +1502,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN12_OAG_OATAILPTR, gtt_offset & GEN12_OAG_OATAILPTR_MASK); @@ -4445,8 +4447,11 @@ int i915_perf_ioctl_version(void) * * 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the * interval for the hrtimer used to check for OA data. + * + * 6: Whitelist OATRIGGER registers to allow user to trigger reports + * into the OA buffer. This applies only to gen8+. */ - return 5; + return 6; } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer @ 2020-07-24 0:18 Umesh Nerlige Ramappa 2020-07-24 0:18 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 0 siblings, 1 reply; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-24 0:18 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson This cover letter is included to trigger "Test-with" an IGT patch. Tests - https://patchwork.freedesktop.org/series/79829/ Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Test-with: 20200724001521.35201-1-umesh.nerlige.ramappa@intel.com Piotr Maciejewski (4): drm/i915/perf: Ensure observation logic is not clock gated drm/i915/perf: Whitelist OA report trigger registers drm/i915/perf: Whitelist OA counter and buffer registers drm/i915/perf: Map OA buffer to user space for gen12 performance query drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 + drivers/gpu/drm/i915/gt/intel_workarounds.c | 60 +++++ .../gpu/drm/i915/gt/selftest_workarounds.c | 38 ++- drivers/gpu/drm/i915/i915_perf.c | 254 +++++++++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 17 ++ drivers/gpu/drm/i915/i915_reg.h | 12 + include/uapi/drm/i915_drm.h | 32 +++ 8 files changed, 411 insertions(+), 6 deletions(-) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-24 0:18 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa @ 2020-07-24 0:18 ` Umesh Nerlige Ramappa 2020-07-24 9:26 ` Chris Wilson 0 siblings, 1 reply; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-24 0:18 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson From: Piotr Maciejewski <piotr.maciejewski@intel.com> OA reports can be triggered into the OA buffer by writing into the OAREPORTTRIG registers. Whitelist the registers to allow user to trigger reports. v2: - Move related change to this patch (Lionel) - Bump up perf revision (Lionel) v3: Pardon whitelisted registers for selftest (Umesh) v4: Document supported gens for the feature (Lionel) Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 26 +++++++++++++++++++ .../gpu/drm/i915/gt/selftest_workarounds.c | 8 ++++++ drivers/gpu/drm/i915/i915_perf.c | 11 +++++--- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index cef1c122696f..a72ebfd115e5 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -1387,6 +1387,20 @@ whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg) whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_ACCESS_RW); } +static void gen9_whitelist_build_performance_counters(struct i915_wa_list *w) +{ + /* OA buffer trigger report 2/6 used by performance query */ + whitelist_reg(w, OAREPORTTRIG2); + whitelist_reg(w, OAREPORTTRIG6); +} + +static void gen12_whitelist_build_performance_counters(struct i915_wa_list *w) +{ + /* OA buffer trigger report 2/6 used by performance query */ + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG2); + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG6); +} + static void gen9_whitelist_build(struct i915_wa_list *w) { /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ @@ -1400,6 +1414,9 @@ static void gen9_whitelist_build(struct i915_wa_list *w) /* WaSendPushConstantsFromMMIO:skl,bxt */ whitelist_reg(w, COMMON_SLICE_CHICKEN2); + + /* Performance counters support */ + gen9_whitelist_build_performance_counters(w); } static void skl_whitelist_build(struct intel_engine_cs *engine) @@ -1493,6 +1510,9 @@ static void cnl_whitelist_build(struct intel_engine_cs *engine) /* WaEnablePreemptionGranularityControlByUMD:cnl */ whitelist_reg(w, GEN8_CS_CHICKEN1); + + /* Performance counters support */ + gen9_whitelist_build_performance_counters(w); } static void icl_whitelist_build(struct intel_engine_cs *engine) @@ -1522,6 +1542,9 @@ static void icl_whitelist_build(struct intel_engine_cs *engine) whitelist_reg_ext(w, PS_INVOCATION_COUNT, RING_FORCE_TO_NONPRIV_ACCESS_RD | RING_FORCE_TO_NONPRIV_RANGE_4); + + /* Performance counters support */ + gen9_whitelist_build_performance_counters(w); break; case VIDEO_DECODE_CLASS: @@ -1572,6 +1595,9 @@ static void tgl_whitelist_build(struct intel_engine_cs *engine) /* Wa_1806527549:tgl */ whitelist_reg(w, HIZ_CHICKEN); + + /* Performance counters support */ + gen12_whitelist_build_performance_counters(w); break; default: whitelist_reg_ext(w, diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c index febc9e6692ba..3b1d3dbcd477 100644 --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c @@ -934,6 +934,10 @@ static bool pardon_reg(struct drm_i915_private *i915, i915_reg_t reg) static const struct regmask pardon[] = { { GEN9_CTX_PREEMPT_REG, INTEL_GEN_MASK(9, 9) }, { GEN8_L3SQCREG4, INTEL_GEN_MASK(9, 9) }, + { OAREPORTTRIG2, INTEL_GEN_MASK(8, 11) }, + { OAREPORTTRIG6, INTEL_GEN_MASK(8, 11) }, + { GEN12_OAG_OAREPORTTRIG2, INTEL_GEN_MASK(12, 12) }, + { GEN12_OAG_OAREPORTTRIG6, INTEL_GEN_MASK(12, 12) }, }; return find_reg(i915, reg, pardon, ARRAY_SIZE(pardon)); @@ -956,6 +960,10 @@ static bool writeonly_reg(struct drm_i915_private *i915, i915_reg_t reg) /* Some registers do not seem to behave and our writes unreadable */ static const struct regmask wo[] = { { GEN9_SLICE_COMMON_ECO_CHICKEN1, INTEL_GEN_MASK(9, 9) }, + { OAREPORTTRIG2, INTEL_GEN_MASK(8, 11) }, + { OAREPORTTRIG6, INTEL_GEN_MASK(8, 11) }, + { GEN12_OAG_OAREPORTTRIG2, INTEL_GEN_MASK(12, 12) }, + { GEN12_OAG_OAREPORTTRIG6, INTEL_GEN_MASK(12, 12) }, }; return find_reg(i915, reg, wo, ARRAY_SIZE(wo)); diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index fe408c327d3c..30f6aeb819aa 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1448,7 +1448,8 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN8_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK); /* Mark that we need updated tail pointers to read from... */ @@ -1501,7 +1502,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN12_OAG_OATAILPTR, gtt_offset & GEN12_OAG_OATAILPTR_MASK); @@ -4445,8 +4447,11 @@ int i915_perf_ioctl_version(void) * * 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the * interval for the hrtimer used to check for OA data. + * + * 6: Whitelist OATRIGGER registers to allow user to trigger reports + * into the OA buffer. This applies only to gen8+. */ - return 5; + return 6; } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-24 0:18 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa @ 2020-07-24 9:26 ` Chris Wilson 2020-07-24 10:07 ` Lionel Landwerlin 0 siblings, 1 reply; 31+ messages in thread From: Chris Wilson @ 2020-07-24 9:26 UTC (permalink / raw) To: Umesh Nerlige Ramappa, intel-gfx Quoting Umesh Nerlige Ramappa (2020-07-24 01:18:59) > From: Piotr Maciejewski <piotr.maciejewski@intel.com> > > OA reports can be triggered into the OA buffer by writing into the > OAREPORTTRIG registers. Whitelist the registers to allow user to trigger > reports. > > v2: > - Move related change to this patch (Lionel) > - Bump up perf revision (Lionel) > > v3: Pardon whitelisted registers for selftest (Umesh) > v4: Document supported gens for the feature (Lionel) > > Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> > Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> > Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 26 +++++++++++++++++++ > .../gpu/drm/i915/gt/selftest_workarounds.c | 8 ++++++ > drivers/gpu/drm/i915/i915_perf.c | 11 +++++--- > 3 files changed, 42 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index cef1c122696f..a72ebfd115e5 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -1387,6 +1387,20 @@ whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg) > whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_ACCESS_RW); > } > > +static void gen9_whitelist_build_performance_counters(struct i915_wa_list *w) > +{ > + /* OA buffer trigger report 2/6 used by performance query */ > + whitelist_reg(w, OAREPORTTRIG2); > + whitelist_reg(w, OAREPORTTRIG6); The other question is: are you sure these are per-context registers? > +} > + > +static void gen12_whitelist_build_performance_counters(struct i915_wa_list *w) > +{ > + /* OA buffer trigger report 2/6 used by performance query */ > + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG2); > + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG6); > +} > + > static void gen9_whitelist_build(struct i915_wa_list *w) > { > /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ > @@ -1400,6 +1414,9 @@ static void gen9_whitelist_build(struct i915_wa_list *w) > > /* WaSendPushConstantsFromMMIO:skl,bxt */ > whitelist_reg(w, COMMON_SLICE_CHICKEN2); > + > + /* Performance counters support */ > + gen9_whitelist_build_performance_counters(w); > } > > static void skl_whitelist_build(struct intel_engine_cs *engine) > @@ -1493,6 +1510,9 @@ static void cnl_whitelist_build(struct intel_engine_cs *engine) > > /* WaEnablePreemptionGranularityControlByUMD:cnl */ > whitelist_reg(w, GEN8_CS_CHICKEN1); > + > + /* Performance counters support */ > + gen9_whitelist_build_performance_counters(w); > } > > static void icl_whitelist_build(struct intel_engine_cs *engine) > @@ -1522,6 +1542,9 @@ static void icl_whitelist_build(struct intel_engine_cs *engine) > whitelist_reg_ext(w, PS_INVOCATION_COUNT, > RING_FORCE_TO_NONPRIV_ACCESS_RD | > RING_FORCE_TO_NONPRIV_RANGE_4); > + > + /* Performance counters support */ > + gen9_whitelist_build_performance_counters(w); > break; > > case VIDEO_DECODE_CLASS: > @@ -1572,6 +1595,9 @@ static void tgl_whitelist_build(struct intel_engine_cs *engine) > > /* Wa_1806527549:tgl */ > whitelist_reg(w, HIZ_CHICKEN); > + > + /* Performance counters support */ > + gen12_whitelist_build_performance_counters(w); > break; > default: > whitelist_reg_ext(w, > diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > index febc9e6692ba..3b1d3dbcd477 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > @@ -934,6 +934,10 @@ static bool pardon_reg(struct drm_i915_private *i915, i915_reg_t reg) > static const struct regmask pardon[] = { > { GEN9_CTX_PREEMPT_REG, INTEL_GEN_MASK(9, 9) }, > { GEN8_L3SQCREG4, INTEL_GEN_MASK(9, 9) }, > + { OAREPORTTRIG2, INTEL_GEN_MASK(8, 11) }, > + { OAREPORTTRIG6, INTEL_GEN_MASK(8, 11) }, > + { GEN12_OAG_OAREPORTTRIG2, INTEL_GEN_MASK(12, 12) }, > + { GEN12_OAG_OAREPORTTRIG6, INTEL_GEN_MASK(12, 12) }, Because we are not making the mistake of exposing more globals, and the pardon is a list of our past sins, not an excuse for more. -Chris --------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-24 9:26 ` Chris Wilson @ 2020-07-24 10:07 ` Lionel Landwerlin 2020-07-24 10:19 ` Chris Wilson 0 siblings, 1 reply; 31+ messages in thread From: Lionel Landwerlin @ 2020-07-24 10:07 UTC (permalink / raw) To: Chris Wilson, Umesh Nerlige Ramappa, intel-gfx On 24/07/2020 12:26, Chris Wilson wrote: > Quoting Umesh Nerlige Ramappa (2020-07-24 01:18:59) >> From: Piotr Maciejewski <piotr.maciejewski@intel.com> >> >> OA reports can be triggered into the OA buffer by writing into the >> OAREPORTTRIG registers. Whitelist the registers to allow user to trigger >> reports. >> >> v2: >> - Move related change to this patch (Lionel) >> - Bump up perf revision (Lionel) >> >> v3: Pardon whitelisted registers for selftest (Umesh) >> v4: Document supported gens for the feature (Lionel) >> >> Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> >> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> >> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> --- >> drivers/gpu/drm/i915/gt/intel_workarounds.c | 26 +++++++++++++++++++ >> .../gpu/drm/i915/gt/selftest_workarounds.c | 8 ++++++ >> drivers/gpu/drm/i915/i915_perf.c | 11 +++++--- >> 3 files changed, 42 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c >> index cef1c122696f..a72ebfd115e5 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c >> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c >> @@ -1387,6 +1387,20 @@ whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg) >> whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_ACCESS_RW); >> } >> >> +static void gen9_whitelist_build_performance_counters(struct i915_wa_list *w) >> +{ >> + /* OA buffer trigger report 2/6 used by performance query */ >> + whitelist_reg(w, OAREPORTTRIG2); >> + whitelist_reg(w, OAREPORTTRIG6); > The other question is: are you sure these are per-context registers? All the registers exposed in this series are global. >> +} >> + >> +static void gen12_whitelist_build_performance_counters(struct i915_wa_list *w) >> +{ >> + /* OA buffer trigger report 2/6 used by performance query */ >> + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG2); >> + whitelist_reg(w, GEN12_OAG_OAREPORTTRIG6); >> +} >> + >> static void gen9_whitelist_build(struct i915_wa_list *w) >> { >> /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ >> @@ -1400,6 +1414,9 @@ static void gen9_whitelist_build(struct i915_wa_list *w) >> >> /* WaSendPushConstantsFromMMIO:skl,bxt */ >> whitelist_reg(w, COMMON_SLICE_CHICKEN2); >> + >> + /* Performance counters support */ >> + gen9_whitelist_build_performance_counters(w); >> } >> >> static void skl_whitelist_build(struct intel_engine_cs *engine) >> @@ -1493,6 +1510,9 @@ static void cnl_whitelist_build(struct intel_engine_cs *engine) >> >> /* WaEnablePreemptionGranularityControlByUMD:cnl */ >> whitelist_reg(w, GEN8_CS_CHICKEN1); >> + >> + /* Performance counters support */ >> + gen9_whitelist_build_performance_counters(w); >> } >> >> static void icl_whitelist_build(struct intel_engine_cs *engine) >> @@ -1522,6 +1542,9 @@ static void icl_whitelist_build(struct intel_engine_cs *engine) >> whitelist_reg_ext(w, PS_INVOCATION_COUNT, >> RING_FORCE_TO_NONPRIV_ACCESS_RD | >> RING_FORCE_TO_NONPRIV_RANGE_4); >> + >> + /* Performance counters support */ >> + gen9_whitelist_build_performance_counters(w); >> break; >> >> case VIDEO_DECODE_CLASS: >> @@ -1572,6 +1595,9 @@ static void tgl_whitelist_build(struct intel_engine_cs *engine) >> >> /* Wa_1806527549:tgl */ >> whitelist_reg(w, HIZ_CHICKEN); >> + >> + /* Performance counters support */ >> + gen12_whitelist_build_performance_counters(w); >> break; >> default: >> whitelist_reg_ext(w, >> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c >> index febc9e6692ba..3b1d3dbcd477 100644 >> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c >> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c >> @@ -934,6 +934,10 @@ static bool pardon_reg(struct drm_i915_private *i915, i915_reg_t reg) >> static const struct regmask pardon[] = { >> { GEN9_CTX_PREEMPT_REG, INTEL_GEN_MASK(9, 9) }, >> { GEN8_L3SQCREG4, INTEL_GEN_MASK(9, 9) }, >> + { OAREPORTTRIG2, INTEL_GEN_MASK(8, 11) }, >> + { OAREPORTTRIG6, INTEL_GEN_MASK(8, 11) }, >> + { GEN12_OAG_OAREPORTTRIG2, INTEL_GEN_MASK(12, 12) }, >> + { GEN12_OAG_OAREPORTTRIG6, INTEL_GEN_MASK(12, 12) }, > Because we are not making the mistake of exposing more globals, and the > pardon is a list of our past sins, not an excuse for more. I'm afraid the HW design leave us no choice on Gen12 :( -Lionel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-24 10:07 ` Lionel Landwerlin @ 2020-07-24 10:19 ` Chris Wilson 2020-07-24 10:23 ` Chris Wilson 2020-07-24 10:31 ` Lionel Landwerlin 0 siblings, 2 replies; 31+ messages in thread From: Chris Wilson @ 2020-07-24 10:19 UTC (permalink / raw) To: Lionel Landwerlin, Umesh Nerlige Ramappa, intel-gfx Quoting Lionel Landwerlin (2020-07-24 11:07:18) > On 24/07/2020 12:26, Chris Wilson wrote: > > Quoting Umesh Nerlige Ramappa (2020-07-24 01:18:59) > >> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > >> index febc9e6692ba..3b1d3dbcd477 100644 > >> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c > >> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > >> @@ -934,6 +934,10 @@ static bool pardon_reg(struct drm_i915_private *i915, i915_reg_t reg) > >> static const struct regmask pardon[] = { > >> { GEN9_CTX_PREEMPT_REG, INTEL_GEN_MASK(9, 9) }, > >> { GEN8_L3SQCREG4, INTEL_GEN_MASK(9, 9) }, > >> + { OAREPORTTRIG2, INTEL_GEN_MASK(8, 11) }, > >> + { OAREPORTTRIG6, INTEL_GEN_MASK(8, 11) }, > >> + { GEN12_OAG_OAREPORTTRIG2, INTEL_GEN_MASK(12, 12) }, > >> + { GEN12_OAG_OAREPORTTRIG6, INTEL_GEN_MASK(12, 12) }, > > Because we are not making the mistake of exposing more globals, and the > > pardon is a list of our past sins, not an excuse for more. > > I'm afraid the HW design leave us no choice on Gen12 :( The question then is how much mischief can a client get up to if they subvert the OA of the privileged user. It's a privilege escalation hole, but is there anything dangerous behind it? Or is it just going to disturb the data being fed to the privileged client... (Which seems scary enough.) -Chris --------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-24 10:19 ` Chris Wilson @ 2020-07-24 10:23 ` Chris Wilson 2020-07-24 10:33 ` Lionel Landwerlin 2020-07-24 10:31 ` Lionel Landwerlin 1 sibling, 1 reply; 31+ messages in thread From: Chris Wilson @ 2020-07-24 10:23 UTC (permalink / raw) To: Lionel Landwerlin, Umesh Nerlige Ramappa, intel-gfx Quoting Chris Wilson (2020-07-24 11:19:05) > Quoting Lionel Landwerlin (2020-07-24 11:07:18) > > On 24/07/2020 12:26, Chris Wilson wrote: > > > Quoting Umesh Nerlige Ramappa (2020-07-24 01:18:59) > > >> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > > >> index febc9e6692ba..3b1d3dbcd477 100644 > > >> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c > > >> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > > >> @@ -934,6 +934,10 @@ static bool pardon_reg(struct drm_i915_private *i915, i915_reg_t reg) > > >> static const struct regmask pardon[] = { > > >> { GEN9_CTX_PREEMPT_REG, INTEL_GEN_MASK(9, 9) }, > > >> { GEN8_L3SQCREG4, INTEL_GEN_MASK(9, 9) }, > > >> + { OAREPORTTRIG2, INTEL_GEN_MASK(8, 11) }, > > >> + { OAREPORTTRIG6, INTEL_GEN_MASK(8, 11) }, > > >> + { GEN12_OAG_OAREPORTTRIG2, INTEL_GEN_MASK(12, 12) }, > > >> + { GEN12_OAG_OAREPORTTRIG6, INTEL_GEN_MASK(12, 12) }, > > > Because we are not making the mistake of exposing more globals, and the > > > pardon is a list of our past sins, not an excuse for more. > > > > I'm afraid the HW design leave us no choice on Gen12 :( > > The question then is how much mischief can a client get up to if they > subvert the OA of the privileged user. It's a privilege escalation hole, > but is there anything dangerous behind it? Or is it just going to disturb > the data being fed to the privileged client... (Which seems scary enough.) The other angle is whether the RING_NONPRIV are context saved; I have vague memories of seeing them in the context image. If they are, we can add them the to privileged OA client. We still need to respect the hard limit on the number, but that would close the hole. -Chris --------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-24 10:23 ` Chris Wilson @ 2020-07-24 10:33 ` Lionel Landwerlin 2020-07-24 11:27 ` Chris Wilson 0 siblings, 1 reply; 31+ messages in thread From: Lionel Landwerlin @ 2020-07-24 10:33 UTC (permalink / raw) To: Chris Wilson, Umesh Nerlige Ramappa, intel-gfx On 24/07/2020 13:23, Chris Wilson wrote: > Quoting Chris Wilson (2020-07-24 11:19:05) >> Quoting Lionel Landwerlin (2020-07-24 11:07:18) >>> On 24/07/2020 12:26, Chris Wilson wrote: >>>> Quoting Umesh Nerlige Ramappa (2020-07-24 01:18:59) >>>>> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c >>>>> index febc9e6692ba..3b1d3dbcd477 100644 >>>>> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c >>>>> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c >>>>> @@ -934,6 +934,10 @@ static bool pardon_reg(struct drm_i915_private *i915, i915_reg_t reg) >>>>> static const struct regmask pardon[] = { >>>>> { GEN9_CTX_PREEMPT_REG, INTEL_GEN_MASK(9, 9) }, >>>>> { GEN8_L3SQCREG4, INTEL_GEN_MASK(9, 9) }, >>>>> + { OAREPORTTRIG2, INTEL_GEN_MASK(8, 11) }, >>>>> + { OAREPORTTRIG6, INTEL_GEN_MASK(8, 11) }, >>>>> + { GEN12_OAG_OAREPORTTRIG2, INTEL_GEN_MASK(12, 12) }, >>>>> + { GEN12_OAG_OAREPORTTRIG6, INTEL_GEN_MASK(12, 12) }, >>>> Because we are not making the mistake of exposing more globals, and the >>>> pardon is a list of our past sins, not an excuse for more. >>> I'm afraid the HW design leave us no choice on Gen12 :( >> The question then is how much mischief can a client get up to if they >> subvert the OA of the privileged user. It's a privilege escalation hole, >> but is there anything dangerous behind it? Or is it just going to disturb >> the data being fed to the privileged client... (Which seems scary enough.) > The other angle is whether the RING_NONPRIV are context saved; I have > vague memories of seeing them in the context image. If they are, we can > add them the to privileged OA client. We still need to respect the > hard limit on the number, but that would close the hole. > -Chris I couldn't find how to do this whitelisting per context. If that's possible, that's probably what we should do. -Lionel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-24 10:33 ` Lionel Landwerlin @ 2020-07-24 11:27 ` Chris Wilson 0 siblings, 0 replies; 31+ messages in thread From: Chris Wilson @ 2020-07-24 11:27 UTC (permalink / raw) To: Lionel Landwerlin, Umesh Nerlige Ramappa, intel-gfx Quoting Lionel Landwerlin (2020-07-24 11:33:38) > On 24/07/2020 13:23, Chris Wilson wrote: > > Quoting Chris Wilson (2020-07-24 11:19:05) > >> Quoting Lionel Landwerlin (2020-07-24 11:07:18) > >>> On 24/07/2020 12:26, Chris Wilson wrote: > >>>> Quoting Umesh Nerlige Ramappa (2020-07-24 01:18:59) > >>>>> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > >>>>> index febc9e6692ba..3b1d3dbcd477 100644 > >>>>> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c > >>>>> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > >>>>> @@ -934,6 +934,10 @@ static bool pardon_reg(struct drm_i915_private *i915, i915_reg_t reg) > >>>>> static const struct regmask pardon[] = { > >>>>> { GEN9_CTX_PREEMPT_REG, INTEL_GEN_MASK(9, 9) }, > >>>>> { GEN8_L3SQCREG4, INTEL_GEN_MASK(9, 9) }, > >>>>> + { OAREPORTTRIG2, INTEL_GEN_MASK(8, 11) }, > >>>>> + { OAREPORTTRIG6, INTEL_GEN_MASK(8, 11) }, > >>>>> + { GEN12_OAG_OAREPORTTRIG2, INTEL_GEN_MASK(12, 12) }, > >>>>> + { GEN12_OAG_OAREPORTTRIG6, INTEL_GEN_MASK(12, 12) }, > >>>> Because we are not making the mistake of exposing more globals, and the > >>>> pardon is a list of our past sins, not an excuse for more. > >>> I'm afraid the HW design leave us no choice on Gen12 :( > >> The question then is how much mischief can a client get up to if they > >> subvert the OA of the privileged user. It's a privilege escalation hole, > >> but is there anything dangerous behind it? Or is it just going to disturb > >> the data being fed to the privileged client... (Which seems scary enough.) > > The other angle is whether the RING_NONPRIV are context saved; I have > > vague memories of seeing them in the context image. If they are, we can > > add them the to privileged OA client. We still need to respect the > > hard limit on the number, but that would close the hole. > > I couldn't find how to do this whitelisting per context. If that's > possible, that's probably what we should do. I've had do luck in finding them in the context image either. Figment of my imagination, or wishful thinking. -Chris --------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-24 10:19 ` Chris Wilson 2020-07-24 10:23 ` Chris Wilson @ 2020-07-24 10:31 ` Lionel Landwerlin 1 sibling, 0 replies; 31+ messages in thread From: Lionel Landwerlin @ 2020-07-24 10:31 UTC (permalink / raw) To: Chris Wilson, Umesh Nerlige Ramappa, intel-gfx On 24/07/2020 13:19, Chris Wilson wrote: > Quoting Lionel Landwerlin (2020-07-24 11:07:18) >> On 24/07/2020 12:26, Chris Wilson wrote: >>> Quoting Umesh Nerlige Ramappa (2020-07-24 01:18:59) >>>> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c >>>> index febc9e6692ba..3b1d3dbcd477 100644 >>>> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c >>>> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c >>>> @@ -934,6 +934,10 @@ static bool pardon_reg(struct drm_i915_private *i915, i915_reg_t reg) >>>> static const struct regmask pardon[] = { >>>> { GEN9_CTX_PREEMPT_REG, INTEL_GEN_MASK(9, 9) }, >>>> { GEN8_L3SQCREG4, INTEL_GEN_MASK(9, 9) }, >>>> + { OAREPORTTRIG2, INTEL_GEN_MASK(8, 11) }, >>>> + { OAREPORTTRIG6, INTEL_GEN_MASK(8, 11) }, >>>> + { GEN12_OAG_OAREPORTTRIG2, INTEL_GEN_MASK(12, 12) }, >>>> + { GEN12_OAG_OAREPORTTRIG6, INTEL_GEN_MASK(12, 12) }, >>> Because we are not making the mistake of exposing more globals, and the >>> pardon is a list of our past sins, not an excuse for more. >> I'm afraid the HW design leave us no choice on Gen12 :( > The question then is how much mischief can a client get up to if they > subvert the OA of the privileged user. It's a privilege escalation hole, > but is there anything dangerous behind it? Or is it just going to disturb > the data being fed to the privileged client... (Which seems scary enough.) > -Chris The trigger registers will allow any other application to generate reports in the global OA buffer. If you've built your OA report parsing correctly, this shouldn't be a problem because you'll filter on context ID prior to the reason of the report being recorded. That's a downside of this patch, that's also a feature because you can use this to monitor particular chunks of work in a global timeline of OA reports for multiple applications. -Lionel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer @ 2020-07-30 23:02 Umesh Nerlige Ramappa 2020-07-30 23:02 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 0 siblings, 1 reply; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-30 23:02 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson This cover letter is included to trigger "Test-with" an IGT patch. Tests - https://patchwork.freedesktop.org/series/80113/ Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Test-with: 20200730230002.55737-1-umesh.nerlige.ramappa@intel.com Piotr Maciejewski (1): drm/i915/perf: Ensure observation logic is not clock gated Umesh Nerlige Ramappa (3): drm/i915/perf: Whitelist OA report trigger registers drm/i915/perf: Whitelist OA counter and buffer registers drm/i915/perf: Map OA buffer to user space for gen12 performance query drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 + drivers/gpu/drm/i915/gt/intel_workarounds.c | 109 +++++++-- drivers/gpu/drm/i915/gt/intel_workarounds.h | 7 + .../gpu/drm/i915/gt/intel_workarounds_types.h | 5 + drivers/gpu/drm/i915/i915_perf.c | 226 +++++++++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 6 + drivers/gpu/drm/i915/i915_reg.h | 10 + include/uapi/drm/i915_drm.h | 18 ++ 9 files changed, 358 insertions(+), 27 deletions(-) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-30 23:02 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa @ 2020-07-30 23:02 ` Umesh Nerlige Ramappa 0 siblings, 0 replies; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-30 23:02 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson OA reports can be triggered into the OA buffer by writing into the OAREPORTTRIG registers. Whitelist the registers to allow non-privileged user to trigger reports. Whitelist registers only if perf_stream_paranoid is set to 0. In i915_perf_open_ioctl, this setting is checked and the whitelist is enabled accordingly. On closing the perf fd, the whitelist is removed. This ensures that the access to the whitelist is gated by perf_stream_paranoid. v2: - Move related change to this patch (Lionel) - Bump up perf revision (Lionel) v3: Pardon whitelisted registers for selftest (Umesh) v4: Document supported gens for the feature (Lionel) v5: Whitelist registers only if perf_stream_paranoid is set to 0 (Jon) v6: Move oa whitelist array to i915_perf (Chris) Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 109 ++++++++++++++---- drivers/gpu/drm/i915/gt/intel_workarounds.h | 7 ++ .../gpu/drm/i915/gt/intel_workarounds_types.h | 5 + drivers/gpu/drm/i915/i915_perf.c | 76 +++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 6 + 5 files changed, 177 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index cef1c122696f..1d337c44d1b0 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -81,10 +81,50 @@ static void wa_init_finish(struct i915_wa_list *wal) wal->wa_count, wal->name, wal->engine_name); } +static int _wa_index(struct i915_wa_list *wal, i915_reg_t reg) +{ + unsigned int addr = i915_mmio_reg_offset(reg); + int start = 0, end = wal->count; + + /* addr and wal->list[].reg, both include the R/W flags */ + while (start < end) { + int mid = start + (end - start) / 2; + + if (i915_mmio_reg_offset(wal->list[mid].reg) < addr) + start = mid + 1; + else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr) + end = mid; + else + return mid; + } + + return -1; +} + +static void _wa_remove(struct i915_wa_list *wal, i915_reg_t reg, u32 flags) +{ + int index; + struct i915_wa *wa = wal->list; + + reg.reg |= flags; + + index = _wa_index(wal, reg); + if (index < 0) + return; + + memset(wa + index, 0, sizeof(*wa)); + + while (index < wal->count - 1) { + swap(wa[index], wa[index + 1]); + index++; + } + + wal->count--; +} + static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) { - unsigned int addr = i915_mmio_reg_offset(wa->reg); - unsigned int start = 0, end = wal->count; + int index; const unsigned int grow = WA_LIST_CHUNK; struct i915_wa *wa_; @@ -106,30 +146,23 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) wal->list = list; } - while (start < end) { - unsigned int mid = start + (end - start) / 2; + index = _wa_index(wal, wa->reg); + if (index >= 0) { + wa_ = &wal->list[index]; - if (i915_mmio_reg_offset(wal->list[mid].reg) < addr) { - start = mid + 1; - } else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr) { - end = mid; - } else { - wa_ = &wal->list[mid]; - - if ((wa->clr | wa_->clr) && !(wa->clr & ~wa_->clr)) { - DRM_ERROR("Discarding overwritten w/a for reg %04x (clear: %08x, set: %08x)\n", - i915_mmio_reg_offset(wa_->reg), - wa_->clr, wa_->set); + if ((wa->clr | wa_->clr) && !(wa->clr & ~wa_->clr)) { + DRM_ERROR("Discarding overwritten w/a for reg %04x (clear: %08x, set: %08x)\n", + i915_mmio_reg_offset(wa_->reg), + wa_->clr, wa_->set); - wa_->set &= ~wa->clr; - } - - wal->wa_count++; - wa_->set |= wa->set; - wa_->clr |= wa->clr; - wa_->read |= wa->read; - return; + wa_->set &= ~wa->clr; } + + wal->wa_count++; + wa_->set |= wa->set; + wa_->clr |= wa->clr; + wa_->read |= wa->read; + return; } wal->wa_count++; @@ -1954,6 +1987,36 @@ void intel_engine_init_workarounds(struct intel_engine_cs *engine) wa_init_finish(wal); } +void intel_engine_allow_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count) +{ + if (!engine || !reg) + return; + + while (count--) { + whitelist_reg_ext(&engine->whitelist, reg->reg, reg->flags); + reg++; + } + + intel_engine_apply_whitelist(engine); +} + +void intel_engine_deny_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count) +{ + if (!engine || !reg) + return; + + while (count--) { + _wa_remove(&engine->whitelist, reg->reg, reg->flags); + reg++; + } + + intel_engine_apply_whitelist(engine); +} + void intel_engine_apply_workarounds(struct intel_engine_cs *engine) { wa_list_apply(engine->uncore, &engine->wa_list); diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h index 8c9c769c2204..6c820c1b751a 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h @@ -37,4 +37,11 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine); int intel_engine_verify_workarounds(struct intel_engine_cs *engine, const char *from); +void intel_engine_allow_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count); +void intel_engine_deny_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count); + #endif diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h index d166a7145720..437981debe14 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h @@ -11,6 +11,11 @@ #include "i915_reg.h" +struct i915_whitelist_reg { + i915_reg_t reg; + u32 flags; +}; + struct i915_wa { i915_reg_t reg; u32 clr; diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index fe408c327d3c..d644cb5118d6 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1347,12 +1347,59 @@ free_noa_wait(struct i915_perf_stream *stream) i915_vma_unpin_and_release(&stream->noa_wait, 0); } +static struct i915_whitelist_reg gen9_oa_wl_regs[] = { + { OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW }, + { OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW }, +}; + +static struct i915_whitelist_reg gen12_oa_wl_regs[] = { + { GEN12_OAG_OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW }, + { GEN12_OAG_OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW }, +}; + +static void intel_engine_apply_oa_whitelist(struct i915_perf_stream *stream) +{ + struct intel_engine_cs *engine = stream->engine; + struct drm_i915_private *i915 = stream->perf->i915; + + if (IS_GEN(i915, 12)) + intel_engine_allow_user_register_access(engine, + gen12_oa_wl_regs, + ARRAY_SIZE(gen12_oa_wl_regs)); + else if (INTEL_GEN(i915) > 8) + intel_engine_allow_user_register_access(engine, + gen9_oa_wl_regs, + ARRAY_SIZE(gen9_oa_wl_regs)); + else + return; +} + +static void intel_engine_remove_oa_whitelist(struct i915_perf_stream *stream) +{ + struct intel_engine_cs *engine = stream->engine; + struct drm_i915_private *i915 = stream->perf->i915; + + if (IS_GEN(i915, 12)) + intel_engine_deny_user_register_access(engine, + gen12_oa_wl_regs, + ARRAY_SIZE(gen12_oa_wl_regs)); + else if (INTEL_GEN(i915) > 8) + intel_engine_deny_user_register_access(engine, + gen9_oa_wl_regs, + ARRAY_SIZE(gen9_oa_wl_regs)); + else + return; +} + static void i915_oa_stream_destroy(struct i915_perf_stream *stream) { struct i915_perf *perf = stream->perf; BUG_ON(stream != perf->exclusive_stream); + if (stream->oa_whitelisted) + intel_engine_remove_oa_whitelist(stream); + /* * Unset exclusive_stream first, it will be checked while disabling * the metric set on gen8+. @@ -1448,7 +1495,8 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN8_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK); /* Mark that we need updated tail pointers to read from... */ @@ -1501,7 +1549,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN12_OAG_OATAILPTR, gtt_offset & GEN12_OAG_OATAILPTR_MASK); @@ -3474,6 +3523,22 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf, if (!(param->flags & I915_PERF_FLAG_DISABLED)) i915_perf_enable_locked(stream); + /* + * OA whitelist allows non-privileged access to some OA counters for + * triggering reports into the OA buffer. This is only allowed if + * perf_stream_paranoid is set to 0 by the sysadmin. + * + * We want to make sure this is almost the last thing we do before + * returning the stream fd. If we do end up checking for errors in code + * that follows this, we MUST call intel_engine_remove_oa_whitelist in + * the error handling path to remove the whitelisted registers. + */ + if (!i915_perf_stream_paranoid && + props->sample_flags & SAMPLE_OA_REPORT) { + intel_engine_apply_oa_whitelist(stream); + stream->oa_whitelisted = true; + } + /* Take a reference on the driver that will be kept with stream_fd * until its release. */ @@ -4445,8 +4510,13 @@ int i915_perf_ioctl_version(void) * * 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the * interval for the hrtimer used to check for OA data. + * + * 6: Whitelist OATRIGGER registers to allow user to trigger reports + * into the OA buffer. This applies only to gen8+. The feature can + * only be accessed if perf_stream_paranoid is set to 0 by privileged + * user. */ - return 5; + return 6; } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h index a36a455ae336..a45ad422b176 100644 --- a/drivers/gpu/drm/i915/i915_perf_types.h +++ b/drivers/gpu/drm/i915/i915_perf_types.h @@ -16,6 +16,7 @@ #include <linux/uuid.h> #include <linux/wait.h> +#include "gt/intel_workarounds.h" #include "gt/intel_sseu.h" #include "i915_reg.h" #include "intel_wakeref.h" @@ -311,6 +312,11 @@ struct i915_perf_stream { * buffer should be checked for available data. */ u64 poll_oa_period; + + /** + * @oa_whitelisted: Indicates that the oa registers are whitelisted. + */ + bool oa_whitelisted; }; /** -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer @ 2020-07-31 6:07 Umesh Nerlige Ramappa 2020-07-31 6:07 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 0 siblings, 1 reply; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-31 6:07 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson Fixes a memory corruption due to addition of OA whitelist on demand. This cover letter is included to trigger "Test-with" an IGT patch. Tests - https://patchwork.freedesktop.org/series/80113/ Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Test-with: 20200730230002.55737-1-umesh.nerlige.ramappa@intel.com Piotr Maciejewski (1): drm/i915/perf: Ensure observation logic is not clock gated Umesh Nerlige Ramappa (3): drm/i915/perf: Whitelist OA report trigger registers drm/i915/perf: Whitelist OA counter and buffer registers drm/i915/perf: Map OA buffer to user space for gen12 performance query drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 + drivers/gpu/drm/i915/gt/intel_workarounds.c | 122 ++++++++-- drivers/gpu/drm/i915/gt/intel_workarounds.h | 7 + .../gpu/drm/i915/gt/intel_workarounds_types.h | 5 + drivers/gpu/drm/i915/i915_perf.c | 226 +++++++++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 6 + drivers/gpu/drm/i915/i915_reg.h | 10 + include/uapi/drm/i915_drm.h | 18 ++ 9 files changed, 370 insertions(+), 28 deletions(-) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-31 6:07 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa @ 2020-07-31 6:07 ` Umesh Nerlige Ramappa 0 siblings, 0 replies; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-31 6:07 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson OA reports can be triggered into the OA buffer by writing into the OAREPORTTRIG registers. Whitelist the registers to allow non-privileged user to trigger reports. Whitelist registers only if perf_stream_paranoid is set to 0. In i915_perf_open_ioctl, this setting is checked and the whitelist is enabled accordingly. On closing the perf fd, the whitelist is removed. This ensures that the access to the whitelist is gated by perf_stream_paranoid. v2: - Move related change to this patch (Lionel) - Bump up perf revision (Lionel) v3: Pardon whitelisted registers for selftest (Umesh) v4: Document supported gens for the feature (Lionel) v5: Whitelist registers only if perf_stream_paranoid is set to 0 (Jon) v6: Move oa whitelist array to i915_perf (Chris) v7: Fix OA writing beyond the wal->list memory (CI) Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 122 ++++++++++++++---- drivers/gpu/drm/i915/gt/intel_workarounds.h | 7 + .../gpu/drm/i915/gt/intel_workarounds_types.h | 5 + drivers/gpu/drm/i915/i915_perf.c | 76 ++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 6 + 5 files changed, 189 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index cef1c122696f..08ddacbf2667 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -60,12 +60,23 @@ static void wa_init_start(struct i915_wa_list *wal, const char *name, const char #define WA_LIST_CHUNK (1 << 4) +/* + * Some of the i915 code like perf OA tries to whitelist registers on demand. + * Such code adds to the wal->list, but that would not work because the list + * is compacted below by wa_init_finish. While _wa_add does have code to grow + * the list, it does not seem to take the compaction into consideration. Leave + * 8 entries free during the compaction until a better mechanism can be put in + * place. + */ +#define WA_LIST_DYNAMIC_ENTRIES 8 + static void wa_init_finish(struct i915_wa_list *wal) { /* Trim unused entries. */ if (!IS_ALIGNED(wal->count, WA_LIST_CHUNK)) { + size_t size = wal->count + WA_LIST_DYNAMIC_ENTRIES; struct i915_wa *list = kmemdup(wal->list, - wal->count * sizeof(*list), + size * sizeof(*list), GFP_KERNEL); if (list) { @@ -81,10 +92,50 @@ static void wa_init_finish(struct i915_wa_list *wal) wal->wa_count, wal->name, wal->engine_name); } +static int _wa_index(struct i915_wa_list *wal, i915_reg_t reg) +{ + unsigned int addr = i915_mmio_reg_offset(reg); + int start = 0, end = wal->count; + + /* addr and wal->list[].reg, both include the R/W flags */ + while (start < end) { + int mid = start + (end - start) / 2; + + if (i915_mmio_reg_offset(wal->list[mid].reg) < addr) + start = mid + 1; + else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr) + end = mid; + else + return mid; + } + + return -1; +} + +static void _wa_remove(struct i915_wa_list *wal, i915_reg_t reg, u32 flags) +{ + int index; + struct i915_wa *wa = wal->list; + + reg.reg |= flags; + + index = _wa_index(wal, reg); + if (index < 0) + return; + + memset(wa + index, 0, sizeof(*wa)); + + while (index < wal->count - 1) { + swap(wa[index], wa[index + 1]); + index++; + } + + wal->count--; +} + static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) { - unsigned int addr = i915_mmio_reg_offset(wa->reg); - unsigned int start = 0, end = wal->count; + int index; const unsigned int grow = WA_LIST_CHUNK; struct i915_wa *wa_; @@ -106,30 +157,23 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) wal->list = list; } - while (start < end) { - unsigned int mid = start + (end - start) / 2; - - if (i915_mmio_reg_offset(wal->list[mid].reg) < addr) { - start = mid + 1; - } else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr) { - end = mid; - } else { - wa_ = &wal->list[mid]; - - if ((wa->clr | wa_->clr) && !(wa->clr & ~wa_->clr)) { - DRM_ERROR("Discarding overwritten w/a for reg %04x (clear: %08x, set: %08x)\n", - i915_mmio_reg_offset(wa_->reg), - wa_->clr, wa_->set); + index = _wa_index(wal, wa->reg); + if (index >= 0) { + wa_ = &wal->list[index]; - wa_->set &= ~wa->clr; - } + if ((wa->clr | wa_->clr) && !(wa->clr & ~wa_->clr)) { + DRM_ERROR("Discarding overwritten w/a for reg %04x (clear: %08x, set: %08x)\n", + i915_mmio_reg_offset(wa_->reg), + wa_->clr, wa_->set); - wal->wa_count++; - wa_->set |= wa->set; - wa_->clr |= wa->clr; - wa_->read |= wa->read; - return; + wa_->set &= ~wa->clr; } + + wal->wa_count++; + wa_->set |= wa->set; + wa_->clr |= wa->clr; + wa_->read |= wa->read; + return; } wal->wa_count++; @@ -1954,6 +1998,36 @@ void intel_engine_init_workarounds(struct intel_engine_cs *engine) wa_init_finish(wal); } +void intel_engine_allow_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count) +{ + if (!engine || !reg) + return; + + while (count--) { + whitelist_reg_ext(&engine->whitelist, reg->reg, reg->flags); + reg++; + } + + intel_engine_apply_whitelist(engine); +} + +void intel_engine_deny_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count) +{ + if (!engine || !reg) + return; + + while (count--) { + _wa_remove(&engine->whitelist, reg->reg, reg->flags); + reg++; + } + + intel_engine_apply_whitelist(engine); +} + void intel_engine_apply_workarounds(struct intel_engine_cs *engine) { wa_list_apply(engine->uncore, &engine->wa_list); diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h index 8c9c769c2204..6c820c1b751a 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h @@ -37,4 +37,11 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine); int intel_engine_verify_workarounds(struct intel_engine_cs *engine, const char *from); +void intel_engine_allow_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count); +void intel_engine_deny_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count); + #endif diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h index d166a7145720..437981debe14 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h @@ -11,6 +11,11 @@ #include "i915_reg.h" +struct i915_whitelist_reg { + i915_reg_t reg; + u32 flags; +}; + struct i915_wa { i915_reg_t reg; u32 clr; diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index fe408c327d3c..d644cb5118d6 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1347,12 +1347,59 @@ free_noa_wait(struct i915_perf_stream *stream) i915_vma_unpin_and_release(&stream->noa_wait, 0); } +static struct i915_whitelist_reg gen9_oa_wl_regs[] = { + { OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW }, + { OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW }, +}; + +static struct i915_whitelist_reg gen12_oa_wl_regs[] = { + { GEN12_OAG_OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW }, + { GEN12_OAG_OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW }, +}; + +static void intel_engine_apply_oa_whitelist(struct i915_perf_stream *stream) +{ + struct intel_engine_cs *engine = stream->engine; + struct drm_i915_private *i915 = stream->perf->i915; + + if (IS_GEN(i915, 12)) + intel_engine_allow_user_register_access(engine, + gen12_oa_wl_regs, + ARRAY_SIZE(gen12_oa_wl_regs)); + else if (INTEL_GEN(i915) > 8) + intel_engine_allow_user_register_access(engine, + gen9_oa_wl_regs, + ARRAY_SIZE(gen9_oa_wl_regs)); + else + return; +} + +static void intel_engine_remove_oa_whitelist(struct i915_perf_stream *stream) +{ + struct intel_engine_cs *engine = stream->engine; + struct drm_i915_private *i915 = stream->perf->i915; + + if (IS_GEN(i915, 12)) + intel_engine_deny_user_register_access(engine, + gen12_oa_wl_regs, + ARRAY_SIZE(gen12_oa_wl_regs)); + else if (INTEL_GEN(i915) > 8) + intel_engine_deny_user_register_access(engine, + gen9_oa_wl_regs, + ARRAY_SIZE(gen9_oa_wl_regs)); + else + return; +} + static void i915_oa_stream_destroy(struct i915_perf_stream *stream) { struct i915_perf *perf = stream->perf; BUG_ON(stream != perf->exclusive_stream); + if (stream->oa_whitelisted) + intel_engine_remove_oa_whitelist(stream); + /* * Unset exclusive_stream first, it will be checked while disabling * the metric set on gen8+. @@ -1448,7 +1495,8 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN8_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK); /* Mark that we need updated tail pointers to read from... */ @@ -1501,7 +1549,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN12_OAG_OATAILPTR, gtt_offset & GEN12_OAG_OATAILPTR_MASK); @@ -3474,6 +3523,22 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf, if (!(param->flags & I915_PERF_FLAG_DISABLED)) i915_perf_enable_locked(stream); + /* + * OA whitelist allows non-privileged access to some OA counters for + * triggering reports into the OA buffer. This is only allowed if + * perf_stream_paranoid is set to 0 by the sysadmin. + * + * We want to make sure this is almost the last thing we do before + * returning the stream fd. If we do end up checking for errors in code + * that follows this, we MUST call intel_engine_remove_oa_whitelist in + * the error handling path to remove the whitelisted registers. + */ + if (!i915_perf_stream_paranoid && + props->sample_flags & SAMPLE_OA_REPORT) { + intel_engine_apply_oa_whitelist(stream); + stream->oa_whitelisted = true; + } + /* Take a reference on the driver that will be kept with stream_fd * until its release. */ @@ -4445,8 +4510,13 @@ int i915_perf_ioctl_version(void) * * 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the * interval for the hrtimer used to check for OA data. + * + * 6: Whitelist OATRIGGER registers to allow user to trigger reports + * into the OA buffer. This applies only to gen8+. The feature can + * only be accessed if perf_stream_paranoid is set to 0 by privileged + * user. */ - return 5; + return 6; } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h index a36a455ae336..a45ad422b176 100644 --- a/drivers/gpu/drm/i915/i915_perf_types.h +++ b/drivers/gpu/drm/i915/i915_perf_types.h @@ -16,6 +16,7 @@ #include <linux/uuid.h> #include <linux/wait.h> +#include "gt/intel_workarounds.h" #include "gt/intel_sseu.h" #include "i915_reg.h" #include "intel_wakeref.h" @@ -311,6 +312,11 @@ struct i915_perf_stream { * buffer should be checked for available data. */ u64 poll_oa_period; + + /** + * @oa_whitelisted: Indicates that the oa registers are whitelisted. + */ + bool oa_whitelisted; }; /** -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer @ 2020-07-31 14:46 Umesh Nerlige Ramappa 2020-07-31 14:46 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 0 siblings, 1 reply; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-31 14:46 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson Fixes a memory corruption due to addition of OA whitelist on demand. This cover letter is included to trigger "Test-with" an IGT patch. Tests - https://patchwork.freedesktop.org/series/80113/ Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Test-with: 20200730230002.55737-1-umesh.nerlige.ramappa@intel.com Piotr Maciejewski (1): drm/i915/perf: Ensure observation logic is not clock gated Umesh Nerlige Ramappa (3): drm/i915/perf: Whitelist OA report trigger registers drm/i915/perf: Whitelist OA counter and buffer registers drm/i915/perf: Map OA buffer to user space for gen12 performance query drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 + drivers/gpu/drm/i915/gt/intel_workarounds.c | 122 ++++++++-- drivers/gpu/drm/i915/gt/intel_workarounds.h | 7 + .../gpu/drm/i915/gt/intel_workarounds_types.h | 5 + drivers/gpu/drm/i915/i915_perf.c | 224 +++++++++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 6 + drivers/gpu/drm/i915/i915_reg.h | 10 + include/uapi/drm/i915_drm.h | 33 +++ 9 files changed, 383 insertions(+), 28 deletions(-) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-31 14:46 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa @ 2020-07-31 14:46 ` Umesh Nerlige Ramappa 0 siblings, 0 replies; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-31 14:46 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson OA reports can be triggered into the OA buffer by writing into the OAREPORTTRIG registers. Whitelist the registers to allow non-privileged user to trigger reports. Whitelist registers only if perf_stream_paranoid is set to 0. In i915_perf_open_ioctl, this setting is checked and the whitelist is enabled accordingly. On closing the perf fd, the whitelist is removed. This ensures that the access to the whitelist is gated by perf_stream_paranoid. v2: - Move related change to this patch (Lionel) - Bump up perf revision (Lionel) v3: Pardon whitelisted registers for selftest (Umesh) v4: Document supported gens for the feature (Lionel) v5: Whitelist registers only if perf_stream_paranoid is set to 0 (Jon) v6: Move oa whitelist array to i915_perf (Chris) v7: Fix OA writing beyond the wal->list memory (CI) Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 122 ++++++++++++++---- drivers/gpu/drm/i915/gt/intel_workarounds.h | 7 + .../gpu/drm/i915/gt/intel_workarounds_types.h | 5 + drivers/gpu/drm/i915/i915_perf.c | 76 ++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 6 + 5 files changed, 189 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index cef1c122696f..08ddacbf2667 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -60,12 +60,23 @@ static void wa_init_start(struct i915_wa_list *wal, const char *name, const char #define WA_LIST_CHUNK (1 << 4) +/* + * Some of the i915 code like perf OA tries to whitelist registers on demand. + * Such code adds to the wal->list, but that would not work because the list + * is compacted below by wa_init_finish. While _wa_add does have code to grow + * the list, it does not seem to take the compaction into consideration. Leave + * 8 entries free during the compaction until a better mechanism can be put in + * place. + */ +#define WA_LIST_DYNAMIC_ENTRIES 8 + static void wa_init_finish(struct i915_wa_list *wal) { /* Trim unused entries. */ if (!IS_ALIGNED(wal->count, WA_LIST_CHUNK)) { + size_t size = wal->count + WA_LIST_DYNAMIC_ENTRIES; struct i915_wa *list = kmemdup(wal->list, - wal->count * sizeof(*list), + size * sizeof(*list), GFP_KERNEL); if (list) { @@ -81,10 +92,50 @@ static void wa_init_finish(struct i915_wa_list *wal) wal->wa_count, wal->name, wal->engine_name); } +static int _wa_index(struct i915_wa_list *wal, i915_reg_t reg) +{ + unsigned int addr = i915_mmio_reg_offset(reg); + int start = 0, end = wal->count; + + /* addr and wal->list[].reg, both include the R/W flags */ + while (start < end) { + int mid = start + (end - start) / 2; + + if (i915_mmio_reg_offset(wal->list[mid].reg) < addr) + start = mid + 1; + else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr) + end = mid; + else + return mid; + } + + return -1; +} + +static void _wa_remove(struct i915_wa_list *wal, i915_reg_t reg, u32 flags) +{ + int index; + struct i915_wa *wa = wal->list; + + reg.reg |= flags; + + index = _wa_index(wal, reg); + if (index < 0) + return; + + memset(wa + index, 0, sizeof(*wa)); + + while (index < wal->count - 1) { + swap(wa[index], wa[index + 1]); + index++; + } + + wal->count--; +} + static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) { - unsigned int addr = i915_mmio_reg_offset(wa->reg); - unsigned int start = 0, end = wal->count; + int index; const unsigned int grow = WA_LIST_CHUNK; struct i915_wa *wa_; @@ -106,30 +157,23 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) wal->list = list; } - while (start < end) { - unsigned int mid = start + (end - start) / 2; - - if (i915_mmio_reg_offset(wal->list[mid].reg) < addr) { - start = mid + 1; - } else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr) { - end = mid; - } else { - wa_ = &wal->list[mid]; - - if ((wa->clr | wa_->clr) && !(wa->clr & ~wa_->clr)) { - DRM_ERROR("Discarding overwritten w/a for reg %04x (clear: %08x, set: %08x)\n", - i915_mmio_reg_offset(wa_->reg), - wa_->clr, wa_->set); + index = _wa_index(wal, wa->reg); + if (index >= 0) { + wa_ = &wal->list[index]; - wa_->set &= ~wa->clr; - } + if ((wa->clr | wa_->clr) && !(wa->clr & ~wa_->clr)) { + DRM_ERROR("Discarding overwritten w/a for reg %04x (clear: %08x, set: %08x)\n", + i915_mmio_reg_offset(wa_->reg), + wa_->clr, wa_->set); - wal->wa_count++; - wa_->set |= wa->set; - wa_->clr |= wa->clr; - wa_->read |= wa->read; - return; + wa_->set &= ~wa->clr; } + + wal->wa_count++; + wa_->set |= wa->set; + wa_->clr |= wa->clr; + wa_->read |= wa->read; + return; } wal->wa_count++; @@ -1954,6 +1998,36 @@ void intel_engine_init_workarounds(struct intel_engine_cs *engine) wa_init_finish(wal); } +void intel_engine_allow_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count) +{ + if (!engine || !reg) + return; + + while (count--) { + whitelist_reg_ext(&engine->whitelist, reg->reg, reg->flags); + reg++; + } + + intel_engine_apply_whitelist(engine); +} + +void intel_engine_deny_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count) +{ + if (!engine || !reg) + return; + + while (count--) { + _wa_remove(&engine->whitelist, reg->reg, reg->flags); + reg++; + } + + intel_engine_apply_whitelist(engine); +} + void intel_engine_apply_workarounds(struct intel_engine_cs *engine) { wa_list_apply(engine->uncore, &engine->wa_list); diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h index 8c9c769c2204..6c820c1b751a 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h @@ -37,4 +37,11 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine); int intel_engine_verify_workarounds(struct intel_engine_cs *engine, const char *from); +void intel_engine_allow_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count); +void intel_engine_deny_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count); + #endif diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h index d166a7145720..437981debe14 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h @@ -11,6 +11,11 @@ #include "i915_reg.h" +struct i915_whitelist_reg { + i915_reg_t reg; + u32 flags; +}; + struct i915_wa { i915_reg_t reg; u32 clr; diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index a43bf4cd337a..7443654ef842 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1347,12 +1347,59 @@ free_noa_wait(struct i915_perf_stream *stream) i915_vma_unpin_and_release(&stream->noa_wait, 0); } +static struct i915_whitelist_reg gen9_oa_wl_regs[] = { + { OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW }, + { OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW }, +}; + +static struct i915_whitelist_reg gen12_oa_wl_regs[] = { + { GEN12_OAG_OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW }, + { GEN12_OAG_OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW }, +}; + +static void intel_engine_apply_oa_whitelist(struct i915_perf_stream *stream) +{ + struct intel_engine_cs *engine = stream->engine; + struct drm_i915_private *i915 = stream->perf->i915; + + if (IS_GEN(i915, 12)) + intel_engine_allow_user_register_access(engine, + gen12_oa_wl_regs, + ARRAY_SIZE(gen12_oa_wl_regs)); + else if (INTEL_GEN(i915) > 8) + intel_engine_allow_user_register_access(engine, + gen9_oa_wl_regs, + ARRAY_SIZE(gen9_oa_wl_regs)); + else + return; +} + +static void intel_engine_remove_oa_whitelist(struct i915_perf_stream *stream) +{ + struct intel_engine_cs *engine = stream->engine; + struct drm_i915_private *i915 = stream->perf->i915; + + if (IS_GEN(i915, 12)) + intel_engine_deny_user_register_access(engine, + gen12_oa_wl_regs, + ARRAY_SIZE(gen12_oa_wl_regs)); + else if (INTEL_GEN(i915) > 8) + intel_engine_deny_user_register_access(engine, + gen9_oa_wl_regs, + ARRAY_SIZE(gen9_oa_wl_regs)); + else + return; +} + static void i915_oa_stream_destroy(struct i915_perf_stream *stream) { struct i915_perf *perf = stream->perf; BUG_ON(stream != perf->exclusive_stream); + if (stream->oa_whitelisted) + intel_engine_remove_oa_whitelist(stream); + /* * Unset exclusive_stream first, it will be checked while disabling * the metric set on gen8+. @@ -1448,7 +1495,8 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN8_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK); /* Mark that we need updated tail pointers to read from... */ @@ -1501,7 +1549,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN12_OAG_OATAILPTR, gtt_offset & GEN12_OAG_OATAILPTR_MASK); @@ -3470,6 +3519,22 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf, if (!(param->flags & I915_PERF_FLAG_DISABLED)) i915_perf_enable_locked(stream); + /* + * OA whitelist allows non-privileged access to some OA counters for + * triggering reports into the OA buffer. This is only allowed if + * perf_stream_paranoid is set to 0 by the sysadmin. + * + * We want to make sure this is almost the last thing we do before + * returning the stream fd. If we do end up checking for errors in code + * that follows this, we MUST call intel_engine_remove_oa_whitelist in + * the error handling path to remove the whitelisted registers. + */ + if (!i915_perf_stream_paranoid && + props->sample_flags & SAMPLE_OA_REPORT) { + intel_engine_apply_oa_whitelist(stream); + stream->oa_whitelisted = true; + } + /* Take a reference on the driver that will be kept with stream_fd * until its release. */ @@ -4441,8 +4506,13 @@ int i915_perf_ioctl_version(void) * * 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the * interval for the hrtimer used to check for OA data. + * + * 6: Whitelist OATRIGGER registers to allow user to trigger reports + * into the OA buffer. This applies only to gen8+. The feature can + * only be accessed if perf_stream_paranoid is set to 0 by privileged + * user. */ - return 5; + return 6; } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h index a36a455ae336..a45ad422b176 100644 --- a/drivers/gpu/drm/i915/i915_perf_types.h +++ b/drivers/gpu/drm/i915/i915_perf_types.h @@ -16,6 +16,7 @@ #include <linux/uuid.h> #include <linux/wait.h> +#include "gt/intel_workarounds.h" #include "gt/intel_sseu.h" #include "i915_reg.h" #include "intel_wakeref.h" @@ -311,6 +312,11 @@ struct i915_perf_stream { * buffer should be checked for available data. */ u64 poll_oa_period; + + /** + * @oa_whitelisted: Indicates that the oa registers are whitelisted. + */ + bool oa_whitelisted; }; /** -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer @ 2020-07-31 23:58 Umesh Nerlige Ramappa 2020-07-31 23:58 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 0 siblings, 1 reply; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-31 23:58 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson - Fixes a memory corruption due to addition of OA whitelist on demand. - Spinlock when applying whitelist This cover letter is included to trigger "Test-with" an IGT patch. Tests - https://patchwork.freedesktop.org/series/80113/ Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Test-with: 20200730230002.55737-1-umesh.nerlige.ramappa@intel.com Piotr Maciejewski (1): drm/i915/perf: Ensure observation logic is not clock gated Umesh Nerlige Ramappa (3): drm/i915/perf: Whitelist OA report trigger registers drm/i915/perf: Whitelist OA counter and buffer registers drm/i915/perf: Map OA buffer to user space for gen12 performance query drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 + drivers/gpu/drm/i915/gt/intel_engine_types.h | 2 + drivers/gpu/drm/i915/gt/intel_workarounds.c | 129 ++++++++-- drivers/gpu/drm/i915/gt/intel_workarounds.h | 7 + .../gpu/drm/i915/gt/intel_workarounds_types.h | 5 + drivers/gpu/drm/i915/i915_perf.c | 224 +++++++++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 6 + drivers/gpu/drm/i915/i915_reg.h | 10 + include/uapi/drm/i915_drm.h | 33 +++ 10 files changed, 392 insertions(+), 28 deletions(-) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-07-31 23:58 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa @ 2020-07-31 23:58 ` Umesh Nerlige Ramappa 0 siblings, 0 replies; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-07-31 23:58 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson OA reports can be triggered into the OA buffer by writing into the OAREPORTTRIG registers. Whitelist the registers to allow non-privileged user to trigger reports. Whitelist registers only if perf_stream_paranoid is set to 0. In i915_perf_open_ioctl, this setting is checked and the whitelist is enabled accordingly. On closing the perf fd, the whitelist is removed. This ensures that the access to the whitelist is gated by perf_stream_paranoid. v2: - Move related change to this patch (Lionel) - Bump up perf revision (Lionel) v3: Pardon whitelisted registers for selftest (Umesh) v4: Document supported gens for the feature (Lionel) v5: Whitelist registers only if perf_stream_paranoid is set to 0 (Jon) v6: Move oa whitelist array to i915_perf (Chris) v7: Fix OA writing beyond the wal->list memory (CI) v8: Protect write to engine whitelist registers Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/gt/intel_engine_types.h | 2 + drivers/gpu/drm/i915/gt/intel_workarounds.c | 129 ++++++++++++++---- drivers/gpu/drm/i915/gt/intel_workarounds.h | 7 + .../gpu/drm/i915/gt/intel_workarounds_types.h | 5 + drivers/gpu/drm/i915/i915_perf.c | 76 ++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 6 + 6 files changed, 198 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index c400aaa2287b..180fc0cdb289 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -406,6 +406,8 @@ struct intel_engine_cs { struct i915_wa_list ctx_wa_list; struct i915_wa_list wa_list; struct i915_wa_list whitelist; + /* lock protects write to engine force_to_nonpriv registers */ + spinlock_t lock; u32 irq_keep_mask; /* always keep these interrupts */ u32 irq_enable_mask; /* bitmask to enable ring interrupt */ diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index cef1c122696f..98520e774481 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -60,12 +60,23 @@ static void wa_init_start(struct i915_wa_list *wal, const char *name, const char #define WA_LIST_CHUNK (1 << 4) +/* + * Some of the i915 code like perf OA tries to whitelist registers on demand. + * Such code adds to the wal->list, but that would not work because the list + * is compacted below by wa_init_finish. While _wa_add does have code to grow + * the list, it does not seem to take the compaction into consideration. Leave + * 8 entries free during the compaction until a better mechanism can be put in + * place. + */ +#define WA_LIST_DYNAMIC_ENTRIES 8 + static void wa_init_finish(struct i915_wa_list *wal) { /* Trim unused entries. */ if (!IS_ALIGNED(wal->count, WA_LIST_CHUNK)) { + size_t size = wal->count + WA_LIST_DYNAMIC_ENTRIES; struct i915_wa *list = kmemdup(wal->list, - wal->count * sizeof(*list), + size * sizeof(*list), GFP_KERNEL); if (list) { @@ -81,10 +92,50 @@ static void wa_init_finish(struct i915_wa_list *wal) wal->wa_count, wal->name, wal->engine_name); } +static int _wa_index(struct i915_wa_list *wal, i915_reg_t reg) +{ + unsigned int addr = i915_mmio_reg_offset(reg); + int start = 0, end = wal->count; + + /* addr and wal->list[].reg, both include the R/W flags */ + while (start < end) { + int mid = start + (end - start) / 2; + + if (i915_mmio_reg_offset(wal->list[mid].reg) < addr) + start = mid + 1; + else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr) + end = mid; + else + return mid; + } + + return -1; +} + +static void _wa_remove(struct i915_wa_list *wal, i915_reg_t reg, u32 flags) +{ + int index; + struct i915_wa *wa = wal->list; + + reg.reg |= flags; + + index = _wa_index(wal, reg); + if (index < 0) + return; + + memset(wa + index, 0, sizeof(*wa)); + + while (index < wal->count - 1) { + swap(wa[index], wa[index + 1]); + index++; + } + + wal->count--; +} + static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) { - unsigned int addr = i915_mmio_reg_offset(wa->reg); - unsigned int start = 0, end = wal->count; + int index; const unsigned int grow = WA_LIST_CHUNK; struct i915_wa *wa_; @@ -106,30 +157,23 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) wal->list = list; } - while (start < end) { - unsigned int mid = start + (end - start) / 2; - - if (i915_mmio_reg_offset(wal->list[mid].reg) < addr) { - start = mid + 1; - } else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr) { - end = mid; - } else { - wa_ = &wal->list[mid]; - - if ((wa->clr | wa_->clr) && !(wa->clr & ~wa_->clr)) { - DRM_ERROR("Discarding overwritten w/a for reg %04x (clear: %08x, set: %08x)\n", - i915_mmio_reg_offset(wa_->reg), - wa_->clr, wa_->set); + index = _wa_index(wal, wa->reg); + if (index >= 0) { + wa_ = &wal->list[index]; - wa_->set &= ~wa->clr; - } + if ((wa->clr | wa_->clr) && !(wa->clr & ~wa_->clr)) { + DRM_ERROR("Discarding overwritten w/a for reg %04x (clear: %08x, set: %08x)\n", + i915_mmio_reg_offset(wa_->reg), + wa_->clr, wa_->set); - wal->wa_count++; - wa_->set |= wa->set; - wa_->clr |= wa->clr; - wa_->read |= wa->read; - return; + wa_->set &= ~wa->clr; } + + wal->wa_count++; + wa_->set |= wa->set; + wa_->clr |= wa->clr; + wa_->read |= wa->read; + return; } wal->wa_count++; @@ -1586,6 +1630,8 @@ void intel_engine_init_whitelist(struct intel_engine_cs *engine) struct drm_i915_private *i915 = engine->i915; struct i915_wa_list *w = &engine->whitelist; + spin_lock_init(&engine->lock); + wa_init_start(w, "whitelist", engine->name); if (IS_GEN(i915, 12)) @@ -1620,11 +1666,14 @@ void intel_engine_apply_whitelist(struct intel_engine_cs *engine) struct intel_uncore *uncore = engine->uncore; const u32 base = engine->mmio_base; struct i915_wa *wa; + unsigned long flags; unsigned int i; if (!wal->count) return; + spin_lock_irqsave(&engine->lock, flags); + for (i = 0, wa = wal->list; i < wal->count; i++, wa++) intel_uncore_write(uncore, RING_FORCE_TO_NONPRIV(base, i), @@ -1635,6 +1684,8 @@ void intel_engine_apply_whitelist(struct intel_engine_cs *engine) intel_uncore_write(uncore, RING_FORCE_TO_NONPRIV(base, i), i915_mmio_reg_offset(RING_NOPID(base))); + + spin_unlock_irqrestore(&engine->lock, flags); } static void @@ -1954,6 +2005,36 @@ void intel_engine_init_workarounds(struct intel_engine_cs *engine) wa_init_finish(wal); } +void intel_engine_allow_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count) +{ + if (!engine || !reg) + return; + + while (count--) { + whitelist_reg_ext(&engine->whitelist, reg->reg, reg->flags); + reg++; + } + + intel_engine_apply_whitelist(engine); +} + +void intel_engine_deny_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count) +{ + if (!engine || !reg) + return; + + while (count--) { + _wa_remove(&engine->whitelist, reg->reg, reg->flags); + reg++; + } + + intel_engine_apply_whitelist(engine); +} + void intel_engine_apply_workarounds(struct intel_engine_cs *engine) { wa_list_apply(engine->uncore, &engine->wa_list); diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h index 8c9c769c2204..6c820c1b751a 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h @@ -37,4 +37,11 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine); int intel_engine_verify_workarounds(struct intel_engine_cs *engine, const char *from); +void intel_engine_allow_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count); +void intel_engine_deny_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count); + #endif diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h index d166a7145720..437981debe14 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h @@ -11,6 +11,11 @@ #include "i915_reg.h" +struct i915_whitelist_reg { + i915_reg_t reg; + u32 flags; +}; + struct i915_wa { i915_reg_t reg; u32 clr; diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index a43bf4cd337a..7443654ef842 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1347,12 +1347,59 @@ free_noa_wait(struct i915_perf_stream *stream) i915_vma_unpin_and_release(&stream->noa_wait, 0); } +static struct i915_whitelist_reg gen9_oa_wl_regs[] = { + { OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW }, + { OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW }, +}; + +static struct i915_whitelist_reg gen12_oa_wl_regs[] = { + { GEN12_OAG_OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW }, + { GEN12_OAG_OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW }, +}; + +static void intel_engine_apply_oa_whitelist(struct i915_perf_stream *stream) +{ + struct intel_engine_cs *engine = stream->engine; + struct drm_i915_private *i915 = stream->perf->i915; + + if (IS_GEN(i915, 12)) + intel_engine_allow_user_register_access(engine, + gen12_oa_wl_regs, + ARRAY_SIZE(gen12_oa_wl_regs)); + else if (INTEL_GEN(i915) > 8) + intel_engine_allow_user_register_access(engine, + gen9_oa_wl_regs, + ARRAY_SIZE(gen9_oa_wl_regs)); + else + return; +} + +static void intel_engine_remove_oa_whitelist(struct i915_perf_stream *stream) +{ + struct intel_engine_cs *engine = stream->engine; + struct drm_i915_private *i915 = stream->perf->i915; + + if (IS_GEN(i915, 12)) + intel_engine_deny_user_register_access(engine, + gen12_oa_wl_regs, + ARRAY_SIZE(gen12_oa_wl_regs)); + else if (INTEL_GEN(i915) > 8) + intel_engine_deny_user_register_access(engine, + gen9_oa_wl_regs, + ARRAY_SIZE(gen9_oa_wl_regs)); + else + return; +} + static void i915_oa_stream_destroy(struct i915_perf_stream *stream) { struct i915_perf *perf = stream->perf; BUG_ON(stream != perf->exclusive_stream); + if (stream->oa_whitelisted) + intel_engine_remove_oa_whitelist(stream); + /* * Unset exclusive_stream first, it will be checked while disabling * the metric set on gen8+. @@ -1448,7 +1495,8 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN8_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK); /* Mark that we need updated tail pointers to read from... */ @@ -1501,7 +1549,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN12_OAG_OATAILPTR, gtt_offset & GEN12_OAG_OATAILPTR_MASK); @@ -3470,6 +3519,22 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf, if (!(param->flags & I915_PERF_FLAG_DISABLED)) i915_perf_enable_locked(stream); + /* + * OA whitelist allows non-privileged access to some OA counters for + * triggering reports into the OA buffer. This is only allowed if + * perf_stream_paranoid is set to 0 by the sysadmin. + * + * We want to make sure this is almost the last thing we do before + * returning the stream fd. If we do end up checking for errors in code + * that follows this, we MUST call intel_engine_remove_oa_whitelist in + * the error handling path to remove the whitelisted registers. + */ + if (!i915_perf_stream_paranoid && + props->sample_flags & SAMPLE_OA_REPORT) { + intel_engine_apply_oa_whitelist(stream); + stream->oa_whitelisted = true; + } + /* Take a reference on the driver that will be kept with stream_fd * until its release. */ @@ -4441,8 +4506,13 @@ int i915_perf_ioctl_version(void) * * 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the * interval for the hrtimer used to check for OA data. + * + * 6: Whitelist OATRIGGER registers to allow user to trigger reports + * into the OA buffer. This applies only to gen8+. The feature can + * only be accessed if perf_stream_paranoid is set to 0 by privileged + * user. */ - return 5; + return 6; } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h index a36a455ae336..a45ad422b176 100644 --- a/drivers/gpu/drm/i915/i915_perf_types.h +++ b/drivers/gpu/drm/i915/i915_perf_types.h @@ -16,6 +16,7 @@ #include <linux/uuid.h> #include <linux/wait.h> +#include "gt/intel_workarounds.h" #include "gt/intel_sseu.h" #include "i915_reg.h" #include "intel_wakeref.h" @@ -311,6 +312,11 @@ struct i915_perf_stream { * buffer should be checked for available data. */ u64 poll_oa_period; + + /** + * @oa_whitelisted: Indicates that the oa registers are whitelisted. + */ + bool oa_whitelisted; }; /** -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer @ 2020-08-04 17:11 Umesh Nerlige Ramappa 2020-08-04 17:11 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 0 siblings, 1 reply; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-08-04 17:11 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson v1: Fixes a memory corruption due to addition of OA whitelist on demand. v2: Spinlock when applying whitelist v3: Use uncore->lock. Do not check for wal->count when applying whitelist. This cover letter is included to trigger "Test-with" an IGT patch. Tests - https://patchwork.freedesktop.org/series/80113/ Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Test-with: 20200730230002.55737-1-umesh.nerlige.ramappa@intel.com Piotr Maciejewski (1): drm/i915/perf: Ensure observation logic is not clock gated Umesh Nerlige Ramappa (3): drm/i915/perf: Whitelist OA report trigger registers drm/i915/perf: Whitelist OA counter and buffer registers drm/i915/perf: Map OA buffer to user space for gen12 performance query drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 + drivers/gpu/drm/i915/gt/intel_workarounds.c | 153 +++++++++--- drivers/gpu/drm/i915/gt/intel_workarounds.h | 7 + .../gpu/drm/i915/gt/intel_workarounds_types.h | 5 + drivers/gpu/drm/i915/i915_perf.c | 224 +++++++++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 5 + drivers/gpu/drm/i915/i915_reg.h | 10 + include/uapi/drm/i915_drm.h | 33 +++ 9 files changed, 401 insertions(+), 40 deletions(-) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-08-04 17:11 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa @ 2020-08-04 17:11 ` Umesh Nerlige Ramappa 0 siblings, 0 replies; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-08-04 17:11 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson OA reports can be triggered into the OA buffer by writing into the OAREPORTTRIG registers. Whitelist the registers to allow non-privileged user to trigger reports. Whitelist registers only if perf_stream_paranoid is set to 0. In i915_perf_open_ioctl, this setting is checked and the whitelist is enabled accordingly. On closing the perf fd, the whitelist is removed. This ensures that the access to the whitelist is gated by perf_stream_paranoid. v2: - Move related change to this patch (Lionel) - Bump up perf revision (Lionel) v3: Pardon whitelisted registers for selftest (Umesh) v4: Document supported gens for the feature (Lionel) v5: Whitelist registers only if perf_stream_paranoid is set to 0 (Jon) v6: Move oa whitelist array to i915_perf (Chris) v7: Fix OA writing beyond the wal->list memory (CI) v8: Protect write to engine whitelist registers v9: (Umesh) - Use uncore->lock to protect write to forcepriv regs - In case wal->count falls to zero on _wa_remove, make sure you still clear the registers. Remove wal->count check when applying whitelist. Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 153 +++++++++++++----- drivers/gpu/drm/i915/gt/intel_workarounds.h | 7 + .../gpu/drm/i915/gt/intel_workarounds_types.h | 5 + drivers/gpu/drm/i915/i915_perf.c | 76 ++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 5 + 5 files changed, 207 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index cef1c122696f..b5075ab73de6 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -60,12 +60,23 @@ static void wa_init_start(struct i915_wa_list *wal, const char *name, const char #define WA_LIST_CHUNK (1 << 4) +/* + * Some of the i915 code like perf OA tries to whitelist registers on demand. + * Such code adds to the wal->list, but that would not work because the list + * is compacted below by wa_init_finish. While _wa_add does have code to grow + * the list, it does not seem to take the compaction into consideration. Leave + * 8 entries free during the compaction until a better mechanism can be put in + * place. + */ +#define WA_LIST_DYNAMIC_ENTRIES 8 + static void wa_init_finish(struct i915_wa_list *wal) { /* Trim unused entries. */ if (!IS_ALIGNED(wal->count, WA_LIST_CHUNK)) { + size_t size = wal->count + WA_LIST_DYNAMIC_ENTRIES; struct i915_wa *list = kmemdup(wal->list, - wal->count * sizeof(*list), + size * sizeof(*list), GFP_KERNEL); if (list) { @@ -81,10 +92,50 @@ static void wa_init_finish(struct i915_wa_list *wal) wal->wa_count, wal->name, wal->engine_name); } +static int _wa_index(struct i915_wa_list *wal, i915_reg_t reg) +{ + unsigned int addr = i915_mmio_reg_offset(reg); + int start = 0, end = wal->count; + + /* addr and wal->list[].reg, both include the R/W flags */ + while (start < end) { + int mid = start + (end - start) / 2; + + if (i915_mmio_reg_offset(wal->list[mid].reg) < addr) + start = mid + 1; + else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr) + end = mid; + else + return mid; + } + + return -1; +} + +static void _wa_remove(struct i915_wa_list *wal, i915_reg_t reg, u32 flags) +{ + int index; + struct i915_wa *wa = wal->list; + + reg.reg |= flags; + + index = _wa_index(wal, reg); + if (index < 0) + return; + + memset(wa + index, 0, sizeof(*wa)); + + while (index < wal->count - 1) { + swap(wa[index], wa[index + 1]); + index++; + } + + wal->count--; +} + static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) { - unsigned int addr = i915_mmio_reg_offset(wa->reg); - unsigned int start = 0, end = wal->count; + int index; const unsigned int grow = WA_LIST_CHUNK; struct i915_wa *wa_; @@ -106,30 +157,23 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) wal->list = list; } - while (start < end) { - unsigned int mid = start + (end - start) / 2; - - if (i915_mmio_reg_offset(wal->list[mid].reg) < addr) { - start = mid + 1; - } else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr) { - end = mid; - } else { - wa_ = &wal->list[mid]; - - if ((wa->clr | wa_->clr) && !(wa->clr & ~wa_->clr)) { - DRM_ERROR("Discarding overwritten w/a for reg %04x (clear: %08x, set: %08x)\n", - i915_mmio_reg_offset(wa_->reg), - wa_->clr, wa_->set); + index = _wa_index(wal, wa->reg); + if (index >= 0) { + wa_ = &wal->list[index]; - wa_->set &= ~wa->clr; - } + if ((wa->clr | wa_->clr) && !(wa->clr & ~wa_->clr)) { + DRM_ERROR("Discarding overwritten w/a for reg %04x (clear: %08x, set: %08x)\n", + i915_mmio_reg_offset(wa_->reg), + wa_->clr, wa_->set); - wal->wa_count++; - wa_->set |= wa->set; - wa_->clr |= wa->clr; - wa_->read |= wa->read; - return; + wa_->set &= ~wa->clr; } + + wal->wa_count++; + wa_->set |= wa->set; + wa_->clr |= wa->clr; + wa_->read |= wa->read; + return; } wal->wa_count++; @@ -1264,7 +1308,8 @@ void intel_gt_init_workarounds(struct drm_i915_private *i915) } static enum forcewake_domains -wal_get_fw_for_rmw(struct intel_uncore *uncore, const struct i915_wa_list *wal) +wal_get_fw(struct intel_uncore *uncore, const struct i915_wa_list *wal, + unsigned int op) { enum forcewake_domains fw = 0; struct i915_wa *wa; @@ -1273,8 +1318,7 @@ wal_get_fw_for_rmw(struct intel_uncore *uncore, const struct i915_wa_list *wal) for (i = 0, wa = wal->list; i < wal->count; i++, wa++) fw |= intel_uncore_forcewake_for_reg(uncore, wa->reg, - FW_REG_READ | - FW_REG_WRITE); + op); return fw; } @@ -1304,7 +1348,7 @@ wa_list_apply(struct intel_uncore *uncore, const struct i915_wa_list *wal) if (!wal->count) return; - fw = wal_get_fw_for_rmw(uncore, wal); + fw = wal_get_fw(uncore, wal, FW_REG_READ | FW_REG_WRITE); spin_lock_irqsave(&uncore->lock, flags); intel_uncore_forcewake_get__locked(uncore, fw); @@ -1616,25 +1660,32 @@ void intel_engine_init_whitelist(struct intel_engine_cs *engine) void intel_engine_apply_whitelist(struct intel_engine_cs *engine) { + enum forcewake_domains fw; const struct i915_wa_list *wal = &engine->whitelist; struct intel_uncore *uncore = engine->uncore; const u32 base = engine->mmio_base; struct i915_wa *wa; + unsigned long flags; unsigned int i; - if (!wal->count) - return; + fw = wal_get_fw(uncore, wal, FW_REG_WRITE); + + spin_lock_irqsave(&uncore->lock, flags); + intel_uncore_forcewake_get__locked(uncore, fw); for (i = 0, wa = wal->list; i < wal->count; i++, wa++) - intel_uncore_write(uncore, - RING_FORCE_TO_NONPRIV(base, i), - i915_mmio_reg_offset(wa->reg)); + intel_uncore_write_fw(uncore, + RING_FORCE_TO_NONPRIV(base, i), + i915_mmio_reg_offset(wa->reg)); /* And clear the rest just in case of garbage */ for (; i < RING_MAX_NONPRIV_SLOTS; i++) - intel_uncore_write(uncore, - RING_FORCE_TO_NONPRIV(base, i), - i915_mmio_reg_offset(RING_NOPID(base))); + intel_uncore_write_fw(uncore, + RING_FORCE_TO_NONPRIV(base, i), + i915_mmio_reg_offset(RING_NOPID(base))); + + intel_uncore_forcewake_put__locked(uncore, fw); + spin_unlock_irqrestore(&uncore->lock, flags); } static void @@ -1954,6 +2005,36 @@ void intel_engine_init_workarounds(struct intel_engine_cs *engine) wa_init_finish(wal); } +void intel_engine_allow_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count) +{ + if (!engine || !reg) + return; + + while (count--) { + whitelist_reg_ext(&engine->whitelist, reg->reg, reg->flags); + reg++; + } + + intel_engine_apply_whitelist(engine); +} + +void intel_engine_deny_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count) +{ + if (!engine || !reg) + return; + + while (count--) { + _wa_remove(&engine->whitelist, reg->reg, reg->flags); + reg++; + } + + intel_engine_apply_whitelist(engine); +} + void intel_engine_apply_workarounds(struct intel_engine_cs *engine) { wa_list_apply(engine->uncore, &engine->wa_list); diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h index 8c9c769c2204..6c820c1b751a 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h @@ -37,4 +37,11 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine); int intel_engine_verify_workarounds(struct intel_engine_cs *engine, const char *from); +void intel_engine_allow_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count); +void intel_engine_deny_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count); + #endif diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h index d166a7145720..437981debe14 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h @@ -11,6 +11,11 @@ #include "i915_reg.h" +struct i915_whitelist_reg { + i915_reg_t reg; + u32 flags; +}; + struct i915_wa { i915_reg_t reg; u32 clr; diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index a43bf4cd337a..7443654ef842 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1347,12 +1347,59 @@ free_noa_wait(struct i915_perf_stream *stream) i915_vma_unpin_and_release(&stream->noa_wait, 0); } +static struct i915_whitelist_reg gen9_oa_wl_regs[] = { + { OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW }, + { OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW }, +}; + +static struct i915_whitelist_reg gen12_oa_wl_regs[] = { + { GEN12_OAG_OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW }, + { GEN12_OAG_OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW }, +}; + +static void intel_engine_apply_oa_whitelist(struct i915_perf_stream *stream) +{ + struct intel_engine_cs *engine = stream->engine; + struct drm_i915_private *i915 = stream->perf->i915; + + if (IS_GEN(i915, 12)) + intel_engine_allow_user_register_access(engine, + gen12_oa_wl_regs, + ARRAY_SIZE(gen12_oa_wl_regs)); + else if (INTEL_GEN(i915) > 8) + intel_engine_allow_user_register_access(engine, + gen9_oa_wl_regs, + ARRAY_SIZE(gen9_oa_wl_regs)); + else + return; +} + +static void intel_engine_remove_oa_whitelist(struct i915_perf_stream *stream) +{ + struct intel_engine_cs *engine = stream->engine; + struct drm_i915_private *i915 = stream->perf->i915; + + if (IS_GEN(i915, 12)) + intel_engine_deny_user_register_access(engine, + gen12_oa_wl_regs, + ARRAY_SIZE(gen12_oa_wl_regs)); + else if (INTEL_GEN(i915) > 8) + intel_engine_deny_user_register_access(engine, + gen9_oa_wl_regs, + ARRAY_SIZE(gen9_oa_wl_regs)); + else + return; +} + static void i915_oa_stream_destroy(struct i915_perf_stream *stream) { struct i915_perf *perf = stream->perf; BUG_ON(stream != perf->exclusive_stream); + if (stream->oa_whitelisted) + intel_engine_remove_oa_whitelist(stream); + /* * Unset exclusive_stream first, it will be checked while disabling * the metric set on gen8+. @@ -1448,7 +1495,8 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN8_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK); /* Mark that we need updated tail pointers to read from... */ @@ -1501,7 +1549,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN12_OAG_OATAILPTR, gtt_offset & GEN12_OAG_OATAILPTR_MASK); @@ -3470,6 +3519,22 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf, if (!(param->flags & I915_PERF_FLAG_DISABLED)) i915_perf_enable_locked(stream); + /* + * OA whitelist allows non-privileged access to some OA counters for + * triggering reports into the OA buffer. This is only allowed if + * perf_stream_paranoid is set to 0 by the sysadmin. + * + * We want to make sure this is almost the last thing we do before + * returning the stream fd. If we do end up checking for errors in code + * that follows this, we MUST call intel_engine_remove_oa_whitelist in + * the error handling path to remove the whitelisted registers. + */ + if (!i915_perf_stream_paranoid && + props->sample_flags & SAMPLE_OA_REPORT) { + intel_engine_apply_oa_whitelist(stream); + stream->oa_whitelisted = true; + } + /* Take a reference on the driver that will be kept with stream_fd * until its release. */ @@ -4441,8 +4506,13 @@ int i915_perf_ioctl_version(void) * * 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the * interval for the hrtimer used to check for OA data. + * + * 6: Whitelist OATRIGGER registers to allow user to trigger reports + * into the OA buffer. This applies only to gen8+. The feature can + * only be accessed if perf_stream_paranoid is set to 0 by privileged + * user. */ - return 5; + return 6; } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h index a36a455ae336..35f8240fd6ce 100644 --- a/drivers/gpu/drm/i915/i915_perf_types.h +++ b/drivers/gpu/drm/i915/i915_perf_types.h @@ -311,6 +311,11 @@ struct i915_perf_stream { * buffer should be checked for available data. */ u64 poll_oa_period; + + /** + * @oa_whitelisted: Indicates that the oa registers are whitelisted. + */ + bool oa_whitelisted; }; /** -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer @ 2020-08-20 18:01 Umesh Nerlige Ramappa 2020-08-20 18:01 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 0 siblings, 1 reply; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-08-20 18:01 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson Allow user to map the OA buffer and also trigger reports into it. CI fixes: v1: Fixes a memory corruption due to addition of OA whitelist on demand. v2: Spinlock when applying whitelist v3: Use uncore->lock. Do not check for wal->count when applying whitelist. v4: Refresh and rerun with newly added test (forked access). Tests: https://patchwork.freedesktop.org/series/80761/ Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Test-with: 20200818203547.24461-1-umesh.nerlige.ramappa@intel.com Piotr Maciejewski (1): drm/i915/perf: Ensure observation logic is not clock gated Umesh Nerlige Ramappa (3): drm/i915/perf: Whitelist OA report trigger registers drm/i915/perf: Whitelist OA counter and buffer registers drm/i915/perf: Map OA buffer to user space for gen12 performance query drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 + drivers/gpu/drm/i915/gt/intel_workarounds.c | 153 +++++++++--- drivers/gpu/drm/i915/gt/intel_workarounds.h | 7 + .../gpu/drm/i915/gt/intel_workarounds_types.h | 5 + drivers/gpu/drm/i915/i915_perf.c | 225 +++++++++++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 5 + drivers/gpu/drm/i915/i915_reg.h | 10 + include/uapi/drm/i915_drm.h | 33 +++ 9 files changed, 402 insertions(+), 40 deletions(-) -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers 2020-08-20 18:01 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa @ 2020-08-20 18:01 ` Umesh Nerlige Ramappa 0 siblings, 0 replies; 31+ messages in thread From: Umesh Nerlige Ramappa @ 2020-08-20 18:01 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson OA reports can be triggered into the OA buffer by writing into the OAREPORTTRIG registers. Whitelist the registers to allow non-privileged user to trigger reports. Whitelist registers only if perf_stream_paranoid is set to 0. In i915_perf_open_ioctl, this setting is checked and the whitelist is enabled accordingly. On closing the perf fd, the whitelist is removed. This ensures that the access to the whitelist is gated by perf_stream_paranoid. v2: - Move related change to this patch (Lionel) - Bump up perf revision (Lionel) v3: Pardon whitelisted registers for selftest (Umesh) v4: Document supported gens for the feature (Lionel) v5: Whitelist registers only if perf_stream_paranoid is set to 0 (Jon) v6: Move oa whitelist array to i915_perf (Chris) v7: Fix OA writing beyond the wal->list memory (CI) v8: Protect write to engine whitelist registers v9: (Umesh) - Use uncore->lock to protect write to forcepriv regs - In case wal->count falls to zero on _wa_remove, make sure you still clear the registers. Remove wal->count check when applying whitelist. Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 153 +++++++++++++----- drivers/gpu/drm/i915/gt/intel_workarounds.h | 7 + .../gpu/drm/i915/gt/intel_workarounds_types.h | 5 + drivers/gpu/drm/i915/i915_perf.c | 76 ++++++++- drivers/gpu/drm/i915/i915_perf_types.h | 5 + 5 files changed, 207 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index be5a4685c991..9fb2a40e00f4 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -78,12 +78,23 @@ static void wa_init_start(struct i915_wa_list *wal, const char *name, const char #define WA_LIST_CHUNK (1 << 4) +/* + * Some of the i915 code like perf OA tries to whitelist registers on demand. + * Such code adds to the wal->list, but that would not work because the list + * is compacted below by wa_init_finish. While _wa_add does have code to grow + * the list, it does not seem to take the compaction into consideration. Leave + * 8 entries free during the compaction until a better mechanism can be put in + * place. + */ +#define WA_LIST_DYNAMIC_ENTRIES 8 + static void wa_init_finish(struct i915_wa_list *wal) { /* Trim unused entries. */ if (!IS_ALIGNED(wal->count, WA_LIST_CHUNK)) { + size_t size = wal->count + WA_LIST_DYNAMIC_ENTRIES; struct i915_wa *list = kmemdup(wal->list, - wal->count * sizeof(*list), + size * sizeof(*list), GFP_KERNEL); if (list) { @@ -99,10 +110,50 @@ static void wa_init_finish(struct i915_wa_list *wal) wal->wa_count, wal->name, wal->engine_name); } +static int _wa_index(struct i915_wa_list *wal, i915_reg_t reg) +{ + unsigned int addr = i915_mmio_reg_offset(reg); + int start = 0, end = wal->count; + + /* addr and wal->list[].reg, both include the R/W flags */ + while (start < end) { + int mid = start + (end - start) / 2; + + if (i915_mmio_reg_offset(wal->list[mid].reg) < addr) + start = mid + 1; + else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr) + end = mid; + else + return mid; + } + + return -1; +} + +static void _wa_remove(struct i915_wa_list *wal, i915_reg_t reg, u32 flags) +{ + int index; + struct i915_wa *wa = wal->list; + + reg.reg |= flags; + + index = _wa_index(wal, reg); + if (index < 0) + return; + + memset(wa + index, 0, sizeof(*wa)); + + while (index < wal->count - 1) { + swap(wa[index], wa[index + 1]); + index++; + } + + wal->count--; +} + static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) { - unsigned int addr = i915_mmio_reg_offset(wa->reg); - unsigned int start = 0, end = wal->count; + int index; const unsigned int grow = WA_LIST_CHUNK; struct i915_wa *wa_; @@ -124,30 +175,23 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) wal->list = list; } - while (start < end) { - unsigned int mid = start + (end - start) / 2; - - if (i915_mmio_reg_offset(wal->list[mid].reg) < addr) { - start = mid + 1; - } else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr) { - end = mid; - } else { - wa_ = &wal->list[mid]; - - if ((wa->clr | wa_->clr) && !(wa->clr & ~wa_->clr)) { - DRM_ERROR("Discarding overwritten w/a for reg %04x (clear: %08x, set: %08x)\n", - i915_mmio_reg_offset(wa_->reg), - wa_->clr, wa_->set); + index = _wa_index(wal, wa->reg); + if (index >= 0) { + wa_ = &wal->list[index]; - wa_->set &= ~wa->clr; - } + if ((wa->clr | wa_->clr) && !(wa->clr & ~wa_->clr)) { + DRM_ERROR("Discarding overwritten w/a for reg %04x (clear: %08x, set: %08x)\n", + i915_mmio_reg_offset(wa_->reg), + wa_->clr, wa_->set); - wal->wa_count++; - wa_->set |= wa->set; - wa_->clr |= wa->clr; - wa_->read |= wa->read; - return; + wa_->set &= ~wa->clr; } + + wal->wa_count++; + wa_->set |= wa->set; + wa_->clr |= wa->clr; + wa_->read |= wa->read; + return; } wal->wa_count++; @@ -1282,7 +1326,8 @@ void intel_gt_init_workarounds(struct drm_i915_private *i915) } static enum forcewake_domains -wal_get_fw_for_rmw(struct intel_uncore *uncore, const struct i915_wa_list *wal) +wal_get_fw(struct intel_uncore *uncore, const struct i915_wa_list *wal, + unsigned int op) { enum forcewake_domains fw = 0; struct i915_wa *wa; @@ -1291,8 +1336,7 @@ wal_get_fw_for_rmw(struct intel_uncore *uncore, const struct i915_wa_list *wal) for (i = 0, wa = wal->list; i < wal->count; i++, wa++) fw |= intel_uncore_forcewake_for_reg(uncore, wa->reg, - FW_REG_READ | - FW_REG_WRITE); + op); return fw; } @@ -1322,7 +1366,7 @@ wa_list_apply(struct intel_uncore *uncore, const struct i915_wa_list *wal) if (!wal->count) return; - fw = wal_get_fw_for_rmw(uncore, wal); + fw = wal_get_fw(uncore, wal, FW_REG_READ | FW_REG_WRITE); spin_lock_irqsave(&uncore->lock, flags); intel_uncore_forcewake_get__locked(uncore, fw); @@ -1634,25 +1678,32 @@ void intel_engine_init_whitelist(struct intel_engine_cs *engine) void intel_engine_apply_whitelist(struct intel_engine_cs *engine) { + enum forcewake_domains fw; const struct i915_wa_list *wal = &engine->whitelist; struct intel_uncore *uncore = engine->uncore; const u32 base = engine->mmio_base; struct i915_wa *wa; + unsigned long flags; unsigned int i; - if (!wal->count) - return; + fw = wal_get_fw(uncore, wal, FW_REG_WRITE); + + spin_lock_irqsave(&uncore->lock, flags); + intel_uncore_forcewake_get__locked(uncore, fw); for (i = 0, wa = wal->list; i < wal->count; i++, wa++) - intel_uncore_write(uncore, - RING_FORCE_TO_NONPRIV(base, i), - i915_mmio_reg_offset(wa->reg)); + intel_uncore_write_fw(uncore, + RING_FORCE_TO_NONPRIV(base, i), + i915_mmio_reg_offset(wa->reg)); /* And clear the rest just in case of garbage */ for (; i < RING_MAX_NONPRIV_SLOTS; i++) - intel_uncore_write(uncore, - RING_FORCE_TO_NONPRIV(base, i), - i915_mmio_reg_offset(RING_NOPID(base))); + intel_uncore_write_fw(uncore, + RING_FORCE_TO_NONPRIV(base, i), + i915_mmio_reg_offset(RING_NOPID(base))); + + intel_uncore_forcewake_put__locked(uncore, fw); + spin_unlock_irqrestore(&uncore->lock, flags); } static void @@ -1972,6 +2023,36 @@ void intel_engine_init_workarounds(struct intel_engine_cs *engine) wa_init_finish(wal); } +void intel_engine_allow_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count) +{ + if (!engine || !reg) + return; + + while (count--) { + whitelist_reg_ext(&engine->whitelist, reg->reg, reg->flags); + reg++; + } + + intel_engine_apply_whitelist(engine); +} + +void intel_engine_deny_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count) +{ + if (!engine || !reg) + return; + + while (count--) { + _wa_remove(&engine->whitelist, reg->reg, reg->flags); + reg++; + } + + intel_engine_apply_whitelist(engine); +} + void intel_engine_apply_workarounds(struct intel_engine_cs *engine) { wa_list_apply(engine->uncore, &engine->wa_list); diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h index 8c9c769c2204..6c820c1b751a 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h @@ -37,4 +37,11 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine); int intel_engine_verify_workarounds(struct intel_engine_cs *engine, const char *from); +void intel_engine_allow_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count); +void intel_engine_deny_user_register_access(struct intel_engine_cs *engine, + struct i915_whitelist_reg *reg, + u32 count); + #endif diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h index d166a7145720..437981debe14 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h @@ -11,6 +11,11 @@ #include "i915_reg.h" +struct i915_whitelist_reg { + i915_reg_t reg; + u32 flags; +}; + struct i915_wa { i915_reg_t reg; u32 clr; diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index a43bf4cd337a..7443654ef842 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1347,12 +1347,59 @@ free_noa_wait(struct i915_perf_stream *stream) i915_vma_unpin_and_release(&stream->noa_wait, 0); } +static struct i915_whitelist_reg gen9_oa_wl_regs[] = { + { OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW }, + { OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW }, +}; + +static struct i915_whitelist_reg gen12_oa_wl_regs[] = { + { GEN12_OAG_OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW }, + { GEN12_OAG_OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW }, +}; + +static void intel_engine_apply_oa_whitelist(struct i915_perf_stream *stream) +{ + struct intel_engine_cs *engine = stream->engine; + struct drm_i915_private *i915 = stream->perf->i915; + + if (IS_GEN(i915, 12)) + intel_engine_allow_user_register_access(engine, + gen12_oa_wl_regs, + ARRAY_SIZE(gen12_oa_wl_regs)); + else if (INTEL_GEN(i915) > 8) + intel_engine_allow_user_register_access(engine, + gen9_oa_wl_regs, + ARRAY_SIZE(gen9_oa_wl_regs)); + else + return; +} + +static void intel_engine_remove_oa_whitelist(struct i915_perf_stream *stream) +{ + struct intel_engine_cs *engine = stream->engine; + struct drm_i915_private *i915 = stream->perf->i915; + + if (IS_GEN(i915, 12)) + intel_engine_deny_user_register_access(engine, + gen12_oa_wl_regs, + ARRAY_SIZE(gen12_oa_wl_regs)); + else if (INTEL_GEN(i915) > 8) + intel_engine_deny_user_register_access(engine, + gen9_oa_wl_regs, + ARRAY_SIZE(gen9_oa_wl_regs)); + else + return; +} + static void i915_oa_stream_destroy(struct i915_perf_stream *stream) { struct i915_perf *perf = stream->perf; BUG_ON(stream != perf->exclusive_stream); + if (stream->oa_whitelisted) + intel_engine_remove_oa_whitelist(stream); + /* * Unset exclusive_stream first, it will be checked while disabling * the metric set on gen8+. @@ -1448,7 +1495,8 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN8_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK); /* Mark that we need updated tail pointers to read from... */ @@ -1501,7 +1549,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream) * bit." */ intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset | - OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT | + GEN7_OABUFFER_EDGE_TRIGGER); intel_uncore_write(uncore, GEN12_OAG_OATAILPTR, gtt_offset & GEN12_OAG_OATAILPTR_MASK); @@ -3470,6 +3519,22 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf, if (!(param->flags & I915_PERF_FLAG_DISABLED)) i915_perf_enable_locked(stream); + /* + * OA whitelist allows non-privileged access to some OA counters for + * triggering reports into the OA buffer. This is only allowed if + * perf_stream_paranoid is set to 0 by the sysadmin. + * + * We want to make sure this is almost the last thing we do before + * returning the stream fd. If we do end up checking for errors in code + * that follows this, we MUST call intel_engine_remove_oa_whitelist in + * the error handling path to remove the whitelisted registers. + */ + if (!i915_perf_stream_paranoid && + props->sample_flags & SAMPLE_OA_REPORT) { + intel_engine_apply_oa_whitelist(stream); + stream->oa_whitelisted = true; + } + /* Take a reference on the driver that will be kept with stream_fd * until its release. */ @@ -4441,8 +4506,13 @@ int i915_perf_ioctl_version(void) * * 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the * interval for the hrtimer used to check for OA data. + * + * 6: Whitelist OATRIGGER registers to allow user to trigger reports + * into the OA buffer. This applies only to gen8+. The feature can + * only be accessed if perf_stream_paranoid is set to 0 by privileged + * user. */ - return 5; + return 6; } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h index a36a455ae336..35f8240fd6ce 100644 --- a/drivers/gpu/drm/i915/i915_perf_types.h +++ b/drivers/gpu/drm/i915/i915_perf_types.h @@ -311,6 +311,11 @@ struct i915_perf_stream { * buffer should be checked for available data. */ u64 poll_oa_period; + + /** + * @oa_whitelisted: Indicates that the oa registers are whitelisted. + */ + bool oa_whitelisted; }; /** -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 31+ messages in thread
end of thread, other threads:[~2020-08-20 18:02 UTC | newest] Thread overview: 31+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-07-18 0:04 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa 2020-07-18 0:04 ` [Intel-gfx] [PATCH 1/4] drm/i915/perf: Ensure observation logic is not clock gated Umesh Nerlige Ramappa 2020-07-18 19:09 ` Lionel Landwerlin 2020-07-18 0:04 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 2020-07-18 19:11 ` Lionel Landwerlin 2020-07-18 0:04 ` [Intel-gfx] [PATCH 3/4] drm/i915/perf: Whitelist OA counter and buffer registers Umesh Nerlige Ramappa 2020-07-18 0:04 ` [Intel-gfx] [PATCH 4/4] drm/i915/perf: Map OA buffer to user space for gen12 performance query Umesh Nerlige Ramappa 2020-07-18 11:44 ` kernel test robot 2020-07-18 11:44 ` kernel test robot 2020-07-18 11:44 ` [Intel-gfx] [RFC PATCH] drm/i915/perf: i915_perf_mmap() can be static kernel test robot 2020-07-18 11:44 ` kernel test robot 2020-07-20 12:21 ` [Intel-gfx] [PATCH 4/4] drm/i915/perf: Map OA buffer to user space for gen12 performance query Chris Wilson 2020-07-18 0:29 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Allow privileged user to map the OA buffer (rev2) Patchwork 2020-07-18 0:30 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork 2020-07-18 0:52 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork -- strict thread matches above, loose matches on Subject: below -- 2020-07-21 2:00 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa 2020-07-21 2:00 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 2020-07-22 5:55 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa 2020-07-22 5:55 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 2020-07-24 0:18 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa 2020-07-24 0:18 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 2020-07-24 9:26 ` Chris Wilson 2020-07-24 10:07 ` Lionel Landwerlin 2020-07-24 10:19 ` Chris Wilson 2020-07-24 10:23 ` Chris Wilson 2020-07-24 10:33 ` Lionel Landwerlin 2020-07-24 11:27 ` Chris Wilson 2020-07-24 10:31 ` Lionel Landwerlin 2020-07-30 23:02 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa 2020-07-30 23:02 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 2020-07-31 6:07 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa 2020-07-31 6:07 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 2020-07-31 14:46 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa 2020-07-31 14:46 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 2020-07-31 23:58 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa 2020-07-31 23:58 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 2020-08-04 17:11 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa 2020-08-04 17:11 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa 2020-08-20 18:01 [Intel-gfx] [PATCH 0/4] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa 2020-08-20 18:01 ` [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers Umesh Nerlige Ramappa
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.