[PATCH 1/8] arm64: cpufeature: Fix the visibility of compat hwcaps

[Amit Kachhap]

Hi,

On 10/26/22 20:45, James Morse wrote:
> Hi Amit,
> 
> (CC: +Suzuki)
> 
> On 26/10/2022 06:58, Amit Daniel Kachhap wrote:
>> Commit 237405ebef58 ("arm64: cpufeature: Force HWCAP to be based on the
>> sysreg visible to user-space") forced the hwcaps to use sanitised
>> user-space view of the id registers. However, the ID register structures
>> used to select few compat cpufeatures (vfp, crc32, ...) are masked and
>> hence such hwcaps do not appear in /proc/cpuinfo anymore for PER_LINUX32
>> personality.
> 
> Oops - I didn't check the 32bit ones. Thanks for catching this!
> 
> 
>> Add the ID register structures explicitly and set them as visible for
>> the compat hwcaps.
> 
> While 32bit user-space can't access these, 64bit with the personality set now can.
> I don't think that's a problem.

ok, Nice that this patch fixes other issues also which I didn't thought 
about.

> 
> 
> Is there a separate posting of this patch on its own? (do I need to reply there too?)
> Mixing fixes with a new-feature series makes the maintainers job harder. Its best to post
> fixes separately so they can be treated more urgently than features.
> 
> 
>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index 6062454a9067..43e5b43ef550 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -428,6 +428,30 @@ static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = {
>>   	ARM64_FTR_END,
>>   };
>>   
>> +static const struct arm64_ftr_bits ftr_mvfr0[] = {
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR0_FPROUND_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR0_FPSHVEC_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR0_FPSQRT_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR0_FPDIVIDE_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR0_FPTRAP_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, MVFR0_FPDP_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR0_FPSP_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR0_SIMD_SHIFT, 4, 0),
>> +	ARM64_FTR_END,
>> +};
>> +
>> +static const struct arm64_ftr_bits ftr_mvfr1[] = {
>> +	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, MVFR1_SIMDFMAC_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR1_FPHP_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR1_SIMDHP_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR1_SIMDSP_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR1_SIMDINT_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR1_SIMDLS_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR1_FPDNAN_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR1_FPFTZ_SHIFT, 4, 0),
>> +	ARM64_FTR_END,
>> +};
> 
> As these two no longer use ftr_generic_32bits[], could you update the comment above
> ftr_generic_32bits[] which lists the id registers that use it?

I just send the v2 with your suggestion. Looks like comment were already 
inconsistent.

> 
> 
>>   static const struct arm64_ftr_bits ftr_mvfr2[] = {
>>   	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR2_FPMISC_SHIFT, 4, 0),
>>   	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, MVFR2_SIMDMISC_SHIFT, 4, 0),
>> @@ -458,10 +482,10 @@ static const struct arm64_ftr_bits ftr_id_isar0[] = {
>>   
>>   static const struct arm64_ftr_bits ftr_id_isar5[] = {
>>   	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_RDM_SHIFT, 4, 0),
>> -	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_CRC32_SHIFT, 4, 0),
>> -	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_SHA2_SHIFT, 4, 0),
>> -	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_SHA1_SHIFT, 4, 0),
>> -	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_AES_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_CRC32_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_SHA2_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_SHA1_SHIFT, 4, 0),
>> +	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_AES_SHIFT, 4, 0),
>>   	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_SEVL_SHIFT, 4, 0),
>>   	ARM64_FTR_END,
>>   };
>> @@ -645,8 +669,8 @@ static const struct __ftr_reg_entry {
>>   	ARM64_FTR_REG(SYS_ID_ISAR6_EL1, ftr_id_isar6),
>>   
>>   	/* Op1 = 0, CRn = 0, CRm = 3 */
>> -	ARM64_FTR_REG(SYS_MVFR0_EL1, ftr_generic_32bits),
>> -	ARM64_FTR_REG(SYS_MVFR1_EL1, ftr_generic_32bits),
>> +	ARM64_FTR_REG(SYS_MVFR0_EL1, ftr_mvfr0),
>> +	ARM64_FTR_REG(SYS_MVFR1_EL1, ftr_mvfr1),
>>   	ARM64_FTR_REG(SYS_MVFR2_EL1, ftr_mvfr2),
>>   	ARM64_FTR_REG(SYS_ID_PFR2_EL1, ftr_id_pfr2),
>>   	ARM64_FTR_REG(SYS_ID_DFR1_EL1, ftr_id_dfr1),
> 
> Reviewed-by: James Morse <james.morse at arm.com>

Thanks for reviewing.

Amit
> 
> 
> Thanks,
> 
> James