diff mbox

[9/9] powerpc: A new cache shape aux vectors

Message ID 20170104051535.9454-9-benh@kernel.crashing.org (mailing list archive)
State Superseded
Headers show

Commit Message

Benjamin Herrenschmidt Jan. 4, 2017, 5:15 a.m. UTC
The definition is loosely based on sh and alpha, modified to
accomodate larger associativity and cache size for future-proofing.

We currently set all the values to -1 which indicates that the
information isn't available.

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(-)

Comments

Tulio Magno Quites Machado Filho Jan. 4, 2017, 1:04 p.m. UTC | #1
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:


> diff --git a/arch/powerpc/include/uapi/asm/auxvec.h b/arch/powerpc/include/uapi/asm/auxvec.h
> index ce17d2c..79183d2 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_CACHESHAPE	41
> +#define AT_L1D_CACHESIZE	42
> +#define AT_L1D_CACHESHAPE	43
> +#define AT_L2_CACHESIZE		44
> +#define AT_L2_CACHESHAPE	45
> +#define AT_L3_CACHESIZE		46
> +#define AT_L3_CACHESHAPE	47

These names will clash with the other ones defined by alpha and sh:

/* Shapes of the caches.  Bits 0-3 contains associativity; bits 4-7 contains
   log2 of line size; mask those to get cache size.  */
#define AT_L1I_CACHESHAPE	34
#define AT_L1D_CACHESHAPE	35
#define AT_L2_CACHESHAPE	36
#define AT_L3_CACHESHAPE	37
Benjamin Herrenschmidt Jan. 4, 2017, 10:03 p.m. UTC | #2
On Wed, 2017-01-04 at 11:04 -0200, Tulio Magno Quites Machado Filho

> > +#define AT_L1I_CACHESIZE	40
> > +#define AT_L1I_CACHESHAPE	41
> > +#define AT_L1D_CACHESIZE	42
> > +#define AT_L1D_CACHESHAPE	43
> > +#define AT_L2_CACHESIZE		44
> > +#define AT_L2_CACHESHAPE	45
> > +#define AT_L3_CACHESIZE		46
> > +#define AT_L3_CACHESHAPE	47
> 
> These names will clash with the other ones defined by alpha and sh:
> 
> /* Shapes of the caches.  Bits 0-3 contains associativity; bits 4-7
> contains
>    log2 of line size; mask those to get cache size.  */
> #define AT_L1I_CACHESHAPE	34
> #define AT_L1D_CACHESHAPE	35
> #define AT_L2_CACHESHAPE	36
> #define AT_L3_CACHESHAPE	37

Is this a problem though ? In the kernel at least these are defined in
arch specific headers so there is no clash.

Otherwise, I can change them to *_CACHEGEOMETRY, is that ok ? Or add a
PPC_ prefix...

Cheers,
Ben.
Tulio Magno Quites Machado Filho Jan. 5, 2017, 11:15 a.m. UTC | #3
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Wed, 2017-01-04 at 11:04 -0200, Tulio Magno Quites Machado Filho
>
>> > +#define AT_L1I_CACHESIZE	40
>> > +#define AT_L1I_CACHESHAPE	41
>> > +#define AT_L1D_CACHESIZE	42
>> > +#define AT_L1D_CACHESHAPE	43
>> > +#define AT_L2_CACHESIZE		44
>> > +#define AT_L2_CACHESHAPE	45
>> > +#define AT_L3_CACHESIZE		46
>> > +#define AT_L3_CACHESHAPE	47
>> 
>> These names will clash with the other ones defined by alpha and sh:
>> 
>> /* Shapes of the caches.  Bits 0-3 contains associativity; bits 4-7
>> contains
>>    log2 of line size; mask those to get cache size.  */
>> #define AT_L1I_CACHESHAPE	34
>> #define AT_L1D_CACHESHAPE	35
>> #define AT_L2_CACHESHAPE	36
>> #define AT_L3_CACHESHAPE	37
>
> Is this a problem though ? In the kernel at least these are defined in
> arch specific headers so there is no clash.

It could become a problem if an architecture tries to use both of these
types.

glibc doesn't distinct between them:
https://sourceware.org/git/?p=glibc.git;a=blob;f=elf/elf.h#l1113

> Otherwise, I can change them to *_CACHEGEOMETRY, is that ok ?

Looks good to me.
Segher Boessenkool Jan. 10, 2017, 1:15 p.m. UTC | #4
On Wed, Jan 04, 2017 at 04:15:35PM +1100, Benjamin Herrenschmidt wrote:
> + * For all these fields, a value of 0 means that the information
> + * is not known.

> +	if (sets == 0)
> +		info->assoc = 0xffff;
> +	else
...

Is this correct?  A fully associative cache should be encoded as sets == 1,
if there are buggy firmwares that set it to 0 instead it should do the same
as 1 here I think?  Or is 0xffff indicating "unknown" here...  But in that
case the comment above says assoc should be 0 instead.


Segher
Benjamin Herrenschmidt Jan. 10, 2017, 3:18 p.m. UTC | #5
On Tue, 2017-01-10 at 07:15 -0600, Segher Boessenkool wrote:
> On Wed, Jan 04, 2017 at 04:15:35PM +1100, Benjamin Herrenschmidt wrote:
> > + * For all these fields, a value of 0 means that the information
> > + * is not known.
> > +     if (sets == 0)
> > +             info->assoc = 0xffff;
> > +     else
> 
> ...
> 
> Is this correct?  A fully associative cache should be encoded as sets == 1,
> if there are buggy firmwares that set it to 0 instead it should do the same
> as 1 here I think?

No, I just made a stupid mistake I think. PAPR does say sets=1 is fully
associative which makes sense. Probably a typo.

>   Or is 0xffff indicating "unknown" here...  But in that
> case the comment above says assoc should be 0 instead.

No it's me.

Cheers,
Ben.
diff mbox

Patch

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..27dece6 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_CACHESHAPE, get_cache_shape(l1i));		\
+	NEW_AUX_ENT(AT_L1D_CACHESIZE, ppc64_caches.l1i.size);		\
+	NEW_AUX_ENT(AT_L1D_CACHESHAPE, get_cache_shape(l1i));		\
+	NEW_AUX_ENT(AT_L2_CACHESIZE, ppc64_caches.l2.size);		\
+	NEW_AUX_ENT(AT_L2_CACHESHAPE, get_cache_shape(l2));		\
+	NEW_AUX_ENT(AT_L3_CACHESIZE, ppc64_caches.l3.size);		\
+	NEW_AUX_ENT(AT_L3_CACHESHAPE, 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..79183d2 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_CACHESHAPE	41
+#define AT_L1D_CACHESIZE	42
+#define AT_L1D_CACHESHAPE	43
+#define AT_L2_CACHESIZE		44
+#define AT_L2_CACHESHAPE	45
+#define AT_L3_CACHESIZE		46
+#define AT_L3_CACHESHAPE	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;
 }