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

[Szabolcs Nagy <szabolcs dot nagy at arm dot com>]

On 22/06/17 20:19, Siddhesh Poyarekar 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.
> 
> 	* elf/dl-tunables.h (tunable_is_name): Move from...
> 	* elf/dl-tunables.c (is_name): ... here.
> 	(parse_tunables, __tunables_init): Adjust.
> 	* manual/tunables.texi: Document glibc.tune.cpu.
> 	* sysdeps/aarch64/dl-tunables.list: New file.
> 	* sysdeps/unix/sysv/linux/aarch64/cpu-features.c (struct
> 	cpu_list): New type.
> 	(cpu_list): New list of CPU names and their MIDR.
> 	(get_midr_from_mcpu): New function.
> 	(init_cpu_features): Override MIDR if necessary.

sounds good.

> +++ b/manual/tunables.texi
> @@ -231,6 +231,14 @@ the ones in @code{sysdeps/x86/cpu-features.h}.
>  This tunable is specific to i386 and x86-64.
>  @end deftp
>  
> +@deftp Tunable glibc.tune.cpu
> +The @code{glibc.tune.cpu=xxx} tunable allows the user to tell @theglibc{} to
> +assume that the CPU is @code{xxx} where xxx may have one of these values:
> +@code{generic}, @code{thunderx}, @code{falkor}.
> +
> +This tunable is specific to aarch64.
> +@end deftp

good, but this way we will have to update this list if glibc
handles some new cpu.

i'm not convinced yet that we need falkor specific memcpy,
in which case adding falkor here is not (yet) necessary.

> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> @@ -20,18 +20,54 @@
>  #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[] = {
> +      {"falkor",	0x510FC000},
> +      {"thunderx",	0x430F0A10},

note that this is not consistent with the gcc aarch64-cores.def
naming where this cpu is named thunderxt88, but it's fine if
this is what we want (i don't know if thunderx ppl will want to
distinguish further variants).

> +      {"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 = UINT_MAX;

UINT_MAX vs UINT64_MAX above is inconsistent.

> +
> +#if HAVE_TUNABLES
> +  /* Get the tunable override.  */
> +  const char *mcpu = TUNABLE_GET (glibc, tune, mcpu, const char *, NULL);
> +  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 == UINT_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;
>  }
>