[PATCH 2/2] aarch64: Accept string values for glibc.cpu.aarch64_gcs tunable

[Adhemerval Zanella Netto]

On 30/03/26 06:40, Yury Khrustalev wrote:
> Hello Adhemerval,
> 
> On Fri, Mar 27, 2026 at 02:44:24PM -0300, Adhemerval Zanella wrote:
>> This patch updates the glibc.cpu.aarch64_gcs tunable to accept
>> human-readable strings in addition to its standard numerical values.
>>
>> The tunable now accepts the strings 'disabled', 'enforced', 'optional',
> 
> Nit: 'now' is ambiguous in a commit message.
> 
>> and 'override', mapping them to their corresponding 0, 1, 2, and 3 internal
>> enum states.
>>
>> To support custom parsing in architecture-specific code, the
>> 'tunable_parse_num' function is moved to the generic dl-tunables-parse.h
>> as an inline function.
>>
>> The '__tunable_print_error' function is exposed globally so that invalid
>> string inputs caught in the newly added 'aarch64_gcs' callback can trigger
>> standard tunable warnings.
> 
> I think that this change is mostly cosmetic and I don't think it's worth
> doing given the changes it needs and the increased complexity of the code
> that parse this tunable value. GCS tunable should take as little code as
> possible to parse it by the nature of it.

It is usability improvement, so not really only 'comestic'.  Sure it is not
strictly required, but it makes the usage and script that eventually need
to setup the GLIBC_TUNABLE more readable to what GCS semantic it wants to
use.

> 
> See more comments below.
> 
>>
>> Checked on aarch64-linux-gnu.
>> ---
>>  elf/dl-tunables.c                             | 29 +++++++++----------
>>  elf/dl-tunables.h                             |  3 ++
>>  manual/tunables.texi                          |  8 ++---
>>  sysdeps/aarch64/dl-tunables.list              |  5 +---
>>  sysdeps/generic/dl-tunables-parse.h           | 13 +++++++++
>>  .../unix/sysv/linux/aarch64/cpu-features.c    | 28 +++++++++++++++++-
>>  6 files changed, 61 insertions(+), 25 deletions(-)
> 
> It would be useful to use these human-readable for Glibc tests to see
> if they work.
> 
>>
>> ...
>>
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> index 1e4f8a86b1..fe634cbded 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> @@ -65,6 +65,31 @@ get_midr_from_mcpu (const struct tunable_str_t *mcpu)
>>    return UINT64_MAX;
>>  }
>>  
>> +static void
>> +TUNABLE_CALLBACK (aarch64_gcs) (tunable_val_t *valp)
>> +{
>> +  tunable_num_t val;
>> +  if (tunable_parse_num_tun (valp, &val))
> 
> The manual says that accepted values right now are 0, 1, 2, and 3. I
> don't think we should bother parsing input as an integer only to have
> to process the obtained value for possible incorrect values.
> 
> We should just compare string value of the tunable with "1", "2", etc.

Alright, we can simplify the parsing with a a more straightforward loop.

> 
>> +    {
>> +      if (tunable_val_lt (val, AARCH64_GCS_POLICY_DISABLED, true))
>> +	val = AARCH64_GCS_POLICY_DISABLED;
>> +      if (tunable_val_gt (val, AARCH64_GCS_POLICY_OVERRIDE, true))
>> +	val = AARCH64_GCS_POLICY_OVERRIDE;
>> +      GL(dl_aarch64_gcs) = val;
>> +    }
> 
> If the value provided by the user is less than 0 or more than 3, it's an
> error, we should not implicitly convert it to one of the supported
> values.

Indeed, it is a mistake in my reading of do_tunable_update_val. If the
value is out of range we should use the default one.

> 
>> +  else if (tunable_strcmp_cte (valp, "disabled"))
>> +    GL(dl_aarch64_gcs) = AARCH64_GCS_POLICY_DISABLED;
>> +  else if (tunable_strcmp_cte (valp, "enforced"))
>> +    GL(dl_aarch64_gcs) = AARCH64_GCS_POLICY_ENFORCED;
>> +  else if (tunable_strcmp_cte (valp, "optional"))
>> +    GL(dl_aarch64_gcs) = AARCH64_GCS_POLICY_OPTIONAL;
>> +  else if (tunable_strcmp_cte (valp, "override"))
>> +    GL(dl_aarch64_gcs) = AARCH64_GCS_POLICY_OVERRIDE;
>> +  else
>> +    __tunable_print_error (valp->strval.str, valp->strval.len,
>> +			   "glibc.cpu.aarch64_gcs");
>> +}
>> +
>>  static inline void
>>  init_cpu_features (struct cpu_features *cpu_features)
>>  {
>> @@ -136,5 +161,6 @@ init_cpu_features (struct cpu_features *cpu_features)
>>  
>>    if (GLRO (dl_hwcap) & HWCAP_GCS)
>>      /* GCS status may be updated later by binary compatibility checks.  */
>> -    GL (dl_aarch64_gcs) = TUNABLE_GET (glibc, cpu, aarch64_gcs, uint64_t, 0);
>> +    TUNABLE_GET (glibc, cpu, aarch64_gcs, tunable_val_t *,
>> +		 TUNABLE_CALLBACK (aarch64_gcs));
> 
> Thanks,
> Yury
>