[U-Boot] [PATCH 1/1] ARM: Move SYS_CACHELINE_SIZE over to Kconfig

[Tom Rini]

On Mon, Aug 22, 2016 at 01:32:52AM +0900, Masahiro Yamada wrote:
> Hi Tom,
> 
> 
> 2016-08-22 0:43 GMT+09:00 Tom Rini <trini at konsulko.com>:
> > On Mon, Aug 22, 2016 at 12:28:19AM +0900, Masahiro Yamada wrote:
> >> Hi Tom,
> >>
> >>
> >>
> >> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >> > index aef901c..15cd66a 100644
> >> > --- a/arch/arm/Kconfig
> >> > +++ b/arch/arm/Kconfig
> >> > @@ -79,6 +79,11 @@ config SYS_ARM_ARCH
> >> >         default 4 if CPU_SA1100
> >> >         default 8 if ARM64
> >> >
> >> > +config SYS_CACHELINE_SIZE
> >> > +       int
> >> > +       default 64 if CPU_V7 || ARM64
> >
> > Note!  I had a brain fart here last night and used 'printf %x' when I
> > thought I was doing 'printf %d', so, no, ARM64 should get moved up to
> > shift 7 / 128 bytes.
> 
> 
> As far as I know, the line size of the cores from ARM Ltd. (CA-53, 57, 72)
> is 64 byte.
> 
> 
> ARM64 Linux increased the line size up to 128 byte for the Cavium's core
> because it is multi-platform.
> 
> 
>  ------------------------------>8--------------------------------------
> 
> commit 97303480753e48fb313dc0e15daaf11b0451cdb8
> Author: Tirumalesh Chalamarla <tchalamarla at cavium.com>
> Date:   Tue Sep 22 19:59:48 2015 +0200
> 
>     arm64: Increase the max granular size
> 
>     Increase the standard cacheline size to avoid having locks in the same
>     cacheline.
> 
>     Cavium's ThunderX core implements cache lines of 128 byte size. With
>     current granulare size of 64 bytes (L1_CACHE_SHIFT=6) two locks could
>     share the same cache line leading a performance degradation.
>     Increasing the size fixes that.
> 
>     Increasing the size has no negative impact to cache invalidation on
>     systems with a smaller cache line. There is an impact on memory usage,
>     but that's not too important for arm64 use cases.
> 
>     Signed-off-by: Tirumalesh Chalamarla <tchalamarla at cavium.com>
>     Signed-off-by: Robert Richter <rrichter at cavium.com>
>     Acked-by: Timur Tabi <timur at codeaurora.org>
>     Signed-off-by: Catalin Marinas <catalin.marinas at arm.com>
> 
> diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
> index bde4499..5082b30 100644
> --- a/arch/arm64/include/asm/cache.h
> +++ b/arch/arm64/include/asm/cache.h
> @@ -18,7 +18,7 @@
> 
>  #include <asm/cachetype.h>
> 
> -#define L1_CACHE_SHIFT         6
> +#define L1_CACHE_SHIFT         7
>  #define L1_CACHE_BYTES         (1 << L1_CACHE_SHIFT)
> 
>  /*
> 
> --------------------------------------8<-------------------------------------
> 
> 
> U-Boot does not adopt multi-platform,
> so we can make the ARCH_THUNDERX select CACHESHIFT_7
> and allow other ARM64 SoCs to stay at CACHESHIFT_6.
> Just my 2 cents.

OK, good point, thanks!

> >> This idea was borrowed from Linux.
> >> (you can grep "_L1_CACHE_SHIFT" in Linux Kconfig files.)
> >
> > I'm agreeable to moving over to shift to more obviously align with the
> > Linux Kernel (and it will make other arches easier to migrate too).  But
> > the UniPhier case currently looks to me like it's overloading what
> > CONFIG_SYS_CACHELINE_SIZE is doing,
> 
> First, I'd like to know if CONFIG_SYS_CACHELINE_SIZE
> means the line size of L1 cache,
> or the largest line size across all the cache levels.

Good question.  This I think ends up being a historical ambiguity.  My
poking around in git log suggests that when CFG_SYS_CACHELINE_SIZE was
added it was implicitly L1.  It's gone on to mean "cache alignment
required for DMA".  It is often defined as L1 and only occasionally
defined differently.  Presumably because it needs to be higher on those
systems for DMA/etc.  In sum, I remove my objection to UniPhier maybe
overloading something that it shouldn't, it's doing the right thing.

> > ie in the Linux Kernel it's not
> > setting the shift to 7, in 32bit.  Or is this a 64bit only feature?
> > Thanks!
> 
> CACHE_UNIPHIER is a full-custom outer cache
> only implemented in its 32bit SoC lineup.
> 
> 
> UniPhier (32bit) is a very unfortunate case
> where outer-cache has a different line size
> than the L1 line size.
> 
> 
> L1  (ARM native, inner):     32byte line (shift 5)
> L2  (UniPhier outer cache):  128byte line (shift 7)
> 
> 
> 
> For Linux, it is just not upstreamed yet.
> 
> 
> If you are interested, check the following:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/400289.html
> 
> 
> If I set the shift to 7, it would affect all the other SoCs in
> multi_v7_defconfig,
> which other vendors would probably be unhappy about.
> 
> Again, it is no problem in U-Boot, which is not multi-platform.

Ah, OK.  So then the question is, are we OK with things like the
following in the .config file:
CONFIG_SYS_CACHE_SHIFT_6=y
CONFIG_SYS_CACHE_SHIFT_7=y
CONFIG_SYS_CACHELINE_SIZE=128

?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160821/bd4c6290/attachment.sig>