[v2] tunables, aarch64: New tunable to override cpu

  On Thu, Jun 29, 2017 at 10:52 AM, Siddhesh Poyarekar
<siddhesh@sourceware.org> wrote:
> Add a new tunable (glibc.tune.cpu) to override CPU identification on
> aarch64.  This is useful in two cases: one where it is desirable to
> pretend to be another CPU for purposes of testing or because routines
> written for that CPU are beneficial for specific workloads and second
> where the underlying kernel does not support emulation of MRS to get
> the MIDR of the CPU.

How was this tested ? I see breakages because ...

<snip>

> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> @@ -20,18 +20,53 @@
>  #include <sys/auxv.h>
>  #include <elf/dl-hwcaps.h>
>
> +#if HAVE_TUNABLES
> +struct cpu_list
> +{
> +  const char *name;
> +  uint64_t midr;
> +};
> +
> +static struct cpu_list cpu_list[] = {
> +      {"thunderxt88",  0x430F0A10},
> +      {"generic",      0x0}
> +};
> +
> +static uint64_t
> +get_midr_from_mcpu (const char *mcpu)
> +{
> +  for (int i = 0; i < sizeof (cpu_list) / sizeof (struct cpu_list); i++)
> +    if (tunable_is_name (mcpu, cpu_list[i].name) == 0)
> +      return cpu_list[i].midr;
> +
> +  return UINT64_MAX;
> +}
> +#endif
> +
>  static inline void
>  init_cpu_features (struct cpu_features *cpu_features)
>  {
>    uint64_t hwcap_mask = GET_HWCAP_MASK();
>    uint64_t hwcap = GLRO (dl_hwcap) & hwcap_mask;
>
> -  if (hwcap & HWCAP_CPUID)
> +  register uint64_t midr = UINT64_MAX;
> +
> +#if HAVE_TUNABLES
> +  /* Get the tunable override.  */
> +  const char *mcpu = TUNABLE_GET (glibc, tune, mcpu, const char *, NULL);


Shouldn't this be

TUNABLE_GET (glibc, tune, cpu, const char * , NULL);

I don't have write privileges in glibc but the attached appears to be
an obvious fix and allows the build to complete. Are there any tests
for tunables btw ?


regards
Ramana

> +  if (mcpu != NULL)
> +    midr = get_midr_from_mcpu (mcpu);
> +#endif
> +
> +  /* If there was no useful tunable override, query the MIDR if the kernel
> +     allows it.  */
> +  if (midr == UINT64_MAX)
>      {
> -      register uint64_t id = 0;
> -      asm volatile ("mrs %0, midr_el1" : "=r"(id));
> -      cpu_features->midr_el1 = id;
> +      if (hwcap & HWCAP_CPUID)
> +       asm volatile ("mrs %0, midr_el1" : "=r"(midr));
> +      else
> +       midr = 0;
>      }
> -  else
> -    cpu_features->midr_el1 = 0;
> +
> +  cpu_features->midr_el1 = midr;
>  }
> --
> 2.7.4
>
  

On Saturday 01 July 2017 03:43 AM, Ramana Radhakrishnan wrote:
> Shouldn't this be
> 
> TUNABLE_GET (glibc, tune, cpu, const char * , NULL);
> 
> I don't have write privileges in glibc but the attached appears to be
> an obvious fix and allows the build to complete. Are there any tests
> for tunables btw ?

Oops, I had tested these using glibc.tune.mcpu and then did a trivial
refactoring (incorrectly, evidently) to call it glibc.tune.cpu.  Fixed
in master.

Some of the earlier tunables have tests (malloc.check for example) but
not all of them.  The recent ifunc selection tunables don't have any
tests in the glibc testsuite.

I should be able to write a trivial test that at least ensures that
tunables are picked up, i.e. make them typo-safe.  I'll give it a shot
next week.

Siddhesh