[PATCH v3 00/42] KVM: arm64: Revamp Fine Grained Trap handling
Marc Zyngier
maz at kernel.org
Tue Apr 29 00:34:17 PDT 2025
On Mon, 28 Apr 2025 19:33:10 +0100,
Ganapatrao Kulkarni <gankulkarni at os.amperecomputing.com> wrote:
>
> Hi Marc,
>
> On 26-04-2025 17:57, Marc Zyngier wrote:
> > This is yet another version of the series last posted at [1].
> >
> > The eagled eye reviewer will have noticed that since v2, the series
> > has more or less doubled in size for any reasonable metric (number of
> > patches, number of lines added or deleted). It is therefore pretty
> > urgent that this gets either merged or forgotten! ;-)
> >
> > See the change log below for the details -- most of it is related to
> > FGT2 (and its rather large dependencies) being added.
> >
> > * From v2:
> >
> > - Added comprehensive support for FEAT_FGT2, as the host kernel is
> > now making use of these registers, without any form of context
> > switch in KVM. What could possibly go wrong?
> >
> > - Reworked some of the FGT description and handling primitives,
> > reducing the boilerplate code and tables that get added over time.
> >
> > - Rebased on 6.15-rc3.
> >
> > [1]: https://lore.kernel.org/r/20250310122505.2857610-1-maz@kernel.org
> >
> > Marc Zyngier (41):
> > arm64: sysreg: Add ID_AA64ISAR1_EL1.LS64 encoding for FEAT_LS64WB
> > arm64: sysreg: Update ID_AA64MMFR4_EL1 description
> > arm64: sysreg: Add layout for HCR_EL2
> > arm64: sysreg: Replace HGFxTR_EL2 with HFG{R,W}TR_EL2
> > arm64: sysreg: Update ID_AA64PFR0_EL1 description
> > arm64: sysreg: Update PMSIDR_EL1 description
> > arm64: sysreg: Update TRBIDR_EL1 description
> > arm64: sysreg: Add registers trapped by HFG{R,W}TR2_EL2
> > arm64: sysreg: Add registers trapped by HDFG{R,W}TR2_EL2
> > arm64: sysreg: Add system instructions trapped by HFGIRT2_EL2
> > arm64: Remove duplicated sysreg encodings
> > arm64: tools: Resync sysreg.h
> > arm64: Add syndrome information for trapped LD64B/ST64B{,V,V0}
> > arm64: Add FEAT_FGT2 capability
> > KVM: arm64: Tighten handling of unknown FGT groups
> > KVM: arm64: Simplify handling of negative FGT bits
> > KVM: arm64: Handle trapping of FEAT_LS64* instructions
> > KVM: arm64: Restrict ACCDATA_EL1 undef to FEAT_ST64_ACCDATA being
> > disabled
> > KVM: arm64: Don't treat HCRX_EL2 as a FGT register
> > KVM: arm64: Plug FEAT_GCS handling
> > KVM: arm64: Compute FGT masks from KVM's own FGT tables
> > KVM: arm64: Add description of FGT bits leading to EC!=0x18
> > KVM: arm64: Use computed masks as sanitisers for FGT registers
> > KVM: arm64: Propagate FGT masks to the nVHE hypervisor
> > KVM: arm64: Use computed FGT masks to setup FGT registers
> > KVM: arm64: Remove hand-crafted masks for FGT registers
> > KVM: arm64: Use KVM-specific HCRX_EL2 RES0 mask
> > KVM: arm64: Handle PSB CSYNC traps
> > KVM: arm64: Switch to table-driven FGU configuration
> > KVM: arm64: Validate FGT register descriptions against RES0 masks
> > KVM: arm64: Use FGT feature maps to drive RES0 bits
> > KVM: arm64: Allow kvm_has_feat() to take variable arguments
> > KVM: arm64: Use HCRX_EL2 feature map to drive fixed-value bits
> > KVM: arm64: Use HCR_EL2 feature map to drive fixed-value bits
> > KVM: arm64: Add FEAT_FGT2 registers to the VNCR page
> > KVM: arm64: Add sanitisation for FEAT_FGT2 registers
> > KVM: arm64: Add trap routing for FEAT_FGT2 registers
> > KVM: arm64: Add context-switch for FEAT_FGT2 registers
> > KVM: arm64: Allow sysreg ranges for FGT descriptors
> > KVM: arm64: Add FGT descriptors for FEAT_FGT2
> > KVM: arm64: Handle TSB CSYNC traps
> >
> > Mark Rutland (1):
> > KVM: arm64: Unconditionally configure fine-grain traps
> >
> > arch/arm64/include/asm/el2_setup.h | 14 +-
> > arch/arm64/include/asm/esr.h | 10 +-
> > arch/arm64/include/asm/kvm_arm.h | 186 ++--
> > arch/arm64/include/asm/kvm_host.h | 56 +-
> > arch/arm64/include/asm/sysreg.h | 26 +-
> > arch/arm64/include/asm/vncr_mapping.h | 5 +
> > arch/arm64/kernel/cpufeature.c | 7 +
> > arch/arm64/kvm/Makefile | 2 +-
> > arch/arm64/kvm/arm.c | 13 +
> > arch/arm64/kvm/config.c | 1085 +++++++++++++++++++++++
> > arch/arm64/kvm/emulate-nested.c | 580 ++++++++----
> > arch/arm64/kvm/handle_exit.c | 77 ++
> > arch/arm64/kvm/hyp/include/hyp/switch.h | 158 ++--
> > arch/arm64/kvm/hyp/nvhe/switch.c | 12 +
> > arch/arm64/kvm/hyp/vgic-v3-sr.c | 8 +-
> > arch/arm64/kvm/nested.c | 223 +----
> > arch/arm64/kvm/sys_regs.c | 68 +-
> > arch/arm64/tools/cpucaps | 1 +
> > arch/arm64/tools/sysreg | 1002 ++++++++++++++++++++-
> > tools/arch/arm64/include/asm/sysreg.h | 65 +-
> > 20 files changed, 2888 insertions(+), 710 deletions(-)
> > create mode 100644 arch/arm64/kvm/config.c
> >
>
> I am trying nv-next branch and I believe these FGT related changes are
> merged. With this, selftest arm64/set_id_regs is failing. From initial
> debug it seems, the register access of SYS_CTR_EL0, SYS_MIDR_EL1,
> SYS_REVIDR_EL1 and SYS_AIDR_EL1 from guest_code is resulting in trap
> to EL2 (HCR_ID1,ID2 are set) and is getting forwarded back to EL1,
> since EL1 sync handler is not installed in the test code, resulting in
> hang(endless guest_exit/entry).
Let's start by calling bullshit on the test itself:
root at semi-fraudulent:/home/maz# grep AA64PFR0 /sys/kernel/debug/kvm/2008-4/idregs
SYS_ID_AA64PFR0_EL1: 0000000020110000
It basically disable anything 64bit at EL{0,1,2,3]. Frankly, all these
tests are pure garbage. I'm baffled that anyone expects this crap to
give any meaningful result.
> It is due to function "triage_sysreg_trap" is returning true.
>
> When guest_code is in EL1 (default case) it is due to return in below if.
>
> if (tc.fgt != __NO_FGT_GROUP__ &&
> (vcpu->kvm->arch.fgu[tc.fgt] & BIT(tc.bit))) {
> kvm_inject_undefined(vcpu);
> return true;
> }
That explains why we end-up here. The 64bit ISA is "disabled", a bunch
of trap bits are advertised as depending on it, so the corresponding
FGU bits are set to "emulate" the requested behaviour.
Works as intended, and this proves once more that what we call testing
is just horseshit.
In retrospect, we should do a few things:
- Prevent writes to ID_AA64PFR0_EL1 disabling the 64bit ISA, breaking
this stupid test for good.
- Flag all the FGT bits depending on FEAT_AA64EL1 as NEVER_FGU,
because that shouldn't happen, by construction (there is no
architecture revision where these sysregs are UNDEFined).
- Mark all these test as unmaintained and deprecated, recognising that
they are utterly pointless (optional).
Full patch below.
M.
diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c
index d4e1218b004dd..666070d4ccd7f 100644
--- a/arch/arm64/kvm/config.c
+++ b/arch/arm64/kvm/config.c
@@ -295,34 +295,34 @@ static const struct reg_bits_to_feat_map hfgrtr_feat_map[] = {
HFGRTR_EL2_APDBKey |
HFGRTR_EL2_APDAKey,
feat_pauth),
- NEEDS_FEAT(HFGRTR_EL2_VBAR_EL1 |
- HFGRTR_EL2_TTBR1_EL1 |
- HFGRTR_EL2_TTBR0_EL1 |
- HFGRTR_EL2_TPIDR_EL0 |
- HFGRTR_EL2_TPIDRRO_EL0 |
- HFGRTR_EL2_TPIDR_EL1 |
- HFGRTR_EL2_TCR_EL1 |
- HFGRTR_EL2_SCTLR_EL1 |
- HFGRTR_EL2_REVIDR_EL1 |
- HFGRTR_EL2_PAR_EL1 |
- HFGRTR_EL2_MPIDR_EL1 |
- HFGRTR_EL2_MIDR_EL1 |
- HFGRTR_EL2_MAIR_EL1 |
- HFGRTR_EL2_ISR_EL1 |
- HFGRTR_EL2_FAR_EL1 |
- HFGRTR_EL2_ESR_EL1 |
- HFGRTR_EL2_DCZID_EL0 |
- HFGRTR_EL2_CTR_EL0 |
- HFGRTR_EL2_CSSELR_EL1 |
- HFGRTR_EL2_CPACR_EL1 |
- HFGRTR_EL2_CONTEXTIDR_EL1 |
- HFGRTR_EL2_CLIDR_EL1 |
- HFGRTR_EL2_CCSIDR_EL1 |
- HFGRTR_EL2_AMAIR_EL1 |
- HFGRTR_EL2_AIDR_EL1 |
- HFGRTR_EL2_AFSR1_EL1 |
- HFGRTR_EL2_AFSR0_EL1,
- FEAT_AA64EL1),
+ NEEDS_FEAT_FLAG(HFGRTR_EL2_VBAR_EL1 |
+ HFGRTR_EL2_TTBR1_EL1 |
+ HFGRTR_EL2_TTBR0_EL1 |
+ HFGRTR_EL2_TPIDR_EL0 |
+ HFGRTR_EL2_TPIDRRO_EL0 |
+ HFGRTR_EL2_TPIDR_EL1 |
+ HFGRTR_EL2_TCR_EL1 |
+ HFGRTR_EL2_SCTLR_EL1 |
+ HFGRTR_EL2_REVIDR_EL1 |
+ HFGRTR_EL2_PAR_EL1 |
+ HFGRTR_EL2_MPIDR_EL1 |
+ HFGRTR_EL2_MIDR_EL1 |
+ HFGRTR_EL2_MAIR_EL1 |
+ HFGRTR_EL2_ISR_EL1 |
+ HFGRTR_EL2_FAR_EL1 |
+ HFGRTR_EL2_ESR_EL1 |
+ HFGRTR_EL2_DCZID_EL0 |
+ HFGRTR_EL2_CTR_EL0 |
+ HFGRTR_EL2_CSSELR_EL1 |
+ HFGRTR_EL2_CPACR_EL1 |
+ HFGRTR_EL2_CONTEXTIDR_EL1|
+ HFGRTR_EL2_CLIDR_EL1 |
+ HFGRTR_EL2_CCSIDR_EL1 |
+ HFGRTR_EL2_AMAIR_EL1 |
+ HFGRTR_EL2_AIDR_EL1 |
+ HFGRTR_EL2_AFSR1_EL1 |
+ HFGRTR_EL2_AFSR0_EL1,
+ NEVER_FGU, FEAT_AA64EL1),
};
static const struct reg_bits_to_feat_map hfgwtr_feat_map[] = {
@@ -368,25 +368,25 @@ static const struct reg_bits_to_feat_map hfgwtr_feat_map[] = {
HFGWTR_EL2_APDBKey |
HFGWTR_EL2_APDAKey,
feat_pauth),
- NEEDS_FEAT(HFGWTR_EL2_VBAR_EL1 |
- HFGWTR_EL2_TTBR1_EL1 |
- HFGWTR_EL2_TTBR0_EL1 |
- HFGWTR_EL2_TPIDR_EL0 |
- HFGWTR_EL2_TPIDRRO_EL0 |
- HFGWTR_EL2_TPIDR_EL1 |
- HFGWTR_EL2_TCR_EL1 |
- HFGWTR_EL2_SCTLR_EL1 |
- HFGWTR_EL2_PAR_EL1 |
- HFGWTR_EL2_MAIR_EL1 |
- HFGWTR_EL2_FAR_EL1 |
- HFGWTR_EL2_ESR_EL1 |
- HFGWTR_EL2_CSSELR_EL1 |
- HFGWTR_EL2_CPACR_EL1 |
- HFGWTR_EL2_CONTEXTIDR_EL1 |
- HFGWTR_EL2_AMAIR_EL1 |
- HFGWTR_EL2_AFSR1_EL1 |
- HFGWTR_EL2_AFSR0_EL1,
- FEAT_AA64EL1),
+ NEEDS_FEAT_FLAG(HFGWTR_EL2_VBAR_EL1 |
+ HFGWTR_EL2_TTBR1_EL1 |
+ HFGWTR_EL2_TTBR0_EL1 |
+ HFGWTR_EL2_TPIDR_EL0 |
+ HFGWTR_EL2_TPIDRRO_EL0 |
+ HFGWTR_EL2_TPIDR_EL1 |
+ HFGWTR_EL2_TCR_EL1 |
+ HFGWTR_EL2_SCTLR_EL1 |
+ HFGWTR_EL2_PAR_EL1 |
+ HFGWTR_EL2_MAIR_EL1 |
+ HFGWTR_EL2_FAR_EL1 |
+ HFGWTR_EL2_ESR_EL1 |
+ HFGWTR_EL2_CSSELR_EL1 |
+ HFGWTR_EL2_CPACR_EL1 |
+ HFGWTR_EL2_CONTEXTIDR_EL1|
+ HFGWTR_EL2_AMAIR_EL1 |
+ HFGWTR_EL2_AFSR1_EL1 |
+ HFGWTR_EL2_AFSR0_EL1,
+ NEVER_FGU, FEAT_AA64EL1),
};
static const struct reg_bits_to_feat_map hdfgrtr_feat_map[] = {
@@ -443,17 +443,17 @@ static const struct reg_bits_to_feat_map hdfgrtr_feat_map[] = {
FEAT_TRBE),
NEEDS_FEAT_FLAG(HDFGRTR_EL2_OSDLR_EL1, NEVER_FGU,
FEAT_DoubleLock),
- NEEDS_FEAT(HDFGRTR_EL2_OSECCR_EL1 |
- HDFGRTR_EL2_OSLSR_EL1 |
- HDFGRTR_EL2_DBGPRCR_EL1 |
- HDFGRTR_EL2_DBGAUTHSTATUS_EL1|
- HDFGRTR_EL2_DBGCLAIM |
- HDFGRTR_EL2_MDSCR_EL1 |
- HDFGRTR_EL2_DBGWVRn_EL1 |
- HDFGRTR_EL2_DBGWCRn_EL1 |
- HDFGRTR_EL2_DBGBVRn_EL1 |
- HDFGRTR_EL2_DBGBCRn_EL1,
- FEAT_AA64EL1)
+ NEEDS_FEAT_FLAG(HDFGRTR_EL2_OSECCR_EL1 |
+ HDFGRTR_EL2_OSLSR_EL1 |
+ HDFGRTR_EL2_DBGPRCR_EL1 |
+ HDFGRTR_EL2_DBGAUTHSTATUS_EL1|
+ HDFGRTR_EL2_DBGCLAIM |
+ HDFGRTR_EL2_MDSCR_EL1 |
+ HDFGRTR_EL2_DBGWVRn_EL1 |
+ HDFGRTR_EL2_DBGWCRn_EL1 |
+ HDFGRTR_EL2_DBGBVRn_EL1 |
+ HDFGRTR_EL2_DBGBCRn_EL1,
+ NEVER_FGU, FEAT_AA64EL1)
};
static const struct reg_bits_to_feat_map hdfgwtr_feat_map[] = {
@@ -503,16 +503,16 @@ static const struct reg_bits_to_feat_map hdfgwtr_feat_map[] = {
FEAT_TRBE),
NEEDS_FEAT_FLAG(HDFGWTR_EL2_OSDLR_EL1,
NEVER_FGU, FEAT_DoubleLock),
- NEEDS_FEAT(HDFGWTR_EL2_OSECCR_EL1 |
- HDFGWTR_EL2_OSLAR_EL1 |
- HDFGWTR_EL2_DBGPRCR_EL1 |
- HDFGWTR_EL2_DBGCLAIM |
- HDFGWTR_EL2_MDSCR_EL1 |
- HDFGWTR_EL2_DBGWVRn_EL1 |
- HDFGWTR_EL2_DBGWCRn_EL1 |
- HDFGWTR_EL2_DBGBVRn_EL1 |
- HDFGWTR_EL2_DBGBCRn_EL1,
- FEAT_AA64EL1),
+ NEEDS_FEAT_FLAG(HDFGWTR_EL2_OSECCR_EL1 |
+ HDFGWTR_EL2_OSLAR_EL1 |
+ HDFGWTR_EL2_DBGPRCR_EL1 |
+ HDFGWTR_EL2_DBGCLAIM |
+ HDFGWTR_EL2_MDSCR_EL1 |
+ HDFGWTR_EL2_DBGWVRn_EL1 |
+ HDFGWTR_EL2_DBGWCRn_EL1 |
+ HDFGWTR_EL2_DBGBVRn_EL1 |
+ HDFGWTR_EL2_DBGBCRn_EL1,
+ NEVER_FGU, FEAT_AA64EL1),
NEEDS_FEAT(HDFGWTR_EL2_TRFCR_EL1, FEAT_TRF),
};
@@ -556,38 +556,38 @@ static const struct reg_bits_to_feat_map hfgitr_feat_map[] = {
HFGITR_EL2_ATS1E1RP,
FEAT_PAN2),
NEEDS_FEAT(HFGITR_EL2_DCCVADP, FEAT_DPB2),
- NEEDS_FEAT(HFGITR_EL2_DCCVAC |
- HFGITR_EL2_SVC_EL1 |
- HFGITR_EL2_SVC_EL0 |
- HFGITR_EL2_ERET |
- HFGITR_EL2_TLBIVAALE1 |
- HFGITR_EL2_TLBIVALE1 |
- HFGITR_EL2_TLBIVAAE1 |
- HFGITR_EL2_TLBIASIDE1 |
- HFGITR_EL2_TLBIVAE1 |
- HFGITR_EL2_TLBIVMALLE1 |
- HFGITR_EL2_TLBIVAALE1IS |
- HFGITR_EL2_TLBIVALE1IS |
- HFGITR_EL2_TLBIVAAE1IS |
- HFGITR_EL2_TLBIASIDE1IS |
- HFGITR_EL2_TLBIVAE1IS |
- HFGITR_EL2_TLBIVMALLE1IS |
- HFGITR_EL2_ATS1E0W |
- HFGITR_EL2_ATS1E0R |
- HFGITR_EL2_ATS1E1W |
- HFGITR_EL2_ATS1E1R |
- HFGITR_EL2_DCZVA |
- HFGITR_EL2_DCCIVAC |
- HFGITR_EL2_DCCVAP |
- HFGITR_EL2_DCCVAU |
- HFGITR_EL2_DCCISW |
- HFGITR_EL2_DCCSW |
- HFGITR_EL2_DCISW |
- HFGITR_EL2_DCIVAC |
- HFGITR_EL2_ICIVAU |
- HFGITR_EL2_ICIALLU |
- HFGITR_EL2_ICIALLUIS,
- FEAT_AA64EL1),
+ NEEDS_FEAT_FLAG(HFGITR_EL2_DCCVAC |
+ HFGITR_EL2_SVC_EL1 |
+ HFGITR_EL2_SVC_EL0 |
+ HFGITR_EL2_ERET |
+ HFGITR_EL2_TLBIVAALE1 |
+ HFGITR_EL2_TLBIVALE1 |
+ HFGITR_EL2_TLBIVAAE1 |
+ HFGITR_EL2_TLBIASIDE1 |
+ HFGITR_EL2_TLBIVAE1 |
+ HFGITR_EL2_TLBIVMALLE1 |
+ HFGITR_EL2_TLBIVAALE1IS |
+ HFGITR_EL2_TLBIVALE1IS |
+ HFGITR_EL2_TLBIVAAE1IS |
+ HFGITR_EL2_TLBIASIDE1IS |
+ HFGITR_EL2_TLBIVAE1IS |
+ HFGITR_EL2_TLBIVMALLE1IS|
+ HFGITR_EL2_ATS1E0W |
+ HFGITR_EL2_ATS1E0R |
+ HFGITR_EL2_ATS1E1W |
+ HFGITR_EL2_ATS1E1R |
+ HFGITR_EL2_DCZVA |
+ HFGITR_EL2_DCCIVAC |
+ HFGITR_EL2_DCCVAP |
+ HFGITR_EL2_DCCVAU |
+ HFGITR_EL2_DCCISW |
+ HFGITR_EL2_DCCSW |
+ HFGITR_EL2_DCISW |
+ HFGITR_EL2_DCIVAC |
+ HFGITR_EL2_ICIVAU |
+ HFGITR_EL2_ICIALLU |
+ HFGITR_EL2_ICIALLUIS,
+ NEVER_FGU, FEAT_AA64EL1),
};
static const struct reg_bits_to_feat_map hafgrtr_feat_map[] = {
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 157de0ace6e7e..28dc778d0d9bb 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1946,6 +1946,12 @@ static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
if ((hw_val & mpam_mask) == (user_val & mpam_mask))
user_val &= ~ID_AA64PFR0_EL1_MPAM_MASK;
+ /* Fail the guest's request to disable the AA64 ISA at EL{0,1,2} */
+ if (!FIELD_GET(ID_AA64PFR0_EL1_EL0, user_val) ||
+ !FIELD_GET(ID_AA64PFR0_EL1_EL1, user_val) ||
+ (vcpu_has_nv(vcpu) && !FIELD_GET(ID_AA64PFR0_EL1_EL2, user_val)))
+ return -EINVAL;
+
return set_id_reg(vcpu, rd, user_val);
}
diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
index 322b9d3b01255..57708de2075df 100644
--- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
@@ -129,10 +129,10 @@ static const struct reg_ftr_bits ftr_id_aa64pfr0_el1[] = {
REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, DIT, 0),
REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, SEL2, 0),
REG_FTR_BITS(FTR_EXACT, ID_AA64PFR0_EL1, GIC, 0),
- REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, EL3, 0),
- REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, EL2, 0),
- REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, EL1, 0),
- REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, EL0, 0),
+ REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, EL3, 1),
+ REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, EL2, 1),
+ REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, EL1, 1),
+ REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, EL0, 1),
REG_FTR_END,
};
--
Jazz isn't dead. It just smells funny.
More information about the linux-arm-kernel
mailing list