[PATCH 2/6] arm64/mm: Override PARange for !LPA2 and use it consistently

[Anshuman Khandual]

On 11/11/24 14:05, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb at kernel.org>
> 
> When FEAT_LPA{,2} are not implemented, the ID_AA64MMFR0_EL1.PARange and
> TCR.IPS values corresponding with 52-bit physical addressing are
> reserved.
> 
> Setting the TCR.IPS field to 0b110 (52-bit physical addressing) has side
> effects, such as how the TTBRn_ELx.BADDR fields are interpreted, and so
> it is important that disabling FEAT_LPA2 (by overriding the
> ID_AA64MMFR0.TGran fields) also presents a PARange field consistent with
> that.
> 
> So limit the field to 48 bits unless LPA2 is enabled, and update
> existing references to use the override consistently.
> 
> Fixes: 352b0395b505 ("arm64: Enable 52-bit virtual addressing for 4k and 16k granule configs")
> Cc: <stable at vger.kernel.org>
> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> ---
>  arch/arm64/include/asm/assembler.h    | 5 +++++
>  arch/arm64/kernel/cpufeature.c        | 2 +-
>  arch/arm64/kernel/pi/idreg-override.c | 9 +++++++++
>  arch/arm64/kernel/pi/map_kernel.c     | 6 ++++++
>  arch/arm64/mm/init.c                  | 2 +-
>  5 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index 3d8d534a7a77..ad63457a05c5 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -343,6 +343,11 @@ alternative_cb_end
>  	// Narrow PARange to fit the PS field in TCR_ELx
>  	ubfx	\tmp0, \tmp0, #ID_AA64MMFR0_EL1_PARANGE_SHIFT, #3
>  	mov	\tmp1, #ID_AA64MMFR0_EL1_PARANGE_MAX
> +#ifdef CONFIG_ARM64_LPA2
> +alternative_if_not ARM64_HAS_VA52
> +	mov	\tmp1, #ID_AA64MMFR0_EL1_PARANGE_48
> +alternative_else_nop_endif
> +#endif

I guess this will only take effect after cpu features have been finalized
but will not be applicable for __cpu_setup() during primary and secondary
cpu bring up during boot.

>  	cmp	\tmp0, \tmp1
>  	csel	\tmp0, \tmp1, \tmp0, hi
>  	bfi	\tcr, \tmp0, \pos, #3
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 37e4c02e0272..6f5137040ff6 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -3399,7 +3399,7 @@ static void verify_hyp_capabilities(void)
>  		return;
>  
>  	safe_mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
> -	mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
> +	mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);

Small nit, should be renamed as safe_mmfr0 to be consistent with safe_mmfr1 ?

>  	mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
>  
>  	/* Verify VMID bits */
> diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c
> index 22159251eb3a..c6b185b885f7 100644
> --- a/arch/arm64/kernel/pi/idreg-override.c
> +++ b/arch/arm64/kernel/pi/idreg-override.c
> @@ -83,6 +83,15 @@ static bool __init mmfr2_varange_filter(u64 val)
>  		id_aa64mmfr0_override.val |=
>  			(ID_AA64MMFR0_EL1_TGRAN_LPA2 - 1) << ID_AA64MMFR0_EL1_TGRAN_SHIFT;
>  		id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_TGRAN_SHIFT;
> +
> +		/*
> +		 * Override PARange to 48 bits - the override will just be
> +		 * ignored if the actual PARange is smaller, but this is
> +		 * unlikely to be the case for LPA2 capable silicon.
> +		 */
> +		id_aa64mmfr0_override.val |=
> +			ID_AA64MMFR0_EL1_PARANGE_48 << ID_AA64MMFR0_EL1_PARANGE_SHIFT;
> +		id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_PARANGE_SHIFT;
Could these be used instead ? 

SYS_FIELD_PREP_ENUM(ID_AA64MMFR0_EL1, PARANGE, 48)
ID_AA64MMFR0_EL1_PARANGE_MASK ?


>  	}
>  #endif
>  	return true;
> diff --git a/arch/arm64/kernel/pi/map_kernel.c b/arch/arm64/kernel/pi/map_kernel.c
> index f374a3e5a5fe..e57b043f324b 100644
> --- a/arch/arm64/kernel/pi/map_kernel.c
> +++ b/arch/arm64/kernel/pi/map_kernel.c
> @@ -136,6 +136,12 @@ static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr)
>  {
>  	u64 sctlr = read_sysreg(sctlr_el1);
>  	u64 tcr = read_sysreg(tcr_el1) | TCR_DS;
> +	u64 mmfr0 = read_sysreg(id_aa64mmfr0_el1);
> +	u64 parange = cpuid_feature_extract_unsigned_field(mmfr0,
> +							   ID_AA64MMFR0_EL1_PARANGE_SHIFT);
> +
> +	tcr &= ~TCR_IPS_MASK;

Could there be a different IPS value in TCR ? OR is this just a normal
clean up instead.

> +	tcr |= parange << TCR_IPS_SHIFT;

Wondering if FIELD_PREP() could be used here.

>  
>  	asm("	msr	sctlr_el1, %0		;"
>  	    "	isb				;"
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index d21f67d67cf5..4db9887b2aef 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -280,7 +280,7 @@ void __init arm64_memblock_init(void)
>  
>  	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
>  		extern u16 memstart_offset_seed;
> -		u64 mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
> +		u64 mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);

Could this have a comment explaining the need for sanitized value ?

>  		int parange = cpuid_feature_extract_unsigned_field(
>  					mmfr0, ID_AA64MMFR0_EL1_PARANGE_SHIFT);
>  		s64 range = linear_region_size -

Otherwise LGTM.