Skip to contentSkip to conversation

KVM: arm64: Add support for FEAT_IDST

PATCH Applied Marc Zyngier 1 participant 11 messages Jan 8 – Jan 15

Subsystems (4)
lore
Share & tools
lore

Marc Zyngier

author maintainer
#0 lore raw

v4 cover KVM: arm64: Add support for FEAT_IDST

FEAT_IDST appeared in ARMv8.4, and allows ID registers to be trapped if they are not implemented. This only concerns 3 registers (GMID_EL1, CCSIDR2_EL1 and SMIDR_EL1), which are part of features that may not be exposed to the guest even if present on the host.

For these registers, the HW should report them with EC=0x18, even if the feature isn't implemented.

Add support for this feature by handling these registers in a specific way and implementing GMID_EL1 support in the process. A very basic selftest checks that these registers behave as expected.

* From v3: [3]

- Added ID_AA64MMFR2_EL1.IDS == EL3 (Ben)

- Introduced in_feat_id_space() helper (Yao)

- Collected RBs, with thanks

* From v2: [2]

- Repainted ID_AA64MMFR2_EL1.IDS description (Oliver)

- Made the IDST handling more generic in the core KVM code, which resulted in the series being restructured a bit

- Added handling to pKVM (in a slightly different way, as pKVM insist on seeing a full enumeration of the trapped registers)

- Some cleanups

- Collected RBs, with thanks

* From v1: [1]

- Fixed commit message in patch #4 (Ben)

- Collected RB, with thanks (Joey)

[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 (9):
arm64: Repaint ID_AA64MMFR2_EL1.IDS description
KVM: arm64: Add trap routing for GMID_EL1
KVM: arm64: Add a generic synchronous exception injection primitive
KVM: arm64: Handle FEAT_IDST for sysregs without specific handlers
KVM: arm64: Handle CSSIDR2_EL1 and SMIDR_EL1 in a generic way
KVM: arm64: Force trap of GMID_EL1 when the guest doesn't have MTE
KVM: arm64: pkvm: Add a generic synchronous exception injection
primitive
KVM: arm64: pkvm: Report optional ID register traps with a 0x18
syndrome
KVM: arm64: selftests: Add a test for FEAT_IDST

 arch/arm64/include/asm/kvm_emulate.h          |   1 +
 arch/arm64/kvm/emulate-nested.c               |  21 ++++
 arch/arm64/kvm/hyp/nvhe/sys_regs.c            |  39 ++++--
 arch/arm64/kvm/inject_fault.c                 |  10 +-
 arch/arm64/kvm/sys_regs.c                     |   4 +-
 arch/arm64/kvm/sys_regs.h                     |  10 ++
 arch/arm64/tools/sysreg                       |   7 +-
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../testing/selftests/kvm/arm64/idreg-idst.c  | 117 ++++++++++++++++++
 9 files changed, 194 insertions(+), 16 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/arm64/idreg-idst.c
Signature · 1 line
2.47.3
author maintainer
#1 lore raw

PATCH v4 1/9 arm64: Repaint ID_AA64MMFR2_EL1.IDS description

ID_AA64MMFR2_EL1.IDS, as described in the sysreg file, is pretty horrible as it diesctly give the ESR value. Repaint it using the usual NI/IMP identifiers to describe the absence/presence of FEAT_IDST.

Also add the new EL3 routing feature, even if we really don't care about it.

---
 arch/arm64/kvm/hyp/nvhe/sys_regs.c | 2 +-
 arch/arm64/tools/sysreg            | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)
arch/arm64/kvm/hyp/nvhe/sys_regs.c +1 -1
index 3108b5185c204..4db0562f5bfa4 100644
@@ -134,7 +134,7 @@ static const struct pvm_ftr_bits pvmid_aa64mmfr2[] = {
 	MAX_FEAT(ID_AA64MMFR2_EL1, UAO, IMP),
 	MAX_FEAT(ID_AA64MMFR2_EL1, IESB, IMP),
 	MAX_FEAT(ID_AA64MMFR2_EL1, AT, IMP),
-	MAX_FEAT_ENUM(ID_AA64MMFR2_EL1, IDS, 0x18),
+	MAX_FEAT(ID_AA64MMFR2_EL1, IDS, IMP),
 	MAX_FEAT(ID_AA64MMFR2_EL1, TTL, IMP),
 	MAX_FEAT(ID_AA64MMFR2_EL1, BBM, 2),
 	MAX_FEAT(ID_AA64MMFR2_EL1, E0PD, IMP),
arch/arm64/tools/sysreg +4 -3
index 8921b51866d64..d0ddfd572b899 100644
@@ -2256,9 +2256,10 @@ UnsignedEnum	43:40	FWB
 	0b0000	NI
 	0b0001	IMP
 EndEnum
-Enum	39:36	IDS
-	0b0000	0x0
-	0b0001	0x18
+UnsignedEnum	39:36	IDS
+	0b0000	NI
+	0b0001	IMP
+	0b0010	EL3
 EndEnum
 UnsignedEnum	35:32	AT
 	0b0000	NI
Signature · 1 line
2.47.3
author maintainer
#2 lore raw

PATCH v4 2/9 KVM: arm64: Add trap routing for GMID_EL1

In reply to Marc Zyngier

HCR_EL2.TID5 is currently ignored by the trap routing infrastructure. Wire it in the routing table so that GMID_EL1, the sole register trapped by this bit, is correctly handled in the NV case.

---
 arch/arm64/kvm/emulate-nested.c | 8 ++++++++
 1 file changed, 8 insertions(+)
arch/arm64/kvm/emulate-nested.c +8 -0
index 834f13fb1fb7d..616eb6ad68701 100644
@@ -70,6 +70,7 @@ enum cgt_group_id {
 	CGT_HCR_ENSCXT,
 	CGT_HCR_TTLBIS,
 	CGT_HCR_TTLBOS,
+	CGT_HCR_TID5,
 
 	CGT_MDCR_TPMCR,
 	CGT_MDCR_TPM,
@@ -308,6 +309,12 @@ static const struct trap_bits coarse_trap_bits[] = {
 		.mask		= HCR_TTLBOS,
 		.behaviour	= BEHAVE_FORWARD_RW,
 	},
+	[CGT_HCR_TID5] = {
+		.index		= HCR_EL2,
+		.value		= HCR_TID5,
+		.mask		= HCR_TID5,
+		.behaviour	= BEHAVE_FORWARD_RW,
+	},
 	[CGT_MDCR_TPMCR] = {
 		.index		= MDCR_EL2,
 		.value		= MDCR_EL2_TPMCR,
@@ -665,6 +672,7 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = {
 	SR_TRAP(SYS_CCSIDR2_EL1,	CGT_HCR_TID2_TID4),
 	SR_TRAP(SYS_CLIDR_EL1,		CGT_HCR_TID2_TID4),
 	SR_TRAP(SYS_CSSELR_EL1,		CGT_HCR_TID2_TID4),
+	SR_TRAP(SYS_GMID_EL1,		CGT_HCR_TID5),
 	SR_RANGE_TRAP(SYS_ID_PFR0_EL1,
 		      sys_reg(3, 0, 0, 7, 7), CGT_HCR_TID3),
 	SR_TRAP(SYS_ICC_SGI0R_EL1,	CGT_HCR_IMO_FMO_ICH_HCR_TC),
Signature · 1 line
2.47.3
author maintainer
#3 lore raw

PATCH v4 3/9 KVM: arm64: Add a generic synchronous exception injection primitive

In reply to Marc Zyngier

Maybe in a surprising way, we don't currently have a generic way to inject a synchronous exception at the EL the vcpu is currently running at.

Extract such primitive from the UNDEF injection code.

---
 arch/arm64/include/asm/kvm_emulate.h |  1 +
 arch/arm64/kvm/inject_fault.c        | 10 +++++++---
 2 files changed, 8 insertions(+), 3 deletions(-)
arch/arm64/include/asm/kvm_emulate.h +1 -0
index c9eab316398e2..df20d47f0d256 100644
@@ -45,6 +45,7 @@ bool kvm_condition_valid32(const struct kvm_vcpu *vcpu);
 void kvm_skip_instr32(struct kvm_vcpu *vcpu);
 
 void kvm_inject_undefined(struct kvm_vcpu *vcpu);
+void kvm_inject_sync(struct kvm_vcpu *vcpu, u64 esr);
 int kvm_inject_serror_esr(struct kvm_vcpu *vcpu, u64 esr);
 int kvm_inject_sea(struct kvm_vcpu *vcpu, bool iabt, u64 addr);
 void kvm_inject_size_fault(struct kvm_vcpu *vcpu);
arch/arm64/kvm/inject_fault.c +7 -3
index dfcd66c655179..7102424a3fa5e 100644
@@ -162,12 +162,16 @@ static void inject_abt64(struct kvm_vcpu *vcpu, bool is_iabt, unsigned long addr
 	vcpu_write_sys_reg(vcpu, esr, exception_esr_elx(vcpu));
 }
 
+void kvm_inject_sync(struct kvm_vcpu *vcpu, u64 esr)
+{
+	pend_sync_exception(vcpu);
+	vcpu_write_sys_reg(vcpu, esr, exception_esr_elx(vcpu));
+}
+
 static void inject_undef64(struct kvm_vcpu *vcpu)
 {
 	u64 esr = (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT);
 
-	pend_sync_exception(vcpu);
-
 	/*
 	 * Build an unknown exception, depending on the instruction
 	 * set.
@@ -175,7 +179,7 @@ static void inject_undef64(struct kvm_vcpu *vcpu)
 	if (kvm_vcpu_trap_il_is32bit(vcpu))
 		esr |= ESR_ELx_IL;
 
-	vcpu_write_sys_reg(vcpu, esr, exception_esr_elx(vcpu));
+	kvm_inject_sync(vcpu, esr);
 }
 
 #define DFSR_FSC_EXTABT_LPAE	0x10
Signature · 1 line
2.47.3
author maintainer
#4 lore raw

PATCH v4 4/9 KVM: arm64: Handle FEAT_IDST for sysregs without specific handlers

In reply to Marc Zyngier

Add a bit of infrastrtcture to triage_sysreg_trap() to handle the case of registers falling into the Feature ID space that do not have a local handler.

For these, we can directly apply the FEAT_IDST semantics and inject an EC=0x18 exception. Otherwise, an UNDEF will do.

---
 arch/arm64/kvm/emulate-nested.c | 13 +++++++++++++
 arch/arm64/kvm/sys_regs.h       | 10 ++++++++++
 2 files changed, 23 insertions(+)
arch/arm64/kvm/emulate-nested.c +13 -0
index 616eb6ad68701..4aabd624c4be5 100644
@@ -2588,6 +2588,19 @@ bool triage_sysreg_trap(struct kvm_vcpu *vcpu, int *sr_index)
 
 		params = esr_sys64_to_params(esr);
 
+		/*
+		 * This implements the pseudocode UnimplementedIDRegister()
+		 * helper for the purpose of dealing with FEAT_IDST.
+		 */
+		if (in_feat_id_space(&params)) {
+			if (kvm_has_feat(vcpu->kvm, ID_AA64MMFR2_EL1, IDS, IMP))
+				kvm_inject_sync(vcpu, kvm_vcpu_get_esr(vcpu));
+			else
+				kvm_inject_undefined(vcpu);
+
+			return true;
+		}
+
 		/*
 		 * Check for the IMPDEF range, as per DDI0487 J.a,
 		 * D18.3.2 Reserved encodings for IMPLEMENTATION
arch/arm64/kvm/sys_regs.h +10 -0
index b3f904472fac5..2a983664220ce 100644
@@ -49,6 +49,16 @@ struct sys_reg_params {
 				  .Op2 = ((esr) >> 17) & 0x7,			\
 				  .is_write = !((esr) & 1) })
 
+/*
+ * The Feature ID space is defined as the System register space in AArch64
+ * with op0==3, op1=={0, 1, 3}, CRn==0, CRm=={0-7}, op2=={0-7}.
+ */
+static inline bool in_feat_id_space(struct sys_reg_params *p)
+{
+	return (p->Op0 == 3 && !(p->Op1 & 0b100) && p->Op1 != 2 &&
+		p->CRn == 0 && !(p->CRm & 0b1000));
+}
+
 struct sys_reg_desc {
 	/* Sysreg string for debug */
 	const char *name;
Signature · 1 line
2.47.3
author maintainer
#5 lore raw

PATCH v4 5/9 KVM: arm64: Handle CSSIDR2_EL1 and SMIDR_EL1 in a generic way

In reply to Marc Zyngier

Now that we can handle ID registers using the FEAT_IDST infrastrcuture, get rid of the handling of CSSIDR2_EL1 and SMIDR_EL1.

---
 arch/arm64/kvm/sys_regs.c | 2 --
 1 file changed, 2 deletions(-)
arch/arm64/kvm/sys_regs.c +0 -2
index c8fd7c6a12a13..a2b14ca2a702b 100644
@@ -3414,8 +3414,6 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_CCSIDR_EL1), access_ccsidr },
 	{ SYS_DESC(SYS_CLIDR_EL1), access_clidr, reset_clidr, CLIDR_EL1,
 	  .set_user = set_clidr, .val = ~CLIDR_EL1_RES0 },
-	{ SYS_DESC(SYS_CCSIDR2_EL1), undef_access },
-	{ SYS_DESC(SYS_SMIDR_EL1), undef_access },
 	IMPLEMENTATION_ID(AIDR_EL1, GENMASK_ULL(63, 0)),
 	{ SYS_DESC(SYS_CSSELR_EL1), access_csselr, reset_unknown, CSSELR_EL1 },
 	ID_FILTERED(CTR_EL0, ctr_el0,
Signature · 1 line
2.47.3
author maintainer
#6 lore raw

PATCH v4 6/9 KVM: arm64: Force trap of GMID_EL1 when the guest doesn't have MTE

In reply to Marc Zyngier

If our host has MTE, but the guest doesn't, make sure we set HCR_EL2.TID5 to force GMID_EL1 being trapped. Such trap will be handled by the FEAT_IDST handling.

---
 arch/arm64/kvm/sys_regs.c | 2 ++
 1 file changed, 2 insertions(+)
arch/arm64/kvm/sys_regs.c +2 -0
index a2b14ca2a702b..3de206b494849 100644
@@ -5576,6 +5576,8 @@ static void vcpu_set_hcr(struct kvm_vcpu *vcpu)
 
 	if (kvm_has_mte(vcpu->kvm))
 		vcpu->arch.hcr_el2 |= HCR_ATA;
+	else
+		vcpu->arch.hcr_el2 |= HCR_TID5;
 
 	/*
 	 * In the absence of FGT, we cannot independently trap TLBI
Signature · 1 line
2.47.3
author maintainer
#7 lore raw

PATCH v4 7/9 KVM: arm64: pkvm: Add a generic synchronous exception injection primitive

In reply to Marc Zyngier

Similarly to the "classic" KVM code, pKVM doesn't have an "anything goes" synchronous exception injection primitive.

Carve one out of the UNDEF injection code.

---
 arch/arm64/kvm/hyp/nvhe/sys_regs.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)
arch/arm64/kvm/hyp/nvhe/sys_regs.c +15 -7
index 4db0562f5bfa4..b197f3eb272c1 100644
@@ -243,16 +243,15 @@ static u64 pvm_calc_id_reg(const struct kvm_vcpu *vcpu, u32 id)
 	}
 }
 
-/*
- * Inject an unknown/undefined exception to an AArch64 guest while most of its
- * sysregs are live.
- */
-static void inject_undef64(struct kvm_vcpu *vcpu)
+static void inject_sync64(struct kvm_vcpu *vcpu, u64 esr)
 {
-	u64 esr = (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT);
-
 	*vcpu_pc(vcpu) = read_sysreg_el2(SYS_ELR);
 	*vcpu_cpsr(vcpu) = read_sysreg_el2(SYS_SPSR);
+
+	/*
+	 * Make sure we have the latest update to VBAR_EL1, as pKVM
+	 * handles traps very early, before sysregs are resync'ed
+	 */
 	__vcpu_assign_sys_reg(vcpu, VBAR_EL1, read_sysreg_el1(SYS_VBAR));
 
 	kvm_pend_exception(vcpu, EXCEPT_AA64_EL1_SYNC);
@@ -265,6 +264,15 @@ static void inject_undef64(struct kvm_vcpu *vcpu)
 	write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR);
 }
 
+/*
+ * Inject an unknown/undefined exception to an AArch64 guest while most of its
+ * sysregs are live.
+ */
+static void inject_undef64(struct kvm_vcpu *vcpu)
+{
+	inject_sync64(vcpu, (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT));
+}
+
 static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 		       struct sys_reg_desc const *r)
 {
Signature · 1 line
2.47.3
author maintainer
#8 lore raw

PATCH v4 8/9 KVM: arm64: pkvm: Report optional ID register traps with a 0x18 syndrome

In reply to Marc Zyngier

With FEAT_IDST, unimplemented system registers in the feature ID space must be reported using EC=0x18 at the closest handling EL, rather than with an UNDEF.

Most of these system registers are always implemented thanks to their dependency on FEAT_AA64, except for a set of (currently) three registers:
GMID_EL1 (depending on MTE2), CCSIDR2_EL1 (depending on FEAT_CCIDX), and SMIDR_EL1 (depending on SME).

For these three registers, report their trap as EC=0x18 if they end-up trapping into KVM and that FEAT_IDST is implemented in the guest. Otherwise, just make them UNDEF.

---
 arch/arm64/kvm/hyp/nvhe/sys_regs.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
arch/arm64/kvm/hyp/nvhe/sys_regs.c +15 -0
index b197f3eb272c1..06d28621722ee 100644
@@ -347,6 +347,18 @@ static bool pvm_gic_read_sre(struct kvm_vcpu *vcpu,
 	return true;
 }
 
+static bool pvm_idst_access(struct kvm_vcpu *vcpu,
+			    struct sys_reg_params *p,
+			    const struct sys_reg_desc *r)
+{
+	if (kvm_has_feat(vcpu->kvm, ID_AA64MMFR2_EL1, IDS, IMP))
+		inject_sync64(vcpu, kvm_vcpu_get_esr(vcpu));
+	else
+		inject_undef64(vcpu);
+
+	return false;
+}
+
 /* Mark the specified system register as an AArch32 feature id register. */
 #define AARCH32(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch32 }
 
@@ -477,6 +489,9 @@ static const struct sys_reg_desc pvm_sys_reg_descs[] = {
 
 	HOST_HANDLED(SYS_CCSIDR_EL1),
 	HOST_HANDLED(SYS_CLIDR_EL1),
+	{ SYS_DESC(SYS_CCSIDR2_EL1), .access = pvm_idst_access },
+	{ SYS_DESC(SYS_GMID_EL1), .access = pvm_idst_access },
+	{ SYS_DESC(SYS_SMIDR_EL1), .access = pvm_idst_access },
 	HOST_HANDLED(SYS_AIDR_EL1),
 	HOST_HANDLED(SYS_CSSELR_EL1),
 	HOST_HANDLED(SYS_CTR_EL0),
Signature · 1 line
2.47.3
author maintainer
#9 lore raw

PATCH v4 9/9 KVM: arm64: selftests: Add a test for FEAT_IDST

In reply to Marc Zyngier

Add a very basic test checking that FEAT_IDST actually works for the {GMID,SMIDR,CSSIDR2}_EL1 registers.

---
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../testing/selftests/kvm/arm64/idreg-idst.c  | 117 ++++++++++++++++++
 2 files changed, 118 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/arm64/idreg-idst.c
tools/testing/selftests/kvm/Makefile.kvm +1 -0
index ba5c2b643efaa..bfa2fad0aba12 100644
@@ -175,6 +175,7 @@ TEST_GEN_PROGS_arm64 += arm64/vgic_irq
 TEST_GEN_PROGS_arm64 += arm64/vgic_lpi_stress
 TEST_GEN_PROGS_arm64 += arm64/vpmu_counter_access
 TEST_GEN_PROGS_arm64 += arm64/no-vgic-v3
+TEST_GEN_PROGS_arm64 += arm64/idreg-idst
 TEST_GEN_PROGS_arm64 += arm64/kvm-uuid
 TEST_GEN_PROGS_arm64 += access_tracking_perf_test
 TEST_GEN_PROGS_arm64 += arch_timer
tools/testing/selftests/kvm/arm64/idreg-idst.c new file +117 -0

117 lines not on this page · raw message

Signature · 1 line
2.47.3
author maintainer Applied
#10 lore raw
In reply to Marc Zyngier
Quoted 9 lines · Marc Zyngier

FEAT_IDST appeared in ARMv8.4, and allows ID registers to be trapped if they are not implemented. This only concerns 3 registers (GMID_EL1, CCSIDR2_EL1 and SMIDR_EL1), which are part of features that may not be exposed to the guest even if present on the host.

For these registers, the HW should report them with EC=0x18, even if the feature isn't implemented.

[...]

Applied to next, thanks!

[1/9] arm64: Repaint ID_AA64MMFR2_EL1.IDS description

      commit: 4a7fe842b8a3f3c173c3075f03c60c3f9f62e299
[2/9] KVM: arm64: Add trap routing for GMID_EL1
      commit: 1ad9767accfcb81f404aa3d37d46b3eb494dce2f
[3/9] KVM: arm64: Add a generic synchronous exception injection primitive
      commit: 19f75678238734ef383f9e10d8e1020873e97170
[4/9] KVM: arm64: Handle FEAT_IDST for sysregs without specific handlers
      commit: d78a14decd494caf72ea0144624621e7e43ae451
[5/9] KVM: arm64: Handle CSSIDR2_EL1 and SMIDR_EL1 in a generic way
      commit: f07ef1bef67ca08799df262cc901971ac274783d
[6/9] KVM: arm64: Force trap of GMID_EL1 when the guest doesn't have MTE
      commit: 70a5ce4efc0e1194718aad6f26332c99e6a119db
[7/9] KVM: arm64: pkvm: Add a generic synchronous exception injection primitive
      commit: e5d40a5a97c1d57e89aa5f324734065c6580b436
[8/9] KVM: arm64: pkvm: Report optional ID register traps with a 0x18 syndrome
      commit: 592dc2c020686536dae1c427c78cf558a3df4414
[9/9] KVM: arm64: selftests: Add a test for FEAT_IDST
      commit: b638a9d0f8965b98403022cb91d8f3b31170eb35

Cheers,

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.