On 09/07/2019 22:09, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-07-09 22:06:17)From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> fls returns bit positions starting from one for the lsb and the MCR register expects zero based (sub)slice addressing. Incorrent MCR programming can have the effect of directing MMIO reads of registers in the 0xb100-0xb3ff range to invalid subslice returning zeroes instead of actual content. Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Fixes: 1e40d4aea57b ("drm/i915/cnl: Implement WaProgramMgsrForCorrectSliceSpecificMmioReads")Makes sense to me, just from my meagre understanding of arrays Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>--- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index bdf279fa3b2e..ee15d1934486 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -975,9 +975,14 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type) u32 intel_calculate_mcr_s_ss_select(struct drm_i915_private *dev_priv) { const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; + unsigned int slice = fls(sseu->slice_mask) - 1;I'd vote for __fls() here instead of fls() - 1.
With fls() I get zero slice mask check for free, in the array out of bounds check below.
+ unsigned int subslice; u32 mcr_s_ss_select; - u32 slice = fls(sseu->slice_mask); - u32 subslice = fls(sseu->subslice_mask[slice]); + + GEM_BUG_ON(slice >= ARRAY_SIZE(sseu->subslice_mask)); + subslice = fls(sseu->subslice_mask[slice]); + GEM_BUG_ON(!subslice); + subslice--;And I think we're a bit late on the BUG_ON here (it's shouldn't change after probing) so could be happily reduced to __fls().
Why late? This one is not checking the array for out of bounds, just if zero subslice mask happens to be in a valid slot. Too paranoid?
Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
