KVM: arm64: Add support for FEAT_IDST
Marc Zyngier
maintainerv3 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
Marc Zyngier
maintainerPATCH 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
@@ -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
@@ -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
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.Signed-off-by Marc Zyngier <[email protected]>
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@@ -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@@ -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 NISignature · 1 line
2.47.3
Re: [PATCH v3 1/9] arm64: Repaint ID_AA64MMFR2_EL1.IDS description
In reply to Marc ZyngierHi 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.Signed-off-by Marc Zyngier <[email protected]>--- 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@@ -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
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
Marc Zyngier
maintainerRe: [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.Signed-off-by Marc Zyngier <[email protected]>--- 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@@ -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
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 IDSShould 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.
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.Signed-off-by Marc Zyngier <[email protected]>--- 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@@ -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
-0Quoted 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 IDSShould 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.
Quoted 8 lines
But at this stage, I'm not confident that this is desirable, let alone necessary.
Thanks,
M.
Thanks,
Ben
Marc Zyngier
maintainerRe: [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.Signed-off-by Marc Zyngier <[email protected]>--- 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@@ -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
-0Quoted 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 IDSShould 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.
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.
Marc Zyngier
maintainerPATCH v3 2/9 KVM: arm64: Add trap routing for GMID_EL1
In reply to Marc ZyngierHCR_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
@@ -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
Marc Zyngier
maintainerPATCH v3 3/9 KVM: arm64: Add a generic synchronous exception injection primitive
In reply to Marc ZyngierMaybe 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
@@ -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
@@ -162,12 +162,16 @@ static void inject_abt64(struct kvm_vcpu *vcpu, bool is_iabt, unsigned long addrvcpu_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
Marc Zyngier
maintainerPATCH v3 4/9 KVM: arm64: Handle FEAT_IDST for sysregs without specific handlers
In reply to Marc ZyngierAdd 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
@@ -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
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_IDSTsemantics and inject an EC=0x18 exception. Otherwise, an UNDEF will do.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/emulate-nested.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)arch/arm64/kvm/emulate-nested.c +0
-0
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
Re: [PATCH v3 4/9] KVM: arm64: Handle FEAT_IDST for sysregs without specific handlers
In reply to Marc ZyngierQuoted 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_IDSTsemantics and inject an EC=0x18 exception. Otherwise, an UNDEF will do.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/emulate-nested.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)arch/arm64/kvm/emulate-nested.c +2
-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()
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
Marc Zyngier
maintainerPATCH v3 5/9 KVM: arm64: Handle CSSIDR2_EL1 and SMIDR_EL1 in a generic way
In reply to Marc ZyngierNow 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
@@ -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
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_IDSTinfrastrcuture, get rid of the handling ofCSSIDR2_EL1andSMIDR_EL1.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/sys_regs.c | 2 -- 1 file changed, 2 deletions(-)arch/arm64/kvm/sys_regs.c +0
-0
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
-1IMPLEMENTATION_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
Marc Zyngier
maintainerPATCH v3 6/9 KVM: arm64: Force trap of GMID_EL1 when the guest doesn't have MTE
In reply to Marc ZyngierIf 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
@@ -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
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_EL1being trapped. Such trap will be handled by theFEAT_IDSThandling.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
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
Marc Zyngier
maintainerPATCH v3 7/9 KVM: arm64: pkvm: Add a generic synchronous exception injection primitive
In reply to Marc ZyngierSimilarly 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
@@ -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
Marc Zyngier
maintainerPATCH v3 8/9 KVM: arm64: pkvm: Report optional ID register traps with a 0x18 syndrome
In reply to Marc ZyngierWith 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
@@ -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
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 onFEAT_CCIDX), andSMIDR_EL1(depending on SME).For these three registers, report their trap as EC=0x18 if they end-up trapping into KVM and that
FEAT_IDSTis implemented in the guest. Otherwise, just make them UNDEF.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/hyp/nvhe/sys_regs.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)arch/arm64/kvm/hyp/nvhe/sys_regs.c +2
-0@@ -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
Marc Zyngier
maintainerRe: [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 onFEAT_CCIDX), andSMIDR_EL1(depending on SME).For these three registers, report their trap as EC=0x18 if they end-up trapping into KVM and that
FEAT_IDSTis implemented in the guest. Otherwise, just make them UNDEF.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/hyp/nvhe/sys_regs.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)arch/arm64/kvm/hyp/nvhe/sys_regs.c +0
-0@@ -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.
Marc Zyngier
maintainerPATCH v3 9/9 KVM: arm64: selftests: Add a test for FEAT_IDST
In reply to Marc ZyngierAdd 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
@@ -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