Re: [PATCH][for-stable-v5.11]] arm64: Unconditionally set virtual cpu id registers

From: Marc Zyngier <maz@kernel.org>
To: Vladimir Murzin <vladimir.murzin@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, stable@vger.kernel.org,
	catalin.marinas@arm.com, will@kernel.org, dbrazdil@google.com
Subject: Re: [PATCH][for-stable-v5.11]] arm64: Unconditionally set virtual cpu id registers
Date: Tue, 16 Mar 2021 13:22:39 +0000	[thread overview]
Message-ID: <878s6nfd28.wl-maz@kernel.org> (raw)
In-Reply-To: <20210316112500.85268-1-vladimir.murzin@arm.com>

Hi Vladimir,

On Tue, 16 Mar 2021 11:25:00 +0000,
Vladimir Murzin <vladimir.murzin@arm.com> wrote:
> 
> Commit 78869f0f0552 ("arm64: Extract parts of el2_setup into a macro")
> reorganized el2 setup in such way that virtual cpu id registers set
> only in nVHE, yet they used (and need) to be set irrespective VHE
> support. Lack of setup causes 32-bit guest stop booting due to MIDR
> stay undefined.

Surely this affects 64bit guests as well, doesn't it? I guess the
32bit code tries to infer stuff such as the architecture revision from
MIDR and falls over, and that the 64bit Linux code has less baggage?

>
> Fixes: 78869f0f0552 ("arm64: Extract parts of el2_setup into a macro")
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
> 
> There is no upstream fix since issue went away due to code there has
> been reworked in 5.12: nVHE comes first, so virtual cpu id register
> are always set.
> 
> Maintainers, please, Ack.
> 
>  arch/arm64/include/asm/el2_setup.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
> index f988e94cdf9e..db87daca6b8c 100644
> --- a/arch/arm64/include/asm/el2_setup.h
> +++ b/arch/arm64/include/asm/el2_setup.h
> @@ -113,7 +113,7 @@
>  .endm
>  
>  /* Virtual CPU ID registers */
> -.macro __init_el2_nvhe_idregs
> +.macro __init_el2_idregs
>  	mrs	x0, midr_el1
>  	mrs	x1, mpidr_el1
>  	msr	vpidr_el2, x0
> @@ -165,6 +165,7 @@
>  	__init_el2_stage2
>  	__init_el2_gicv3
>  	__init_el2_hstr
> +	__init_el2_idregs
>  
>  	/*
>  	 * When VHE is not in use, early init of EL2 needs to be done here.
> @@ -173,7 +174,6 @@
>  	 * will be done via the _EL1 system register aliases in __cpu_setup.
>  	 */
>  .ifeqs "\mode", "nvhe"
> -	__init_el2_nvhe_idregs
>  	__init_el2_nvhe_cptr
>  	__init_el2_nvhe_sve
>  	__init_el2_nvhe_prepare_eret

The couple of VHE systems I have around don't suffer from this issue,
but it looks like I can trigger it on the FVP model (probably because
the model is nasty enough to not have VPIDR_EL2 default to anything
sensible!).

Anyway, good catch. If you can respin it to drop the reference to
32bit guests, I'll happily ack it!

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel