[PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Lichao Liu]

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

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Jiaxun Yang]

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

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Lichao Liu]

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

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Thomas Bogendoerfer]

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 ]

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[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

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[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.

>> 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.

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Lichao Liu]

在 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.

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Lichao Liu]

在 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.

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Jiaxun Yang]

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.

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Thomas Bogendoerfer]

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 ]

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Thomas Bogendoerfer]

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 ]

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Jiaxun Yang]

于 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

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Thomas Bogendoerfer]

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 ]

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Thomas Bogendoerfer]

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 ]

Re: [PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Jiaxun Yang]

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!

[...]

[PATCH] MIPS: CPU_LOONGSON2EF need software to maintain cache consistency

[Lichao Liu]

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