Re: [PATCH v3] PowerPC: Influence cpu/arch hwcap features via GLIBC_TUNABLES.

[MAHESH BODAPATI <bmahi496@linux.ibm.com>]

On 07/07/23 2:35 am, Peter Bergner wrote:
> On 7/6/23 7:25 AM, bmahi496@linux.ibm.com wrote:
>> +  /* Copy the features from dl_powerpc_cpu_features, which contains the
>> +     features provided by AT_HWCAP and AT_HWCAP2.  */
>> +  struct cpu_features *cpu_features = &GLRO(dl_powerpc_cpu_features);
>> +  unsigned long int tcbv_hwcap = cpu_features->hwcap;
>> +  unsigned long int tcbv_hwcap2 = cpu_features->hwcap2;
> [snip]
>> +	      else
>> +		{
>> +		  /* Enable the features and also checking that no unsupported
>> +		     features were enabled by user.  */
>> +		  if (hwcap_tunables[i].id)
>> +		    cpu_features->hwcap2 |= (tcbv_hwcap2 & hwcap_tunables[i].mask);
>> +		  else
>> +		    cpu_features->hwcap |= (tcbv_hwcap & hwcap_tunables[i].mask);
>> +		}
> I don't see how this code can ever "add" new bits to cpu_features->hwcap
> or cpu_features->hwcap2.  We cache their values at the top of the loop
> and then OR in the tunable mask, but only if they're not already set
> in the cached values.  If the tunable mask has a bit that isn't already
> present in cpu_features->hwcap/cpu_features->hwcap2, we'll never set
> them.  It seems your code as is, can only ever remove bits from hwcap/hwcap2.
>
> Question for you or anyone else, is there a scenario where we can execute
> our set_hwcaps tunable callback function where the set of HWCAP/2 feature
> bits is a subset of the HWCAP/2 bits that the kernel passed to us?

I don't see any scenario like that.
It will be helpful when user set multiple entries of tunable.
For example "-arch_3_1,-arch_3_00,-vsx,arch_3_00" here we have 2 entries 
of arch_3_00
where it's disabling at first place and enabling at second place.

>
> For example, could our set_hwcaps tunable callback function be called multiple
> times in some scenario like via a fork or ???  If yes, then tcbv_hwcap and
> tcbv_hwcap2 really should be set to the HWCAP/2 values the kernel gives us
> and not some possibly modified version in cpu_features->hwcap/2.  If no, then
> I think we probably don't even need to support trying to enable/add bits to
> the cpu_features->hwcap/2 masks at all, since we'd only try setting bits
> that are already set or bits we're not supposed to set since they aren't
> in the kernel's version of the hwcaps.
>
>
> Peter
>
>