| Message ID | 1413809642-12931-1-git-send-email-js07.lee@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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.
> >> 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.
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"); }
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(-)