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

[Ramana Radhakrishnan <ramana dot gcc at googlemail dot com>]

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
>

Attachment: p1.txt
Description: Text document