[PATCH 9/9] powerpc: A new cache geometry aux vectors

[Paul Clarke]

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;
>  }
>