[PATCH] arm: fix flush_pfn_alias

[PATCH] arm: fix flush_pfn_alias

[Jungseung Lee]

L1_CACHE_BYTES could be larger than real L1 cache line size.
In that case, flush_pfn_alias function would omit to flush last bytes
as much as L1_CACHE_BYTES - real cache line size.

So fix end address to "to + PAGE_SIZE - 1". The bottom bits of the address
is LINELEN. that is ignored by mcrr.

Signed-off-by: Jungseung Lee <js07.lee@gmail.com>
---
 arch/arm/mm/flush.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index 265b836..34b66af 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -33,7 +33,7 @@ static void flush_pfn_alias(unsigned long pfn, unsigned long vaddr)
 	asm(	"mcrr	p15, 0, %1, %0, c14\n"
 	"	mcr	p15, 0, %2, c7, c10, 4"
 	    :
-	    : "r" (to), "r" (to + PAGE_SIZE - L1_CACHE_BYTES), "r" (zero)
+	    : "r" (to), "r" (to + PAGE_SIZE - 1), "r" (zero)
 	    : "cc");
 }
 
-- 
1.9.1

[PATCH] arm: fix flush_pfn_alias

[Arnd Bergmann]

On Monday 20 October 2014 21:54:02 Jungseung Lee wrote:
> L1_CACHE_BYTES could be larger than real L1 cache line size.
> In that case, flush_pfn_alias function would omit to flush last bytes
> as much as L1_CACHE_BYTES - real cache line size.

Can you list an example on what CPU this would happen in the
patch description? Isn't the L1 cache line size always 32 bytes on ARM?

> So fix end address to "to + PAGE_SIZE - 1". The bottom bits of the address
> is LINELEN. that is ignored by mcrr.
> 
> Signed-off-by: Jungseung Lee <js07.lee@gmail.com>

Is this needed in stable backports?

	Arnd

[PATCH] arm: fix flush_pfn_alias

[Will Deacon]

On Mon, Oct 20, 2014 at 02:39:44PM +0100, Arnd Bergmann wrote:
> On Monday 20 October 2014 21:54:02 Jungseung Lee wrote:
> > L1_CACHE_BYTES could be larger than real L1 cache line size.
> > In that case, flush_pfn_alias function would omit to flush last bytes
> > as much as L1_CACHE_BYTES - real cache line size.
> 
> Can you list an example on what CPU this would happen in the
> patch description? Isn't the L1 cache line size always 32 bytes on ARM?

It's 64 bytes on A15, but I suspect it's always 32 bytes for this codepath
(VIPT aliasing D-side).

Will

[PATCH] arm: fix flush_pfn_alias

[Russell King - ARM Linux]

On Mon, Oct 20, 2014 at 03:39:44PM +0200, Arnd Bergmann wrote:
> On Monday 20 October 2014 21:54:02 Jungseung Lee wrote:
> > L1_CACHE_BYTES could be larger than real L1 cache line size.
> > In that case, flush_pfn_alias function would omit to flush last bytes
> > as much as L1_CACHE_BYTES - real cache line size.
> 
> Can you list an example on what CPU this would happen in the
> patch description? Isn't the L1 cache line size always 32 bytes on ARM?

No, there are 64-byte cache lines in some v7 CPUs.

> Is this needed in stable backports?

The MCRR instruction is deprecated (presumably because, as I discovered
in the early days, it can prevent the system making progress under high
interrupt load), but is only used on ARMv6 CPUs with aliasing cachces -
which have a cache line size of 32.  However, when built alongside ARMv7,
it is possible that L1_CACHE_SIZE could be 64.

The patch seems sane on the face of it.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

[PATCH] arm: fix flush_pfn_alias

[Jungseung Lee]

> >> L1_CACHE_BYTES could be larger than real L1 cache line size.
> >> In that case, flush_pfn_alias function would omit to flush last bytes
> >> as much as L1_CACHE_BYTES - real cache line size.
> >
> >Can you list an example on what CPU this would happen in the
> >patch description? Isn't the L1 cache line size always 32 bytes on ARM?
>

It's 64bytes on A15 and 32bytes on A9.
However, L1_CACHE_BYTES is 64 on ARMv7.

>
> > So fix end address to "to + PAGE_SIZE - 1". The bottom bits of the address
> > is LINELEN. that is ignored by mcrr.
> >
> > Signed-off-by: Jungseung Lee <js07.lee@gmail.com>
>
> Is this needed in stable backports?

Maybe no.

As Russell and Will said, it always would be 32bytes for this codpath.
So that would not make any problem. In my CA9 kernel image, the
function is not included also.

But the code that using L1_CACHE_BYTES should be modified ,
since that is not real cache line size and cause misunderstanding.