* [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
@ 2020-05-26 11:14 Lichao Liu
2020-05-26 11:38 ` Jiaxun Yang
2020-05-27 10:49 ` Thomas Bogendoerfer
0 siblings, 2 replies; 16+ messages in thread
From: Lichao Liu @ 2020-05-26 11:14 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Paul Burton, Robin Murphy, Geert Uytterhoeven, Max Filippov,
Lichao Liu, yuanjunqing, linux-mips
CPU_LOONGSON2EF need software to maintain cache consistency,
so modify the 'cpu_needs_post_dma_flush' function to return true
when the cpu type is CPU_LOONGSON2EF.
---
arch/mips/mm/dma-noncoherent.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/mips/mm/dma-noncoherent.c b/arch/mips/mm/dma-noncoherent.c
index fcea92d95d86..563c2c0d0c81 100644
--- a/arch/mips/mm/dma-noncoherent.c
+++ b/arch/mips/mm/dma-noncoherent.c
@@ -33,6 +33,7 @@ static inline bool cpu_needs_post_dma_flush(void)
case CPU_R10000:
case CPU_R12000:
case CPU_BMIPS5000:
+ case CPU_LOONGSON2EF:
return true;
default:
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-26 11:14 [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency Lichao Liu
@ 2020-05-26 11:38 ` Jiaxun Yang
2020-05-26 12:40 ` Lichao Liu
2020-05-27 10:49 ` Thomas Bogendoerfer
1 sibling, 1 reply; 16+ messages in thread
From: Jiaxun Yang @ 2020-05-26 11:38 UTC (permalink / raw)
To: Lichao Liu
Cc: Thomas Bogendoerfer, Paul Burton, Robin Murphy,
Geert Uytterhoeven, Max Filippov, yuanjunqing, linux-mips
On Tue, 26 May 2020 19:14:38 +0800
Lichao Liu <liulichao@loongson.cn> wrote:
> CPU_LOONGSON2EF need software to maintain cache consistency,
> so modify the 'cpu_needs_post_dma_flush' function to return true
> when the cpu type is CPU_LOONGSON2EF.
Hi Lichao,
I don't think that's required for Loongson-2EF,
According to the comment in code:
The affected CPUs below in 'cpu_needs_post_dma_flush()' can
speculatively
fill random cachelines with stale data at any time, requiring an
extra flush post-DMA.
And according to my understanding that's not going to happen on
Loongson-2EF. We're always allocating coherent DMA memory in uncached
range, Loongson-2EF's writeback policy will ensure it won't writeback
random lines to the memory but only modified dirty lines.
We've been fine without post flush for almost 10 years, there is no
stability issue revealed.
Btw: Please keep me CCed for Loongson-2EF patches. I'm not very active
on 2EF development but I'll still review patches.
Thanks.
> ---
> arch/mips/mm/dma-noncoherent.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/mips/mm/dma-noncoherent.c
> b/arch/mips/mm/dma-noncoherent.c index fcea92d95d86..563c2c0d0c81
> 100644 --- a/arch/mips/mm/dma-noncoherent.c
> +++ b/arch/mips/mm/dma-noncoherent.c
> @@ -33,6 +33,7 @@ static inline bool cpu_needs_post_dma_flush(void)
> case CPU_R10000:
> case CPU_R12000:
> case CPU_BMIPS5000:
> + case CPU_LOONGSON2EF:
> return true;
> default:
> /*
--
Jiaxun Yang
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-26 11:38 ` Jiaxun Yang
@ 2020-05-26 12:40 ` Lichao Liu
2020-05-26 13:01 ` Thomas Bogendoerfer
2020-05-26 13:25 ` Jiaxun Yang
0 siblings, 2 replies; 16+ messages in thread
From: Lichao Liu @ 2020-05-26 12:40 UTC (permalink / raw)
To: Jiaxun Yang
Cc: Thomas Bogendoerfer, Paul Burton, Robin Murphy,
Geert Uytterhoeven, Max Filippov, yuanjunqing, linux-mips
On 2020/5/26 下午7:38, Jiaxun Yang wrote:
> On Tue, 26 May 2020 19:14:38 +0800
> Lichao Liu <liulichao@loongson.cn> wrote:
>
>> CPU_LOONGSON2EF need software to maintain cache consistency,
>> so modify the 'cpu_needs_post_dma_flush' function to return true
>> when the cpu type is CPU_LOONGSON2EF.
>
> Hi Lichao,
>
> I don't think that's required for Loongson-2EF,
>
> According to the comment in code:
>
> The affected CPUs below in 'cpu_needs_post_dma_flush()' can
> speculatively
> fill random cachelines with stale data at any time, requiring an
> extra flush post-DMA.
>
> And according to my understanding that's not going to happen on
> Loongson-2EF. We're always allocating coherent DMA memory in uncached
> range, Loongson-2EF's writeback policy will ensure it won't writeback
> random lines to the memory but only modified dirty lines.
>
> We've been fine without post flush for almost 10 years, there is no
> stability issue revealed.
>
> Btw: Please keep me CCed for Loongson-2EF patches. I'm not very active
> on 2EF development but I'll still review patches.
>
> Thanks.
>
Hi Jiaxun,
Loongson-2EF need software maintain cache consistency, So when using
streaming DMA, software needs to maintain consistency.
dma_map_single() is correct, but dma_unmap_single is wrong.
The function call path:
'dma_unmap_single->dma_unmap_page_attrs->dma_direct_unmap_page->
dma_direct_sync_single_for_cpu->arch_sync_dma_for_cpu->
cpu_needs_post_dma_flush'
In current version, 'cpu_needs_post_dma_flush' will return false
at Loongon-2EF platform, and dma_unmap_single will not invalidate cache,
driver may access wrong dma data.
I don't know what's the exact meaning of "fill random cachelines with
stale data at any time". I always think 'cpu_needs_post_dma_flush()'
means whether this platform needs software to maintain cache consistency.
I found this problem in 4.19.90 kernel's ethernet driver,
and this patch can fix this problem.
Thanks.
>> ---
>> arch/mips/mm/dma-noncoherent.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/mips/mm/dma-noncoherent.c
>> b/arch/mips/mm/dma-noncoherent.c index fcea92d95d86..563c2c0d0c81
>> 100644 --- a/arch/mips/mm/dma-noncoherent.c
>> +++ b/arch/mips/mm/dma-noncoherent.c
>> @@ -33,6 +33,7 @@ static inline bool cpu_needs_post_dma_flush(void)
>> case CPU_R10000:
>> case CPU_R12000:
>> case CPU_BMIPS5000:
>> + case CPU_LOONGSON2EF:
>> return true;
>> default:
>> /*
> --
> Jiaxun Yang
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-26 12:40 ` Lichao Liu
@ 2020-05-26 13:01 ` Thomas Bogendoerfer
2020-05-26 13:29 ` Robin Murphy
2020-05-26 13:25 ` Jiaxun Yang
1 sibling, 1 reply; 16+ messages in thread
From: Thomas Bogendoerfer @ 2020-05-26 13:01 UTC (permalink / raw)
To: Lichao Liu
Cc: Jiaxun Yang, Paul Burton, Robin Murphy, Geert Uytterhoeven,
Max Filippov, yuanjunqing, linux-mips
On Tue, May 26, 2020 at 08:40:28PM +0800, Lichao Liu wrote:
> Loongson-2EF need software maintain cache consistency, So when using
> streaming DMA, software needs to maintain consistency.
>
> dma_map_single() is correct, but dma_unmap_single is wrong.
>
> The function call path:
> 'dma_unmap_single->dma_unmap_page_attrs->dma_direct_unmap_page->
> dma_direct_sync_single_for_cpu->arch_sync_dma_for_cpu->
> cpu_needs_post_dma_flush'
>
> In current version, 'cpu_needs_post_dma_flush' will return false
> at Loongon-2EF platform, and dma_unmap_single will not invalidate cache,
> driver may access wrong dma data.
why should it ? CPU must not touch data while it's mapped for DMA.
> I don't know what's the exact meaning of "fill random cachelines with
> stale data at any time". I always think 'cpu_needs_post_dma_flush()'
> means whether this platform needs software to maintain cache consistency.
this will only happen, if cpu speculates creates dirty cache lines
by speculation as R10k type of CPUs do.
> I found this problem in 4.19.90 kernel's ethernet driver,
> and this patch can fix this problem.
if CPU isn't affected by the R10k speculation problem, it sounds more
like wrong usage of DMA api.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-26 13:01 ` Thomas Bogendoerfer
@ 2020-05-26 13:29 ` Robin Murphy
2020-05-26 15:01 ` Lichao Liu
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Robin Murphy @ 2020-05-26 13:29 UTC (permalink / raw)
To: Thomas Bogendoerfer, Lichao Liu
Cc: Jiaxun Yang, Paul Burton, Geert Uytterhoeven, Max Filippov,
yuanjunqing, linux-mips
On 2020-05-26 14:01, Thomas Bogendoerfer wrote:
> On Tue, May 26, 2020 at 08:40:28PM +0800, Lichao Liu wrote:
>> Loongson-2EF need software maintain cache consistency, So when using
>> streaming DMA, software needs to maintain consistency.
>>
>> dma_map_single() is correct, but dma_unmap_single is wrong.
>>
>> The function call path:
>> 'dma_unmap_single->dma_unmap_page_attrs->dma_direct_unmap_page->
>> dma_direct_sync_single_for_cpu->arch_sync_dma_for_cpu->
>> cpu_needs_post_dma_flush'
>>
>> In current version, 'cpu_needs_post_dma_flush' will return false
>> at Loongon-2EF platform, and dma_unmap_single will not invalidate cache,
>> driver may access wrong dma data.
>
> why should it ? CPU must not touch data while it's mapped for DMA.
>
>> I don't know what's the exact meaning of "fill random cachelines with
>> stale data at any time". I always think 'cpu_needs_post_dma_flush()'
>> means whether this platform needs software to maintain cache consistency.
>
> this will only happen, if cpu speculates creates dirty cache lines
> by speculation as R10k type of CPUs do.
Will it? The usual pattern for this problem is that the CPU
speculatively fills a (clean) cache line after a DMA_FROM_DEVICE
operation has begun, but before the device has actually written to that
part of the buffer. Thus a subsequent CPU read after the operation is
complete can hit in the cache and return the previous data rather than
the updated data that the device wrote. I don't know about MIPS
specifically, but that can certainly happen on Arm.
>> I found this problem in 4.19.90 kernel's ethernet driver,
>> and this patch can fix this problem.
>
> if CPU isn't affected by the R10k speculation problem, it sounds more
> like wrong usage of DMA api.
Sure, explicit accesses to the mapped buffer would be a software error,
but if legitimate non-overlapping accesses to other data nearby can
trigger the CPU to speculatively fetch lines from the DMA-mapped region,
that's generally beyond software's control.
Robin.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-26 13:29 ` Robin Murphy
@ 2020-05-26 15:01 ` Lichao Liu
2020-05-26 15:25 ` Jiaxun Yang
2020-05-26 16:16 ` Thomas Bogendoerfer
2 siblings, 0 replies; 16+ messages in thread
From: Lichao Liu @ 2020-05-26 15:01 UTC (permalink / raw)
To: Robin Murphy, Thomas Bogendoerfer
Cc: Jiaxun Yang, Paul Burton, Geert Uytterhoeven, Max Filippov,
yuanjunqing, linux-mips
在 2020/5/26 下午9:29, Robin Murphy 写道:
> On 2020-05-26 14:01, Thomas Bogendoerfer wrote:
>> On Tue, May 26, 2020 at 08:40:28PM +0800, Lichao Liu wrote:
>>> Loongson-2EF need software maintain cache consistency, So when using
>>> streaming DMA, software needs to maintain consistency.
>>>
>>> dma_map_single() is correct, but dma_unmap_single is wrong.
>>>
>>> The function call path:
>>> 'dma_unmap_single->dma_unmap_page_attrs->dma_direct_unmap_page->
>>> dma_direct_sync_single_for_cpu->arch_sync_dma_for_cpu->
>>> cpu_needs_post_dma_flush'
>>>
>>> In current version, 'cpu_needs_post_dma_flush' will return false
>>> at Loongon-2EF platform, and dma_unmap_single will not invalidate cache,
>>> driver may access wrong dma data.
>>
>> why should it ? CPU must not touch data while it's mapped for DMA.
>>
>>> I don't know what's the exact meaning of "fill random cachelines with
>>> stale data at any time". I always think 'cpu_needs_post_dma_flush()'
>>> means whether this platform needs software to maintain cache consistency.
>>
>> this will only happen, if cpu speculates creates dirty cache lines
>> by speculation as R10k type of CPUs do.
>
> Will it? The usual pattern for this problem is that the CPU speculatively fills a (clean) cache line after a DMA_FROM_DEVICE operation has begun, but before the device has actually written to that part of the buffer. Thus a subsequent CPU read after the operation is complete can hit in the cache and return the previous data rather than the updated data that the device wrote. I don't know about MIPS specifically, but that can certainly happen on Arm.
Yes, I think the essence of this problem is speculative execution.
>
>>> I found this problem in 4.19.90 kernel's ethernet driver,
>>> and this patch can fix this problem.
>>
>> if CPU isn't affected by the R10k speculation problem, it sounds more
>> like wrong usage of DMA api.
>
> Sure, explicit accesses to the mapped buffer would be a software error, but if legitimate non-overlapping accesses to other data nearby can trigger the CPU to speculatively fetch lines from the DMA-mapped region, that's generally beyond software's control.
>
> Robin.
The usage of DMA api is correct, driver only access DMA-mapped region after 'dmp_unmap_single'.
Sorry for the previous wrong message format.
Thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-26 13:29 ` Robin Murphy
2020-05-26 15:01 ` Lichao Liu
@ 2020-05-26 15:25 ` Jiaxun Yang
2020-05-26 16:00 ` Thomas Bogendoerfer
2020-05-26 16:16 ` Thomas Bogendoerfer
2 siblings, 1 reply; 16+ messages in thread
From: Jiaxun Yang @ 2020-05-26 15:25 UTC (permalink / raw)
To: Robin Murphy
Cc: Thomas Bogendoerfer, Lichao Liu, Paul Burton, Geert Uytterhoeven,
Max Filippov, yuanjunqing, linux-mips
On Tue, 26 May 2020 14:29:50 +0100
Robin Murphy <robin.murphy@arm.com> wrote:
> On 2020-05-26 14:01, Thomas Bogendoerfer wrote:
> > On Tue, May 26, 2020 at 08:40:28PM +0800, Lichao Liu wrote:
> >> Loongson-2EF need software maintain cache consistency, So when
> >> using streaming DMA, software needs to maintain consistency.
> >>
> >> dma_map_single() is correct, but dma_unmap_single is wrong.
> >>
> >> The function call path:
> >> 'dma_unmap_single->dma_unmap_page_attrs->dma_direct_unmap_page->
> >> dma_direct_sync_single_for_cpu->arch_sync_dma_for_cpu->
> >> cpu_needs_post_dma_flush'
> >>
> >> In current version, 'cpu_needs_post_dma_flush' will return false
> >> at Loongon-2EF platform, and dma_unmap_single will not invalidate
> >> cache, driver may access wrong dma data.
> >
> > why should it ? CPU must not touch data while it's mapped for DMA.
> >
> >> I don't know what's the exact meaning of "fill random cachelines
> >> with stale data at any time". I always think
> >> 'cpu_needs_post_dma_flush()' means whether this platform needs
> >> software to maintain cache consistency.
> >
> > this will only happen, if cpu speculates creates dirty cache lines
> > by speculation as R10k type of CPUs do.
>
> Will it? The usual pattern for this problem is that the CPU
> speculatively fills a (clean) cache line after a DMA_FROM_DEVICE
> operation has begun, but before the device has actually written to
> that part of the buffer. Thus a subsequent CPU read after the
> operation is complete can hit in the cache and return the previous
> data rather than the updated data that the device wrote. I don't know
> about MIPS specifically, but that can certainly happen on Arm.
Checked the manual of Loongson-2F again and I must admit the case may
happen on Loongson-2EF processor.
R4k manual didn't show the details of speculative policy but I think
that should be applied to all R4k like processors?
So what we need is a post Invalidate after DMA_FROM_DEVICE?
DMA to device should not affected at all.
The origin R10k errata looks like a different story. Both from and to
device is affected because cache line was incorrectly marked as dirty
and it may writeback old data to memory already modified by uncached
write even CPU doesn't perform any cached write to that line.
See details at Page 22 of R10k UM[1].
Not sure if BMIPS here is the same case.
Thanks.
[1]:http://www.ece.mtu.edu/faculty/rmkieckh/cla/4173/REFERENCES/MIPS-R10K-uman1.pdf
- Jiaxun
>
> >> I found this problem in 4.19.90 kernel's ethernet driver,
> >> and this patch can fix this problem.
> >
> > if CPU isn't affected by the R10k speculation problem, it sounds
> > more like wrong usage of DMA api.
>
> Sure, explicit accesses to the mapped buffer would be a software
> error, but if legitimate non-overlapping accesses to other data
> nearby can trigger the CPU to speculatively fetch lines from the
> DMA-mapped region, that's generally beyond software's control.
>
> Robin.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-26 15:25 ` Jiaxun Yang
@ 2020-05-26 16:00 ` Thomas Bogendoerfer
2020-05-26 16:42 ` Jiaxun Yang
0 siblings, 1 reply; 16+ messages in thread
From: Thomas Bogendoerfer @ 2020-05-26 16:00 UTC (permalink / raw)
To: Jiaxun Yang
Cc: Robin Murphy, Lichao Liu, Paul Burton, Geert Uytterhoeven,
Max Filippov, yuanjunqing, linux-mips
On Tue, May 26, 2020 at 11:25:56PM +0800, Jiaxun Yang wrote:
> On Tue, 26 May 2020 14:29:50 +0100
> Robin Murphy <robin.murphy@arm.com> wrote:
>
> > On 2020-05-26 14:01, Thomas Bogendoerfer wrote:
> > > On Tue, May 26, 2020 at 08:40:28PM +0800, Lichao Liu wrote:
> > >> Loongson-2EF need software maintain cache consistency, So when
> > >> using streaming DMA, software needs to maintain consistency.
> > >>
> > >> dma_map_single() is correct, but dma_unmap_single is wrong.
> > >>
> > >> The function call path:
> > >> 'dma_unmap_single->dma_unmap_page_attrs->dma_direct_unmap_page->
> > >> dma_direct_sync_single_for_cpu->arch_sync_dma_for_cpu->
> > >> cpu_needs_post_dma_flush'
> > >>
> > >> In current version, 'cpu_needs_post_dma_flush' will return false
> > >> at Loongon-2EF platform, and dma_unmap_single will not invalidate
> > >> cache, driver may access wrong dma data.
> > >
> > > why should it ? CPU must not touch data while it's mapped for DMA.
> > >
> > >> I don't know what's the exact meaning of "fill random cachelines
> > >> with stale data at any time". I always think
> > >> 'cpu_needs_post_dma_flush()' means whether this platform needs
> > >> software to maintain cache consistency.
> > >
> > > this will only happen, if cpu speculates creates dirty cache lines
> > > by speculation as R10k type of CPUs do.
> >
> > Will it? The usual pattern for this problem is that the CPU
> > speculatively fills a (clean) cache line after a DMA_FROM_DEVICE
> > operation has begun, but before the device has actually written to
> > that part of the buffer. Thus a subsequent CPU read after the
> > operation is complete can hit in the cache and return the previous
> > data rather than the updated data that the device wrote. I don't know
> > about MIPS specifically, but that can certainly happen on Arm.
>
> Checked the manual of Loongson-2F again and I must admit the case may
> happen on Loongson-2EF processor.
so the patch is correct ?
>
> R4k manual didn't show the details of speculative policy but I think
> that should be applied to all R4k like processors?
R4k doesn't speculate at all.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-26 16:00 ` Thomas Bogendoerfer
@ 2020-05-26 16:42 ` Jiaxun Yang
0 siblings, 0 replies; 16+ messages in thread
From: Jiaxun Yang @ 2020-05-26 16:42 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Robin Murphy, Lichao Liu, Paul Burton, Geert Uytterhoeven,
Max Filippov, yuanjunqing, linux-mips
于 2020年5月27日 GMT+08:00 上午12:00:45, Thomas Bogendoerfer <tsbogend@alpha.franken.de> 写到:
>On Tue, May 26, 2020 at 11:25:56PM +0800, Jiaxun Yang wrote:
>> On Tue, 26 May 2020 14:29:50 +0100
>> Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> > On 2020-05-26 14:01, Thomas Bogendoerfer wrote:
>> > > On Tue, May 26, 2020 at 08:40:28PM +0800, Lichao Liu wrote:
>> > >> Loongson-2EF need software maintain cache consistency, So when
>> > >> using streaming DMA, software needs to maintain consistency.
>> > >>
>> > >> dma_map_single() is correct, but dma_unmap_single is wrong.
>> > >>
>> > >> The function call path:
>> > >> 'dma_unmap_single->dma_unmap_page_attrs->dma_direct_unmap_page->
>> > >> dma_direct_sync_single_for_cpu->arch_sync_dma_for_cpu->
>> > >> cpu_needs_post_dma_flush'
>> > >>
>> > >> In current version, 'cpu_needs_post_dma_flush' will return false
>> > >> at Loongon-2EF platform, and dma_unmap_single will not invalidate
>> > >> cache, driver may access wrong dma data.
>> > >
>> > > why should it ? CPU must not touch data while it's mapped for DMA.
>> > >
>> > >> I don't know what's the exact meaning of "fill random cachelines
>> > >> with stale data at any time". I always think
>> > >> 'cpu_needs_post_dma_flush()' means whether this platform needs
>> > >> software to maintain cache consistency.
>> > >
>> > > this will only happen, if cpu speculates creates dirty cache lines
>> > > by speculation as R10k type of CPUs do.
>> >
>> > Will it? The usual pattern for this problem is that the CPU
>> > speculatively fills a (clean) cache line after a DMA_FROM_DEVICE
>> > operation has begun, but before the device has actually written to
>> > that part of the buffer. Thus a subsequent CPU read after the
>> > operation is complete can hit in the cache and return the previous
>> > data rather than the updated data that the device wrote. I don't know
>> > about MIPS specifically, but that can certainly happen on Arm.
>>
>> Checked the manual of Loongson-2F again and I must admit the case may
>> happen on Loongson-2EF processor.
>
>so the patch is correct ?
I'm not sure. If we only have to care the case raised by Robin
then we just need to care DMA_FROM_DEVICE.
But sync unconditionally seems much more safer.
Thanks.
>>
>> R4k manual didn't show the details of speculative policy but I think
>> that should be applied to all R4k like processors?
>
>R4k doesn't speculate at all.
>
>Thomas.
>
--
Jiaxun Yang
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-26 13:29 ` Robin Murphy
2020-05-26 15:01 ` Lichao Liu
2020-05-26 15:25 ` Jiaxun Yang
@ 2020-05-26 16:16 ` Thomas Bogendoerfer
2 siblings, 0 replies; 16+ messages in thread
From: Thomas Bogendoerfer @ 2020-05-26 16:16 UTC (permalink / raw)
To: Robin Murphy
Cc: Lichao Liu, Jiaxun Yang, Paul Burton, Geert Uytterhoeven,
Max Filippov, yuanjunqing, linux-mips
On Tue, May 26, 2020 at 02:29:50PM +0100, Robin Murphy wrote:
> On 2020-05-26 14:01, Thomas Bogendoerfer wrote:
> >On Tue, May 26, 2020 at 08:40:28PM +0800, Lichao Liu wrote:
> >>Loongson-2EF need software maintain cache consistency, So when using
> >>streaming DMA, software needs to maintain consistency.
> >>
> >>dma_map_single() is correct, but dma_unmap_single is wrong.
> >>
> >>The function call path:
> >>'dma_unmap_single->dma_unmap_page_attrs->dma_direct_unmap_page->
> >> dma_direct_sync_single_for_cpu->arch_sync_dma_for_cpu->
> >> cpu_needs_post_dma_flush'
> >>
> >>In current version, 'cpu_needs_post_dma_flush' will return false
> >>at Loongon-2EF platform, and dma_unmap_single will not invalidate cache,
> >>driver may access wrong dma data.
> >
> >why should it ? CPU must not touch data while it's mapped for DMA.
> >
> >>I don't know what's the exact meaning of "fill random cachelines with
> >>stale data at any time". I always think 'cpu_needs_post_dma_flush()'
> >>means whether this platform needs software to maintain cache consistency.
> >
> >this will only happen, if cpu speculates creates dirty cache lines
> >by speculation as R10k type of CPUs do.
>
> Will it? The usual pattern for this problem is that the CPU speculatively
> fills a (clean) cache line after a DMA_FROM_DEVICE operation has begun, but
you are right, it will already happen on speculative read accesses.
And that's taken care by the cache flush after dma_umap.
R10ks have the additional problem that they will sepculate writes to the
cache, which will leave the cache line dirty even when the speculation was
wrong. So there is race that the cache line is written back to memory
after DMA transfer and before the cache flush in dma_unmap. Solution
to this is either cache coherence in hardware or the special gcc option,
which generates speculation barriers before memory writes.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-26 12:40 ` Lichao Liu
2020-05-26 13:01 ` Thomas Bogendoerfer
@ 2020-05-26 13:25 ` Jiaxun Yang
2020-05-26 14:50 ` Lichao Liu
1 sibling, 1 reply; 16+ messages in thread
From: Jiaxun Yang @ 2020-05-26 13:25 UTC (permalink / raw)
To: Lichao Liu
Cc: Thomas Bogendoerfer, Paul Burton, Robin Murphy,
Geert Uytterhoeven, Max Filippov, yuanjunqing, linux-mips
On Tue, 26 May 2020 20:40:28 +0800
Lichao Liu <liulichao@loongson.cn> wrote:
> On 2020/5/26 下午7:38, Jiaxun Yang wrote:
> > On Tue, 26 May 2020 19:14:38 +0800
> > Lichao Liu <liulichao@loongson.cn> wrote:
> >
> >> CPU_LOONGSON2EF need software to maintain cache consistency,
> >> so modify the 'cpu_needs_post_dma_flush' function to return true
> >> when the cpu type is CPU_LOONGSON2EF.
> >
> > Hi Lichao,
> >
> > I don't think that's required for Loongson-2EF,
> >
> > According to the comment in code:
> >
> > The affected CPUs below in 'cpu_needs_post_dma_flush()' can
> > speculatively
> > fill random cachelines with stale data at any time, requiring an
> > extra flush post-DMA.
> >
> > And according to my understanding that's not going to happen on
> > Loongson-2EF. We're always allocating coherent DMA memory in
> > uncached range, Loongson-2EF's writeback policy will ensure it
> > won't writeback random lines to the memory but only modified dirty
> > lines.
> >
> > We've been fine without post flush for almost 10 years, there is no
> > stability issue revealed.
> >
> > Btw: Please keep me CCed for Loongson-2EF patches. I'm not very
> > active on 2EF development but I'll still review patches.
> >
> > Thanks.
> >
> Hi Jiaxun,
>
> Loongson-2EF need software maintain cache consistency, So when using
> streaming DMA, software needs to maintain consistency.
>
> dma_map_single() is correct, but dma_unmap_single is wrong.
>
> The function call path:
> 'dma_unmap_single->dma_unmap_page_attrs->dma_direct_unmap_page->
> dma_direct_sync_single_for_cpu->arch_sync_dma_for_cpu->
> cpu_needs_post_dma_flush'
>
> In current version, 'cpu_needs_post_dma_flush' will return false
> at Loongon-2EF platform, and dma_unmap_single will not invalidate
> cache, driver may access wrong dma data.
>
> I don't know what's the exact meaning of "fill random cachelines with
> stale data at any time". I always think 'cpu_needs_post_dma_flush()'
> means whether this platform needs software to maintain cache
> consistency.
>
> I found this problem in 4.19.90 kernel's ethernet driver,
> and this patch can fix this problem.
Which machine with which ethernet card?
My Fuloong 2F is still serving as printer server and looks like there
is no stability issue.
Thanks.
--
Jiaxun Yang
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-26 13:25 ` Jiaxun Yang
@ 2020-05-26 14:50 ` Lichao Liu
0 siblings, 0 replies; 16+ messages in thread
From: Lichao Liu @ 2020-05-26 14:50 UTC (permalink / raw)
To: Jiaxun Yang
Cc: Thomas Bogendoerfer, Paul Burton, Robin Murphy,
Geert Uytterhoeven, Max Filippov, yuanjunqing, linux-mips
在 2020/5/26 下午9:25, Jiaxun Yang 写道:
> On Tue, 26 May 2020 20:40:28 +0800
> Lichao Liu <liulichao@loongson.cn> wrote:
>
>> On 2020/5/26 下午7:38, Jiaxun Yang wrote:
>>> On Tue, 26 May 2020 19:14:38 +0800
>>> Lichao Liu <liulichao@loongson.cn> wrote:
>>>
>>>> CPU_LOONGSON2EF need software to maintain cache consistency,
>>>> so modify the 'cpu_needs_post_dma_flush' function to return true
>>>> when the cpu type is CPU_LOONGSON2EF.
>>> Hi Lichao,
>>>
>>> I don't think that's required for Loongson-2EF,
>>>
>>> According to the comment in code:
>>>
>>> The affected CPUs below in 'cpu_needs_post_dma_flush()' can
>>> speculatively
>>> fill random cachelines with stale data at any time, requiring an
>>> extra flush post-DMA.
>>>
>>> And according to my understanding that's not going to happen on
>>> Loongson-2EF. We're always allocating coherent DMA memory in
>>> uncached range, Loongson-2EF's writeback policy will ensure it
>>> won't writeback random lines to the memory but only modified dirty
>>> lines.
>>>
>>> We've been fine without post flush for almost 10 years, there is no
>>> stability issue revealed.
>>>
>>> Btw: Please keep me CCed for Loongson-2EF patches. I'm not very
>>> active on 2EF development but I'll still review patches.
>>>
>>> Thanks.
>>>
>> Hi Jiaxun,
>>
>> Loongson-2EF need software maintain cache consistency, So when using
>> streaming DMA, software needs to maintain consistency.
>>
>> dma_map_single() is correct, but dma_unmap_single is wrong.
>>
>> The function call path:
>> 'dma_unmap_single->dma_unmap_page_attrs->dma_direct_unmap_page->
>> dma_direct_sync_single_for_cpu->arch_sync_dma_for_cpu->
>> cpu_needs_post_dma_flush'
>>
>> In current version, 'cpu_needs_post_dma_flush' will return false
>> at Loongon-2EF platform, and dma_unmap_single will not invalidate
>> cache, driver may access wrong dma data.
>>
>> I don't know what's the exact meaning of "fill random cachelines with
>> stale data at any time". I always think 'cpu_needs_post_dma_flush()'
>> means whether this platform needs software to maintain cache
>> consistency.
>>
>> I found this problem in 4.19.90 kernel's ethernet driver,
>> and this patch can fix this problem.
>
> Which machine with which ethernet card?
>
> My Fuloong 2F is still serving as printer server and looks like there
> is no stability issue.
>
> Thanks.
> --
> Jiaxun Yang
I think a common intel igb or e1000 nic can reproduce the problem,
I have encountered this problem on both 2.6 and 4.19 kernel.
Try use tftp to get a large file, such as 200M, then compare their md5.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-26 11:14 [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency Lichao Liu
2020-05-26 11:38 ` Jiaxun Yang
@ 2020-05-27 10:49 ` Thomas Bogendoerfer
1 sibling, 0 replies; 16+ messages in thread
From: Thomas Bogendoerfer @ 2020-05-27 10:49 UTC (permalink / raw)
To: Lichao Liu
Cc: Paul Burton, Robin Murphy, Geert Uytterhoeven, Max Filippov,
yuanjunqing, linux-mips
On Tue, May 26, 2020 at 07:14:38PM +0800, Lichao Liu wrote:
> CPU_LOONGSON2EF need software to maintain cache consistency,
> so modify the 'cpu_needs_post_dma_flush' function to return true
> when the cpu type is CPU_LOONGSON2EF.
> ---
> arch/mips/mm/dma-noncoherent.c | 1 +
> 1 file changed, 1 insertion(+)
Please add your signed-off-by.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
@ 2020-05-28 1:10 Lichao Liu
2020-05-28 6:05 ` Jiaxun Yang
2020-05-28 7:44 ` Thomas Bogendoerfer
0 siblings, 2 replies; 16+ messages in thread
From: Lichao Liu @ 2020-05-28 1:10 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Paul Burton, Robin Murphy, Geert Uytterhoeven, Max Filippov,
Lichao Liu, jiaxun.yang, yuanjunqing, linux-mips, stable
CPU_LOONGSON2EF need software to maintain cache consistency,
so modify the 'cpu_needs_post_dma_flush' function to return true
when the cpu type is CPU_LOONGSON2EF.
Cc: stable@vger.kernel.org
Signed-off-by: Lichao Liu <liulichao@loongson.cn>
---
arch/mips/mm/dma-noncoherent.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/mips/mm/dma-noncoherent.c b/arch/mips/mm/dma-noncoherent.c
index fcea92d95d86..563c2c0d0c81 100644
--- a/arch/mips/mm/dma-noncoherent.c
+++ b/arch/mips/mm/dma-noncoherent.c
@@ -33,6 +33,7 @@ static inline bool cpu_needs_post_dma_flush(void)
case CPU_R10000:
case CPU_R12000:
case CPU_BMIPS5000:
+ case CPU_LOONGSON2EF:
return true;
default:
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-28 1:10 Lichao Liu
@ 2020-05-28 6:05 ` Jiaxun Yang
2020-05-28 7:44 ` Thomas Bogendoerfer
1 sibling, 0 replies; 16+ messages in thread
From: Jiaxun Yang @ 2020-05-28 6:05 UTC (permalink / raw)
To: Lichao Liu
Cc: Thomas Bogendoerfer, Paul Burton, Robin Murphy,
Geert Uytterhoeven, Max Filippov, yuanjunqing, linux-mips, stable
On Thu, 28 May 2020 09:10:31 +0800
Lichao Liu <liulichao@loongson.cn> wrote:
> CPU_LOONGSON2EF need software to maintain cache consistency,
> so modify the 'cpu_needs_post_dma_flush' function to return true
> when the cpu type is CPU_LOONGSON2EF.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Lichao Liu <liulichao@loongson.cn>
Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Thanks!
[...]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency
2020-05-28 1:10 Lichao Liu
2020-05-28 6:05 ` Jiaxun Yang
@ 2020-05-28 7:44 ` Thomas Bogendoerfer
1 sibling, 0 replies; 16+ messages in thread
From: Thomas Bogendoerfer @ 2020-05-28 7:44 UTC (permalink / raw)
To: Lichao Liu
Cc: Paul Burton, Robin Murphy, Geert Uytterhoeven, Max Filippov,
jiaxun.yang, yuanjunqing, linux-mips, stable
On Thu, May 28, 2020 at 09:10:31AM +0800, Lichao Liu wrote:
> CPU_LOONGSON2EF need software to maintain cache consistency,
> so modify the 'cpu_needs_post_dma_flush' function to return true
> when the cpu type is CPU_LOONGSON2EF.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Lichao Liu <liulichao@loongson.cn>
> ---
> arch/mips/mm/dma-noncoherent.c | 1 +
> 1 file changed, 1 insertion(+)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-05-28 7:46 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-26 11:14 [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency Lichao Liu
2020-05-26 11:38 ` Jiaxun Yang
2020-05-26 12:40 ` Lichao Liu
2020-05-26 13:01 ` Thomas Bogendoerfer
2020-05-26 13:29 ` Robin Murphy
2020-05-26 15:01 ` Lichao Liu
2020-05-26 15:25 ` Jiaxun Yang
2020-05-26 16:00 ` Thomas Bogendoerfer
2020-05-26 16:42 ` Jiaxun Yang
2020-05-26 16:16 ` Thomas Bogendoerfer
2020-05-26 13:25 ` Jiaxun Yang
2020-05-26 14:50 ` Lichao Liu
2020-05-27 10:49 ` Thomas Bogendoerfer
-- strict thread matches above, loose matches on Subject: below --
2020-05-28 1:10 Lichao Liu
2020-05-28 6:05 ` Jiaxun Yang
2020-05-28 7:44 ` Thomas Bogendoerfer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox