| Message ID | 20170108233150.15353-9-benh@kernel.crashing.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 01/08/2017 05:31 PM, Benjamin Herrenschmidt wrote: > This adds AUX vectors for the L1I,D, L2 and L3 cache levels > providing for each cache level the size of the cache in bytes > and the geometry (line size and number of ways). > > We chose to not use the existing alpha/sh definition which > packs all the information in a single entry per cache level as > it is too restricted to represent some of the geometries used > on POWER. > diff --git a/arch/powerpc/include/asm/cache.h b/arch/powerpc/include/asm/cache.h > index 3987bd9..1557d26 100644 > --- a/arch/powerpc/include/asm/cache.h > +++ b/arch/powerpc/include/asm/cache.h > @@ -35,6 +35,7 @@ struct ppc_cache_info { > u32 log_block_size; > u32 blocks_per_page; > u32 sets; > + u32 assoc; Associativity is defined as u32... > }; > > struct ppc64_caches { > diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h > index 730c27e..a128836 100644 > --- a/arch/powerpc/include/asm/elf.h > +++ b/arch/powerpc/include/asm/elf.h > +extern long il1cache_shape; > +extern long dl1cache_shape; > +extern long l2cache_shape; > +extern long l3cache_shape; shapes are "long"... > @@ -136,6 +140,9 @@ extern int arch_setup_additional_pages(struct linux_binprm *bprm, > > #endif /* CONFIG_SPU_BASE */ > > +#define get_cache_shape(level) \ > + (ppc64_caches.level.assoc << 16 | ppc64_caches.level.line_size) Now we stuff u32 assoc into 16 bits. What's the reason behind combining associativity and cache line size into a single field? Likely the most requested values will be cache line size and (maybe) cache size. The latter is a simple query with these changes, but the former is now a query and some awkward mask/shift. I don't think we're currently providing accessor macros, although I guess those can be added to glibc. Would it not be simpler, though, to just add independent queries for each of cache line size and associativity? - AT_L1I_CACHESIZE - AT_L1I_CACHE_LINE_SIZE - AT_L1I_CACHE_ASSOC etc. Is there a big "con" to having a few more AUXV entries? Regards, PC > @@ -156,6 +163,14 @@ do { \ > NEW_AUX_ENT(AT_ICACHEBSIZE, icache_bsize); \ > NEW_AUX_ENT(AT_UCACHEBSIZE, ucache_bsize); \ > VDSO_AUX_ENT(AT_SYSINFO_EHDR, current->mm->context.vdso_base); \ > + NEW_AUX_ENT(AT_L1I_CACHESIZE, ppc64_caches.l1i.size); \ > + NEW_AUX_ENT(AT_L1I_CACHEGEOMETRY, get_cache_shape(l1i)); \ > + NEW_AUX_ENT(AT_L1D_CACHESIZE, ppc64_caches.l1i.size); \ > + NEW_AUX_ENT(AT_L1D_CACHEGEOMETRY, get_cache_shape(l1i)); \ > + NEW_AUX_ENT(AT_L2_CACHESIZE, ppc64_caches.l2.size); \ > + NEW_AUX_ENT(AT_L2_CACHEGEOMETRY, get_cache_shape(l2)); \ > + NEW_AUX_ENT(AT_L3_CACHESIZE, ppc64_caches.l3.size); \ > + NEW_AUX_ENT(AT_L3_CACHEGEOMETRY, get_cache_shape(l3)); \ > } while (0) > > #endif /* _ASM_POWERPC_ELF_H */ > diff --git a/arch/powerpc/include/uapi/asm/auxvec.h b/arch/powerpc/include/uapi/asm/auxvec.h > index ce17d2c..be6e94e 100644 > --- a/arch/powerpc/include/uapi/asm/auxvec.h > +++ b/arch/powerpc/include/uapi/asm/auxvec.h > @@ -16,6 +16,37 @@ > */ > #define AT_SYSINFO_EHDR 33 > > -#define AT_VECTOR_SIZE_ARCH 6 /* entries in ARCH_DLINFO */ > +/* > + * AT_*CACHEBSIZE above represent the cache *block* size which is > + * the size that is affected by the cache management instructions. > + * > + * It doesn't nececssarily matches the cache *line* size which is > + * more of a performance tuning hint. Additionally the latter can > + * be different for the different cache levels. > + * > + * The set of entries below represent more extensive information > + * about the caches, in the form of two entry per cache type, > + * one entry containing the cache size in bytes, and the other > + * containing the cache line size in bytes in the bottom 16 bits > + * and the cache associativity in the next 16 bits. > + * > + * The associativity is such that if N is the 16-bit value, the > + * cache is N way set associative. A value if 0xffff means fully > + * associative, a value of 1 means directly mapped. > + * > + * For all these fields, a value of 0 means that the information > + * is not known. > + */ > + > +#define AT_L1I_CACHESIZE 40 > +#define AT_L1I_CACHEGEOMETRY 41 > +#define AT_L1D_CACHESIZE 42 > +#define AT_L1D_CACHEGEOMETRY 43 > +#define AT_L2_CACHESIZE 44 > +#define AT_L2_CACHEGEOMETRY 45 > +#define AT_L3_CACHESIZE 46 > +#define AT_L3_CACHEGEOMETRY 47 > + > +#define AT_VECTOR_SIZE_ARCH 14 /* entries in ARCH_DLINFO */ > > #endif > diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c > index e0eeed4..cfa2a06 100644 > --- a/arch/powerpc/kernel/setup-common.c > +++ b/arch/powerpc/kernel/setup-common.c > @@ -94,7 +94,10 @@ EXPORT_SYMBOL_GPL(boot_cpuid); > int dcache_bsize; > int icache_bsize; > int ucache_bsize; > - > +long il1cache_shape = -1; > +long dl1cache_shape = -1; > +long l2cache_shape = -1; > +long l3cache_shape = -1; > > unsigned long klimit = (unsigned long) _end; > > diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c > index b3c93d8..16cb0b7 100644 > --- a/arch/powerpc/kernel/setup_64.c > +++ b/arch/powerpc/kernel/setup_64.c > @@ -451,6 +451,10 @@ static bool __init parse_cache_info(struct device_node *np, > info->block_size = bsize; > info->log_block_size = __ilog2(bsize); > info->blocks_per_page = PAGE_SIZE / bsize; > + if (sets == 0) > + info->assoc = 0xffff; > + else > + info->assoc = size / (sets * lsize); > > return success; > } >
On Tue, 2017-01-10 at 10:16 -0600, Paul Clarke wrote: > On 01/08/2017 05:31 PM, Benjamin Herrenschmidt wrote: > > This adds AUX vectors for the L1I,D, L2 and L3 cache levels > > providing for each cache level the size of the cache in bytes > > and the geometry (line size and number of ways). > > > > We chose to not use the existing alpha/sh definition which > > packs all the information in a single entry per cache level as > > it is too restricted to represent some of the geometries used > > on POWER. > > diff --git a/arch/powerpc/include/asm/cache.h > > b/arch/powerpc/include/asm/cache.h > > index 3987bd9..1557d26 100644 > > --- a/arch/powerpc/include/asm/cache.h > > +++ b/arch/powerpc/include/asm/cache.h > > @@ -35,6 +35,7 @@ struct ppc_cache_info { > > u32 log_block_size; > > u32 blocks_per_page; > > u32 sets; > > + u32 assoc; > > Associativity is defined as u32... > > > }; > > > > struct ppc64_caches { > > diff --git a/arch/powerpc/include/asm/elf.h > > b/arch/powerpc/include/asm/elf.h > > index 730c27e..a128836 100644 > > --- a/arch/powerpc/include/asm/elf.h > > +++ b/arch/powerpc/include/asm/elf.h > > +extern long il1cache_shape; > > +extern long dl1cache_shape; > > +extern long l2cache_shape; > > +extern long l3cache_shape; > > shapes are "long"... Those are obsolete definitions from the previous implementation, I forgot to remove them. > > @@ -136,6 +140,9 @@ extern int arch_setup_additional_pages(struct > > linux_binprm *bprm, > > > > #endif /* CONFIG_SPU_BASE */ > > > > +#define get_cache_shape(level) \ > > + (ppc64_caches.level.assoc << 16 | > > ppc64_caches.level.line_size) > > Now we stuff u32 assoc into 16 bits. Right. Not a huge deal. > What's the reason behind combining associativity and cache line size > into a single field? Because we are already creating way too many AT_* entries, I don't want to add 4 more. > Likely the most requested values will be cache line size and (maybe) > cache size. The latter is a simple query with these changes, but the > former is now a query and some awkward mask/shift. A simple mask, not *that* awkward. > I don't think we're currently providing accessor macros, although I > guess those can be added to glibc. Would it not be simpler, though, > to just add independent queries for each of cache line size and > associativity? > - AT_L1I_CACHESIZE > - AT_L1I_CACHE_LINE_SIZE > - AT_L1I_CACHE_ASSOC > etc. > > Is there a big "con" to having a few more AUXV entries? We already *multiplied* the number of arch entries and I think we're the only arch to do that... at that point, I'm starting to prefer something in the VDSO... Ben. > Regards, > PC > > > @@ -156,6 +163,14 @@ do { > > \ > > NEW_AUX_ENT(AT_ICACHEBSIZE, icache_bsize); > > \ > > NEW_AUX_ENT(AT_UCACHEBSIZE, ucache_bsize); > > \ > > VDSO_AUX_ENT(AT_SYSINFO_EHDR, current->mm- > > >context.vdso_base); \ > > + NEW_AUX_ENT(AT_L1I_CACHESIZE, ppc64_caches.l1i.size); > > \ > > + NEW_AUX_ENT(AT_L1I_CACHEGEOMETRY, get_cache_shape(l1i)); > > \ > > + NEW_AUX_ENT(AT_L1D_CACHESIZE, ppc64_caches.l1i.size); > > \ > > + NEW_AUX_ENT(AT_L1D_CACHEGEOMETRY, get_cache_shape(l1i)); > > \ > > + NEW_AUX_ENT(AT_L2_CACHESIZE, ppc64_caches.l2.size); > > \ > > + NEW_AUX_ENT(AT_L2_CACHEGEOMETRY, get_cache_shape(l2)); > > \ > > + NEW_AUX_ENT(AT_L3_CACHESIZE, ppc64_caches.l3.size); > > \ > > + NEW_AUX_ENT(AT_L3_CACHEGEOMETRY, get_cache_shape(l3)); > > \ > > } while (0) > > > > #endif /* _ASM_POWERPC_ELF_H */ > > diff --git a/arch/powerpc/include/uapi/asm/auxvec.h > > b/arch/powerpc/include/uapi/asm/auxvec.h > > index ce17d2c..be6e94e 100644 > > --- a/arch/powerpc/include/uapi/asm/auxvec.h > > +++ b/arch/powerpc/include/uapi/asm/auxvec.h > > @@ -16,6 +16,37 @@ > > */ > > #define AT_SYSINFO_EHDR 33 > > > > -#define AT_VECTOR_SIZE_ARCH 6 /* entries in ARCH_DLINFO */ > > +/* > > + * AT_*CACHEBSIZE above represent the cache *block* size which is > > + * the size that is affected by the cache management instructions. > > + * > > + * It doesn't nececssarily matches the cache *line* size which is > > + * more of a performance tuning hint. Additionally the latter can > > + * be different for the different cache levels. > > + * > > + * The set of entries below represent more extensive information > > + * about the caches, in the form of two entry per cache type, > > + * one entry containing the cache size in bytes, and the other > > + * containing the cache line size in bytes in the bottom 16 bits > > + * and the cache associativity in the next 16 bits. > > + * > > + * The associativity is such that if N is the 16-bit value, the > > + * cache is N way set associative. A value if 0xffff means fully > > + * associative, a value of 1 means directly mapped. > > + * > > + * For all these fields, a value of 0 means that the information > > + * is not known. > > + */ > > + > > +#define AT_L1I_CACHESIZE 40 > > +#define AT_L1I_CACHEGEOMETRY 41 > > +#define AT_L1D_CACHESIZE 42 > > +#define AT_L1D_CACHEGEOMETRY 43 > > +#define AT_L2_CACHESIZE 44 > > +#define AT_L2_CACHEGEOMETRY 45 > > +#define AT_L3_CACHESIZE 46 > > +#define AT_L3_CACHEGEOMETRY 47 > > + > > +#define AT_VECTOR_SIZE_ARCH 14 /* entries in ARCH_DLINFO */ > > > > #endif > > diff --git a/arch/powerpc/kernel/setup-common.c > > b/arch/powerpc/kernel/setup-common.c > > index e0eeed4..cfa2a06 100644 > > --- a/arch/powerpc/kernel/setup-common.c > > +++ b/arch/powerpc/kernel/setup-common.c > > @@ -94,7 +94,10 @@ EXPORT_SYMBOL_GPL(boot_cpuid); > > int dcache_bsize; > > int icache_bsize; > > int ucache_bsize; > > - > > +long il1cache_shape = -1; > > +long dl1cache_shape = -1; > > +long l2cache_shape = -1; > > +long l3cache_shape = -1; > > > > unsigned long klimit = (unsigned long) _end; > > > > diff --git a/arch/powerpc/kernel/setup_64.c > > b/arch/powerpc/kernel/setup_64.c > > index b3c93d8..16cb0b7 100644 > > --- a/arch/powerpc/kernel/setup_64.c > > +++ b/arch/powerpc/kernel/setup_64.c > > @@ -451,6 +451,10 @@ static bool __init parse_cache_info(struct > > device_node *np, > > info->block_size = bsize; > > info->log_block_size = __ilog2(bsize); > > info->blocks_per_page = PAGE_SIZE / bsize; > > + if (sets == 0) > > + info->assoc = 0xffff; > > + else > > + info->assoc = size / (sets * lsize); > > > > return success; > > } > >
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h > index 730c27e..a128836 100644 > --- a/arch/powerpc/include/asm/elf.h > +++ b/arch/powerpc/include/asm/elf.h > @@ -156,6 +163,14 @@ do { \ > NEW_AUX_ENT(AT_ICACHEBSIZE, icache_bsize); \ > NEW_AUX_ENT(AT_UCACHEBSIZE, ucache_bsize); \ > VDSO_AUX_ENT(AT_SYSINFO_EHDR, current->mm->context.vdso_base); \ > + NEW_AUX_ENT(AT_L1I_CACHESIZE, ppc64_caches.l1i.size); \ > + NEW_AUX_ENT(AT_L1I_CACHEGEOMETRY, get_cache_shape(l1i)); \ > + NEW_AUX_ENT(AT_L1D_CACHESIZE, ppc64_caches.l1i.size); \ > + NEW_AUX_ENT(AT_L1D_CACHEGEOMETRY, get_cache_shape(l1i)); \ > + NEW_AUX_ENT(AT_L2_CACHESIZE, ppc64_caches.l2.size); \ > + NEW_AUX_ENT(AT_L2_CACHEGEOMETRY, get_cache_shape(l2)); \ > + NEW_AUX_ENT(AT_L3_CACHESIZE, ppc64_caches.l3.size); \ > + NEW_AUX_ENT(AT_L3_CACHEGEOMETRY, get_cache_shape(l3)); \ > } while (0) This breaks the 32-bit build: ../fs/binfmt_elf.c: In function ‘create_elf_tables’: ../arch/powerpc/include/asm/elf.h:166:32: error: ‘ppc64_caches’ undeclared (first use in this function) NEW_AUX_ENT(AT_L1I_CACHESIZE, ppc64_caches.l1i.size); \ ^ ../fs/binfmt_elf.c:222:26: note: in definition of macro ‘NEW_AUX_ENT’ elf_info[ei_index++] = val; \ ^~~ ../fs/binfmt_elf.c:232:2: note: in expansion of macro ‘ARCH_DLINFO’ ARCH_DLINFO; ^~~~~~~~~~~ ../arch/powerpc/include/asm/elf.h:166:32: note: each undeclared identifier is reported only once for each function it appears in NEW_AUX_ENT(AT_L1I_CACHESIZE, ppc64_caches.l1i.size); \ ^ ../fs/binfmt_elf.c:222:26: note: in definition of macro ‘NEW_AUX_ENT’ elf_info[ei_index++] = val; \ ^~~ ../fs/binfmt_elf.c:232:2: note: in expansion of macro ‘ARCH_DLINFO’ ARCH_DLINFO; ^~~~~~~~~~~ make[2]: *** [fs/binfmt_elf.o] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [fs] Error 2 make: *** [sub-make] Error 2 Will need some more macro foo. cheers
On Tue, 2017-01-31 at 07:27 +1100, Michael Ellerman wrote:
> Will need some more macro foo.
That or make ppc64_caches ppc_caches, and make it common...
I'll look into it.
Cheers,
Ben.
diff --git a/arch/powerpc/include/asm/cache.h b/arch/powerpc/include/asm/cache.h index 3987bd9..1557d26 100644 --- a/arch/powerpc/include/asm/cache.h +++ b/arch/powerpc/include/asm/cache.h @@ -35,6 +35,7 @@ struct ppc_cache_info { u32 log_block_size; u32 blocks_per_page; u32 sets; + u32 assoc; }; struct ppc64_caches { diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h index 730c27e..a128836 100644 --- a/arch/powerpc/include/asm/elf.h +++ b/arch/powerpc/include/asm/elf.h @@ -108,13 +108,17 @@ do { \ */ # define elf_read_implies_exec(ex, exec_stk) (is_32bit_task() ? \ (exec_stk == EXSTACK_DEFAULT) : 0) -#else +#else # define elf_read_implies_exec(ex, exec_stk) (exec_stk == EXSTACK_DEFAULT) #endif /* __powerpc64__ */ extern int dcache_bsize; extern int icache_bsize; extern int ucache_bsize; +extern long il1cache_shape; +extern long dl1cache_shape; +extern long l2cache_shape; +extern long l3cache_shape; /* vDSO has arch_setup_additional_pages */ #define ARCH_HAS_SETUP_ADDITIONAL_PAGES @@ -136,6 +140,9 @@ extern int arch_setup_additional_pages(struct linux_binprm *bprm, #endif /* CONFIG_SPU_BASE */ +#define get_cache_shape(level) \ + (ppc64_caches.level.assoc << 16 | ppc64_caches.level.line_size) + /* * The requirements here are: * - keep the final alignment of sp (sp & 0xf) @@ -156,6 +163,14 @@ do { \ NEW_AUX_ENT(AT_ICACHEBSIZE, icache_bsize); \ NEW_AUX_ENT(AT_UCACHEBSIZE, ucache_bsize); \ VDSO_AUX_ENT(AT_SYSINFO_EHDR, current->mm->context.vdso_base); \ + NEW_AUX_ENT(AT_L1I_CACHESIZE, ppc64_caches.l1i.size); \ + NEW_AUX_ENT(AT_L1I_CACHEGEOMETRY, get_cache_shape(l1i)); \ + NEW_AUX_ENT(AT_L1D_CACHESIZE, ppc64_caches.l1i.size); \ + NEW_AUX_ENT(AT_L1D_CACHEGEOMETRY, get_cache_shape(l1i)); \ + NEW_AUX_ENT(AT_L2_CACHESIZE, ppc64_caches.l2.size); \ + NEW_AUX_ENT(AT_L2_CACHEGEOMETRY, get_cache_shape(l2)); \ + NEW_AUX_ENT(AT_L3_CACHESIZE, ppc64_caches.l3.size); \ + NEW_AUX_ENT(AT_L3_CACHEGEOMETRY, get_cache_shape(l3)); \ } while (0) #endif /* _ASM_POWERPC_ELF_H */ diff --git a/arch/powerpc/include/uapi/asm/auxvec.h b/arch/powerpc/include/uapi/asm/auxvec.h index ce17d2c..be6e94e 100644 --- a/arch/powerpc/include/uapi/asm/auxvec.h +++ b/arch/powerpc/include/uapi/asm/auxvec.h @@ -16,6 +16,37 @@ */ #define AT_SYSINFO_EHDR 33 -#define AT_VECTOR_SIZE_ARCH 6 /* entries in ARCH_DLINFO */ +/* + * AT_*CACHEBSIZE above represent the cache *block* size which is + * the size that is affected by the cache management instructions. + * + * It doesn't nececssarily matches the cache *line* size which is + * more of a performance tuning hint. Additionally the latter can + * be different for the different cache levels. + * + * The set of entries below represent more extensive information + * about the caches, in the form of two entry per cache type, + * one entry containing the cache size in bytes, and the other + * containing the cache line size in bytes in the bottom 16 bits + * and the cache associativity in the next 16 bits. + * + * The associativity is such that if N is the 16-bit value, the + * cache is N way set associative. A value if 0xffff means fully + * associative, a value of 1 means directly mapped. + * + * For all these fields, a value of 0 means that the information + * is not known. + */ + +#define AT_L1I_CACHESIZE 40 +#define AT_L1I_CACHEGEOMETRY 41 +#define AT_L1D_CACHESIZE 42 +#define AT_L1D_CACHEGEOMETRY 43 +#define AT_L2_CACHESIZE 44 +#define AT_L2_CACHEGEOMETRY 45 +#define AT_L3_CACHESIZE 46 +#define AT_L3_CACHEGEOMETRY 47 + +#define AT_VECTOR_SIZE_ARCH 14 /* entries in ARCH_DLINFO */ #endif diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c index e0eeed4..cfa2a06 100644 --- a/arch/powerpc/kernel/setup-common.c +++ b/arch/powerpc/kernel/setup-common.c @@ -94,7 +94,10 @@ EXPORT_SYMBOL_GPL(boot_cpuid); int dcache_bsize; int icache_bsize; int ucache_bsize; - +long il1cache_shape = -1; +long dl1cache_shape = -1; +long l2cache_shape = -1; +long l3cache_shape = -1; unsigned long klimit = (unsigned long) _end; diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c index b3c93d8..16cb0b7 100644 --- a/arch/powerpc/kernel/setup_64.c +++ b/arch/powerpc/kernel/setup_64.c @@ -451,6 +451,10 @@ static bool __init parse_cache_info(struct device_node *np, info->block_size = bsize; info->log_block_size = __ilog2(bsize); info->blocks_per_page = PAGE_SIZE / bsize; + if (sets == 0) + info->assoc = 0xffff; + else + info->assoc = size / (sets * lsize); return success; }
This adds AUX vectors for the L1I,D, L2 and L3 cache levels providing for each cache level the size of the cache in bytes and the geometry (line size and number of ways). We chose to not use the existing alpha/sh definition which packs all the information in a single entry per cache level as it is too restricted to represent some of the geometries used on POWER. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- arch/powerpc/include/asm/cache.h | 1 + arch/powerpc/include/asm/elf.h | 17 ++++++++++++++++- arch/powerpc/include/uapi/asm/auxvec.h | 33 ++++++++++++++++++++++++++++++++- arch/powerpc/kernel/setup-common.c | 5 ++++- arch/powerpc/kernel/setup_64.c | 4 ++++ 5 files changed, 57 insertions(+), 3 deletions(-)