Re: [PATCH 02/21] drm/i915/gtt: Workaround for HW preload not flushing pdps

Mika Kuoppala
2015-06-11T13:57:46Z

Dave Gordon <david.s.gordon@intel.com> writes:

> On 10/06/15 12:42, Michel Thierry wrote:
>> On 5/29/2015 1:53 PM, Michel Thierry wrote:
>>> On 5/29/2015 12:05 PM, Michel Thierry wrote:
>>>> On 5/22/2015 6:04 PM, Mika Kuoppala wrote:
>>>>> With BDW/SKL and 32bit addressing mode only, the hardware preloads
>>>>> pdps. However the TLB invalidation only has effect on levels below
>>>>> the pdps. This means that if pdps change, hw might access with
>>>>> stale pdp entry.
>>>>>
>>>>> To combat this problem, preallocate the top pdps so that hw sees
>>>>> them as immutable for each context.
>>>>>
>>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>> Cc: Rafael Barbalho <rafael.barbalho@intel.com>
>>>>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>>>>> ---
>>>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 50
>>>>> +++++++++++++++++++++++++++++++++++++
>>>>> drivers/gpu/drm/i915/i915_reg.h | 17 +++++++++++++
>>>>> drivers/gpu/drm/i915/intel_lrc.c | 15 +----------
>>>>> 3 files changed, 68 insertions(+), 14 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> index 0ffd459..1a5ad4c 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> @@ -941,6 +941,48 @@ err_out:
>>>>> return ret;
>>>>> }
>>>>>
>>>>> +/* With some architectures and 32bit legacy mode, hardware pre-loads
>>>>> the
>>>>> + * top level pdps but the tlb invalidation only invalidates the
>>>>> lower levels.
>>>>> + * This might lead to hw fetching with stale pdp entries if top level
>>>>> + * structure changes, ie va space grows with dynamic page tables.
>>>>> + */
>
> Is this still necessary if we reload PDPs via LRI instructions whenever
> the address map has changed? That always (AFAICT) causes sufficient
> invalidation, so then we might not need to preallocate at all :)
>

LRI reload gets my vote. Please ignore this patch.
-Mika

> .Dave.