mbox series

[v3,0/2] Don't use stolen memory or BAR mappings for ring buffers

Message ID 20230216011101.1909009-1-John.C.Harrison@Intel.com (mailing list archive)
Headers show
Series Don't use stolen memory or BAR mappings for ring buffers | expand

Message

John Harrison Feb. 16, 2023, 1:10 a.m. UTC
From: John Harrison <John.C.Harrison@Intel.com>

Instruction from hardware arch is that stolen memory and BAR mappings
are unsafe for use as ring buffers. There can be issues with cache
aliasing due to the CPU access going to memory via the BAR. So, don't
do it.

v2: Dont use BAR mappings either.
Make conditional on LLC so as not to change platforms that don't need
to change (Daniele).
Add 'Fixes' tags (Tvrtko).
v3: Fix dumb typo.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>


John Harrison (2):
  drm/i915: Don't use stolen memory for ring buffers with LLC
  drm/i915: Don't use BAR mappings for ring buffers with LLC

 drivers/gpu/drm/i915/gt/intel_ring.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Hogander, Jouni Feb. 17, 2023, 8:39 a.m. UTC | #1
On Wed, 2023-02-15 at 17:10 -0800, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Instruction from hardware arch is that stolen memory and BAR mappings
> are unsafe for use as ring buffers. There can be issues with cache
> aliasing due to the CPU access going to memory via the BAR. So, don't
> do it.

Tested these patches for GPU Hang I was debugging. Seem to fix that one
as well:

Tested-by: Jouni Högander <jouni.hogander@intel.com>

> 
> v2: Dont use BAR mappings either.
> Make conditional on LLC so as not to change platforms that don't need
> to change (Daniele).
> Add 'Fixes' tags (Tvrtko).
> v3: Fix dumb typo.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> 
> 
> John Harrison (2):
>   drm/i915: Don't use stolen memory for ring buffers with LLC
>   drm/i915: Don't use BAR mappings for ring buffers with LLC
> 
>  drivers/gpu/drm/i915/gt/intel_ring.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
John Harrison Feb. 17, 2023, 5:18 p.m. UTC | #2
On 2/17/2023 00:39, Hogander, Jouni wrote:
> On Wed, 2023-02-15 at 17:10 -0800, John.C.Harrison@Intel.com wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> Instruction from hardware arch is that stolen memory and BAR mappings
>> are unsafe for use as ring buffers. There can be issues with cache
>> aliasing due to the CPU access going to memory via the BAR. So, don't
>> do it.
> Tested these patches for GPU Hang I was debugging. Seem to fix that one
> as well:
>
> Tested-by: Jouni Högander <jouni.hogander@intel.com>
Sweet! Out of interest, which platform was that? And how reproducible 
was it? It would be interesting to know if an IGT was actually regularly 
showing the issue and we had just been ignoring it!

John.

>
>> v2: Dont use BAR mappings either.
>> Make conditional on LLC so as not to change platforms that don't need
>> to change (Daniele).
>> Add 'Fixes' tags (Tvrtko).
>> v3: Fix dumb typo.
>>
>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>
>>
>> John Harrison (2):
>>    drm/i915: Don't use stolen memory for ring buffers with LLC
>>    drm/i915: Don't use BAR mappings for ring buffers with LLC
>>
>>   drivers/gpu/drm/i915/gt/intel_ring.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
Hogander, Jouni Feb. 20, 2023, 6:50 a.m. UTC | #3
On Fri, 2023-02-17 at 09:18 -0800, John Harrison wrote:
> On 2/17/2023 00:39, Hogander, Jouni wrote:
> > On Wed, 2023-02-15 at 17:10 -0800, John.C.Harrison@Intel.com wrote:
> > > From: John Harrison <John.C.Harrison@Intel.com>
> > > 
> > > Instruction from hardware arch is that stolen memory and BAR
> > > mappings
> > > are unsafe for use as ring buffers. There can be issues with
> > > cache
> > > aliasing due to the CPU access going to memory via the BAR. So,
> > > don't
> > > do it.
> > Tested these patches for GPU Hang I was debugging. Seem to fix that
> > one
> > as well:
> > 
> > Tested-by: Jouni Högander <jouni.hogander@intel.com>
> Sweet! Out of interest, which platform was that? And how reproducible
> was it? It would be interesting to know if an IGT was actually
> regularly 
> showing the issue and we had just been ignoring it!

It was Alderlake. It wasn't igt test. Opened several browser
instances: 

https://webglsamples.org/aquarium/aquarium.html

Took max. couple of minutes to reproduce. For some reason PSR2
DEEP_SLEEP entry/exit was some kind of trigger for this. Without PSR2
DEEP_SLEEP I couldn't reproduce the issue.

BR,

Jouni Högander


> John.
> 
> > 
> > > v2: Dont use BAR mappings either.
> > > Make conditional on LLC so as not to change platforms that don't
> > > need
> > > to change (Daniele).
> > > Add 'Fixes' tags (Tvrtko).
> > > v3: Fix dumb typo.
> > > 
> > > Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> > > 
> > > 
> > > John Harrison (2):
> > >    drm/i915: Don't use stolen memory for ring buffers with LLC
> > >    drm/i915: Don't use BAR mappings for ring buffers with LLC
> > > 
> > >   drivers/gpu/drm/i915/gt/intel_ring.c | 6 +++---
> > >   1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
>
Tvrtko Ursulin Feb. 20, 2023, 8:39 a.m. UTC | #4
On 16/02/2023 01:10, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Instruction from hardware arch is that stolen memory and BAR mappings
> are unsafe for use as ring buffers. There can be issues with cache
> aliasing due to the CPU access going to memory via the BAR. So, don't
> do it.
> 
> v2: Dont use BAR mappings either.
> Make conditional on LLC so as not to change platforms that don't need
> to change (Daniele).
> Add 'Fixes' tags (Tvrtko).
> v3: Fix dumb typo.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> 
> 
> John Harrison (2):
>    drm/i915: Don't use stolen memory for ring buffers with LLC
>    drm/i915: Don't use BAR mappings for ring buffers with LLC
> 
>   drivers/gpu/drm/i915/gt/intel_ring.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

It is doing what it laid out as the problem statement so series looks 
good to me.

Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko