powerpc: Support auxilliary vector components for cache geometries

Submitted by Paul Clarke on March 9, 2017, 3:57 p.m.

Details

Message ID 09eaab05-8cfe-a800-a033-750a02e417a5@us.ibm.com
State New
Headers show

Commit Message

Paul Clarke March 9, 2017, 3:57 p.m.
Use getauxval() to get L1, L2, L3 cache sizes, cache line sizes, and
cache associativities.  The new types for getauxval() were added in
the stream for Linux kernel v4.11 in commit
98a5f361b8625c6f4841d6ba013bbf0e80d08147.

Add test case which retrieves above values using getauxval().

	* elf/elf.h: Add auxvec identifiers from kernel
	arch/powerpc/include/uapi/asm/auxvec.h.
	* glibc/sysdeps/powerpc/tst-getauxval.c: New test to
	retrieve new auxvec values.
	* glibc/sysdeps/powerpc/Makefile (tests): Add tst-getauxval.
---
  elf/elf.h                       | 12 ++++++
  sysdeps/powerpc/Makefile        |  1 +
  sysdeps/powerpc/tst-getauxval.c | 86 +++++++++++++++++++++++++++++++++++++++++
  3 files changed, 99 insertions(+)
  create mode 100644 sysdeps/powerpc/tst-getauxval.c

Comments

Mike Frysinger March 12, 2017, 2:01 a.m.
On 09 Mar 2017 09:57, Paul Clarke wrote:
> Use getauxval() to get L1, L2, L3 cache sizes, cache line sizes, and
> cache associativities.  The new types for getauxval() were added in
> the stream for Linux kernel v4.11 in commit
> 98a5f361b8625c6f4841d6ba013bbf0e80d08147.
> 
> Add test case which retrieves above values using getauxval().
> 
> 	* elf/elf.h: Add auxvec identifiers from kernel
> 	arch/powerpc/include/uapi/asm/auxvec.h.
> 	* glibc/sysdeps/powerpc/tst-getauxval.c: New test to
> 	retrieve new auxvec values.
> 	* glibc/sysdeps/powerpc/Makefile (tests): Add tst-getauxval.

the defines have been added to the common elf/elf.h.  and the way
you've written the test is not arch specific.  so should it live
in the common code paths instead of under powerpc/ ?

also, there is no "glibc/" subdir.

> +static int
> +do_test (void)
> +{
> +  int rc = 0;
> +  unsigned long val;
> +  val = getauxval (AT_L1I_CACHESIZE);
> +  if (val)
> +    printf("AT_L1I_CACHESIZE: %ld (0x%lx)\n",val,val);

bad whitespace/style here.  this comes up multiple times.

also, you want %lu, not %ld, with val.  this comes up multiple times.

> +  val = getauxval (AT_L1I_CACHEGEOMETRY);
> +  if (val)
> +    printf("AT_L1I_CACHEGEOMETRY: associativity %ld; line size %ld\n",
> +	   (val & 0xffff0000) >> 16, val & 0x0000ffff);
> +  else
> +    rc = EXIT_UNSUPPORTED;

honestly, what is the value of this test ?  you basically just
printf the values everywhere, or you exit unsupported.  there is
no actual "test" here that i can see as you don't validate the
results anywhere.
-mike
Paul Clarke March 13, 2017, 2:20 a.m.
On 03/11/2017 08:01 PM, Mike Frysinger wrote:
> On 09 Mar 2017 09:57, Paul Clarke wrote:
>> Use getauxval() to get L1, L2, L3 cache sizes, cache line sizes, and
>> cache associativities.  The new types for getauxval() were added in
>> the stream for Linux kernel v4.11 in commit
>> 98a5f361b8625c6f4841d6ba013bbf0e80d08147.
>>
>> Add test case which retrieves above values using getauxval().
>>
>> 	* elf/elf.h: Add auxvec identifiers from kernel
>> 	arch/powerpc/include/uapi/asm/auxvec.h.
>> 	* glibc/sysdeps/powerpc/tst-getauxval.c: New test to
>> 	retrieve new auxvec values.
>> 	* glibc/sysdeps/powerpc/Makefile (tests): Add tst-getauxval.
>
> the defines have been added to the common elf/elf.h.  and the way
> you've written the test is not arch specific.  so should it live
> in the common code paths instead of under powerpc/ ?

The support on the kernel side is arch specific.  I didn't see a good reason to hide the new constants, though.  They should probably be reserved across arches, though, as the new queries allow for larger cache sizes and could be readily adopted by other arches as needed.

> also, there is no "glibc/" subdir.

cut-and-paste error on my part.  Thanks for the attention to detail!

>> +static int
>> +do_test (void)
>> +{
>> +  int rc = 0;
>> +  unsigned long val;
>> +  val = getauxval (AT_L1I_CACHESIZE);
>> +  if (val)
>> +    printf("AT_L1I_CACHESIZE: %ld (0x%lx)\n",val,val);
>
> bad whitespace/style here.  this comes up multiple times.

I presume space before parentheses, and after commas. Thanks!

> also, you want %lu, not %ld, with val.  this comes up multiple times.

Yep, my mistakes.  Will clean up and repost, subject to resolving the final issue, below...

>> +  val = getauxval (AT_L1I_CACHEGEOMETRY);
>> +  if (val)
>> +    printf("AT_L1I_CACHEGEOMETRY: associativity %ld; line size %ld\n",
>> +	   (val & 0xffff0000) >> 16, val & 0x0000ffff);
>> +  else
>> +    rc = EXIT_UNSUPPORTED;
>
> honestly, what is the value of this test ?  you basically just
> printf the values everywhere, or you exit unsupported.  there is
> no actual "test" here that i can see as you don't validate the
> results anywhere.

I debated this with colleagues before sending (and perhaps should've deferred to their experience).  I was reluctant to add new code without exercising it, at least a successful compile and run.  However it's difficult to determine a true "failure" case without knowing too much about the kernel.  I also like that it provides an example of use.  If those reasons are not sufficient, I can also remove it from the patch.

Regards,
Paul Clarke

Patch hide | download patch | download mbox

diff --git a/elf/elf.h b/elf/elf.h
index 6d3b356..fff893d 100644
--- a/elf/elf.h
+++ b/elf/elf.h
@@ -1170,6 +1170,18 @@  typedef struct
  #define AT_L2_CACHESHAPE	36
  #define AT_L3_CACHESHAPE	37
  
+/* Shapes of the caches, with more room to describe them.
+   *GEOMETRY are comprised of cache line size in bytes in the bottom 16 bits
+   and the cache associativity in the next 16 bits.  */
+#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
+
  /* Note section contents.  Each entry in the note section begins with
     a header of a fixed form.  */
  
diff --git a/sysdeps/powerpc/Makefile b/sysdeps/powerpc/Makefile
index 933810f..5f10f68 100644
--- a/sysdeps/powerpc/Makefile
+++ b/sysdeps/powerpc/Makefile
@@ -35,6 +35,7 @@  ifeq ($(subdir),misc)
  sysdep_headers += sys/platform/ppc.h
  tests += test-gettimebase
  tests += tst-set_ppr
+tests += tst-getauxval
  endif
  
  ifneq (,$(filter %le,$(config-machine)))
diff --git a/sysdeps/powerpc/tst-getauxval.c b/sysdeps/powerpc/tst-getauxval.c
new file mode 100644
index 0000000..1f5e9f0
--- /dev/null
+++ b/sysdeps/powerpc/tst-getauxval.c
@@ -0,0 +1,86 @@ 
+/* Test the implementation of getauxval functions for
+   cache size, cache line size, and associativity for L1, L2, L3 caches.
+
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <sys/auxv.h>
+
+#include <support/test-driver.h>
+
+static int
+do_test (void)
+{
+  int rc = 0;
+  unsigned long val;
+  val = getauxval (AT_L1I_CACHESIZE);
+  if (val)
+    printf("AT_L1I_CACHESIZE: %ld (0x%lx)\n",val,val);
+  else
+    rc = EXIT_UNSUPPORTED;
+
+  val = getauxval (AT_L1I_CACHEGEOMETRY);
+  if (val)
+    printf("AT_L1I_CACHEGEOMETRY: associativity %ld; line size %ld\n",
+	   (val & 0xffff0000) >> 16, val & 0x0000ffff);
+  else
+    rc = EXIT_UNSUPPORTED;
+
+  val = getauxval (AT_L1D_CACHESIZE);
+  if (val)
+      printf("AT_L1D_CACHESIZE: %ld (0x%lx)\n",val,val);
+  else
+    rc = EXIT_UNSUPPORTED;
+
+  val = getauxval (AT_L1D_CACHEGEOMETRY);
+  if (val)
+    printf("AT_L1D_CACHEGEOMETRY: associativity %ld; line size %ld\n",
+	   (val & 0xffff0000) >> 16, val & 0x0000ffff);
+  else
+    rc = EXIT_UNSUPPORTED;
+
+  val = getauxval (AT_L2_CACHESIZE);
+  if (val)
+    printf("AT_L2_CACHESIZE: %ld (0x%lx)\n",val,val);
+  else
+    rc = EXIT_UNSUPPORTED;
+
+  val = getauxval (AT_L2_CACHEGEOMETRY);
+  if (val)
+    printf("AT_L2_CACHEGEOMETRY: associativity %ld; line size %ld\n",
+	   (val & 0xffff0000) >> 16, val & 0x0000ffff);
+  else
+    rc = EXIT_UNSUPPORTED;
+
+  val = getauxval (AT_L3_CACHESIZE);
+  if (val)
+      printf("AT_L3_CACHESIZE: %ld (0x%lx)\n",val,val);
+  else
+    rc = EXIT_UNSUPPORTED;
+
+  val = getauxval (AT_L3_CACHEGEOMETRY);
+  if (val)
+    printf("AT_L3_CACHEGEOMETRY: associativity %ld; line size %ld\n",
+	   (val & 0xffff0000) >> 16, val & 0x0000ffff);
+  else
+    rc = EXIT_UNSUPPORTED;
+
+  return rc;
+}
+
+#include <support/test-driver.c>