Skip to contentSkip to conversation

KVM: arm64: Add support for FEAT_IDST

PATCH Superseded Marc Zyngier 4 participants 21 messages Dec 4, 2025 – Dec 5, 2025

Subsystems (4)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Marc Zyngier

author maintainer
#0 lore raw

v3 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 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]

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               |  28 +++++
 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/tools/sysreg                       |   4 +-
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../testing/selftests/kvm/arm64/idreg-idst.c  | 117 ++++++++++++++++++
 8 files changed, 189 insertions(+), 15 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/arm64/idreg-idst.c
Signature · 1 line
2.47.3
author maintainer
#1 lore raw

PATCH v3 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.

---
 arch/arm64/kvm/hyp/nvhe/sys_regs.c | 2 +-
 arch/arm64/tools/sysreg            | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
arch/arm64/kvm/hyp/nvhe/sys_regs.c +1 -1
index 82da9b03692d4..107d62921b168 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_ENUM(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 +2 -2
index 1c6cdf9d54bba..3261e8791ac03 100644
@@ -2257,8 +2257,8 @@ UnsignedEnum	43:40	FWB
 	0b0001	IMP
 EndEnum
 Enum	39:36	IDS
-	0b0000	0x0
-	0b0001	0x18
+	0b0000	NI
+	0b0001	IMP
 EndEnum
 UnsignedEnum	35:32	AT
 	0b0000	NI
Signature · 1 line
2.47.3
reviewer Reviewed-by on v3 1/9
#2 lore raw

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

Quoted 2 lines · Marc Zyngier

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

typo: directly

Quoted 3 lines

identifiers to describe the absence/presence of FEAT_IDST.

Quoted 36 lines
---
 arch/arm64/kvm/hyp/nvhe/sys_regs.c | 2 +-
 arch/arm64/tools/sysreg            | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
arch/arm64/kvm/hyp/nvhe/sys_regs.c +1 -1
index 82da9b03692d4..107d62921b168 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_ENUM(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 +2 -2
index 1c6cdf9d54bba..3261e8791ac03 100644
@@ -2257,8 +2257,8 @@ UnsignedEnum	43:40	FWB
 	0b0001	IMP
 EndEnum
 Enum	39:36	IDS
-	0b0000	0x0
-	0b0001	0x18
+	0b0000	NI
+	0b0001	IMP
 EndEnum
 UnsignedEnum	35:32	AT
 	0b0000	NI
Signature · 1 line
2.47.3
#3 lore raw

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

In reply to Marc Zyngier

Hi Marc,

Quoted 28 lines · Marc Zyngier

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.

---
 arch/arm64/kvm/hyp/nvhe/sys_regs.c | 2 +-
 arch/arm64/tools/sysreg            | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
arch/arm64/kvm/hyp/nvhe/sys_regs.c +1 -1
index 82da9b03692d4..107d62921b168 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_ENUM(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 +0 -0
index 1c6cdf9d54bba..3261e8791ac03 100644

arch/arm64/tools/sysreg UnsignedEnum 43:40 FWB · line 2259

Quoted 3 lines
arch/arm64/tools/sysreg +0 -0
@@ -2257,8 +2257,8 @@ UnsignedEnum	43:40	FWB
 	0b0001	IMP
 EndEnum
 Enum	39:36	IDS

Should this also be changed to an UnsignedEnum?

arch/arm64/tools/sysreg UnsignedEnum 43:40 FWB · line 2264

Quoted 7 lines
arch/arm64/tools/sysreg +2 -2
-	0b0000	0x0
-	0b0001	0x18
+	0b0000	NI
+	0b0001	IMP
 EndEnum
 UnsignedEnum	35:32	AT
 	0b0000	NI

Thanks,

Ben

author maintainer
#4 lore raw

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

Quoted 29 lines · Ben Horgan

Hi Marc,

Quoted 25 lines · Marc Zyngier

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.

---
 arch/arm64/kvm/hyp/nvhe/sys_regs.c | 2 +-
 arch/arm64/tools/sysreg            | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
arch/arm64/kvm/hyp/nvhe/sys_regs.c +1 -1
index 82da9b03692d4..107d62921b168 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_ENUM(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 +0 -0
index 1c6cdf9d54bba..3261e8791ac03 100644
Quoted 8 lines

arch/arm64/tools/sysreg UnsignedEnum 43:40 FWB · line 2259

Quoted 6 lines
arch/arm64/tools/sysreg +0 -0
@@ -2257,8 +2257,8 @@ UnsignedEnum	43:40	FWB
 	0b0001	IMP
 EndEnum
 Enum	39:36	IDS

Should this also be changed to an UnsignedEnum?

I'm not sure this brings much when you only have two values. If IDS was growing a third value, and that there was an actual order in the numbering scheme, then yes, that'd be useful.

But at this stage, I'm not confident that this is desirable, let alone necessary.

Thanks,

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

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

Hi Marc,

Quoted 40 lines · Marc Zyngier
Quoted 29 lines · Ben Horgan

Hi Marc,

Quoted 25 lines · Marc Zyngier

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.

---
 arch/arm64/kvm/hyp/nvhe/sys_regs.c | 2 +-
 arch/arm64/tools/sysreg            | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
arch/arm64/kvm/hyp/nvhe/sys_regs.c +1 -1
index 82da9b03692d4..107d62921b168 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_ENUM(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 +0 -0
index 1c6cdf9d54bba..3261e8791ac03 100644
Quoted 8 lines

arch/arm64/tools/sysreg UnsignedEnum 43:40 FWB · line 2259

Quoted 6 lines
arch/arm64/tools/sysreg +0 -0
@@ -2257,8 +2257,8 @@ UnsignedEnum	43:40	FWB
 	0b0001	IMP
 EndEnum
 Enum	39:36	IDS

Should this also be changed to an UnsignedEnum?

Quoted 3 lines

I'm not sure this brings much when you only have two values. If IDS was growing a third value, and that there was an actual order in the numbering scheme, then yes, that'd be useful.

Joey just pointed out to me that there is a new third field. Not in the arm reference manual yet, but mentioned in the xml. I'm unsure if it's necessary to consider this at the moment though.

https://developer.arm.com/documentation/ddi0601/2025-09/AArch64-Registers/ID-AA64MMFR2-EL1--AArch64-Memory-Model-Feature-Register-2?lang=en

Quoted 8 lines

But at this stage, I'm not confident that this is desirable, let alone necessary.

Thanks,

	M.

Thanks,

Ben

author maintainer
#6 lore raw

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

Quoted 48 lines · Ben Horgan

Hi Marc,

Quoted 40 lines · Marc Zyngier
Quoted 29 lines · Ben Horgan

Hi Marc,

Quoted 25 lines · Marc Zyngier

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.

---
 arch/arm64/kvm/hyp/nvhe/sys_regs.c | 2 +-
 arch/arm64/tools/sysreg            | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
arch/arm64/kvm/hyp/nvhe/sys_regs.c +1 -1
index 82da9b03692d4..107d62921b168 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_ENUM(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 +0 -0
index 1c6cdf9d54bba..3261e8791ac03 100644
Quoted 8 lines

arch/arm64/tools/sysreg UnsignedEnum 43:40 FWB · line 2259

Quoted 6 lines
arch/arm64/tools/sysreg +0 -0
@@ -2257,8 +2257,8 @@ UnsignedEnum	43:40	FWB
 	0b0001	IMP
 EndEnum
 Enum	39:36	IDS

Should this also be changed to an UnsignedEnum?

Quoted 3 lines

I'm not sure this brings much when you only have two values. If IDS was growing a third value, and that there was an actual order in the numbering scheme, then yes, that'd be useful.

Quoted 5 lines

Joey just pointed out to me that there is a new third field. Not in the arm reference manual yet, but mentioned in the xml. I'm unsure if it's necessary to consider this at the moment though.

https://developer.arm.com/documentation/ddi0601/2025-09/AArch64-Registers/ID-AA64MMFR2-EL1--AArch64-Memory-Model-Feature-Register-2?lang=en

Ah, nice catch. In which case I agree that this should become UnsignedEnum, as the new EL3 feature is a strict superset of 0b0001.

I'll add fix for that on top.

Thanks,

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

PATCH v3 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
#8 lore raw

PATCH v3 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
#9 lore raw

PATCH v3 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 | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
arch/arm64/kvm/emulate-nested.c +20 -0
index 616eb6ad68701..fac2707221b47 100644
@@ -2588,6 +2588,26 @@ 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 fealing with FEAT_IDST.
+		 *
+		 * 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}.
+		 */
+		if (params.Op0 == 3 &&
+		    !(params.Op1 & 0b100) && params.Op1 != 2 &&
+		    params.CRn == 0 &&
+		    !(params.CRm & 0b1000)) {
+			if (kvm_has_feat_enum(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
Signature · 1 line
2.47.3
reviewer Reviewed-by on v3 4/9
#10 lore raw

Re: [PATCH v3 4/9] KVM: arm64: Handle FEAT_IDST for sysregs without specific handlers

Quoted 17 lines · 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 | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
arch/arm64/kvm/emulate-nested.c +0 -0
index 616eb6ad68701..fac2707221b47 100644

arch/arm64/kvm/emulate-nested.c bool triage_sysreg_trap(struct kvm_vcpu *vcpu, int *sr_index) · line 2593

Quoted 6 lines
arch/arm64/kvm/emulate-nested.c +3 -0
@@ -2588,6 +2588,26 @@ 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 fealing with FEAT_IDST.

*dealing

Quoted 23 lines
arch/arm64/kvm/emulate-nested.c +17 -0
+		 *
+		 * 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}.
+		 */
+		if (params.Op0 == 3 &&
+		    !(params.Op1 & 0b100) && params.Op1 != 2 &&
+		    params.CRn == 0 &&
+		    !(params.CRm & 0b1000)) {
+			if (kvm_has_feat_enum(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
Signature · 1 line
2.47.3
Reviewed-by on v3 4/9
#11 lore raw

Re: [PATCH v3 4/9] KVM: arm64: Handle FEAT_IDST for sysregs without specific handlers

In reply to Marc Zyngier
Quoted 22 lines · 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 | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
arch/arm64/kvm/emulate-nested.c +2 -0
index 616eb6ad68701..fac2707221b47 100644
@@ -2588,6 +2588,26 @@ bool triage_sysreg_trap(struct kvm_vcpu *vcpu, int *sr_index)
 
 		params = esr_sys64_to_params(esr);
 
+		/*
+		 * This implements the pseudocode UnimplementedIDRegister()

arch/arm64/kvm/emulate-nested.c bool triage_sysreg_trap(struct kvm_vcpu *vcpu, int *sr_index) · line 2602

Quoted 10 lines
arch/arm64/kvm/emulate-nested.c +10 -0
+		 * helper for the purpose of fealing with FEAT_IDST.
+		 *
+		 * 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}.
+		 */
+		if (params.Op0 == 3 &&
+		    !(params.Op1 & 0b100) && params.Op1 != 2 &&
+		    params.CRn == 0 &&
+		    !(params.CRm & 0b1000)) {

Hi Marc,

May give a macro/inline function or local variable w/ name like "is_feat_id_space()" can simplify the long comment here. Others LGTM:

Quoted 13 lines
arch/arm64/kvm/emulate-nested.c +8 -1
+			if (kvm_has_feat_enum(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
--

2.47.3

author maintainer
#12 lore raw

PATCH v3 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 ec3fbe0b8d525..ae1e72df1ed45 100644
@@ -3399,8 +3399,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
Reviewed-by on v3 5/9
#13 lore raw

Re: [PATCH v3 5/9] KVM: arm64: Handle CSSIDR2_EL1 and SMIDR_EL1 in a generic way

Quoted 13 lines · 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 -0
index ec3fbe0b8d525..ae1e72df1ed45 100644

arch/arm64/kvm/sys_regs.c static const struct sys_reg_desc sys_reg_descs[] = { · line 3403

Quoted 5 lines
arch/arm64/kvm/sys_regs.c +0 -2
@@ -3399,8 +3399,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 },

Hi Marc,

I checked the arm ARM to make sure these 2 belong to ID space.

Quoted 5 lines
arch/arm64/kvm/sys_regs.c +0 -1
 	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,
--

2.47.3

author maintainer
#14 lore raw

PATCH v3 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 ae1e72df1ed45..2e94c423594eb 100644
@@ -5558,6 +5558,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
Reviewed-by on v3 6/9
#15 lore raw

Re: [PATCH v3 6/9] KVM: arm64: Force trap of GMID_EL1 when the guest doesn't have MTE

Hi Marc,

Quoted 15 lines · 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 +0 -0
index ae1e72df1ed45..2e94c423594eb 100644

arch/arm64/kvm/sys_regs.c static void vcpu_set_hcr(struct kvm_vcpu *vcpu) · line 5562

Quoted 5 lines
arch/arm64/kvm/sys_regs.c +2 -0
@@ -5558,6 +5558,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;
Quoted 5 lines
arch/arm64/kvm/sys_regs.c +0 -1
 	/*
 	 * In the absence of FGT, we cannot independently trap TLBI
--

2.47.3

author maintainer
#16 lore raw

PATCH v3 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 107d62921b168..876b36d3d4788 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
#17 lore raw

PATCH v3 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 876b36d3d4788..efc36645f4b5a 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_enum(vcpu->kvm, ID_AA64MMFR2_EL1, IDS, NI))
+		inject_undef64(vcpu);
+	else
+		inject_sync64(vcpu, kvm_vcpu_get_esr(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 }
 
@@ -472,6 +484,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
#18 lore raw

Re: [PATCH v3 8/9] KVM: arm64: pkvm: Report optional ID register traps with a 0x18 syndrome

Hi Marc,

Quoted 28 lines · 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 +2 -0
index 876b36d3d4788..efc36645f4b5a 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,

arch/arm64/kvm/hyp/nvhe/sys_regs.c static bool pvm_gic_read_sre(struct kvm_vcpu *vcpu, · line 361

Quoted 10 lines
arch/arm64/kvm/hyp/nvhe/sys_regs.c +10 -0
+			    const struct sys_reg_desc *r)
+{
+	if (kvm_has_feat_enum(vcpu->kvm, ID_AA64MMFR2_EL1, IDS, NI))
+		inject_undef64(vcpu);
+	else
+		inject_sync64(vcpu, kvm_vcpu_get_esr(vcpu));
+
+	return false;
+}
+

Just wondering, why is the pkvm version register specific? You changed the non-pkvm from register specific to generic.

arch/arm64/kvm/hyp/nvhe/sys_regs.c static const struct sys_reg_desc pvm_sys_reg_descs[] = { · line 492

Quoted 13 lines
arch/arm64/kvm/hyp/nvhe/sys_regs.c +3 -0
 /* Mark the specified system register as an AArch32 feature id register. */
 #define AARCH32(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch32 }
 
@@ -472,6 +484,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),

Thanks,

Ben

author maintainer
#19 lore raw

Re: [PATCH v3 8/9] KVM: arm64: pkvm: Report optional ID register traps with a 0x18 syndrome

Quoted 30 lines · Ben Horgan

Hi Marc,

Quoted 26 lines · 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 +0 -0
index 876b36d3d4788..efc36645f4b5a 100644
@@ -347,6 +347,18 @@ static bool pvm_gic_read_sre(struct kvm_vcpu *vcpu,
 	return true;
 }
 
Quoted 15 lines

arch/arm64/kvm/hyp/nvhe/sys_regs.c static bool pvm_gic_read_sre(struct kvm_vcpu *vcpu, · line 361

Quoted 12 lines
arch/arm64/kvm/hyp/nvhe/sys_regs.c +12 -0
+static bool pvm_idst_access(struct kvm_vcpu *vcpu,
+			    struct sys_reg_params *p,
+			    const struct sys_reg_desc *r)
+{
+	if (kvm_has_feat_enum(vcpu->kvm, ID_AA64MMFR2_EL1, IDS, NI))
+		inject_undef64(vcpu);
+	else
+		inject_sync64(vcpu, kvm_vcpu_get_esr(vcpu));
+
+	return false;
+}
+

Just wondering, why is the pkvm version register specific? You changed the non-pkvm from register specific to generic.

Because pKVM relies on a full enumeration of the registers as a design principle, and refrains from having wide-casting nets as a catch-all.

That's to ensure that there is a "surprise trap" always results in an UNDEF. Different model from the rest of KVM.

Thanks,

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

PATCH v3 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 148d427ff24be..fa44e6d9afc35 100644
@@ -171,6 +171,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