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

[Siddhesh Poyarekar <siddhesh at sourceware dot org>]

On Friday 23 June 2017 07:31 PM, Szabolcs Nagy wrote:
>> +++ 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.

Yeah, maybe we should auto-generate this later.


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

Ack, I'll drop falkor from the list for now.

> 
>> +++ 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).

I intended it to be the same as gcc, so I'll change it to thunderxt88.
And I'll drop falkor from here for now.

>> +      {"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.

Ugh, yes, I'll fix that.

Siddhesh