Skip to contentSkip to conversation

arm64: Add support for FEAT_E2H0, or lack thereof

PATCH In review Marc Zyngier 5 participants 28 messages Jan 22, 2024 – Feb 12, 2024

Subsystems (2)
arm64 KVM arm64
lore
Share & tools
lore

Marc Zyngier

author maintainer
#0 lore raw

v4 cover arm64: Add support for FEAT_E2H0, or lack thereof

Since ARMv8.1, the architecture has grown the VHE feature, which makes EL2 a superset of EL1. With ARMv9.5 (and retroactively allowed from ARMv8.1), the architecture allows implementations to have VHE as the *only* implemented behaviour, meaning that HCR_EL2.E2H can be implemented as RES1. As a follow-up, HCR_EL2.NV1 can also be implemented as RES0, making the VHE-ness of the architecture recursive.

This has a number of consequences, both at boot time and for KVM, though the changes at that level are pretty minor.

The real meat of this series is on the cpufeature front, as FEAT_E2H0 is a *negative* feature, where 0b1111 (-1) represents E2H being RES1 and 0b1110 (-2) additionally indicates that NV1 is RES0. Fun, isn't it?

As a bonus, a popular crop of non-compliant HW gets promoted as the first batch of non-zero E2H0 users, making things easy for KVM.

This series is a prefix for the NV support.

* From v3 [3]:
- Dropped the capability for E2H being RES1
- Dropped the MIDR-based IDreg override
- Rebased on 6.8-rc1

* From v2 [2]:
- Moved some SYS_FIELD_VALUE() usage to the correct patch
- Fixed a couple of spelling mistakes
- Picked RBs from Suzuki (thanks!)

* From v1 [1]:
- Added a SYS_FIELD_VALUE() helper to handle the concatenation of various fields
- Only test for the top bit of ID_AA64MMFR4_EL1.E2H0 to decide whether HCR_EL2.E2H is RES1.
- Picked RBs from Oliver (thanks!)
- Rebased on 6.7-rc2

[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]

Marc Zyngier (10):
arm64: Add macro to compose a sysreg field value
arm64: cpufeatures: Correctly handle signed values
arm64: cpufeature: Correctly display signed override values
arm64: sysreg: Add layout for ID_AA64MMFR4_EL1
arm64: cpufeature: Add ID_AA64MMFR4_EL1 handling
arm64: cpufeature: Detect HCR_EL2.NV1 being RES0
arm64: Treat HCR_EL2.E2H as RES1 when ID_AA64MMFR4_EL1.E2H0 is
negative
KVM: arm64: Expose ID_AA64MMFR4_EL1 to guests
KVM: arm64: Force guest's HCR_EL2.E2H RES1 when NV1 is not implemented
KVM: arm64: Handle Apple M2 as not having HCR_EL2.NV1 implemented

 arch/arm64/include/asm/cpu.h         |   1 +
 arch/arm64/include/asm/cpufeature.h  |   1 +
 arch/arm64/include/asm/kvm_emulate.h |   3 +-
 arch/arm64/include/asm/sysreg.h      |   5 +-
 arch/arm64/kernel/cpufeature.c       | 103 ++++++++++++++++++++++++---
 arch/arm64/kernel/cpuinfo.c          |   1 +
 arch/arm64/kernel/head.S             |  23 +++---
 arch/arm64/kvm/nested.c              |   7 ++
 arch/arm64/kvm/sys_regs.c            |  17 ++++-
 arch/arm64/tools/cpucaps             |   1 +
 arch/arm64/tools/sysreg              |  37 ++++++++++
 11 files changed, 176 insertions(+), 23 deletions(-)
Signature · 1 line
2.39.2
author maintainer
#1 lore raw

PATCH v4 1/10 arm64: Add macro to compose a sysreg field value

A common idiom is to compose a tupple (reg, field, val) into a symbol matching an autogenerated definition.

Add a help performing the concatenation and replace it when open-coded implementations exist.

Suggested-by Oliver Upton <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/sysreg.h | 5 ++++-
 arch/arm64/kvm/sys_regs.c       | 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)
arch/arm64/include/asm/sysreg.h +4 -1
index c3b19b376c86..9e8999592f3a 100644
@@ -1181,6 +1181,8 @@
 	par;								\
 })
 
+#define SYS_FIELD_VALUE(reg, field, val)	reg##_##field##_##val
+
 #define SYS_FIELD_GET(reg, field, val)		\
 		 FIELD_GET(reg##_##field##_MASK, val)
 

@@ -1188,7 +1190,8 @@
 		 FIELD_PREP(reg##_##field##_MASK, val)
 
 #define SYS_FIELD_PREP_ENUM(reg, field, val)		\
-		 FIELD_PREP(reg##_##field##_MASK, reg##_##field##_##val)
+		 FIELD_PREP(reg##_##field##_MASK,	\
+			    SYS_FIELD_VALUE(reg, field, val))
 
 #endif
arch/arm64/kvm/sys_regs.c +2 -1
index 30253bd19917..88b8fbeafaa0 100644
@@ -1685,7 +1685,8 @@ static u64 read_sanitised_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
 	u64 __f_val = FIELD_GET(reg##_##field##_MASK, val);		       \
 	(val) &= ~reg##_##field##_MASK;					       \
 	(val) |= FIELD_PREP(reg##_##field##_MASK,			       \
-			min(__f_val, (u64)reg##_##field##_##limit));	       \
+			    min(__f_val,				       \
+				(u64)SYS_FIELD_VALUE(reg, field, limit)));     \
 	(val);								       \
 })
Signature · 1 line
2.39.2
maintainer Reviewed-by on v4 1/10
#2 lore raw

Re: [PATCH v4 01/10] arm64: Add macro to compose a sysreg field value

Quoted 8 lines · Marc Zyngier

A common idiom is to compose a tupple (reg, field, val) into a symbol matching an autogenerated definition.

Add a help performing the concatenation and replace it when open-coded implementations exist.

Suggested-by Oliver Upton <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
Reviewed-by Catalin Marinas <[email protected]>
author maintainer
#3 lore raw

PATCH v4 2/10 arm64: cpufeatures: Correctly handle signed values

In reply to Marc Zyngier

Although we've had signed values for some features such as PMUv3 and FP, the code that handles the comparaison with some limit has a couple of annoying issues:

- the min_field_value is always unsigned, meaning that we cannot easily compare it with a negative value

- it is not possible to have a range of values, let alone a range of negative values

Fix this by:

- adding an upper limit to the comparison, defaulting to all bits being set to the maximum positive value

- ensuring that the signess of the min and max values are taken into account

A ARM64_CPUID_FIELDS_NEG() macro is provided for signed features, but nothing is using it yet.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/cpufeature.h |  1 +
 arch/arm64/kernel/cpufeature.c      | 65 +++++++++++++++++++++++++----
 2 files changed, 57 insertions(+), 9 deletions(-)
arch/arm64/include/asm/cpufeature.h +1 -0
index 21c824edf8ce..a98d95f3492b 100644
@@ -363,6 +363,7 @@ struct arm64_cpu_capabilities {
 			u8 field_pos;
 			u8 field_width;
 			u8 min_field_value;
+			u8 max_field_value;
 			u8 hwcap_type;
 			bool sign;
 			unsigned long hwcap;
arch/arm64/kernel/cpufeature.c +56 -9

79 lines not on this page · raw message

Signature · 1 line
2.39.2
maintainer Reviewed-by on v4 2/10
#4 lore raw

Re: [PATCH v4 02/10] arm64: cpufeatures: Correctly handle signed values

Quoted 15 lines · Marc Zyngier

Although we've had signed values for some features such as PMUv3 and FP, the code that handles the comparaison with some limit has a couple of annoying issues:

- the min_field_value is always unsigned, meaning that we cannot easily compare it with a negative value

- it is not possible to have a range of values, let alone a range of negative values

Fix this by:

- adding an upper limit to the comparison, defaulting to all bits being set to the maximum positive value

Quoted 7 lines

- ensuring that the signess of the min and max values are taken into account

A ARM64_CPUID_FIELDS_NEG() macro is provided for signed features, but nothing is using it yet.

Signed-off-by Marc Zyngier <[email protected]>
Reviewed-by Catalin Marinas <[email protected]>
author maintainer
#5 lore raw

PATCH v4 3/10 arm64: cpufeature: Correctly display signed override values

In reply to Marc Zyngier

When a field gets overriden, the kernel indicates the result of the override in dmesg. This works well with unsigned fields, but results in a pretty ugly output when the field is signed.

Truncate the field to its width before displaying it.

Reviewed-by Oliver Upton <[email protected]>
Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm64/kernel/cpufeature.c +2 -1
index 92b1546f2622..bae5d0655262 100644
@@ -949,7 +949,8 @@ static void init_cpu_ftr_reg(u32 sys_reg, u64 new)
 				pr_warn("%s[%d:%d]: %s to %llx\n",
 					reg->name,
 					ftrp->shift + ftrp->width - 1,
-					ftrp->shift, str, tmp);
+					ftrp->shift, str,
+					tmp & (BIT(ftrp->width) - 1));
 		} else if ((ftr_mask & reg->override->val) == ftr_mask) {
 			reg->override->val &= ~ftr_mask;
 			pr_warn("%s[%d:%d]: impossible override, ignored\n",
Signature · 1 line
2.39.2
maintainer Reviewed-by on v4 3/10
#6 lore raw

Re: [PATCH v4 03/10] arm64: cpufeature: Correctly display signed override values

Quoted 9 lines · Marc Zyngier

When a field gets overriden, the kernel indicates the result of the override in dmesg. This works well with unsigned fields, but results in a pretty ugly output when the field is signed.

Truncate the field to its width before displaying it.

Reviewed-by Oliver Upton <[email protected]>
Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
Reviewed-by Catalin Marinas <[email protected]>
author maintainer
#7 lore raw

PATCH v4 4/10 arm64: sysreg: Add layout for ID_AA64MMFR4_EL1

In reply to Marc Zyngier

ARMv9.5 has infroduced ID_AA64MMFR4_EL1 with a bunch of new features. Add the corresponding layout.

This is extracted from the public ARM SysReg_xml_A_profile-2023-09 delivery, timestamped d55f5af8e09052abe92a02adf820deea2eaed717.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/tools/sysreg | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
arch/arm64/tools/sysreg +37 -0
index 4c9b67934367..fa3fe0856880 100644
@@ -1791,6 +1791,43 @@ UnsignedEnum	3:0	TCRX
 EndEnum
 EndSysreg
 
+Sysreg	ID_AA64MMFR4_EL1	3	0	0	7	4
+Res0	63:40
+UnsignedEnum	39:36	E3DSE
+	0b0000	NI
+	0b0001	IMP
+EndEnum
+Res0	35:28
+SignedEnum	27:24	E2H0
+	0b0000	IMP
+	0b1110	NI_NV1
+	0b1111	NI
+EndEnum
+UnsignedEnum	23:20	NV_frac
+	0b0000	NV_NV2
+	0b0001	NV2_ONLY
+EndEnum
+UnsignedEnum	19:16	FGWTE3
+	0b0000	NI
+	0b0001	IMP
+EndEnum
+UnsignedEnum	15:12	HACDBS
+	0b0000	NI
+	0b0001	IMP
+EndEnum
+UnsignedEnum	11:8	ASID2
+	0b0000	NI
+	0b0001	IMP
+EndEnum
+SignedEnum	7:4	EIESB
+	0b0000	NI
+	0b0001	ToEL3
+	0b0010	ToELx
+	0b1111	ANY
+EndEnum
+Res0	3:0
+EndSysreg
+
 Sysreg	SCTLR_EL1	3	0	1	0	0
 Field	63	TIDCP
 Field	62	SPINTMASK
Signature · 1 line
2.39.2
maintainer Reviewed-by on v4 4/10
#8 lore raw

Re: [PATCH v4 04/10] arm64: sysreg: Add layout for ID_AA64MMFR4_EL1

Quoted 8 lines · Marc Zyngier

ARMv9.5 has infroduced ID_AA64MMFR4_EL1 with a bunch of new features. Add the corresponding layout.

This is extracted from the public ARM SysReg_xml_A_profile-2023-09 delivery, timestamped d55f5af8e09052abe92a02adf820deea2eaed717.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
Reviewed-by Catalin Marinas <[email protected]>
Reviewed-by on v4 4/10
#9 lore raw

Re: [PATCH v4 04/10] arm64: sysreg: Add layout for ID_AA64MMFR4_EL1

In reply to Marc Zyngier

Hi Marc,

Quoted 49 lines · Marc Zyngier

ARMv9.5 has infroduced ID_AA64MMFR4_EL1 with a bunch of new features. Add the corresponding layout.

This is extracted from the public ARM SysReg_xml_A_profile-2023-09 delivery, timestamped d55f5af8e09052abe92a02adf820deea2eaed717.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>

---
arch/arm64/tools/sysreg | 37 +++++++++++++++++++++++++++++++++++++

1 file changed, 37 insertions(+)
arch/arm64/tools/sysreg +0 -0
index 4c9b67934367..fa3fe0856880 100644
@@ -1791,6 +1791,43 @@ UnsignedEnum 3:0 TCRX

EndEnum
EndSysreg

arch/arm64/tools/sysreg +27 -0
+Sysreg ID_AA64MMFR4_EL1 3 0 0 7 4
+Res0 63:40
+UnsignedEnum 39:36 E3DSE
+ 0b0000 NI
+ 0b0001 IMP
+EndEnum
+Res0 35:28
+SignedEnum 27:24 E2H0
+ 0b0000 IMP
+ 0b1110 NI_NV1
+ 0b1111 NI
+EndEnum
+UnsignedEnum 23:20 NV_frac
+ 0b0000 NV_NV2
+ 0b0001 NV2_ONLY
+EndEnum
+UnsignedEnum 19:16 FGWTE3
+ 0b0000 NI
+ 0b0001 IMP
+EndEnum
+UnsignedEnum 15:12 HACDBS
+ 0b0000 NI
+ 0b0001 IMP
+EndEnum
+UnsignedEnum 11:8 ASID2
+ 0b0000 NI
+ 0b0001 IMP

arch/arm64/tools/sysreg UnsignedEnum 3:0 TCRX · line 1830

Quoted 10 lines
arch/arm64/tools/sysreg +10 -0
+EndEnum
+SignedEnum 7:4 EIESB
+ 0b0000 NI
+ 0b0001 ToEL3
+ 0b0010 ToELx
+ 0b1111 ANY
+EndEnum
+Res0 3:0
+EndSysreg
+
Reviewed-by Miguel Luis <[email protected]>

Would you please help me understand how the kernel would cope with E2H0’s 0b0000 value on systems prior to ARMv9.5 where a read to ID_AA64MMFR4_EL1 would be RES0 ?

Thanks

Miguel

Quoted 11 lines

Sysreg SCTLR_EL1 3 0 1 0 0
Field 63 TIDCP
Field 62 SPINTMASK

Signature · 7 lines
2.39.2


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
author maintainer
#10 lore raw

Re: [PATCH v4 04/10] arm64: sysreg: Add layout for ID_AA64MMFR4_EL1

Quoted 65 lines · Miguel Luis

Hi Marc,

Quoted 49 lines · Marc Zyngier

ARMv9.5 has infroduced ID_AA64MMFR4_EL1 with a bunch of new features. Add the corresponding layout.

This is extracted from the public ARM SysReg_xml_A_profile-2023-09 delivery, timestamped d55f5af8e09052abe92a02adf820deea2eaed717.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>

---
arch/arm64/tools/sysreg | 37 +++++++++++++++++++++++++++++++++++++

1 file changed, 37 insertions(+)
arch/arm64/tools/sysreg +0 -0
index 4c9b67934367..fa3fe0856880 100644
@@ -1791,6 +1791,43 @@ UnsignedEnum 3:0 TCRX

EndEnum
EndSysreg

arch/arm64/tools/sysreg +27 -0
+Sysreg ID_AA64MMFR4_EL1 3 0 0 7 4
+Res0 63:40
+UnsignedEnum 39:36 E3DSE
+ 0b0000 NI
+ 0b0001 IMP
+EndEnum
+Res0 35:28
+SignedEnum 27:24 E2H0
+ 0b0000 IMP
+ 0b1110 NI_NV1
+ 0b1111 NI
+EndEnum
+UnsignedEnum 23:20 NV_frac
+ 0b0000 NV_NV2
+ 0b0001 NV2_ONLY
+EndEnum
+UnsignedEnum 19:16 FGWTE3
+ 0b0000 NI
+ 0b0001 IMP
+EndEnum
+UnsignedEnum 15:12 HACDBS
+ 0b0000 NI
+ 0b0001 IMP
+EndEnum
+UnsignedEnum 11:8 ASID2
+ 0b0000 NI
+ 0b0001 IMP

arch/arm64/tools/sysreg UnsignedEnum 3:0 TCRX · line 1830

Quoted 10 lines
arch/arm64/tools/sysreg +10 -0
+EndEnum
+SignedEnum 7:4 EIESB
+ 0b0000 NI
+ 0b0001 ToEL3
+ 0b0010 ToELx
+ 0b1111 ANY
+EndEnum
+Res0 3:0
+EndSysreg
+
Reviewed-by Miguel Luis <[email protected]>
Quoted 3 lines

Would you please help me understand how the kernel would cope with E2H0’s 0b0000 value on systems prior to ARMv9.5 where a read to ID_AA64MMFR4_EL1 would be RES0 ?

E2H0==0 means that E2H can be set to 0. Which is what MMFR4 always indicated before the introduction of this anti-feature. On any system where E2H0 reports 0, there is no change at all for SW. Only if you want to correctly deal with a system where E2H0 reports something other than 0 would you need to do something different. Which is what this series is about, mostly.

ARMv9.5 doesn't come into play, as this is an allowed anti-feature from ARMv8.1.

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
author maintainer
#11 lore raw

PATCH v4 5/10 arm64: cpufeature: Add ID_AA64MMFR4_EL1 handling

In reply to Marc Zyngier

Add ID_AA64MMFR4_EL1 to the list of idregs the kernel knows about, and describe the E2H0 field.

Reviewed-by Oliver Upton <[email protected]>
Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/cpu.h   | 1 +
 arch/arm64/kernel/cpufeature.c | 7 +++++++
 arch/arm64/kernel/cpuinfo.c    | 1 +
 3 files changed, 9 insertions(+)
arch/arm64/include/asm/cpu.h +1 -0
index b1e43f56ee46..6c13fd47e170 100644
@@ -56,6 +56,7 @@ struct cpuinfo_arm64 {
 	u64		reg_id_aa64mmfr1;
 	u64		reg_id_aa64mmfr2;
 	u64		reg_id_aa64mmfr3;
+	u64		reg_id_aa64mmfr4;
 	u64		reg_id_aa64pfr0;
 	u64		reg_id_aa64pfr1;
 	u64		reg_id_aa64zfr0;
arch/arm64/kernel/cpufeature.c +7 -0
index bae5d0655262..ad3753fbdcb1 100644
@@ -437,6 +437,11 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr3[] = {
 	ARM64_FTR_END,
 };
 
+static const struct arm64_ftr_bits ftr_id_aa64mmfr4[] = {
+	S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR4_EL1_E2H0_SHIFT, 4, 0),
+	ARM64_FTR_END,
+};
+
 static const struct arm64_ftr_bits ftr_ctr[] = {
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_EXACT, 31, 1, 1), /* RES1 */
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, CTR_EL0_DIC_SHIFT, 1, 1),

@@ -754,6 +759,7 @@ static const struct __ftr_reg_entry {
 			       &id_aa64mmfr1_override),
 	ARM64_FTR_REG(SYS_ID_AA64MMFR2_EL1, ftr_id_aa64mmfr2),
 	ARM64_FTR_REG(SYS_ID_AA64MMFR3_EL1, ftr_id_aa64mmfr3),
+	ARM64_FTR_REG(SYS_ID_AA64MMFR4_EL1, ftr_id_aa64mmfr4),
 
 	/* Op1 = 1, CRn = 0, CRm = 0 */
 	ARM64_FTR_REG(SYS_GMID_EL1, ftr_gmid),

@@ -1078,6 +1084,7 @@ void __init init_cpu_features(struct cpuinfo_arm64 *info)
 	init_cpu_ftr_reg(SYS_ID_AA64MMFR1_EL1, info->reg_id_aa64mmfr1);
 	init_cpu_ftr_reg(SYS_ID_AA64MMFR2_EL1, info->reg_id_aa64mmfr2);
 	init_cpu_ftr_reg(SYS_ID_AA64MMFR3_EL1, info->reg_id_aa64mmfr3);
+	init_cpu_ftr_reg(SYS_ID_AA64MMFR4_EL1, info->reg_id_aa64mmfr4);
 	init_cpu_ftr_reg(SYS_ID_AA64PFR0_EL1, info->reg_id_aa64pfr0);
 	init_cpu_ftr_reg(SYS_ID_AA64PFR1_EL1, info->reg_id_aa64pfr1);
 	init_cpu_ftr_reg(SYS_ID_AA64ZFR0_EL1, info->reg_id_aa64zfr0);
arch/arm64/kernel/cpuinfo.c +1 -0
index 47043c0d95ec..7ca3fbd200f0 100644
@@ -447,6 +447,7 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
 	info->reg_id_aa64mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
 	info->reg_id_aa64mmfr2 = read_cpuid(ID_AA64MMFR2_EL1);
 	info->reg_id_aa64mmfr3 = read_cpuid(ID_AA64MMFR3_EL1);
+	info->reg_id_aa64mmfr4 = read_cpuid(ID_AA64MMFR4_EL1);
 	info->reg_id_aa64pfr0 = read_cpuid(ID_AA64PFR0_EL1);
 	info->reg_id_aa64pfr1 = read_cpuid(ID_AA64PFR1_EL1);
 	info->reg_id_aa64zfr0 = read_cpuid(ID_AA64ZFR0_EL1);
Signature · 1 line
2.39.2
author maintainer
#13 lore raw

PATCH v4 6/10 arm64: cpufeature: Detect HCR_EL2.NV1 being RES0

In reply to Marc Zyngier

A variant of FEAT_E2H0 not being implemented exists in the form of HCR_EL2.E2H being RES1 *and* HCR_EL2.NV1 being RES0 (indicating that only VHE is supported on the host and nested guests).

Add the necessary infrastructure for this new CPU capability.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 12 ++++++++++++
 arch/arm64/tools/cpucaps       |  1 +
 2 files changed, 13 insertions(+)
arch/arm64/kernel/cpufeature.c +12 -0
index ad3753fbdcb1..91249d20883b 100644
@@ -1794,6 +1794,11 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
 	return !meltdown_safe;
 }
 
+static bool has_nv1(const struct arm64_cpu_capabilities *entry, int scope)
+{
+	return !has_cpuid_feature(entry, scope);
+}
+
 #if defined(ID_AA64MMFR0_EL1_TGRAN_LPA2) && defined(ID_AA64MMFR0_EL1_TGRAN_2_SUPPORTED_LPA2)
 static bool has_lpa2_at_stage1(u64 mmfr0)
 {

@@ -2794,6 +2799,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
 		.matches = has_lpa2,
 	},
+	{
+		.desc = "NV1",
+		.capability = ARM64_HAS_HCR_NV1,
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.matches = has_nv1,
+		ARM64_CPUID_FIELDS_NEG(ID_AA64MMFR4_EL1, E2H0, NI_NV1)
+	},
 	{},
 };
arch/arm64/tools/cpucaps +1 -0
index b912b1409fc0..65090dd34641 100644
@@ -35,6 +35,7 @@ HAS_GENERIC_AUTH_IMP_DEF
 HAS_GIC_CPUIF_SYSREGS
 HAS_GIC_PRIO_MASKING
 HAS_GIC_PRIO_RELAXED_SYNC
+HAS_HCR_NV1
 HAS_HCX
 HAS_LDAPR
 HAS_LPA2
Signature · 1 line
2.39.2
maintainer Reviewed-by on v4 6/10
#14 lore raw

Re: [PATCH v4 06/10] arm64: cpufeature: Detect HCR_EL2.NV1 being RES0

Quoted 8 lines · Marc Zyngier

A variant of FEAT_E2H0 not being implemented exists in the form of HCR_EL2.E2H being RES1 *and* HCR_EL2.NV1 being RES0 (indicating that only VHE is supported on the host and nested guests).

Add the necessary infrastructure for this new CPU capability.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
Reviewed-by Catalin Marinas <[email protected]>
maintainer Reviewed-by on v4 6/10
#15 lore raw

Re: [PATCH v4 06/10] arm64: cpufeature: Detect HCR_EL2.NV1 being RES0

In reply to Marc Zyngier
Quoted 8 lines · Marc Zyngier

A variant of FEAT_E2H0 not being implemented exists in the form of HCR_EL2.E2H being RES1 *and* HCR_EL2.NV1 being RES0 (indicating that only VHE is supported on the host and nested guests).

Add the necessary infrastructure for this new CPU capability.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
Reviewed-by Catalin Marinas <[email protected]>
#16 lore raw

Re: [PATCH v4 06/10] arm64: cpufeature: Detect HCR_EL2.NV1 being RES0

In reply to Marc Zyngier

Dead All,

Quoted 9 lines · Marc Zyngier

A variant of FEAT_E2H0 not being implemented exists in the form of HCR_EL2.E2H being RES1 *and* HCR_EL2.NV1 being RES0 (indicating that only VHE is supported on the host and nested guests).

Add the necessary infrastructure for this new CPU capability.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>

---

This patch landed recently in linux-next as commit da9af5071b25 ("arm64:
cpufeature: Detect HCR_EL2.NV1 being RES0"). I found that it causes a following regression in the CPU hot-plug operation:

# for i in /sys/devices/system/cpu/cpu[1-9]; do echo 1 >$i/online; done
------------[ cut here ]------------
kernel BUG at arch/arm64/kernel/cpufeature.c:1468!
Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
Modules linked in: dw_hdmi_cec dw_hdmi_i2s_audio crct10dif_ce 
snd_soc_simple_card rockchip_saradc industrialio_triggered_buffer 
hantro_vpu kfifo_buf snd_soc_simple_card_utils rockchip_thermal 
phy_rockchip_naneng_combphy display_connector gpio_ir_recv v4l2_vp9 
v4l2_h264 v4l2_mem2mem dwmac_rk videobuf2_dma_contig stmmac_platform 
videobuf2_memops videobuf2_v4l2 stmmac rockchipdrm videodev 
snd_soc_rockchip_i2s_tdm snd_soc_rk817 pcs_xpcs panfrost rtc_rk808 
rk805_pwrkey analogix_dp rk817_charger spi_rockchip_sfc dw_mipi_dsi 
videobuf2_common dw_hdmi drm_shmem_helper mc gpu_sched 
drm_display_helper ahci_dwc ip_tables x_tables ipv6
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 6.8.0-rc1+ #14563
Hardware name: Hardkernel ODROID-M1 (DT)
pstate: 800001c9 (Nzcv dAIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __read_sysreg_by_encoding+0x38c/0x390
lr : read_scoped_sysreg+0x4c/0x70
...
Call trace:
  __read_sysreg_by_encoding+0x38c/0x390
  read_scoped_sysreg+0x4c/0x70
  has_nv1+0x18/0x48
  verify_local_cpu_caps+0x54/0x124
  check_local_cpu_capabilities+0x28/0x208
  secondary_start_kernel+0xb0/0x154
  __secondary_switched+0xb8/0xbc
Code: d53802f3 17ffff3b d5380253 17ffff39 (d4210000)
---[ end trace 0000000000000000 ]---
Kernel panic - not syncing: Attempted to kill the idle task!
SMP: stopping secondary CPUs
Kernel Offset: disabled
CPU features: 0x1,80000000,80050295,2100721b
Memory Limit: none
---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---

It looks that some additional checks are needed for the CPU hot-plug case.

Quoted 39 lines
  arch/arm64/kernel/cpufeature.c | 12 ++++++++++++
  arch/arm64/tools/cpucaps       |  1 +
  2 files changed, 13 insertions(+)
arch/arm64/kernel/cpufeature.c +12 -0
index ad3753fbdcb1..91249d20883b 100644
@@ -1794,6 +1794,11 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
  	return !meltdown_safe;
  }
 
+static bool has_nv1(const struct arm64_cpu_capabilities *entry, int scope)
+{
+	return !has_cpuid_feature(entry, scope);
+}
+
  #if defined(ID_AA64MMFR0_EL1_TGRAN_LPA2) && defined(ID_AA64MMFR0_EL1_TGRAN_2_SUPPORTED_LPA2)
  static bool has_lpa2_at_stage1(u64 mmfr0)
  {

@@ -2794,6 +2799,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
  		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
  		.matches = has_lpa2,
  	},
+	{
+		.desc = "NV1",
+		.capability = ARM64_HAS_HCR_NV1,
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.matches = has_nv1,
+		ARM64_CPUID_FIELDS_NEG(ID_AA64MMFR4_EL1, E2H0, NI_NV1)
+	},
  	{},
  };
arch/arm64/tools/cpucaps +0 -0
index b912b1409fc0..65090dd34641 100644

arch/arm64/tools/cpucaps HAS_GENERIC_AUTH_IMP_DEF · line 41

Quoted 7 lines
arch/arm64/tools/cpucaps +1 -0
@@ -35,6 +35,7 @@ HAS_GENERIC_AUTH_IMP_DEF
  HAS_GIC_CPUIF_SYSREGS
  HAS_GIC_PRIO_MASKING
  HAS_GIC_PRIO_RELAXED_SYNC
+HAS_HCR_NV1
  HAS_HCX
  HAS_LDAPR
  HAS_LPA2

Best regards

Signature · 2 lines
Marek Szyprowski, PhD
Samsung R&D Institute Poland
author maintainer
#17 lore raw

Re: [PATCH v4 06/10] arm64: cpufeature: Detect HCR_EL2.NV1 being RES0

Quoted 55 lines · Marek Szyprowski

Dead All,

Quoted 9 lines · Marc Zyngier

A variant of FEAT_E2H0 not being implemented exists in the form of HCR_EL2.E2H being RES1 *and* HCR_EL2.NV1 being RES0 (indicating that only VHE is supported on the host and nested guests).

Add the necessary infrastructure for this new CPU capability.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>

---

This patch landed recently in linux-next as commit da9af5071b25 ("arm64:
cpufeature: Detect HCR_EL2.NV1 being RES0"). I found that it causes a following regression in the CPU hot-plug operation:

# for i in /sys/devices/system/cpu/cpu[1-9]; do echo 1 >$i/online; done
------------[ cut here ]------------
kernel BUG at arch/arm64/kernel/cpufeature.c:1468!
Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
Modules linked in: dw_hdmi_cec dw_hdmi_i2s_audio crct10dif_ce 
snd_soc_simple_card rockchip_saradc industrialio_triggered_buffer 
hantro_vpu kfifo_buf snd_soc_simple_card_utils rockchip_thermal 
phy_rockchip_naneng_combphy display_connector gpio_ir_recv v4l2_vp9 
v4l2_h264 v4l2_mem2mem dwmac_rk videobuf2_dma_contig stmmac_platform 
videobuf2_memops videobuf2_v4l2 stmmac rockchipdrm videodev 
snd_soc_rockchip_i2s_tdm snd_soc_rk817 pcs_xpcs panfrost rtc_rk808 
rk805_pwrkey analogix_dp rk817_charger spi_rockchip_sfc dw_mipi_dsi 
videobuf2_common dw_hdmi drm_shmem_helper mc gpu_sched 
drm_display_helper ahci_dwc ip_tables x_tables ipv6
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 6.8.0-rc1+ #14563
Hardware name: Hardkernel ODROID-M1 (DT)
pstate: 800001c9 (Nzcv dAIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __read_sysreg_by_encoding+0x38c/0x390
lr : read_scoped_sysreg+0x4c/0x70
...
Call trace:
  __read_sysreg_by_encoding+0x38c/0x390
  read_scoped_sysreg+0x4c/0x70
  has_nv1+0x18/0x48
  verify_local_cpu_caps+0x54/0x124
  check_local_cpu_capabilities+0x28/0x208
  secondary_start_kernel+0xb0/0x154
  __secondary_switched+0xb8/0xbc
Code: d53802f3 17ffff3b d5380253 17ffff39 (d4210000)
---[ end trace 0000000000000000 ]---
Kernel panic - not syncing: Attempted to kill the idle task!
SMP: stopping secondary CPUs
Kernel Offset: disabled
CPU features: 0x1,80000000,80050295,2100721b
Memory Limit: none
---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---
Quoted 1 line

It looks that some additional checks are needed for the CPU hot-plug case.

Nah, that's much simpler.

I simply missed a spot where ID_AA64MMFR4_EL1 must be handled (it is getting annoying that we have more than a single place where these "read all the ID registers" are handled).

Can you please give the following a go? If that works for you (it does for me here), I'll post an actual fix (plus another fix for another buglet I just noticed).

Thanks,

	M.
arch/arm64/kernel/cpufeature.c +1 -0
index 0f29ac43c7a2..2f8958f27e9e 100644
@@ -1456,6 +1456,7 @@ u64 __read_sysreg_by_encoding(u32 sys_id)
 	read_sysreg_case(SYS_ID_AA64MMFR1_EL1);
 	read_sysreg_case(SYS_ID_AA64MMFR2_EL1);
 	read_sysreg_case(SYS_ID_AA64MMFR3_EL1);
+	read_sysreg_case(SYS_ID_AA64MMFR4_EL1);
 	read_sysreg_case(SYS_ID_AA64ISAR0_EL1);
 	read_sysreg_case(SYS_ID_AA64ISAR1_EL1);
 	read_sysreg_case(SYS_ID_AA64ISAR2_EL1);
Signature · 1 line
Without deviation from the norm, progress is not possible.
Tested-by
#18 lore raw

Re: [PATCH v4 06/10] arm64: cpufeature: Detect HCR_EL2.NV1 being RES0

Quoted 56 lines · Marc Zyngier
Quoted 51 lines · Marek Szyprowski
Quoted 9 lines · Marc Zyngier

A variant of FEAT_E2H0 not being implemented exists in the form of HCR_EL2.E2H being RES1 *and* HCR_EL2.NV1 being RES0 (indicating that only VHE is supported on the host and nested guests).

Add the necessary infrastructure for this new CPU capability.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>

---

This patch landed recently in linux-next as commit da9af5071b25 ("arm64:
cpufeature: Detect HCR_EL2.NV1 being RES0"). I found that it causes a following regression in the CPU hot-plug operation:

# for i in /sys/devices/system/cpu/cpu[1-9]; do echo 1 >$i/online; done
------------[ cut here ]------------
kernel BUG at arch/arm64/kernel/cpufeature.c:1468!
Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
Modules linked in: dw_hdmi_cec dw_hdmi_i2s_audio crct10dif_ce
snd_soc_simple_card rockchip_saradc industrialio_triggered_buffer
hantro_vpu kfifo_buf snd_soc_simple_card_utils rockchip_thermal
phy_rockchip_naneng_combphy display_connector gpio_ir_recv v4l2_vp9
v4l2_h264 v4l2_mem2mem dwmac_rk videobuf2_dma_contig stmmac_platform
videobuf2_memops videobuf2_v4l2 stmmac rockchipdrm videodev
snd_soc_rockchip_i2s_tdm snd_soc_rk817 pcs_xpcs panfrost rtc_rk808
rk805_pwrkey analogix_dp rk817_charger spi_rockchip_sfc dw_mipi_dsi
videobuf2_common dw_hdmi drm_shmem_helper mc gpu_sched
drm_display_helper ahci_dwc ip_tables x_tables ipv6
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 6.8.0-rc1+ #14563
Hardware name: Hardkernel ODROID-M1 (DT)
pstate: 800001c9 (Nzcv dAIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __read_sysreg_by_encoding+0x38c/0x390
lr : read_scoped_sysreg+0x4c/0x70
...
Call trace:
   __read_sysreg_by_encoding+0x38c/0x390
   read_scoped_sysreg+0x4c/0x70
   has_nv1+0x18/0x48
   verify_local_cpu_caps+0x54/0x124
   check_local_cpu_capabilities+0x28/0x208
   secondary_start_kernel+0xb0/0x154
   __secondary_switched+0xb8/0xbc
Code: d53802f3 17ffff3b d5380253 17ffff39 (d4210000)
---[ end trace 0000000000000000 ]---
Kernel panic - not syncing: Attempted to kill the idle task!
SMP: stopping secondary CPUs
Kernel Offset: disabled
CPU features: 0x1,80000000,80050295,2100721b
Memory Limit: none
---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---
Quoted 1 line

It looks that some additional checks are needed for the CPU hot-plug case.

Nah, that's much simpler.

Quoted 7 lines

I simply missed a spot where ID_AA64MMFR4_EL1 must be handled (it is getting annoying that we have more than a single place where these "read all the ID registers" are handled).

Can you please give the following a go? If that works for you (it does for me here), I'll post an actual fix (plus another fix for another buglet I just noticed).

Works fine now.

Reported-by Marek Szyprowski <[email protected]>
Tested-by Marek Szyprowski <[email protected]>
Quoted 10 lines

Thanks,

	M.
arch/arm64/kernel/cpufeature.c +0 -0
index 0f29ac43c7a2..2f8958f27e9e 100644

arch/arm64/kernel/cpufeature.c u64 __read_sysreg_by_encoding(u32 sys_id) · line 1462

Quoted 7 lines
arch/arm64/kernel/cpufeature.c +1 -0
@@ -1456,6 +1456,7 @@ u64 __read_sysreg_by_encoding(u32 sys_id)
  	read_sysreg_case(SYS_ID_AA64MMFR1_EL1);
  	read_sysreg_case(SYS_ID_AA64MMFR2_EL1);
  	read_sysreg_case(SYS_ID_AA64MMFR3_EL1);
+	read_sysreg_case(SYS_ID_AA64MMFR4_EL1);
  	read_sysreg_case(SYS_ID_AA64ISAR0_EL1);
  	read_sysreg_case(SYS_ID_AA64ISAR1_EL1);
  	read_sysreg_case(SYS_ID_AA64ISAR2_EL1);

Best regards

Signature · 2 lines
Marek Szyprowski, PhD
Samsung R&D Institute Poland
author maintainer
#19 lore raw

PATCH v4 7/10 arm64: Treat HCR_EL2.E2H as RES1 when ID_AA64MMFR4_EL1.E2H0 is negative

In reply to Marc Zyngier

For CPUs that have ID_AA64MMFR4_EL1.E2H0 as negative, it is important to avoid the boot path that sets HCR_EL2.E2H=0. Fortunately, we already have this path to cope with fruity CPUs.

Tweak init_el2 to look at ID_AA64MMFR4_EL1.E2H0 first.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/head.S | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)
arch/arm64/kernel/head.S +15 -8
index cab7f91949d8..5bdafbcff009 100644
@@ -584,25 +584,32 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
 	mov_q	x1, INIT_SCTLR_EL1_MMU_OFF
 
 	/*
-	 * Fruity CPUs seem to have HCR_EL2.E2H set to RES1,
-	 * making it impossible to start in nVHE mode. Is that
-	 * compliant with the architecture? Absolutely not!
+	 * Compliant CPUs advertise their VHE-onlyness with
+	 * ID_AA64MMFR4_EL1.E2H0 < 0. HCR_EL2.E2H can be
+	 * RES1 in that case.
+	 *
+	 * Fruity CPUs seem to have HCR_EL2.E2H set to RES1, but
+	 * don't advertise it (they predate this relaxation).
 	 */
+	mrs_s	x0, SYS_ID_AA64MMFR4_EL1
+	ubfx	x0, x0, #ID_AA64MMFR4_EL1_E2H0_SHIFT, #ID_AA64MMFR4_EL1_E2H0_WIDTH
+	tbnz	x0, #(ID_AA64MMFR4_EL1_E2H0_SHIFT + ID_AA64MMFR4_EL1_E2H0_WIDTH - 1), 1f
+
 	mrs	x0, hcr_el2
 	and	x0, x0, #HCR_E2H
-	cbz	x0, 1f
-
+	cbz	x0, 2f
+1:
 	/* Set a sane SCTLR_EL1, the VHE way */
 	pre_disable_mmu_workaround
 	msr_s	SYS_SCTLR_EL12, x1
 	mov	x2, #BOOT_CPU_FLAG_E2H
-	b	2f
+	b	3f
 
-1:
+2:
 	pre_disable_mmu_workaround
 	msr	sctlr_el1, x1
 	mov	x2, xzr
-2:
+3:
 	__init_el2_nvhe_prepare_eret
 
 	mov	w0, #BOOT_CPU_MODE_EL2
Signature · 1 line
2.39.2
maintainer Reviewed-by on v4 7/10
#20 lore raw

Re: [PATCH v4 07/10] arm64: Treat HCR_EL2.E2H as RES1 when ID_AA64MMFR4_EL1.E2H0 is negative

Quoted 8 lines · Marc Zyngier

For CPUs that have ID_AA64MMFR4_EL1.E2H0 as negative, it is important to avoid the boot path that sets HCR_EL2.E2H=0. Fortunately, we already have this path to cope with fruity CPUs.

Tweak init_el2 to look at ID_AA64MMFR4_EL1.E2H0 first.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
Reviewed-by Catalin Marinas <[email protected]>
author maintainer
#21 lore raw

PATCH v4 8/10 KVM: arm64: Expose ID_AA64MMFR4_EL1 to guests

In reply to Marc Zyngier

We can now expose ID_AA64MMFR4_EL1 to guests, and let NV guests understand that they cannot really switch HCR_EL2.E2H to 0 on some platforms.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/nested.c   | 7 +++++++
 arch/arm64/kvm/sys_regs.c | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)
arch/arm64/kvm/nested.c +7 -0
index ba95d044bc98..d55e809e26cb 100644
@@ -133,6 +133,13 @@ static u64 limit_nv_id_reg(u32 id, u64 val)
 		val |= FIELD_PREP(NV_FTR(MMFR2, TTL), 0b0001);
 		break;
 
+	case SYS_ID_AA64MMFR4_EL1:
+		val = 0;
+		if (!cpus_have_final_cap(ARM64_HAS_HCR_NV1))
+			val |= FIELD_PREP(NV_FTR(MMFR4, E2H0),
+					  ID_AA64MMFR4_EL1_E2H0_NI_NV1);
+		break;
+
 	case SYS_ID_AA64DFR0_EL1:
 		/* Only limited support for PMU, Debug, BPs and WPs */
 		val &= (NV_FTR(DFR0, PMUVer)	|
arch/arm64/kvm/sys_regs.c +1 -1
index 88b8fbeafaa0..08a9571fa809 100644
@@ -2350,7 +2350,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 					ID_AA64MMFR2_EL1_NV |
 					ID_AA64MMFR2_EL1_CCIDX)),
 	ID_SANITISED(ID_AA64MMFR3_EL1),
-	ID_UNALLOCATED(7,4),
+	ID_SANITISED(ID_AA64MMFR4_EL1),
 	ID_UNALLOCATED(7,5),
 	ID_UNALLOCATED(7,6),
 	ID_UNALLOCATED(7,7),
Signature · 1 line
2.39.2
author maintainer
#22 lore raw

PATCH v4 9/10 KVM: arm64: Force guest's HCR_EL2.E2H RES1 when NV1 is not implemented

In reply to Marc Zyngier

If NV1 isn't supported on a system, make sure we always evaluate the guest's HCR_EL2.E2H as RES1, irrespective of what the guest may have written there.

Reviewed-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/kvm_emulate.h |  3 ++-
 arch/arm64/kvm/sys_regs.c            | 12 +++++++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)
arch/arm64/include/asm/kvm_emulate.h +2 -1
index b804fe832184..debc3753d2ef 100644
@@ -209,7 +209,8 @@ static inline bool vcpu_is_el2(const struct kvm_vcpu *vcpu)
 
 static inline bool __vcpu_el2_e2h_is_set(const struct kvm_cpu_context *ctxt)
 {
-	return ctxt_sys_reg(ctxt, HCR_EL2) & HCR_E2H;
+	return (!cpus_have_final_cap(ARM64_HAS_HCR_NV1) ||
+		(ctxt_sys_reg(ctxt, HCR_EL2) & HCR_E2H));
 }
 
 static inline bool vcpu_el2_e2h_is_set(const struct kvm_vcpu *vcpu)
arch/arm64/kvm/sys_regs.c +11 -1
index 08a9571fa809..041b11825578 100644
@@ -2175,6 +2175,16 @@ static bool access_spsr(struct kvm_vcpu *vcpu,
 	return true;
 }
 
+static u64 reset_hcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
+{
+	u64 val = r->val;
+
+	if (!cpus_have_final_cap(ARM64_HAS_HCR_NV1))
+		val |= HCR_E2H;
+
+	return __vcpu_sys_reg(vcpu, r->reg) = val;
+}
+
 /*
  * Architected system registers.
  * Important: Must be sorted ascending by Op0, Op1, CRn, CRm, Op2

@@ -2666,7 +2676,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	EL2_REG_VNCR(VMPIDR_EL2, reset_unknown, 0),
 	EL2_REG(SCTLR_EL2, access_rw, reset_val, SCTLR_EL2_RES1),
 	EL2_REG(ACTLR_EL2, access_rw, reset_val, 0),
-	EL2_REG_VNCR(HCR_EL2, reset_val, 0),
+	EL2_REG_VNCR(HCR_EL2, reset_hcr, 0),
 	EL2_REG(MDCR_EL2, access_rw, reset_val, 0),
 	EL2_REG(CPTR_EL2, access_rw, reset_val, CPTR_NVHE_EL2_RES1),
 	EL2_REG_VNCR(HSTR_EL2, reset_val, 0),
Signature · 1 line
2.39.2
author maintainer
#23 lore raw

PATCH v4 10/10 KVM: arm64: Handle Apple M2 as not having HCR_EL2.NV1 implemented

In reply to Marc Zyngier

Although the Apple M2 family of CPUs can have HCR_EL2.NV1 being set and clear, with the change in trap behaviour being OK, they explode spectacularily on an EL2 S1 page table using the nVHE format. This is no good.

Let's pretend this HW doesn't have NV1, and move along.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
arch/arm64/kernel/cpufeature.c +17 -1
index 91249d20883b..0f29ac43c7a2 100644
@@ -1796,7 +1796,23 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
 
 static bool has_nv1(const struct arm64_cpu_capabilities *entry, int scope)
 {
-	return !has_cpuid_feature(entry, scope);
+	/*
+	 * Although the Apple M2 family appears to support NV1, the
+	 * PTW barfs on the nVHE EL2 S1 page table format. Pretend
+	 * that it doesn't support NV1 at all.
+	 */
+	static const struct midr_range nv1_ni_list[] = {
+		MIDR_ALL_VERSIONS(MIDR_APPLE_M2_BLIZZARD),
+		MIDR_ALL_VERSIONS(MIDR_APPLE_M2_AVALANCHE),
+		MIDR_ALL_VERSIONS(MIDR_APPLE_M2_BLIZZARD_PRO),
+		MIDR_ALL_VERSIONS(MIDR_APPLE_M2_AVALANCHE_PRO),
+		MIDR_ALL_VERSIONS(MIDR_APPLE_M2_BLIZZARD_MAX),
+		MIDR_ALL_VERSIONS(MIDR_APPLE_M2_AVALANCHE_MAX),
+		{}
+	};
+
+	return !(has_cpuid_feature(entry, scope) ||
+		 is_midr_in_range_list(read_cpuid_id(), nv1_ni_list));
 }
 
 #if defined(ID_AA64MMFR0_EL1_TGRAN_LPA2) && defined(ID_AA64MMFR0_EL1_TGRAN_2_SUPPORTED_LPA2)
Signature · 1 line
2.39.2
maintainer Acked-by on v4 10/10
#24 lore raw

Re: [PATCH v4 10/10] KVM: arm64: Handle Apple M2 as not having HCR_EL2.NV1 implemented

Quoted 8 lines · Marc Zyngier

Although the Apple M2 family of CPUs can have HCR_EL2.NV1 being set and clear, with the change in trap behaviour being OK, they explode spectacularily on an EL2 S1 page table using the nVHE format. This is no good.

Let's pretend this HW doesn't have NV1, and move along.

Signed-off-by Marc Zyngier <[email protected]>
Acked-by Catalin Marinas <[email protected]>
#25 lore raw
In reply to Marc Zyngier
Quoted 8 lines · Marc Zyngier

Since ARMv8.1, the architecture has grown the VHE feature, which makes EL2 a superset of EL1. With ARMv9.5 (and retroactively allowed from ARMv8.1), the architecture allows implementations to have VHE as the *only* implemented behaviour, meaning that HCR_EL2.E2H can be implemented as RES1. As a follow-up, HCR_EL2.NV1 can also be implemented as RES0, making the VHE-ness of the architecture recursive.

Quoted 7 lines

This has a number of consequences, both at boot time and for KVM, though the changes at that level are pretty minor.

The real meat of this series is on the cpufeature front, as FEAT_E2H0 is a *negative* feature, where 0b1111 (-1) represents E2H being RES1 and 0b1110 (-2) additionally indicates that NV1 is RES0. Fun, isn't it?

This looks good to me. Catalin + Will, are you comfortable with the cpufeature changes at this point?

No strong opinions on which tree these patches get applied to. Most of the series is non-KVM code, so I'd understand if y'all wanted it to go through arm64. OTOH, there are some other KVM features that might appear in 6.9 that build on top of this, so at minimum I'd probably need a shared branch.

LMK what you think.

Signature · 2 lines
Thanks,
Oliver
maintainer
#26 lore raw
Quoted 12 lines · Oliver Upton
Quoted 11 lines · Marc Zyngier

Since ARMv8.1, the architecture has grown the VHE feature, which makes EL2 a superset of EL1. With ARMv9.5 (and retroactively allowed from ARMv8.1), the architecture allows implementations to have VHE as the *only* implemented behaviour, meaning that HCR_EL2.E2H can be implemented as RES1. As a follow-up, HCR_EL2.NV1 can also be implemented as RES0, making the VHE-ness of the architecture recursive.

This has a number of consequences, both at boot time and for KVM, though the changes at that level are pretty minor.

Quoted 7 lines
Quoted 4 lines

The real meat of this series is on the cpufeature front, as FEAT_E2H0 is a *negative* feature, where 0b1111 (-1) represents E2H being RES1 and 0b1110 (-2) additionally indicates that NV1 is RES0. Fun, isn't it?

This looks good to me. Catalin + Will, are you comfortable with the cpufeature changes at this point?

Yes, I acked those touching the arm64 files.

Quoted 5 lines

No strong opinions on which tree these patches get applied to. Most of the series is non-KVM code, so I'd understand if y'all wanted it to go through arm64. OTOH, there are some other KVM features that might appear in 6.9 that build on top of this, so at minimum I'd probably need a shared branch.

Please take it through the arm64 kvm tree but keep it on a separate stable branch in case I need to pull it.

Signature · 1 line
Catalin
#27 lore raw
In reply to Marc Zyngier
Quoted 9 lines · Marc Zyngier

Since ARMv8.1, the architecture has grown the VHE feature, which makes EL2 a superset of EL1. With ARMv9.5 (and retroactively allowed from ARMv8.1), the architecture allows implementations to have VHE as the *only* implemented behaviour, meaning that HCR_EL2.E2H can be implemented as RES1. As a follow-up, HCR_EL2.NV1 can also be implemented as RES0, making the VHE-ness of the architecture recursive.

[...]

Thanks folks for having taken a look at the series. I've pushed it out to kvmarm/next.

Catalin, as requested here is the shared branch:

https://git.kernel.org/pub/scm/linux/kernel/git/oupton/linux.git/log/?h=kvm-arm64/feat_e2h0

[01/10] arm64: Add macro to compose a sysreg field value

        https://git.kernel.org/kvmarm/kvmarm/c/53eaeb7fbe27
[02/10] arm64: cpufeatures: Correctly handle signed values
        https://git.kernel.org/kvmarm/kvmarm/c/d9a065914dcc
[03/10] arm64: cpufeature: Correctly display signed override values
        https://git.kernel.org/kvmarm/kvmarm/c/d42bf63fd4db
[04/10] arm64: sysreg: Add layout for ID_AA64MMFR4_EL1
        https://git.kernel.org/kvmarm/kvmarm/c/cfc680bb04c5
[05/10] arm64: cpufeature: Add ID_AA64MMFR4_EL1 handling
        https://git.kernel.org/kvmarm/kvmarm/c/805bb61f8279
[06/10] arm64: cpufeature: Detect HCR_EL2.NV1 being RES0
        https://git.kernel.org/kvmarm/kvmarm/c/da9af5071b25
[07/10] arm64: Treat HCR_EL2.E2H as RES1 when ID_AA64MMFR4_EL1.E2H0 is negative
        https://git.kernel.org/kvmarm/kvmarm/c/3944382fa6f2
[08/10] KVM: arm64: Expose ID_AA64MMFR4_EL1 to guests
        https://git.kernel.org/kvmarm/kvmarm/c/c21df6e43f0e
[09/10] KVM: arm64: Force guest's HCR_EL2.E2H RES1 when NV1 is not implemented
        https://git.kernel.org/kvmarm/kvmarm/c/94f29ab2d801
[10/10] KVM: arm64: Handle Apple M2 as not having HCR_EL2.NV1 implemented
        https://git.kernel.org/kvmarm/kvmarm/c/aade38faca63

--
Best,
Oliver