| Message ID | CAK7LNASNzbCyLcRUS6gkHPDXYgLt_9upqSQBNEMX1baZCAS3jA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Hi Tom, 2016-08-22 5:18 GMT+09:00 Tom Rini <trini@konsulko.com>: > 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@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@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@cavium.com> >> Signed-off-by: Robert Richter <rrichter@cavium.com> >> Acked-by: Timur Tabi <timur@codeaurora.org> >> Signed-off-by: Catalin Marinas <catalin.marinas@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. Right. Given that likely(CONFIG_SYS_CACHELINE_SIZE == ARCH_DMA_ALIGN) I have to set CONFIG_SYS_CACHELINE_SIZE to 128 when the outer cache is enabled because DMA engines are located outside of the cache topology. >> > 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 > > ? Yes, I think it is OK. -- Best Regards Masahiro Yamada _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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) /*