Re: [PATCH] powerpc: Support auxilliary vector components for cache geometries

[Mike Frysinger <vapier at gentoo dot org>]

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

Attachment: signature.asc
Description: Digital signature