[PATCH 0/3] Two minor fixups around FEAT_E2H0

[PATCH 0/3] Two minor fixups around FEAT_E2H0

[Yicong Yang]

From: Yicong Yang <yangyicong@hisilicon.com>

Two minor fixups for the FEAT_E2H0:
1) add missing feature register update
2) fix boot warning with kvm-arm.mode=nvhe if !FEAT_E2H0

Add a cpucap identify the HCR_EL2.E2H RES1 (!FEAT_E2H0) support.

Yicong Yang (3):
  arm64/cpufeature: Add missing id_aa64mmfr4 feature reg update
  arm64/cpufeature: Add cpucap for HCR_EL2.E2H RES1 (!FEAT_E2H0)
  KVM: arm64: Fix boot warning with kvm-arm.mode=nvhe on !FEAT_E2H0
    platforms

 arch/arm64/include/asm/cpufeature.h | 23 +++++++++++++++++++++++
 arch/arm64/kernel/cpufeature.c      | 14 ++++++++++++++
 arch/arm64/kvm/arm.c                | 14 +++++++++++---
 arch/arm64/tools/cpucaps            |  1 +
 4 files changed, 49 insertions(+), 3 deletions(-)

-- 
2.24.0

[PATCH 1/3] arm64/cpufeature: Add missing id_aa64mmfr4 feature reg update

[Yicong Yang]

From: Yicong Yang <yangyicong@hisilicon.com>

Add missing id_aa64mmfr4 feature register check and update in
update_cpu_features(). Update the taint status as well.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 arch/arm64/kernel/cpufeature.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9c4d6d552b25..75ef319c1ef8 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1403,6 +1403,8 @@ void update_cpu_features(int cpu,
 				      info->reg_id_aa64mmfr2, boot->reg_id_aa64mmfr2);
 	taint |= check_update_ftr_reg(SYS_ID_AA64MMFR3_EL1, cpu,
 				      info->reg_id_aa64mmfr3, boot->reg_id_aa64mmfr3);
+	taint |= check_update_ftr_reg(SYS_ID_AA64MMFR4_EL1, cpu,
+				      info->reg_id_aa64mmfr4, boot->reg_id_aa64mmfr4);
 
 	taint |= check_update_ftr_reg(SYS_ID_AA64PFR0_EL1, cpu,
 				      info->reg_id_aa64pfr0, boot->reg_id_aa64pfr0);
-- 
2.24.0

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

[Yicong Yang]

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);
+	val = cpuid_feature_extract_signed_field(mmfr4,
+						 ID_AA64MMFR4_EL1_E2H0_SHIFT);
+
+	return val != ID_AA64MMFR4_EL1_E2H0_IMP;
+}
+
 static inline bool cpu_has_pan(void)
 {
 	u64 mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 75ef319c1ef8..64e99330b380 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2473,6 +2473,12 @@ test_has_mpam_hcr(const struct arm64_cpu_capabilities *entry, int scope)
 	return idr & MPAMIDR_EL1_HAS_HCR;
 }
 
+static bool
+has_e2h_res1(const struct arm64_cpu_capabilities *entry, int __unused)
+{
+	return cpu_has_e2h_res1();
+}
+
 static const struct arm64_cpu_capabilities arm64_features[] = {
 	{
 		.capability = ARM64_ALWAYS_BOOT,
@@ -3043,6 +3049,12 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.matches = has_pmuv3,
 	},
 #endif
+	{
+		.desc = "HCR_EL2.E2H RES1",
+		.capability = ARM64_HAS_E2H_RES1,
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.matches = has_e2h_res1,
+	},
 	{},
 };
 
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 772c1b008e43..dd6631886d67 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -22,6 +22,7 @@ HAS_DCPODP
 HAS_DCPOP
 HAS_DIT
 HAS_E0PD
+HAS_E2H_RES1
 HAS_ECV
 HAS_ECV_CNTPOFF
 HAS_EPAN
-- 
2.24.0

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

[Marc Zyngier]

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.

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

[Yicong Yang]

On 2025/3/29 16:12, Marc Zyngier wrote:
> 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?
> 

No. If any use required in the hotpath, check ARM64_HAS_E2H_RES1 by
alternative_has_cap* instead. We cannot check the capabilites in
the early_param() (in early_kvm_mode_cfg()) since they are not initialized,
so we can only rely on the registers directly.

>> +	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.

currently maybe only a wrong declaration of HCR_EL2.E2H RES1 and we can have
a workaround for the apple? considering the only use case of this is in the
early_kvm_mode_cfg() described below.

> 
> 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?
> 

Only one usecase introduced in patch 3/3, avoid triggering the warning on
!E2H0 platforms when booting with "kvm-arm.mode=nvhe". In such case "nvhe"
is inavailable but user has no hint on this, booting with "kvm-arm.mode=nvhe"
will trigger the warn [1]. Need to give some hint if user try to boot with
"nvhe" mode on !FEAT_E2H0 platforms. But we need a ARM64_CPUCAP_SYSTEM_FEATURE
capability to make this feature system wide consistent.

[1] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kvm/arm.c#n2917

Thanks.

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

[Marc Zyngier]

On Sat, 29 Mar 2025 08:41:09 +0000,
Yicong Yang <yangyicong@huawei.com> wrote:
> 
> On 2025/3/29 16:12, Marc Zyngier wrote:
> > 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?
> > 
> 
> No. If any use required in the hotpath, check ARM64_HAS_E2H_RES1 by
> alternative_has_cap* instead. We cannot check the capabilites in
> the early_param() (in early_kvm_mode_cfg()) since they are not initialized,
> so we can only rely on the registers directly.

Then I think an explicit comment would help clarifying what is
expected to be used. I also don't think the __always_inline is
mandated here. Specially if the helper needs to account for the broken
Apple stuff.

> 
> >> +	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.
> 
> currently maybe only a wrong declaration of HCR_EL2.E2H RES1 and we can have
> a workaround for the apple? considering the only use case of this is in the
> early_kvm_mode_cfg() described below.

Indeed, I think you would need to handle the Apple behaviour here.

> 
> > 
> > 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?
> > 
> 
> Only one usecase introduced in patch 3/3, avoid triggering the warning on
> !E2H0 platforms when booting with "kvm-arm.mode=nvhe". In such case "nvhe"
> is inavailable but user has no hint on this, booting with "kvm-arm.mode=nvhe"
> will trigger the warn [1]. Need to give some hint if user try to boot with
> "nvhe" mode on !FEAT_E2H0 platforms. But we need a ARM64_CPUCAP_SYSTEM_FEATURE
> capability to make this feature system wide consistent.
> 
> [1] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kvm/arm.c#n2917

But *why* do we need an extra helper for this only functionnality? If
we reach the point where early_kvm_mode_cfg() gets called, that we are
still at EL2, and that the requested mode is "nvhe", then we know, by
construction, that we couldn't switch to E2H==0.

That's because idreg-override.c defines this:

 { "kvm_arm.mode=nvhe",		"arm64_sw.hvhe=0 id_aa64mmfr1.vh=0" },

and "id_aa64mmfr1.vh=0" gets filtered out by mmfr1_vh_filter().

Or am I missing something obvious?

Thanks,

	M.

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

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

[Yicong Yang]

On 2025/3/29 18:41, Marc Zyngier wrote:
> On Sat, 29 Mar 2025 08:41:09 +0000,
> Yicong Yang <yangyicong@huawei.com> wrote:
>>
>> On 2025/3/29 16:12, Marc Zyngier wrote:
>>> 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?
>>>
>>
>> No. If any use required in the hotpath, check ARM64_HAS_E2H_RES1 by
>> alternative_has_cap* instead. We cannot check the capabilites in
>> the early_param() (in early_kvm_mode_cfg()) since they are not initialized,
>> so we can only rely on the registers directly.
> 
> Then I think an explicit comment would help clarifying what is
> expected to be used. I also don't think the __always_inline is
> mandated here. Specially if the helper needs to account for the broken
> Apple stuff.
> 

sure.

>>
>>>> +	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.
>>
>> currently maybe only a wrong declaration of HCR_EL2.E2H RES1 and we can have
>> a workaround for the apple? considering the only use case of this is in the
>> early_kvm_mode_cfg() described below.
> 
> Indeed, I think you would need to handle the Apple behaviour here.
> 

okay will add a workaround for apple on this feature.

>>
>>>
>>> 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?
>>>
>>
>> Only one usecase introduced in patch 3/3, avoid triggering the warning on
>> !E2H0 platforms when booting with "kvm-arm.mode=nvhe". In such case "nvhe"
>> is inavailable but user has no hint on this, booting with "kvm-arm.mode=nvhe"
>> will trigger the warn [1]. Need to give some hint if user try to boot with
>> "nvhe" mode on !FEAT_E2H0 platforms. But we need a ARM64_CPUCAP_SYSTEM_FEATURE
>> capability to make this feature system wide consistent.
>>
>> [1] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kvm/arm.c#n2917
> 
> But *why* do we need an extra helper for this only functionnality? If
> we reach the point where early_kvm_mode_cfg() gets called, that we are
> still at EL2, and that the requested mode is "nvhe", then we know, by
> construction, that we couldn't switch to E2H==0.
> 
> That's because idreg-override.c defines this:
> 
>  { "kvm_arm.mode=nvhe",		"arm64_sw.hvhe=0 id_aa64mmfr1.vh=0" },
> 
> and "id_aa64mmfr1.vh=0" gets filtered out by mmfr1_vh_filter().
> 
> Or am I missing something obvious?
> 

nop, but the problem is not about the KVM function itself. The problem is that
trying to use "nvhe" mode on a E2H0 unavailable platform, and doesn't pass the
check of nvhe mode in early_kvm_mode_cfg(), explicitly:

early_kvm_mode_cfg()
[...]
	if (strcmp(arg, "nvhe") == 0 && !WARN_ON(is_kernel_in_hyp_mode())) { // Trigger the WARN_ON here and a call trace in the dmesg.
		kvm_mode = KVM_MODE_DEFAULT;
		return 0;
	}

finally we'll enter with VHE mode so the function of KVM is fine. we want
to eliminate this call trace and notify the user "nvhe" is not supported
due to !FEAT_E2H0.

The helper will be used in 2 places (sorry for the mistake in the last
reply):
1) in the HAS_E2H_RES1 capcap's detection has_e2h_res1()
2) in early_kvm_mode_cfg() for checking HAS_E2H_RES1 support and avoid
   checking for "nvhe".

is it suggested to check the mmfr4 directly in those 2 places without
using a wrapper?

Thanks.

[PATCH 3/3] KVM: arm64: Fix boot warning with kvm-arm.mode=nvhe on !FEAT_E2H0 platforms

[Yicong Yang]

From: Yicong Yang <yangyicong@hisilicon.com>

If platforms don't support FEAT_E2H0, HCR_EL2.E2H is RES1 and nVHE is
not available. The warning in early_kvm_mode_cfg() will be triggered
if boot with kvm-arm.mode=nvhe and no indication for this. Fix this by
skipping the non-VHE checking on !FEAT_E2H0 platform.

Since this should only affect the "nvhe" mode, move "nested" mode check
ahead to avoid being affected.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 arch/arm64/kvm/arm.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 68fec8c95fee..a0c0bd936a53 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -2905,6 +2905,11 @@ static int __init early_kvm_mode_cfg(char *arg)
 		return 0;
 	}
 
+	if (strcmp(arg, "nested") == 0 && !WARN_ON(!is_kernel_in_hyp_mode())) {
+		kvm_mode = KVM_MODE_NV;
+		return 0;
+	}
+
 	if (strcmp(arg, "protected") == 0) {
 		if (!is_kernel_in_hyp_mode())
 			kvm_mode = KVM_MODE_PROTECTED;
@@ -2914,13 +2919,16 @@ static int __init early_kvm_mode_cfg(char *arg)
 		return 0;
 	}
 
-	if (strcmp(arg, "nvhe") == 0 && !WARN_ON(is_kernel_in_hyp_mode())) {
+	/* If system doesn't support FEAT_E2H0, nVHE host is not available */
+	if (cpu_has_e2h_res1()) {
+		WARN_ON(!is_kernel_in_hyp_mode());
 		kvm_mode = KVM_MODE_DEFAULT;
+		pr_warn_once("FEAT_E2H0 not supported. Ignoring kvm-arm.mode\n");
 		return 0;
 	}
 
-	if (strcmp(arg, "nested") == 0 && !WARN_ON(!is_kernel_in_hyp_mode())) {
-		kvm_mode = KVM_MODE_NV;
+	if (strcmp(arg, "nvhe") == 0 && !WARN_ON(is_kernel_in_hyp_mode())) {
+		kvm_mode = KVM_MODE_DEFAULT;
 		return 0;
 	}
 
-- 
2.24.0

Re: [PATCH 0/3] Two minor fixups around FEAT_E2H0

[Will Deacon]

On Sat, 29 Mar 2025 11:44:06 +0800, Yicong Yang wrote:
> Two minor fixups for the FEAT_E2H0:
> 1) add missing feature register update
> 2) fix boot warning with kvm-arm.mode=nvhe if !FEAT_E2H0
> 
> Add a cpucap identify the HCR_EL2.E2H RES1 (!FEAT_E2H0) support.
> 
> Yicong Yang (3):
>   arm64/cpufeature: Add missing id_aa64mmfr4 feature reg update
>   arm64/cpufeature: Add cpucap for HCR_EL2.E2H RES1 (!FEAT_E2H0)
>   KVM: arm64: Fix boot warning with kvm-arm.mode=nvhe on !FEAT_E2H0
>     platforms
> 
> [...]

Applied first patch only to arm64 (for-next/cpufeature), thanks!

[1/3] arm64/cpufeature: Add missing id_aa64mmfr4 feature reg update
      https://git.kernel.org/arm64/c/35382a364640

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev