KVM: arm64: Add support for FEAT_IDST

If our host has MTE, but the guest doesn't, make sure we set HCR_EL2.TID5 to force GMID_EL1 being trapped.

Reviewed-by Joey Gouly <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kvm/sys_regs.c | 2 ++
 1 file changed, 2 insertions(+)

arch/arm64/kvm/sys_regs.c +0 -0

index 9e4c46fbfd802..2ca6862e935b5 100644

arch/arm64/kvm/sys_regs.c +0 -1

 	/*
 	 * In the absence of FGT, we cannot independently trap TLBI
--

2.47.3

If our host has MTE, but the guest doesn't, make sure we set HCR_EL2.TID5 to force GMID_EL1 being trapped.

Reviewed-by Joey Gouly <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kvm/sys_regs.c | 2 ++
 1 file changed, 2 insertions(+)

arch/arm64/kvm/sys_regs.c +0 -0

index 9e4c46fbfd802..2ca6862e935b5 100644

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.

Reviewed-by Joey Gouly <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kvm/sys_regs.c | 2 ++
 1 file changed, 2 insertions(+)

arch/arm64/kvm/sys_regs.c +0 -0

index 9e4c46fbfd802..2ca6862e935b5 100644
@@ -5561,6 +5561,8 @@ static void vcpu_set_hcr(struct kvm_vcpu *vcpu)
 

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

Quoted 3 lines

arch/arm64/kvm/sys_regs.c +1 -0

 	if (kvm_has_mte(vcpu->kvm))
 		vcpu->arch.hcr_el2 |= HCR_ATA;
+	else if (id_aa64pfr1_mte(read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1)))

This helper is ugly!

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

Quoted 2 lines

arch/arm64/kvm/sys_regs.c +1 -0

+		vcpu->arch.hcr_el2 |= HCR_TID5;

How about setting the trap unconditionally when !kvm_has_mte()? Even in the case of asymmetry we'd want GMID_EL1 to trap.

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.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/include/asm/kvm_emulate.h |  1 +
 arch/arm64/kvm/inject_fault.c        | 10 +++++++---
 2 files changed, 8 insertions(+), 3 deletions(-)

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.

Signed-off-by Marc Zyngier <[email protected]>

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

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

2.47.3

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 not implemented in the guest. Otherwise, just make them UNDEF.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kvm/sys_regs.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Hi Marc,

Quoted 4 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.

Quoted 8 lines

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 not implemented in the guest. Otherwise, just make them UNDEF.

Missed it before but there is a stray 'not' in there. You get the EC=0x18 trap if FEAT_IDST is implemented.

Quoted 1 line

Quoted 4 lines

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kvm/sys_regs.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

The code looks good.

Reviewed-by Ben Horgan

Thanks,

Ben

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 not implemented in the guest. Otherwise, just make them UNDEF.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kvm/sys_regs.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

arch/arm64/kvm/sys_regs.c +0 -0

index 2ca6862e935b5..7705f703e7c6d 100644

arch/arm64/kvm/sys_regs.c +9 -4

+		return undef_access(vcpu, p, r);
+
+	kvm_inject_sync(vcpu, kvm_vcpu_get_esr(vcpu));
+	return false;
+}
+
 enum sr_loc_attr {
 	SR_LOC_MEMORY	= 0,	  /* Register definitely in memory */
 	SR_LOC_LOADED	= BIT(0), /* Register on CPU, unless it cannot */

@@ -3399,9 +3409,9 @@ 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_GMID_EL1), undef_access },
-	{ SYS_DESC(SYS_SMIDR_EL1), undef_access },
+	{ SYS_DESC(SYS_CCSIDR2_EL1), idst_access },
+	{ SYS_DESC(SYS_GMID_EL1), idst_access },
+	{ SYS_DESC(SYS_SMIDR_EL1), idst_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,
--

2.47.3

Quoted 23 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 not implemented in the guest. Otherwise, just make them UNDEF.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kvm/sys_regs.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

arch/arm64/kvm/sys_regs.c +0 -0

index 2ca6862e935b5..7705f703e7c6d 100644

arch/arm64/kvm/sys_regs.c static bool write_to_read_only(struct kvm_vcpu *vcpu, · line 88

Quoted 7 lines

arch/arm64/kvm/sys_regs.c +4 -0

@@ -82,6 +82,16 @@ static bool write_to_read_only(struct kvm_vcpu *vcpu,
 			"sys_reg write to read-only register");
 }
 
+static bool 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, 0x0))

Hi Marc,

Minor: maybe beter readability if use NI instead of 0x0, just like things in feat_nv2() below, but depends on you.

Quoted 34 lines · Yao Yuan

Quoted 23 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 not implemented in the guest. Otherwise, just make them UNDEF.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kvm/sys_regs.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

arch/arm64/kvm/sys_regs.c +0 -0

index 2ca6862e935b5..7705f703e7c6d 100644

arch/arm64/kvm/sys_regs.c static bool write_to_read_only(struct kvm_vcpu *vcpu, · line 88

Quoted 7 lines

arch/arm64/kvm/sys_regs.c +4 -0

@@ -82,6 +82,16 @@ static bool write_to_read_only(struct kvm_vcpu *vcpu,
 			"sys_reg write to read-only register");
 }
 
+static bool 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, 0x0))

Hi Marc,

Quoted 2 lines

Minor: maybe beter readability if use NI instead of 0x0, just like things in feat_nv2() below, but depends on you.

+1, using the ESR value as an identifier in the sysreg tables is just terrible. This reads like a literal.

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.