Re: [PATCH 2/3] arm64/cpufeature: Add cpucap for HCR_EL2.E2H RES1 (!FEAT_E2H0)

[Marc Zyngier <maz@kernel.org>]

On Sat, 29 Mar 2025 03:44:08 +0000,
Yicong Yang <yangyicong@huawei.com> wrote:
> 
> From: Yicong Yang <yangyicong@hisilicon.com>
> 
> Arm introduced a "new" feature FEAT_E2H0 indicates that HCR_EL2.E2H can
> be programmed to the value 0 for legacy hardwares supported VHE. The
> feature is indicated by ID_AA64MMFR4_EL1.E2H0 == 0. It is needed to
> detect this feature for KVM mode initialization. Instead of bothering
> the existed hardwares, introduce a new cpucap HAS_E2H_RES1 to indicate
> FEAT_E2H0 is not supported. Make this a ARM64_CPUCAP_SYSTEM_FEATURE
> just like VHE.
> 
> Introduce cpu_has_e2h_res1() for checking the feature's support
> which can be used in the early boot stage where CPU capabilities
> are not initialized.
> 
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
>  arch/arm64/include/asm/cpufeature.h | 23 +++++++++++++++++++++++
>  arch/arm64/kernel/cpufeature.c      | 12 ++++++++++++
>  arch/arm64/tools/cpucaps            |  1 +
>  3 files changed, 36 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index c4326f1cb917..b35d393da28d 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -889,6 +889,29 @@ static inline bool cpu_has_hw_af(void)
>  						ID_AA64MMFR1_EL1_HAFDBS_SHIFT);
>  }
>  
> +/*
> + * Check whether FEAT_E2H0 is not supported, in which case HCR_EL2.E2H
> + * is implemented as RES1.
> + */
> +static __always_inline bool cpu_has_e2h_res1(void)
> +{
> +	u64 mmfr4;
> +	u32 val;
> +
> +	/*
> +	 * It's also used for checking the kvm mode cfg in early_param()
> +	 * where boot capabilities is not initialized. In such case read
> +	 * mmfr4 directly. This works same after boot stage since
> +	 * ARM64_HAS_E2H_RES1 is a system feature, the cached sanitised
> +	 * value keeps same with every single CPU.
> +	 */
> +	mmfr4 = read_sysreg_s(SYS_ID_AA64MMFR4_EL1);

This will result in traps to EL2 with nested. Are you expecting this
to be used on any form of hot paths?

> +	val = cpuid_feature_extract_signed_field(mmfr4,
> +						 ID_AA64MMFR4_EL1_E2H0_SHIFT);
> +
> +	return val != ID_AA64MMFR4_EL1_E2H0_IMP;

This is going to break badly on Apple HW, which predate the
"!FEAT_E2H0" relaxation and yet have HCR_EL2.E2H RAO/WI and
ID_AA64MMFR4_EL1.E2H0==0.

The curent code was carefully designed to *avoid* doing this, because
the kernel doesn't really need to know anything about FEAT_E2H0 apart
from the very early boot.

What do we gain with this?

	M.

-- 
Jazz isn't dead. It just smells funny.