[PATCH v1] arm64: mm: fix booting with 52-bit address space

[Ard Biesheuvel]

Hi Joey,

On Thu, 30 Jun 2022 at 13:00, Joey Gouly <joey.gouly at arm.com> wrote:
>
> __cpu_setup() used `vabits_actual` before it had been set to the
> correct value (in arm64_memblock_init()), if 52-bit address space
> was supported.
>
> Use the register value directly so that TCR_EL1.T1SZ will be set
> to the correct value.
>
> This fixes booting on QEMU and the FVP model.
>
> Fixes: 0d9b1ffefabe ("arm64: mm: make vabits_actual a build time constant if possible")
> Signed-off-by: Joey Gouly <joey.gouly at arm.com>
> Cc: Ard Biesheuvel <ardb at kernel.org>
> Cc: Will Deacon <will at kernel.org>
> Cc: Anshuman Khandual <anshuman.khandual at arm.com>
> Cc: Mark Rutland <mark.rutland at arm.com>
> Signed-off-by: Joey Gouly <joey.gouly at arm.com>
> ---
>  arch/arm64/mm/proc.S | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index 605c6640f94b..375448b2de9e 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -466,7 +466,16 @@ SYM_FUNC_START(__cpu_setup)
>         tcr_clear_errata_bits tcr, x9, x5
>
>  #ifdef CONFIG_ARM64_VA_BITS_52
> -       ldr_l           x9, vabits_actual
> +       /*
> +        * vabits_actual has not been set at this point, check the
> +        * ID_AA64MMFR2_EL1 register directly.
> +        */
> +       mrs_s   x10, SYS_ID_AA64MMFR2_EL1
> +       and     x10, x10, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> +       mov     x9, #52
> +       cbnz    x10, 1f
> +       mov     x9, #VA_BITS_MIN
> +1:
>         sub             x9, xzr, x9
>         add             x9, x9, #64
>         tcr_set_t1sz    tcr, x9

Oops, sorry about that!

I think this might give the wrong result on heterogeneous systems,
e.g., if you boot on a CPU that does not have LVA but have secondaries
that do. Given the the issue only affects the boot CPU, perhaps we
should initialize vabits_actual to 0x0 statically, and assign the
correct value in arm64_memblock_init(). Or alternatively, pass the
correct value as a parameter to __cpu_setup(), in which case we can
use the above approach for the primary, and load vabits_actual for the
secondary.

I'm also more than happy to clean up my own mess, and take this as a
reported-by.