On 11/11/24 14:05, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> ---
> 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.
