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

[James Morse]

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.


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?


>  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,

James