KVM: arm64: Add LR overflow infrastructure (the final one, I swear!)
Marc Zyngier
maintainerv4 cover KVM: arm64: Add LR overflow infrastructure (the final one, I swear!)
As $SUBJECT says, I really hope this is the last dance for this particular series -- I'm done with it! It was supposed to be a 5 patch job, and we're close to 50. Something went really wrong...
Most of the fixes have now been squashed back into the base patches, and the only new patch is plugging the deactivation helper into the NV code, making it more correct.
Special thanks to Fuad for going the extra mile and testing this series with pKVM.
* From v3 [3]:
- Squashed most of the previous fixes in their original patch
- Plug the L1 LR handling into the deactivation helper
* From v2 [2]:
- Fix no-vgic-v3. Again.
- Fix clearing of trap bits when running an EL1 host
- Fix the NV handling of the MI, forcing the synchronising of LRs and VMCR early in order to deliver the MI as early as possible
- Clean some leftovers of previous rework
- Force a read-back of ICH_MISR_EL2 when disabling the vgic, making NV2 suck a bit less
- Extra fixes made it out of the series:
- Fix pKVM's lack of handling of GICv3 traps, and remember the VM's vgic type
- Don't explode on non-Apple, non-GICv3 VHE HW by checking ICH_VTR_EL2 when there is none to check
* From v1 [1]:
- Fixed the ICH_HCR_EL2.TDIR detection code to include the Apple stuff, and to deal with GICv5's legacy mode
- Fixed compilation issue for old toolchains that don't understand the GICv3 sysreg names
- Allow GICv3 in-LR deactivation even when DIR trapping is enabled
- Dropped the split overflow list, once I convinced myself it wasn't bringing much to the table
- Turned kvm_vgic_vcpu_enable() into a vgic reset helper
- Remove IPI-ing on GICv3 systems without TDIR
- Fixed the out-of-LR deactivation when dealing with asymmetric SPI deactivation
- Fixed broken MMIO offset computation
- Added group enable to the GIC selftest library
- Added fixes and improvements to the vgic_irq selftest:
- Fixed definition of spurious interrupt
- Fixed config/enable ordering
- Prevent timer interrupts from being injected from userspace
- Removed limit of 4 interrupts being injected at any given time
- Added an asymmetric SPI deactivation test case
- Added a Group-0 enable test case
- Added a timer interrupt + SPI interrupt test case
- Fixed a couple of spelling mistakes (and added many more, I'm sure)
- Reordered the series slightly
[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 (49):
irqchip/gic: Add missing GICH_HCR control bits irqchip/gic: Expose CPU interface VA to KVM irqchip/apple-aic: Spit out ICH_MISR_EL2 value on spurious vGIC MI
KVM: arm64: Turn vgic-v3 errata traps into a patched-in constant
KVM: arm64: vgic-v3: Fix GICv3 trapping in protected mode
KVM: arm64: GICv3: Detect and work around the lack of ICV_DIR_EL1
trapping
KVM: arm64: Repack struct vgic_irq fields
KVM: arm64: Add tracking of vgic_irq being present in a LR
KVM: arm64: Add LR overflow handling documentation
KVM: arm64: GICv3: Drop LPI active state when folding LRs
KVM: arm64: GICv3: Preserve EOIcount on exit
KVM: arm64: GICv3: Decouple ICH_HCR_EL2 programming from LRs
KVM: arm64: GICv3: Extract LR folding primitive
KVM: arm64: GICv3: Extract LR computing primitive
KVM: arm64: GICv2: Preserve EOIcount on exit
KVM: arm64: GICv2: Decouple GICH_HCR programming from LRs being loaded
KVM: arm64: GICv2: Extract LR folding primitive
KVM: arm64: GICv2: Extract LR computing primitive
KVM: arm64: Compute vgic state irrespective of the number of
interrupts
KVM: arm64: Eagerly save VMCR on exit
KVM: arm64: Revamp vgic maintenance interrupt configuration
KVM: arm64: Turn kvm_vgic_vcpu_enable() into kvm_vgic_vcpu_reset()
KVM: arm64: Make vgic_target_oracle() globally available
KVM: arm64: Invert ap_list sorting to push active interrupts out
KVM: arm64: Move undeliverable interrupts to the end of ap_list
KVM: arm64: Use MI to detect groups being enabled/disabled
KVM: arm64: GICv3: Handle LR overflow when EOImode==0
KVM: arm64: GICv3: Handle deactivation via ICV_DIR_EL1 traps
KVM: arm64: GICv3: Add GICv2 SGI handling to deactivation primitive
KVM: arm64: GICv3: Set ICH_HCR_EL2.TDIR when interrupts overflow LR
capacity
KVM: arm64: GICv3: Add SPI tracking to handle asymmetric deactivation
KVM: arm64: GICv3: Handle in-LR deactivation when possible
KVM: arm64: GICv3: Avoid broadcast kick on CPUs lacking TDIR
KVM: arm64: GICv3: nv: Resync LRs/VMCR/HCR early for better MI
emulation
KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
KVM: arm64: GICv3: Force exit to sync ICH_HCR_EL2.En
KVM: arm64: GICv2: Handle LR overflow when EOImode==0
KVM: arm64: GICv2: Handle deactivation via GICV_DIR traps
KVM: arm64: GICv2: Always trap GICV_DIR register
KVM: arm64: selftests: gic_v3: Add irq group setting helper
KVM: arm64: selftests: gic_v3: Disable Group-0 interrupts by default
KVM: arm64: selftests: vgic_irq: Fix GUEST_ASSERT_IAR_EMPTY() helper
KVM: arm64: selftests: vgic_irq: Change configuration before enabling
interrupt
KVM: arm64: selftests: vgic_irq: Exclude timer-controlled interrupts
KVM: arm64: selftests: vgic_irq: Remove LR-bound limitation
KVM: arm64: selftests: vgic_irq: Perform EOImode==1 deactivation in
ack order
KVM: arm64: selftests: vgic_irq: Add asymmetric SPI deaectivation test
KVM: arm64: selftests: vgic_irq: Add Group-0 enable test
KVM: arm64: selftests: vgic_irq: Add timer deactivation test
arch/arm64/include/asm/kvm_asm.h | 2 +- arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/include/asm/kvm_hyp.h | 3 +- arch/arm64/include/asm/virt.h | 7 +- arch/arm64/kernel/cpufeature.c | 52 +++ arch/arm64/kernel/hyp-stub.S | 5 + arch/arm64/kernel/image-vars.h | 1 + arch/arm64/kvm/arm.c | 7 +- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 7 +- arch/arm64/kvm/hyp/nvhe/pkvm.c | 3 + arch/arm64/kvm/hyp/nvhe/sys_regs.c | 5 + arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c | 4 + arch/arm64/kvm/hyp/vgic-v3-sr.c | 96 ++-- arch/arm64/kvm/sys_regs.c | 19 +- arch/arm64/kvm/vgic/vgic-init.c | 9 +- arch/arm64/kvm/vgic/vgic-mmio-v2.c | 24 + arch/arm64/kvm/vgic/vgic-mmio.h | 1 + arch/arm64/kvm/vgic/vgic-v2.c | 291 +++++++++--- arch/arm64/kvm/vgic/vgic-v3-nested.c | 104 ++--- arch/arm64/kvm/vgic/vgic-v3.c | 426 ++++++++++++++---- arch/arm64/kvm/vgic/vgic-v4.c | 5 +- arch/arm64/kvm/vgic/vgic.c | 298 +++++++----- arch/arm64/kvm/vgic/vgic.h | 43 +- arch/arm64/tools/cpucaps | 1 + drivers/irqchip/irq-apple-aic.c | 7 +- drivers/irqchip/irq-gic.c | 3 + include/kvm/arm_vgic.h | 29 +- include/linux/irqchip/arm-gic.h | 6 + include/linux/irqchip/arm-vgic-info.h | 2 + tools/testing/selftests/kvm/arm64/vgic_irq.c | 285 +++++++++++- .../testing/selftests/kvm/include/arm64/gic.h | 1 + tools/testing/selftests/kvm/lib/arm64/gic.c | 6 + .../selftests/kvm/lib/arm64/gic_private.h | 1 + .../testing/selftests/kvm/lib/arm64/gic_v3.c | 17 + 34 files changed, 1352 insertions(+), 419 deletions(-)
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 1/49 irqchip/gic: Add missing GICH_HCR control bits
The GICH_HCR description is missing a bunch of control bits that control the maintenance interrupt. Add them.
--- include/linux/irqchip/arm-gic.h | 6 ++++++ 1 file changed, 6 insertions(+)
include/linux/irqchip/arm-gic.h +6 -0
@@ -86,7 +86,13 @@#define GICH_HCR_EN (1 << 0)#define GICH_HCR_UIE (1 << 1) +#define GICH_HCR_LRENPIE (1 << 2)#define GICH_HCR_NPIE (1 << 3) +#define GICH_HCR_VGrp0EIE (1 << 4) +#define GICH_HCR_VGrp0DIE (1 << 5) +#define GICH_HCR_VGrp1EIE (1 << 6) +#define GICH_HCR_VGrp1DIE (1 << 7) +#define GICH_HCR_EOICOUNT GENMASK(31, 27)#define GICH_LR_VIRTUALID (0x3ff << 0)#define GICH_LR_PHYSID_CPUID_SHIFT (10)
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 2/49 irqchip/gic: Expose CPU interface VA to KVM
In reply to Marc ZyngierFuture changes will require KVM to be able to perform deactivations by writing to the physical CPU interface. Add the corresponding VA to the kvm_info structure, and let KVM stash it.
--- arch/arm64/kvm/vgic/vgic-v2.c | 1 + drivers/irqchip/irq-gic.c | 3 +++ include/kvm/arm_vgic.h | 3 +++ include/linux/irqchip/arm-vgic-info.h | 2 ++ 4 files changed, 9 insertions(+)
arch/arm64/kvm/vgic/vgic-v2.c +1 -0
@@ -385,6 +385,7 @@ int vgic_v2_probe(const struct gic_kvm_info *info) kvm_vgic_global_state.can_emulate_gicv2 = true; kvm_vgic_global_state.vcpu_base = info->vcpu.start; + kvm_vgic_global_state.gicc_base = info->gicc_base; kvm_vgic_global_state.type =VGIC_V2 ; kvm_vgic_global_state.max_gic_vcpus =VGIC_V2_MAX_CPUS ;
drivers/irqchip/irq-gic.c +3 -0
@@ -1459,6 +1459,8 @@ static void __init gic_of_setup_kvm_info(struct device_node *node)if (ret)return ; + gic_v2_kvm_info.gicc_base = gic_data[0 ].cpu_base.common_base; +if (static_branch_likely (&supports_deactivate_key))vgic_set_kvm_info (&gic_v2_kvm_info); }
@@ -1620,6 +1622,7 @@ static void __init gic_acpi_setup_kvm_info(void)return ; gic_v2_kvm_info.maint_irq = irq; + gic_v2_kvm_info.gicc_base = gic_data[0 ].cpu_base.common_base;vgic_set_kvm_info (&gic_v2_kvm_info); }
include/kvm/arm_vgic.h +3 -0
@@ -59,6 +59,9 @@ struct vgic_global {/* virtual control interface mapping, HYP VA */ void __iomem *vctrl_hyp; +/* Physical CPU interface, kernel VA */ +void __iomem *gicc_base; +/* Number of implemented list registers */ int nr_lr;
include/linux/irqchip/arm-vgic-info.h +2 -0
@@ -24,6 +24,8 @@ struct gic_kvm_info {enum gic_type type;/* Virtual CPU interface */ struct resource vcpu; +/* GICv2 GICC VA */ +void __iomem *gicc_base;/* Interrupt number */ unsigned int maint_irq;/* No interrupt mask, no need to use the above field */
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 3/49 irqchip/apple-aic: Spit out ICH_MISR_EL2 value on spurious vGIC MI
In reply to Marc ZyngierIt is all good and well to scream about spurious vGIC maintenance interrupts. It would be even better to output the reason why, which is already checked, but not printed out.
The unsuspecting kernel tinkerer thanks you.
--- drivers/irqchip/irq-apple-aic.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/irqchip/irq-apple-aic.c +5 -2
@@ -411,12 +411,15 @@ static void __exception_irq_entry aic_handle_irq(struct pt_regs *regs)if (is_kernel_in_hyp_mode () && (read_sysreg_s (SYS_ICH_HCR_EL2 ) & ICH_HCR_EL2_En) &&read_sysreg_s (SYS_ICH_MISR_EL2 ) !=0 ) { +u64 val; +generic_handle_domain_irq (aic_irqc->hw_domain,AIC_FIQ_HWIRQ (AIC_VGIC_MI ));if (unlikely ((read_sysreg_s (SYS_ICH_HCR_EL2 ) & ICH_HCR_EL2_En) && -read_sysreg_s (SYS_ICH_MISR_EL2 ))) { -pr_err_ratelimited ("vGIC IRQ fired and not handled by KVM, disabling.\n" ); + (val =read_sysreg_s (SYS_ICH_MISR_EL2 )))) { +pr_err_ratelimited ("vGIC IRQ fired and not handled by KVM (MISR=%llx), disabling.\n" , + val);sysreg_clear_set_s (SYS_ICH_HCR_EL2 , ICH_HCR_EL2_En,0 ); } }
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 4/49 KVM: arm64: Turn vgic-v3 errata traps into a patched-in constant
In reply to Marc ZyngierThe trap bits are currently only set to manage CPU errata. However, we are about to make use of them for purposes beyond beating broken CPUs into submission.
For this purpose, turn these errata-driven bits into a patched-in constant that is merged with the KVM-driven value at the point of programming the ICH_HCR_EL2 register, rather than being directly stored with with the shadow value..
This allows the KVM code to distinguish between a trap being handled for the purpose of an erratum workaround, or for KVM's own need.
--- arch/arm64/kernel/image-vars.h | 1 + arch/arm64/kvm/hyp/vgic-v3-sr.c | 21 ++++--- arch/arm64/kvm/vgic/vgic-v3-nested.c | 12 +--- arch/arm64/kvm/vgic/vgic-v3.c | 83 +++++++++++++++++----------- arch/arm64/kvm/vgic/vgic.h | 16 ++++++ 5 files changed, 84 insertions(+), 49 deletions(-)
arch/arm64/kernel/image-vars.h +1 -0
@@ -91,6 +91,7 @@ KVM_NVHE_ALIAS(spectre_bhb_patch_loop_mitigation_enable);KVM_NVHE_ALIAS (spectre_bhb_patch_wa3);KVM_NVHE_ALIAS (spectre_bhb_patch_clearbhb);KVM_NVHE_ALIAS (alt_cb_patch_nops); +KVM_NVHE_ALIAS (kvm_compute_ich_hcr_trap_bits);/* Global kernel state accessed by nVHE hyp code. */ KVM_NVHE_ALIAS (kvm_vgic_global_state);
arch/arm64/kvm/hyp/vgic-v3-sr.c +14 -7
@@ -14,6 +14,8 @@#include <asm/kvm_hyp.h> #include <asm/kvm_mmu.h> +#include "../../vgic/vgic.h" +#define vtr_to_max_lr_idx (v) ((v) & 0xf)#define vtr_to_nr_pre_bits (v) ((((u32)(v) >> 26) & 7) + 1)#define vtr_to_nr_apr_regs (v) (1 << (vtr_to_nr_pre_bits(v) - 5))
@@ -196,6 +198,11 @@ static u32 __vgic_v3_read_ap1rn(int n)return val; } +static u64 compute_ich_hcr (struct vgic_v3_cpu_if *cpu_if) +{ +return cpu_if->vgic_hcr |vgic_ich_hcr_trap_bits (); +} +void __vgic_v3_save_state (struct vgic_v3_cpu_if *cpu_if) {u64 used_lrs = cpu_if->used_lrs;
@@ -218,7 +225,7 @@ void __vgic_v3_save_state(struct vgic_v3_cpu_if *cpu_if) elrsr =read_gicreg (ICH_ELRSR_EL2 ); -write_gicreg (cpu_if->vgic_hcr & ~ICH_HCR_EL2_En,ICH_HCR_EL2 ); +write_gicreg (0 ,ICH_HCR_EL2 );for (i =0 ; i < used_lrs; i++) {if (elrsr & (1 << i))
@@ -237,7 +244,7 @@ void __vgic_v3_restore_state(struct vgic_v3_cpu_if *cpu_if)int i;if (used_lrs || cpu_if->its_vpe.its_vm) { -write_gicreg (cpu_if->vgic_hcr,ICH_HCR_EL2 ); +write_gicreg (compute_ich_hcr (cpu_if),ICH_HCR_EL2 );for (i =0 ; i < used_lrs; i++)__gic_v3_set_lr (cpu_if->vgic_lr[i], i);
@@ -307,14 +314,14 @@ void __vgic_v3_activate_traps(struct vgic_v3_cpu_if *cpu_if) }/* -* If we need to trap system registers, we must write -* ICH_HCR_EL2 anyway, even if no interrupts are being -* injected. Note that this also applies if we don't expect -* any system register access (no vgic at all). +* If we need to trap system registers, we must write ICH_HCR_EL2 +* anyway, even if no interrupts are being injected. Note that this +* also applies if we don't expect any system register access (no +* vgic at all). In any case, no need to provide MI configuration. */ if (static_branch_unlikely (&vgic_v3_cpuif_trap) || cpu_if->its_vpe.its_vm || !cpu_if->vgic_sre) -write_gicreg (cpu_if->vgic_hcr,ICH_HCR_EL2 ); +write_gicreg (vgic_ich_hcr_trap_bits () | ICH_HCR_EL2_En,ICH_HCR_EL2 ); }void __vgic_v3_deactivate_traps (struct vgic_v3_cpu_if *cpu_if)
arch/arm64/kvm/vgic/vgic-v3-nested.c +1 -11
@@ -298,19 +298,9 @@ static void vgic_v3_create_shadow_state(struct kvm_vcpu *vcpu, struct vgic_v3_cpu_if *s_cpu_if) {struct vgic_v3_cpu_if *host_if = &vcpu->arch.vgic_cpu.vgic_v3; -u64 val =0 ;int i; -/* -* If we're on a system with a broken vgic that requires -* trapping, propagate the trapping requirements. -* -* Ah, the smell of rotten fruits... -*/ -if (static_branch_unlikely (&vgic_v3_cpuif_trap)) - val = host_if->vgic_hcr & (ICH_HCR_EL2_TALL0 |ICH_HCR_EL2_TALL1 | -ICH_HCR_EL2_TC |ICH_HCR_EL2_TDIR ); - s_cpu_if->vgic_hcr =__vcpu_sys_reg (vcpu,ICH_HCR_EL2 ) | val; + s_cpu_if->vgic_hcr =__vcpu_sys_reg (vcpu,ICH_HCR_EL2 ); s_cpu_if->vgic_vmcr =__vcpu_sys_reg (vcpu,ICH_VMCR_EL2 ); s_cpu_if->vgic_sre = host_if->vgic_sre;
arch/arm64/kvm/vgic/vgic-v3.c +52 -31
115 lines not on this page · raw message
arch/arm64/kvm/vgic/vgic.h +16 -0
@@ -164,6 +164,22 @@ static inline int vgic_write_guest_lock(struct kvm *kvm, gpa_t gpa,return ret; } +void kvm_compute_ich_hcr_trap_bits (struct alt_instr *alt, +__le32 *origptr,__le32 *updptr,int nr_inst); + +static inline u64 vgic_ich_hcr_trap_bits (void ) +{ +u64 hcr; + +/* All the traps are in the bottom 16bits */ + asm volatile(ALTERNATIVE_CB("movz %0, #0\n", + ARM64_ALWAYS_SYSTEM, + kvm_compute_ich_hcr_trap_bits) + : "=r" (hcr)); + +return hcr; +} +/* * This struct provides an intermediate representation of the fields contained * in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 5/49 KVM: arm64: vgic-v3: Fix GICv3 trapping in protected mode
In reply to Marc ZyngierAs we are about to start trapping a bunch of extra things, augment the pKVM trap description with all the registers trapped by ICH_HCR_EL2.TC, making them legal instead of resulting in a UNDEF injection in the guest.
While we're at it, ensure that pKVM captures the vgic model so that it can be checked by the emulation code.
--- arch/arm64/kvm/hyp/nvhe/pkvm.c | 3 +++ arch/arm64/kvm/hyp/nvhe/sys_regs.c | 5 +++++ 2 files changed, 8 insertions(+)
arch/arm64/kvm/hyp/nvhe/pkvm.c +3 -0
@@ -337,6 +337,9 @@ static void pkvm_init_features_from_host(struct pkvm_hyp_vm *hyp_vm, const struc/* CTR_EL0 is always under host control, even for protected VMs. */ hyp_vm->kvm.arch.ctr_el0 = host_kvm->arch.ctr_el0; +/* Preserve the vgic model so that GICv3 emulation works */ + hyp_vm->kvm.arch.vgic.vgic_model = host_kvm->arch.vgic.vgic_model; +if (test_bit (KVM_ARCH_FLAG_MTE_ENABLED , &host_kvm->arch.flags))set_bit (KVM_ARCH_FLAG_MTE_ENABLED , &kvm->arch.flags);
arch/arm64/kvm/hyp/nvhe/sys_regs.c +5 -0
@@ -444,6 +444,8 @@ static const struct sys_reg_desc pvm_sys_reg_descs[] = {/* Scalable Vector Registers are restricted. */ +HOST_HANDLED (SYS_ICC_PMR_EL1 ), +RAZ_WI (SYS_ERRIDR_EL1 ),RAZ_WI (SYS_ERRSELR_EL1 ), RAZ_WI(SYS_ERXFR_EL1),
@@ -457,9 +459,12 @@ static const struct sys_reg_desc pvm_sys_reg_descs[] = {/* Limited Ordering Regions Registers are restricted. */ +HOST_HANDLED (SYS_ICC_DIR_EL1 ), +HOST_HANDLED (SYS_ICC_RPR_EL1 ),HOST_HANDLED (SYS_ICC_SGI1R_EL1 ),HOST_HANDLED (SYS_ICC_ASGI1R_EL1 ),HOST_HANDLED (SYS_ICC_SGI0R_EL1 ), +HOST_HANDLED (SYS_ICC_CTLR_EL1 ), { SYS_DESC(SYS_ICC_SRE_EL1), .access = pvm_gic_read_sre, }, HOST_HANDLED(SYS_CCSIDR_EL1),
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 6/49 KVM: arm64: GICv3: Detect and work around the lack of ICV_DIR_EL1 trapping
In reply to Marc ZyngierA long time ago, an unsuspecting architect forgot to add a trap bit for ICV_DIR_EL1 in ICH_HCR_EL2. Which was unfortunate, but what's a bit of spec between friends? Thankfully, this was fixed in a later revision, and ARM "deprecates" the lack of trapping ability.
Unfortuantely, a few (billion) CPUs went out with that defect, anything ARMv8.0 from ARM, give or take. And on these CPUs, you can't trap DIR on its own, full stop.
As the next best thing, we can trap everything in the common group, which is a tad expensive, but hey ho, that's what you get. You can otherwise recycle the HW in the neaby bin.
--- arch/arm64/include/asm/virt.h | 7 ++++- arch/arm64/kernel/cpufeature.c | 52 ++++++++++++++++++++++++++++++++++ arch/arm64/kernel/hyp-stub.S | 5 ++++ arch/arm64/kvm/vgic/vgic-v3.c | 3 ++ arch/arm64/tools/cpucaps | 1 + 5 files changed, 67 insertions(+), 1 deletion(-)
arch/arm64/include/asm/virt.h +6 -1
@@ -40,8 +40,13 @@*/ #define HVC_FINALISE_EL2 3 +/* +* HVC_GET_ICH_VTR_EL2 - Retrieve the ICH_VTR_EL2 value +*/ +#define HVC_GET_ICH_VTR_EL2 4 +/* Max number of HYP stub hypercalls */ -#define HVC_STUB_HCALL_NR 4 +#define HVC_STUB_HCALL_NR 5/* Error returned when an invalid stub number is passed into x0 */ #define HVC_STUB_ERR 0xbadca11
arch/arm64/kernel/cpufeature.c +52 -0
64 lines not on this page · raw message
arch/arm64/kernel/hyp-stub.S +5 -0
@@ -54,6 +54,11 @@ SYM_CODE_START_LOCAL(elx_sync) 1: cmp x0, #HVC_FINALISE_EL2 b.eq __finalise_el2 + cmp x0, #HVC_GET_ICH_VTR_EL2 + b.ne 2f + mrs_s x1, SYS_ICH_VTR_EL2 + b 9f + 2: cmp x0, #HVC_SOFT_RESTART b.ne 3f mov x0, x2
arch/arm64/kvm/vgic/vgic-v3.c +3 -0
@@ -648,6 +648,9 @@ void noinstr kvm_compute_ich_hcr_trap_bits(struct alt_instr *alt, dir_trap = true; } +if (!cpus_have_cap (ARM64_HAS_ICH_HCR_EL2_TDIR )) + common_trap = true; +if (group0_trap) hcr |=ICH_HCR_EL2_TALL0 ;if (group1_trap)
arch/arm64/tools/cpucaps +1 -0
@@ -40,6 +40,7 @@ HAS_GICV5_CPUIF HAS_GICV5_LEGACY HAS_GIC_PRIO_MASKING HAS_GIC_PRIO_RELAXED_SYNC +HAS_ICH_HCR_EL2_TDIR HAS_HCR_NV1 HAS_HCX HAS_LDAPR
Signature · 1 line
2.47.3
Suzuki K Poulose
reviewerRe: [PATCH v4 06/49] KVM: arm64: GICv3: Detect and work around the lack of ICV_DIR_EL1 trapping
Quoted 77 lines · Marc Zyngier
A long time ago, an unsuspecting architect forgot to add a trap bit for
ICV_DIR_EL1inICH_HCR_EL2. Which was unfortunate, but what's a bit of spec between friends? Thankfully, this was fixed in a later revision, and ARM "deprecates" the lack of trapping ability.Unfortuantely, a few (billion) CPUs went out with that defect, anything ARMv8.0 from ARM, give or take. And on these CPUs, you can't trap DIR on its own, full stop.
As the next best thing, we can trap everything in the common group, which is a tad expensive, but hey ho, that's what you get. You can otherwise recycle the HW in the neaby bin.
Tested-by Fuad Tabba <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/include/asm/virt.h | 7 ++++- arch/arm64/kernel/cpufeature.c | 52 ++++++++++++++++++++++++++++++++++ arch/arm64/kernel/hyp-stub.S | 5 ++++ arch/arm64/kvm/vgic/vgic-v3.c | 3 ++ arch/arm64/tools/cpucaps | 1 + 5 files changed, 67 insertions(+), 1 deletion(-)arch/arm64/include/asm/virt.h +6
-1@@ -40,8 +40,13 @@*/ #define HVC_FINALISE_EL2 3 +/* +* HVC_GET_ICH_VTR_EL2 - Retrieve the ICH_VTR_EL2 value +*/ +#define HVC_GET_ICH_VTR_EL2 4 +/* Max number of HYP stub hypercalls */ -#define HVC_STUB_HCALL_NR 4 +#define HVC_STUB_HCALL_NR 5/* Error returned when an invalid stub number is passed into x0 */ #define HVC_STUB_ERR 0xbadca11arch/arm64/kernel/cpufeature.c +26
-0@@ -2303,6 +2303,49 @@ static bool has_gic_prio_relaxed_sync(const struct arm64_cpu_capabilities *entry }#endif +static bool can_trap_icv_dir_el1 (const struct arm64_cpu_capabilities *entry, +int scope) +{ +static const struct midr_range has_vgic_v3[] = { +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE_MAX ), + {}, + }; +struct arm_smccc_res res = {}; + +BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV3_CPUIF ); +BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV5_LEGACY ); +if (!cpus_have_cap (ARM64_HAS_GICV3_CPUIF ) && + !is_midr_in_range_list (has_vgic_v3)) +return false; +
arch/arm64/kernel/cpufeature.c static bool has_gic_prio_relaxed_sync(const struct arm64_cpu_capabilities *entry · line 2341
Quoted 10 lines
arch/arm64/kernel/cpufeature.c +10
-0+if (!is_hyp_mode_available ()) +return false; + +if (cpus_have_cap (ARM64_HAS_GICV5_LEGACY )) +return true; + +if (is_kernel_in_hyp_mode ()) + res.a1 =read_sysreg_s (SYS_ICH_VTR_EL2 ); +else +arm_smccc_1_1_hvc (HVC_GET_ICH_VTR_EL2 , &res);
We are reading the register on the current CPU and this capability,
being a SYSTEM_FEATURE, relies on the "probing CPU". If there CPUs
with differing values (which I don't think is practical, but hey,
never say..). This is would better be a ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE, which would run through all boot CPUs and would set the capability when it matches.
That means, you may need to replace cpus_have_cap() with this_cpu_has_cap() for both GICV3_CPUIF and GICV5_LEGACY.
Quoted 11 lines
arch/arm64/kernel/cpufeature.c +7
-0+ +if (res.a0 ==HVC_STUB_ERR ) +return false; + +return res.a1 &ICH_VTR_EL2_TDS ; +} +#ifdef CONFIG_ARM64_BTI static void bti_enable (const struct arm64_cpu_capabilities *__unused) {
arch/arm64/kernel/cpufeature.c static const struct arm64_cpu_capabilities arm64_features[] = { · line 2866
Quoted 10 lines
arch/arm64/kernel/cpufeature.c +7
-0@@ -2814,6 +2857,15 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .matches = has_gic_prio_relaxed_sync, },#endif + { +/* +* Depends on having GICv3 +*/ + .desc = "ICV_DIR_EL1 trapping", + .capability = ARM64_HAS_ICH_HCR_EL2_TDIR, + .type = ARM64_CPUCAP_SYSTEM_FEATURE,
As mentioned above, this needs to be a ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE.
Suzuki
Marc Zyngier
maintainerRe: [PATCH v4 06/49] KVM: arm64: GICv3: Detect and work around the lack of ICV_DIR_EL1 trapping
Quoted 87 lines · Suzuki K Poulose
Quoted 85 lines · Marc Zyngier
A long time ago, an unsuspecting architect forgot to add a trap bit for
ICV_DIR_EL1inICH_HCR_EL2. Which was unfortunate, but what's a bit of spec between friends? Thankfully, this was fixed in a later revision, and ARM "deprecates" the lack of trapping ability.Unfortuantely, a few (billion) CPUs went out with that defect, anything ARMv8.0 from ARM, give or take. And on these CPUs, you can't trap DIR on its own, full stop.
As the next best thing, we can trap everything in the common group, which is a tad expensive, but hey ho, that's what you get. You can otherwise recycle the HW in the neaby bin.
Tested-by Fuad Tabba <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/include/asm/virt.h | 7 ++++- arch/arm64/kernel/cpufeature.c | 52 ++++++++++++++++++++++++++++++++++ arch/arm64/kernel/hyp-stub.S | 5 ++++ arch/arm64/kvm/vgic/vgic-v3.c | 3 ++ arch/arm64/tools/cpucaps | 1 + 5 files changed, 67 insertions(+), 1 deletion(-)arch/arm64/include/asm/virt.h +5
-1@@ -40,8 +40,13 @@*/ #define HVC_FINALISE_EL2 3 +/* +* HVC_GET_ICH_VTR_EL2 - Retrieve the ICH_VTR_EL2 value +*/ +#define HVC_GET_ICH_VTR_EL2 4 +/* Max number of HYP stub hypercalls */ -#define HVC_STUB_HCALL_NR 4 +#define HVC_STUB_HCALL_NR 5/* Error returned when an invalid stub number is passed into x0 */ #define HVC_STUB_ERR 0xbadca11arch/arm64/kernel/cpufeature.c +34
-0@@ -2303,6 +2303,49 @@ static bool has_gic_prio_relaxed_sync(const struct arm64_cpu_capabilities *entry }#endif +static bool can_trap_icv_dir_el1(const structarm64_cpu_capabilities *entry, +int scope) +{ +static const struct midr_range has_vgic_v3[] = { +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE_MAX ), + {}, + }; +struct arm_smccc_res res = {}; + +BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV3_CPUIF ); +BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV5_LEGACY ); +if (!cpus_have_cap (ARM64_HAS_GICV3_CPUIF ) && + !is_midr_in_range_list (has_vgic_v3)) +return false; + +if (!is_hyp_mode_available ()) +return false; + +if (cpus_have_cap (ARM64_HAS_GICV5_LEGACY )) +return true; + +if (is_kernel_in_hyp_mode ()) + res.a1 =read_sysreg_s (SYS_ICH_VTR_EL2 ); +else
Quoted 8 lines
arch/arm64/kernel/cpufeature.c static bool has_gic_prio_relaxed_sync(const struct arm64_cpu_capabilities *entry · line 2341
Quoted 1 line
arch/arm64/kernel/cpufeature.c +1
-0+arm_smccc_1_1_hvc (HVC_GET_ICH_VTR_EL2 , &res);We are reading the register on the current CPU and this capability, being a
SYSTEM_FEATURE, relies on the "probing CPU". If there CPUs with differing values (which I don't think is practical, but hey, never say..). This is would better be aARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE, which would run through all boot CPUs and would set the capability when it matches.
While I agree that SYSTEM_FEATURE is most probably the wrong thing, I can't help but notice that
- ARM64_HAS_GICV3_CPUIF,
- ARM64_HAS_GIC_PRIO_MASKING
- ARM64_HAS_GIC_PRIO_RELAXED_SYNC
are all ARM64_CPUCAP_STRICT_BOOT_CPU_FEATURE.
On the other ARM64_HAS_GICV5_LEGACY is ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE.
Given that ARM64_HAS_ICH_HCR_EL2_TDIR is dependent on both ARM64_HAS_GICV3_CPUIF and ARM64_HAS_GICV5_LEGACY, shouldn't these two (and their dependencies) be aligned to have the same behaviour?
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Suzuki K Poulose
reviewerRe: [PATCH v4 06/49] KVM: arm64: GICv3: Detect and work around the lack of ICV_DIR_EL1 trapping
Quoted 98 lines · Marc Zyngier
Quoted 87 lines · Suzuki K Poulose
Quoted 85 lines · Marc Zyngier
A long time ago, an unsuspecting architect forgot to add a trap bit for
ICV_DIR_EL1inICH_HCR_EL2. Which was unfortunate, but what's a bit of spec between friends? Thankfully, this was fixed in a later revision, and ARM "deprecates" the lack of trapping ability.Unfortuantely, a few (billion) CPUs went out with that defect, anything ARMv8.0 from ARM, give or take. And on these CPUs, you can't trap DIR on its own, full stop.
As the next best thing, we can trap everything in the common group, which is a tad expensive, but hey ho, that's what you get. You can otherwise recycle the HW in the neaby bin.
Tested-by Fuad Tabba <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/include/asm/virt.h | 7 ++++- arch/arm64/kernel/cpufeature.c | 52 ++++++++++++++++++++++++++++++++++ arch/arm64/kernel/hyp-stub.S | 5 ++++ arch/arm64/kvm/vgic/vgic-v3.c | 3 ++ arch/arm64/tools/cpucaps | 1 + 5 files changed, 67 insertions(+), 1 deletion(-)arch/arm64/include/asm/virt.h +5
-1@@ -40,8 +40,13 @@*/ #define HVC_FINALISE_EL2 3 +/* +* HVC_GET_ICH_VTR_EL2 - Retrieve the ICH_VTR_EL2 value +*/ +#define HVC_GET_ICH_VTR_EL2 4 +/* Max number of HYP stub hypercalls */ -#define HVC_STUB_HCALL_NR 4 +#define HVC_STUB_HCALL_NR 5/* Error returned when an invalid stub number is passed into x0 */ #define HVC_STUB_ERR 0xbadca11arch/arm64/kernel/cpufeature.c +34
-0@@ -2303,6 +2303,49 @@ static bool has_gic_prio_relaxed_sync(const struct arm64_cpu_capabilities *entry }#endif +static bool can_trap_icv_dir_el1(const structarm64_cpu_capabilities *entry, +int scope) +{ +static const struct midr_range has_vgic_v3[] = { +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE_MAX ), + {}, + }; +struct arm_smccc_res res = {}; + +BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV3_CPUIF ); +BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV5_LEGACY ); +if (!cpus_have_cap (ARM64_HAS_GICV3_CPUIF ) && + !is_midr_in_range_list (has_vgic_v3)) +return false; + +if (!is_hyp_mode_available ()) +return false; + +if (cpus_have_cap (ARM64_HAS_GICV5_LEGACY )) +return true; + +if (is_kernel_in_hyp_mode ()) + res.a1 =read_sysreg_s (SYS_ICH_VTR_EL2 ); +else Quoted 8 lines
arch/arm64/kernel/cpufeature.c static bool has_gic_prio_relaxed_sync(const struct arm64_cpu_capabilities *entry · line 2341
Quoted 1 line
arch/arm64/kernel/cpufeature.c +1
-0+arm_smccc_1_1_hvc (HVC_GET_ICH_VTR_EL2 , &res);We are reading the register on the current CPU and this capability, being a
SYSTEM_FEATURE, relies on the "probing CPU". If there CPUs with differing values (which I don't think is practical, but hey, never say..). This is would better be aARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE, which would run through all boot CPUs and would set the capability when it matches.
Quoted 8 lines
While I agree that
SYSTEM_FEATUREis most probably the wrong thing, I can't help but notice that-
ARM64_HAS_GICV3_CPUIF,
-ARM64_HAS_GIC_PRIO_MASKING
-ARM64_HAS_GIC_PRIO_RELAXED_SYNCare all
ARM64_CPUCAP_STRICT_BOOT_CPU_FEATURE.
Which means, if GICV3_CPUIF is set any booting CPU must have them.
Quoted 6 lines
On the other
ARM64_HAS_GICV5_LEGACYisARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE.Given that
ARM64_HAS_ICH_HCR_EL2_TDIRis dependent on bothARM64_HAS_GICV3_CPUIFandARM64_HAS_GICV5_LEGACY, shouldn't these two (and their dependencies) be aligned to have the same behaviour?
Yes, but it also depends on how you are detecting the feature.
BOOT_CPU_FEATURES are finalized with BOOT CPU and thusanything that boots up later must have it. So it is fine, as long as a SYSTEM cap depends on it and can do its own additional checks
in a system wide safe manner.
But in your case, we a have SYSTEM cap, which only performs the check on the "running CPU" and not considering a system wide safe value for ICH_VTR_EL2_TDS. In order to do this, we need to switch to something like the GICV5_LEGACY cap, which chooses to perform the check on all booting CPUs (as we don't keep the system wide safe value for the bit it is looking for).
e.g., your cap may be incorrectly SET, when the BOOT cpu (which mostly runs the SYSTEM scope), but another CPU didn't have the ICH_VTR_EL2_TDS.
Does that help ?
Suzuki
Quoted 5 lines
Thanks,
M.
Marc Zyngier
maintainerRe: [PATCH v4 06/49] KVM: arm64: GICv3: Detect and work around the lack of ICV_DIR_EL1 trapping
Quoted 118 lines · Suzuki K Poulose
Quoted 98 lines · Marc Zyngier
Quoted 87 lines · Suzuki K Poulose
Quoted 85 lines · Marc Zyngier
A long time ago, an unsuspecting architect forgot to add a trap bit for
ICV_DIR_EL1inICH_HCR_EL2. Which was unfortunate, but what's a bit of spec between friends? Thankfully, this was fixed in a later revision, and ARM "deprecates" the lack of trapping ability.Unfortuantely, a few (billion) CPUs went out with that defect, anything ARMv8.0 from ARM, give or take. And on these CPUs, you can't trap DIR on its own, full stop.
As the next best thing, we can trap everything in the common group, which is a tad expensive, but hey ho, that's what you get. You can otherwise recycle the HW in the neaby bin.
Tested-by Fuad Tabba <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/include/asm/virt.h | 7 ++++- arch/arm64/kernel/cpufeature.c | 52 ++++++++++++++++++++++++++++++++++ arch/arm64/kernel/hyp-stub.S | 5 ++++ arch/arm64/kvm/vgic/vgic-v3.c | 3 ++ arch/arm64/tools/cpucaps | 1 + 5 files changed, 67 insertions(+), 1 deletion(-)arch/arm64/include/asm/virt.h +5
-1@@ -40,8 +40,13 @@*/ #define HVC_FINALISE_EL2 3 +/* +* HVC_GET_ICH_VTR_EL2 - Retrieve the ICH_VTR_EL2 value +*/ +#define HVC_GET_ICH_VTR_EL2 4 +/* Max number of HYP stub hypercalls */ -#define HVC_STUB_HCALL_NR 4 +#define HVC_STUB_HCALL_NR 5/* Error returned when an invalid stub number is passed into x0 */ #define HVC_STUB_ERR 0xbadca11arch/arm64/kernel/cpufeature.c +34
-0@@ -2303,6 +2303,49 @@ static bool has_gic_prio_relaxed_sync(const struct arm64_cpu_capabilities *entry }#endif +static bool can_trap_icv_dir_el1(const structarm64_cpu_capabilities *entry, +int scope) +{ +static const struct midr_range has_vgic_v3[] = { +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE_MAX ), + {}, + }; +struct arm_smccc_res res = {}; + +BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV3_CPUIF ); +BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV5_LEGACY ); +if (!cpus_have_cap (ARM64_HAS_GICV3_CPUIF ) && + !is_midr_in_range_list (has_vgic_v3)) +return false; + +if (!is_hyp_mode_available ()) +return false; + +if (cpus_have_cap (ARM64_HAS_GICV5_LEGACY )) +return true; + +if (is_kernel_in_hyp_mode ()) + res.a1 =read_sysreg_s (SYS_ICH_VTR_EL2 ); +else Quoted 8 lines
arch/arm64/kernel/cpufeature.c static bool has_gic_prio_relaxed_sync(const struct arm64_cpu_capabilities *entry · line 2341
Quoted 1 line
arch/arm64/kernel/cpufeature.c +1
-0+arm_smccc_1_1_hvc (HVC_GET_ICH_VTR_EL2 , &res);We are reading the register on the current CPU and this capability, being a
SYSTEM_FEATURE, relies on the "probing CPU". If there CPUs with differing values (which I don't think is practical, but hey, never say..). This is would better be aARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE, which would run through all boot CPUs and would set the capability when it matches.Quoted 8 lines
While I agree that
SYSTEM_FEATUREis most probably the wrong thing, I can't help but notice that-
ARM64_HAS_GICV3_CPUIF,
-ARM64_HAS_GIC_PRIO_MASKING
-ARM64_HAS_GIC_PRIO_RELAXED_SYNCare all
ARM64_CPUCAP_STRICT_BOOT_CPU_FEATURE.Which means, if
GICV3_CPUIFis set any booting CPU must have them.Quoted 6 lines
On the other
ARM64_HAS_GICV5_LEGACYisARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE.Given that
ARM64_HAS_ICH_HCR_EL2_TDIRis dependent on bothARM64_HAS_GICV3_CPUIFandARM64_HAS_GICV5_LEGACY, shouldn't these two (and their dependencies) be aligned to have the same behaviour?
Quoted 6 lines
Yes, but it also depends on how you are detecting the feature.
BOOT_CPU_FEATURESare finalized with BOOT CPU and thusanything that boots up later must have it. So it is fine, as long
as a SYSTEM cap depends on it and can do its own additional checks in a system wide safe manner.
I had a quick go at it, and things are a bit murky. As it turns out, the GICv3 stuff is STRICT_BOOT_CPU_FEATURE because we need the GIC super early, before secondary CPUs can be brought up.
However, virtualisation-specific features only get used way down the line, so both ICH_HCR_EL2_TDIR and GICV5_LEGACY can be delayed until all CPUs have booted.
Quoted 10 lines
But in your case, we a have SYSTEM cap, which only performs the check on the "running CPU" and not considering a system wide safe value for
ICH_VTR_EL2_TDS. In order to do this, we need to switch to something like theGICV5_LEGACYcap, which chooses to perform the check on all booting CPUs (as we don't keep the system wide safe value for the bit it is looking for).e.g., your cap may be incorrectly SET, when the BOOT cpu (which mostly runs the SYSTEM scope), but another CPU didn't have the
ICH_VTR_EL2_TDS.
Yup, that's the conclusion I also came to.
Quoted 1 line
Does that help ?
It does. I came up with the fix below, which I'll spin as a patch shortly.
Thanks,
M.
arch/arm64/kernel/cpufeature.c +3 -3
@@ -2326,14 +2326,14 @@ static bool can_trap_icv_dir_el1(const struct arm64_cpu_capabilities *entry,BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV3_CPUIF );BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV5_LEGACY ); -if (!cpus_have_cap (ARM64_HAS_GICV3_CPUIF ) && +if (!this_cpu_has_cap (ARM64_HAS_GICV3_CPUIF ) && !is_midr_in_range_list (has_vgic_v3))return false;if (!is_hyp_mode_available ())return false; -if (cpus_have_cap (ARM64_HAS_GICV5_LEGACY )) +if (this_cpu_has_cap (ARM64_HAS_GICV5_LEGACY ))return true;if (is_kernel_in_hyp_mode ())
@@ -2864,7 +2864,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {*/ .desc = "ICV_DIR_EL1 trapping", .capability = ARM64_HAS_ICH_HCR_EL2_TDIR, - .type = ARM64_CPUCAP_SYSTEM_FEATURE, + .type = ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE, .matches = can_trap_icv_dir_el1, },#ifdef CONFIG_ARM64_E0PD
Signature · 1 line
Without deviation from the norm, progress is not possible.
Suzuki K Poulose
reviewerRe: [PATCH v4 06/49] KVM: arm64: GICv3: Detect and work around the lack of ICV_DIR_EL1 trapping
Quoted 146 lines · Marc Zyngier
Quoted 118 lines · Suzuki K Poulose
Quoted 98 lines · Marc Zyngier
Quoted 87 lines · Suzuki K Poulose
Quoted 85 lines · Marc Zyngier
A long time ago, an unsuspecting architect forgot to add a trap bit for
ICV_DIR_EL1inICH_HCR_EL2. Which was unfortunate, but what's a bit of spec between friends? Thankfully, this was fixed in a later revision, and ARM "deprecates" the lack of trapping ability.Unfortuantely, a few (billion) CPUs went out with that defect, anything ARMv8.0 from ARM, give or take. And on these CPUs, you can't trap DIR on its own, full stop.
As the next best thing, we can trap everything in the common group, which is a tad expensive, but hey ho, that's what you get. You can otherwise recycle the HW in the neaby bin.
Tested-by Fuad Tabba <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/include/asm/virt.h | 7 ++++- arch/arm64/kernel/cpufeature.c | 52 ++++++++++++++++++++++++++++++++++ arch/arm64/kernel/hyp-stub.S | 5 ++++ arch/arm64/kvm/vgic/vgic-v3.c | 3 ++ arch/arm64/tools/cpucaps | 1 + 5 files changed, 67 insertions(+), 1 deletion(-)arch/arm64/include/asm/virt.h +5
-1@@ -40,8 +40,13 @@*/ #define HVC_FINALISE_EL2 3 +/* +* HVC_GET_ICH_VTR_EL2 - Retrieve the ICH_VTR_EL2 value +*/ +#define HVC_GET_ICH_VTR_EL2 4 +/* Max number of HYP stub hypercalls */ -#define HVC_STUB_HCALL_NR 4 +#define HVC_STUB_HCALL_NR 5/* Error returned when an invalid stub number is passed into x0 */ #define HVC_STUB_ERR 0xbadca11arch/arm64/kernel/cpufeature.c +34
-0@@ -2303,6 +2303,49 @@ static bool has_gic_prio_relaxed_sync(const struct arm64_cpu_capabilities *entry }#endif +static bool can_trap_icv_dir_el1(const structarm64_cpu_capabilities *entry, +int scope) +{ +static const struct midr_range has_vgic_v3[] = { +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_ICESTORM_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M1_FIRESTORM_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE_PRO ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_BLIZZARD_MAX ), +MIDR_ALL_VERSIONS (MIDR_APPLE_M2_AVALANCHE_MAX ), + {}, + }; +struct arm_smccc_res res = {}; + +BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV3_CPUIF ); +BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV5_LEGACY ); +if (!cpus_have_cap (ARM64_HAS_GICV3_CPUIF ) && + !is_midr_in_range_list (has_vgic_v3)) +return false; + +if (!is_hyp_mode_available ()) +return false; + +if (cpus_have_cap (ARM64_HAS_GICV5_LEGACY )) +return true; + +if (is_kernel_in_hyp_mode ()) + res.a1 =read_sysreg_s (SYS_ICH_VTR_EL2 ); +else Quoted 8 lines
arch/arm64/kernel/cpufeature.c static bool has_gic_prio_relaxed_sync(const struct arm64_cpu_capabilities *entry · line 2341
Quoted 1 line
arch/arm64/kernel/cpufeature.c +1
-0+arm_smccc_1_1_hvc (HVC_GET_ICH_VTR_EL2 , &res);We are reading the register on the current CPU and this capability, being a
SYSTEM_FEATURE, relies on the "probing CPU". If there CPUs with differing values (which I don't think is practical, but hey, never say..). This is would better be aARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE, which would run through all boot CPUs and would set the capability when it matches.Quoted 8 lines
While I agree that
SYSTEM_FEATUREis most probably the wrong thing, I can't help but notice that-
ARM64_HAS_GICV3_CPUIF,
-ARM64_HAS_GIC_PRIO_MASKING
-ARM64_HAS_GIC_PRIO_RELAXED_SYNCare all
ARM64_CPUCAP_STRICT_BOOT_CPU_FEATURE.Which means, if
GICV3_CPUIFis set any booting CPU must have them.Quoted 6 lines
On the other
ARM64_HAS_GICV5_LEGACYisARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE.Given that
ARM64_HAS_ICH_HCR_EL2_TDIRis dependent on bothARM64_HAS_GICV3_CPUIFandARM64_HAS_GICV5_LEGACY, shouldn't these two (and their dependencies) be aligned to have the same behaviour?Quoted 6 lines
Yes, but it also depends on how you are detecting the feature.
BOOT_CPU_FEATURESare finalized with BOOT CPU and thusanything that boots up later must have it. So it is fine, as long
as a SYSTEM cap depends on it and can do its own additional checks in a system wide safe manner.I had a quick go at it, and things are a bit murky. As it turns out, the GICv3 stuff is
STRICT_BOOT_CPU_FEATUREbecause we need the GIC super early, before secondary CPUs can be brought up.However, virtualisation-specific features only get used way down the line, so both
ICH_HCR_EL2_TDIRandGICV5_LEGACYcan be delayed until all CPUs have booted.Quoted 10 lines
But in your case, we a have SYSTEM cap, which only performs the check on the "running CPU" and not considering a system wide safe value for
ICH_VTR_EL2_TDS. In order to do this, we need to switch to something like theGICV5_LEGACYcap, which chooses to perform the check on all booting CPUs (as we don't keep the system wide safe value for the bit it is looking for).e.g., your cap may be incorrectly SET, when the BOOT cpu (which mostly runs the SYSTEM scope), but another CPU didn't have the
ICH_VTR_EL2_TDS.
Quoted 7 lines
Yup, that's the conclusion I also came to.
Quoted 1 line
Does that help ?
It does. I came up with the fix below, which I'll spin as a patch shortly.
Thanks,
Thanks, the below looks good to me.
Cheers
Suzuki
Quoted 34 lines
M.arch/arm64/kernel/cpufeature.c +3
-3@@ -2326,14 +2326,14 @@ static bool can_trap_icv_dir_el1(const struct arm64_cpu_capabilities *entry,BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV3_CPUIF );BUILD_BUG_ON (ARM64_HAS_ICH_HCR_EL2_TDIR <=ARM64_HAS_GICV5_LEGACY ); -if (!cpus_have_cap (ARM64_HAS_GICV3_CPUIF ) && +if (!this_cpu_has_cap (ARM64_HAS_GICV3_CPUIF ) && !is_midr_in_range_list (has_vgic_v3))return false;if (!is_hyp_mode_available ())return false; -if (cpus_have_cap (ARM64_HAS_GICV5_LEGACY )) +if (this_cpu_has_cap (ARM64_HAS_GICV5_LEGACY ))return true;if (is_kernel_in_hyp_mode ())@@ -2864,7 +2864,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {*/ .desc = "ICV_DIR_EL1 trapping", .capability = ARM64_HAS_ICH_HCR_EL2_TDIR, - .type = ARM64_CPUCAP_SYSTEM_FEATURE, + .type = ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE, .matches = can_trap_icv_dir_el1, },#ifdef CONFIG_ARM64_E0PD
Marc Zyngier
maintainerPATCH v4 7/49 KVM: arm64: Repack struct vgic_irq fields
In reply to Marc Zyngierstruct vgic_irq has grown over the years, in a rather bad way. Repack it using bitfields so that the individual flags, and move things around a bit so that it a bit smaller.
--- arch/arm64/kvm/vgic/vgic-v4.c | 5 ++++- include/kvm/arm_vgic.h | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-)
arch/arm64/kvm/vgic/vgic-v4.c +4 -1
@@ -163,6 +163,7 @@ static void vgic_v4_disable_vsgis(struct kvm_vcpu *vcpu)struct vgic_irq *irq =vgic_get_vcpu_irq (vcpu, i);struct irq_desc *desc;unsigned long flags; +bool pending;int ret;raw_spin_lock_irqsave (&irq->irq_lock, flags);
@@ -173,9 +174,11 @@ static void vgic_v4_disable_vsgis(struct kvm_vcpu *vcpu) irq->hw = false; ret =irq_get_irqchip_state (irq->host_irq,IRQCHIP_STATE_PENDING , - &irq->pending_latch); + &pending);WARN_ON (ret); + irq->pending_latch = pending; + desc =irq_to_desc (irq->host_irq);irq_domain_deactivate_irq (irq_desc_get_irq_data (desc)); unlock:
include/kvm/arm_vgic.h +10 -10
@@ -123,6 +123,7 @@ struct irq_ops {struct vgic_irq {raw_spinlock_t irq_lock;/* Protects the content of the struct */ +u32 intid;/* Guest visible INTID */ struct rcu_head rcu;struct list_head ap_list;
@@ -137,17 +138,17 @@ struct vgic_irq {* affinity reg (v3). */ -u32 intid;/* Guest visible INTID */ -bool line_level;/* Level only */ -bool pending_latch;/* The pending latch state used to calculate -* the pending state for both level -* and edge triggered IRQs. */ -bool active; -bool pending_release;/* Used for LPIs only, unreferenced IRQ + bool pending_release:1; /* Used for LPIs only, unreferenced IRQ* pending a release */ -bool enabled; -bool hw;/* Tied to HW IRQ */ + bool pending_latch:1; /* The pending latch state used to calculate +* the pending state for both level +* and edge triggered IRQs. */ + enum vgic_irq_config config:1; /* Level or edge */ + bool line_level:1; /* Level only */ + bool enabled:1; + bool active:1; + bool hw:1; /* Tied to HW IRQ */refcount_t refcount;/* Used for LPIs */ u32 hwintid;/* HW INTID number */ unsigned int host_irq;/* linux irq corresponding to hwintid */
@@ -159,7 +160,6 @@ struct vgic_irq {u8 active_source;/* GICv2 SGIs only */ u8 priority;u8 group;/* 0 == group 0, 1 == group 1 */ -enum vgic_irq_config config;/* Level or edge */ struct irq_ops *ops;
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 8/49 KVM: arm64: Add tracking of vgic_irq being present in a LR
In reply to Marc ZyngierWe currently cannot identify whether an interrupt is queued into a LR. It wasn't needed until now, but that's about to change.
Add yet another flag to track that state.
--- arch/arm64/kvm/vgic/vgic-v2.c | 6 ++++++ arch/arm64/kvm/vgic/vgic-v3.c | 6 ++++++ include/kvm/arm_vgic.h | 1 + 3 files changed, 13 insertions(+)
arch/arm64/kvm/vgic/vgic-v2.c +6 -0
@@ -101,6 +101,8 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)/* Handle resampling for mapped interrupts if required */ vgic_irq_handle_resampling (irq, deactivated, val &GICH_LR_PENDING_BIT ); + irq->on_lr = false; +raw_spin_unlock (&irq->irq_lock);vgic_put_irq (vcpu->kvm, irq); }
@@ -124,6 +126,8 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)u32 val = irq->intid;bool allow_pending = true; +WARN_ON (irq->on_lr); +if (irq->active) { val |=GICH_LR_ACTIVE_BIT ;if (vgic_irq_is_sgi (irq->intid))
@@ -194,6 +198,8 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)/* The GICv2 LR only holds five bits of priority. */ val |= (irq->priority >>3 ) <<GICH_LR_PRIORITY_SHIFT ; + irq->on_lr = true; + vcpu->arch.vgic_cpu.vgic_v2.vgic_lr[lr] = val; }
arch/arm64/kvm/vgic/vgic-v3.c +6 -0
@@ -97,6 +97,8 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)/* Handle resampling for mapped interrupts if required */ vgic_irq_handle_resampling (irq, deactivated, val &ICH_LR_PENDING_BIT ); + irq->on_lr = false; +raw_spin_unlock (&irq->irq_lock);vgic_put_irq (vcpu->kvm, irq); }
@@ -111,6 +113,8 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)u64 val = irq->intid;bool allow_pending = true, is_v2_sgi; +WARN_ON (irq->on_lr); + is_v2_sgi = (vgic_irq_is_sgi (irq->intid) && model ==KVM_DEV_TYPE_ARM_VGIC_V2 );
@@ -185,6 +189,8 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) val |= (u64 )irq->priority <<ICH_LR_PRIORITY_SHIFT ; vcpu->arch.vgic_cpu.vgic_v3.vgic_lr[lr] = val; + + irq->on_lr = true; }void vgic_v3_clear_lr (struct kvm_vcpu *vcpu,int lr)
include/kvm/arm_vgic.h +1 -0
@@ -149,6 +149,7 @@ struct vgic_irq { bool enabled:1; bool active:1; bool hw:1; /* Tied to HW IRQ */ + bool on_lr:1; /* Present in a CPU LR */refcount_t refcount;/* Used for LPIs */ u32 hwintid;/* HW INTID number */ unsigned int host_irq;/* linux irq corresponding to hwintid */
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 9/49 KVM: arm64: Add LR overflow handling documentation
In reply to Marc ZyngierAdd a bit of documentation describing how we are dealing with LR overflow. This is mostly a braindump of how things are expected to work. For now anyway.
--- arch/arm64/kvm/vgic/vgic.c | 81 +++++++++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-)
arch/arm64/kvm/vgic/vgic.c +80 -1
@@ -825,7 +825,86 @@ static int compute_ap_list_depth(struct kvm_vcpu *vcpu,return count; } -/* Requires the VCPU's ap_list_lock to be held. */ +/* +* Dealing with LR overflow is close to black magic -- dress accordingly. +* +* We have to present an almost infinite number of interrupts through a very +* limited number of registers. Therefore crucial decisions must be made to +* ensure we feed the most relevant interrupts into the LRs, and yet have +* some facilities to let the guest interact with those that are not there. +* +* All considerations below are in the context of interrupts targeting a +* single vcpu with non-idle state (either pending, active, or both), +* colloquially called the ap_list: +* +* - Pending interrupts must have priority over active interrupts. This also +* excludes pending+active interrupts. This ensures that a guest can +* perform priority drops on any number of interrupts, and yet be +* presented the next pending one. +* +* - Deactivation of interrupts outside of the LRs must be tracked by using +* either the EOIcount-driven maintenance interrupt, and sometimes by +* trapping the DIR register. +* +* - For EOImode=0, a non-zero EOIcount means walking the ap_list past the +* point that made it into the LRs, and deactivate interrupts that would +* have made it onto the LRs if we had the space. +* +* - The MI-generation bits must be used to try and force an exit when the +* guest has done enough changes to the LRs that we want to reevaluate the +* situation: +* +* - if the total number of pending interrupts exceeds the number of +* LR, NPIE must be set in order to exit once no pending interrupts +* are present in the LRs, allowing us to populate the next batch. +* +* - if there are active interrupts outside of the LRs, then LRENPIE +* must be set so that we exit on deactivation of one of these, and +* work out which one is to be deactivated. Note that this is not +* enough to deal with EOImode=1, see below. +* +* - if the overall number of interrupts exceeds the number of LRs, +* then UIE must be set to allow refilling of the LRs once the +* majority of them has been processed. +* +* - as usual, MI triggers are only an optimisation, since we cannot +* rely on the MI being delivered in timely manner... +* +* - EOImode=1 creates some additional problems: +* +* - deactivation can happen in any order, and we cannot rely on +* EOImode=0's coupling of priority-drop and deactivation which +* imposes strict reverse Ack order. This means that DIR must +* trap if we have active interrupts outside of the LRs. +* +* - deactivation of SPIs can occur on any CPU, while the SPI is only +* present in the ap_list of the CPU that actually ack-ed it. In that +* case, EOIcount doesn't provide enough information, and we must +* resort to trapping DIR even if we don't overflow the LRs. Bonus +* point for not trapping DIR when no SPIs are pending or active in +* the whole VM. +* +* - LPIs do not suffer the same problem as SPIs on deactivation, as we +* have to essentially discard the active state, see below. +* +* - Virtual LPIs have an active state (surprise!), which gets removed on +* priority drop (EOI). However, EOIcount doesn't get bumped when the LPI +* is not present in the LR (surprise again!). Special care must therefore +* be taken to remove the active state from any activated LPI when exiting +* from the guest. This is in a way no different from what happens on the +* physical side. We still rely on the running priority to have been +* removed from the APRs, irrespective of the LPI being present in the LRs +* or not. +* +* - Virtual SGIs directly injected via GICv4.1 must not affect EOIcount, as +* they are not managed in SW and don't have a true active state. So only +* set vSGIEOICount when no SGIs are in the ap_list. +* +* - GICv2 SGIs with multiple sources are injected one source at a time, as +* if they were made pending sequentially. This may mean that we don't +* always present the HPPI if other interrupts with lower priority are +* pending in the LRs. Big deal. +*/ static void vgic_flush_lr_state (struct kvm_vcpu *vcpu) {struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 10/49 KVM: arm64: GICv3: Drop LPI active state when folding LRs
In reply to Marc ZyngierDespite LPIs not having an active state, *virtual* LPIs do have one, which gets cleared on EOI. So far, so good.
However, this leads to a small problem: when an active LPI is not in the LRs, that EOImode==0 and that the guest EOIs it, EOIcount doesn't get bumped up. Which means that in these condition, the LPI would stay active forever.
Clearly, we can't have that. So if we spot an active LPI, we drop that state. It's pretty pointless anyway, and only serves as a way to trip SW over.
--- arch/arm64/kvm/vgic/vgic-v3.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
arch/arm64/kvm/vgic/vgic-v3.c +3 -1
@@ -72,7 +72,9 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)raw_spin_lock (&irq->irq_lock); -/* Always preserve the active bit, note deactivation */ +/* Always preserve the active bit for !LPIs, note deactivation */ +if (irq->intid >=VGIC_MIN_LPI ) + val &= ~ICH_LR_ACTIVE_BIT ; deactivated = irq->active && !(val &ICH_LR_ACTIVE_BIT ); irq->active = !!(val &ICH_LR_ACTIVE_BIT );
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 11/49 KVM: arm64: GICv3: Preserve EOIcount on exit
In reply to Marc ZyngierEOIcount is how the virtual CPU interface signals that the guest is deactivating interrupts outside of the LRs when EOImode==0.
We therefore need to preserve that information so that we can find out what actually needs deactivating.
--- arch/arm64/kvm/hyp/vgic-v3-sr.c | 6 ++++++ 1 file changed, 6 insertions(+)
arch/arm64/kvm/hyp/vgic-v3-sr.c +6 -0
@@ -225,6 +225,12 @@ void __vgic_v3_save_state(struct vgic_v3_cpu_if *cpu_if) elrsr =read_gicreg (ICH_ELRSR_EL2 ); +if (cpu_if->vgic_hcr &ICH_HCR_EL2_LRENPIE ) { +u64 val =read_gicreg (ICH_HCR_EL2 ); + cpu_if->vgic_hcr &= ~ICH_HCR_EL2_EOIcount; + cpu_if->vgic_hcr |= val & ICH_HCR_EL2_EOIcount; + } +write_gicreg (0 ,ICH_HCR_EL2 );for (i =0 ; i < used_lrs; i++) {
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 12/49 KVM: arm64: GICv3: Decouple ICH_HCR_EL2 programming from LRs
In reply to Marc ZyngierNot programming ICH_HCR_EL2 while no LRs are populated is a bit of an issue, as we otherwise don't see any maintenance interrupt when the guest interacts with the LRs.
Decouple the two and always program the control register, even when we don't have to touch the LRs.
--- arch/arm64/kvm/hyp/vgic-v3-sr.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-)
arch/arm64/kvm/hyp/vgic-v3-sr.c +12 -14
@@ -219,20 +219,12 @@ void __vgic_v3_save_state(struct vgic_v3_cpu_if *cpu_if) } } -if (used_lrs || cpu_if->its_vpe.its_vm) { +if (used_lrs) {int i;u32 elrsr; elrsr =read_gicreg (ICH_ELRSR_EL2 ); -if (cpu_if->vgic_hcr &ICH_HCR_EL2_LRENPIE ) { -u64 val =read_gicreg (ICH_HCR_EL2 ); - cpu_if->vgic_hcr &= ~ICH_HCR_EL2_EOIcount; - cpu_if->vgic_hcr |= val & ICH_HCR_EL2_EOIcount; - } - -write_gicreg (0 ,ICH_HCR_EL2 ); -for (i =0 ; i < used_lrs; i++) {if (elrsr & (1 << i)) cpu_if->vgic_lr[i] &= ~ICH_LR_STATE ;
@@ -242,6 +234,14 @@ void __vgic_v3_save_state(struct vgic_v3_cpu_if *cpu_if)__gic_v3_set_lr (0 , i); } } + +if (cpu_if->vgic_hcr &ICH_HCR_EL2_LRENPIE ) { +u64 val =read_gicreg (ICH_HCR_EL2 ); + cpu_if->vgic_hcr &= ~ICH_HCR_EL2_EOIcount; + cpu_if->vgic_hcr |= val & ICH_HCR_EL2_EOIcount; + } + +write_gicreg (0 ,ICH_HCR_EL2 ); }void __vgic_v3_restore_state (struct vgic_v3_cpu_if *cpu_if)
@@ -249,12 +249,10 @@ void __vgic_v3_restore_state(struct vgic_v3_cpu_if *cpu_if)u64 used_lrs = cpu_if->used_lrs;int i; -if (used_lrs || cpu_if->its_vpe.its_vm) { -write_gicreg (compute_ich_hcr (cpu_if),ICH_HCR_EL2 ); +write_gicreg (compute_ich_hcr (cpu_if),ICH_HCR_EL2 ); -for (i =0 ; i < used_lrs; i++) -__gic_v3_set_lr (cpu_if->vgic_lr[i], i); - } +for (i =0 ; i < used_lrs; i++) +__gic_v3_set_lr (cpu_if->vgic_lr[i], i);/* * Ensure that writes to the LRs, and on non-VHE systems ensure that
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 13/49 KVM: arm64: GICv3: Extract LR folding primitive
In reply to Marc ZyngierAs we are going to need to handle deactivation for interrupts that are not in the LRs, split vgic_v3_fold_lr_state() into a helper that deals with a single interrupt, and the function that loops over the used LRs.
--- arch/arm64/kvm/vgic/vgic-v3.c | 88 +++++++++++++++++------------------ 1 file changed, 43 insertions(+), 45 deletions(-)
arch/arm64/kvm/vgic/vgic-v3.c +43 -45
@@ -33,78 +33,76 @@ static bool lr_signals_eoi_mi(u64 lr_val) !(lr_val &ICH_LR_HW ); } -void vgic_v3_fold_lr_state (struct kvm_vcpu *vcpu) +static void vgic_v3_fold_lr (struct kvm_vcpu *vcpu,u64 val) { -struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; -struct vgic_v3_cpu_if *cpuif = &vgic_cpu->vgic_v3; -u32 model = vcpu->kvm->arch.vgic.vgic_model; -int lr; - -DEBUG_SPINLOCK_BUG_ON (!irqs_disabled ()); - - cpuif->vgic_hcr &= ~ICH_HCR_EL2_UIE ; - -for (lr =0 ; lr < cpuif->used_lrs; lr++) { -u64 val = cpuif->vgic_lr[lr]; -u32 intid, cpuid; -struct vgic_irq *irq; -bool is_v2_sgi = false; -bool deactivated; - - cpuid = val &GICH_LR_PHYSID_CPUID ; - cpuid >>=GICH_LR_PHYSID_CPUID_SHIFT ; - -if (model ==KVM_DEV_TYPE_ARM_VGIC_V3 ) { - intid = val &ICH_LR_VIRTUAL_ID_MASK ; - }else { - intid = val &GICH_LR_VIRTUALID ; - is_v2_sgi =vgic_irq_is_sgi (intid); - } +struct vgic_irq *irq; +bool is_v2_sgi = false; +bool deactivated; +u32 intid; -/* Notify fds when the guest EOI'ed a level-triggered IRQ */ -if (lr_signals_eoi_mi (val) &&vgic_valid_spi (vcpu->kvm, intid)) -kvm_notify_acked_irq (vcpu->kvm,0 , - intid -VGIC_NR_PRIVATE_IRQS ); +if (vcpu->kvm->arch.vgic.vgic_model ==KVM_DEV_TYPE_ARM_VGIC_V3 ) { + intid = val &ICH_LR_VIRTUAL_ID_MASK ; + }else { + intid = val &GICH_LR_VIRTUALID ; + is_v2_sgi =vgic_irq_is_sgi (intid); + } - irq =vgic_get_vcpu_irq (vcpu, intid); -if (!irq)/* An LPI could have been unmapped. */ -continue ; + irq =vgic_get_vcpu_irq (vcpu, intid); +if (!irq)/* An LPI could have been unmapped. */ +return ; -raw_spin_lock (&irq->irq_lock); +/* Notify fds when the guest EOI'ed a level-triggered IRQ */ +if (lr_signals_eoi_mi (val) &&vgic_valid_spi (vcpu->kvm, intid)) +kvm_notify_acked_irq (vcpu->kvm,0 , + intid -VGIC_NR_PRIVATE_IRQS ); + scoped_guard(raw_spinlock, &irq->irq_lock) {/* Always preserve the active bit for !LPIs, note deactivation */ if (irq->intid >=VGIC_MIN_LPI ) val &= ~ICH_LR_ACTIVE_BIT ; deactivated = irq->active && !(val &ICH_LR_ACTIVE_BIT ); irq->active = !!(val &ICH_LR_ACTIVE_BIT ); -if (irq->active && is_v2_sgi) - irq->active_source = cpuid; -/* Edge is the only case where we preserve the pending bit */ if (irq->config ==VGIC_CONFIG_EDGE && - (val &ICH_LR_PENDING_BIT )) { + (val &ICH_LR_PENDING_BIT )) irq->pending_latch = true; -if (is_v2_sgi) - irq->source |= (1 << cpuid); - } -/* * Clear soft pending state when level irqs have been acked. */ if (irq->config ==VGIC_CONFIG_LEVEL && !(val &ICH_LR_STATE )) irq->pending_latch = false; +if (is_v2_sgi) { +u8 cpuid =FIELD_GET (GICH_LR_PHYSID_CPUID , val); + +if (irq->active) + irq->active_source = cpuid; + +if (val &ICH_LR_PENDING_BIT ) + irq->source |=BIT (cpuid); + } +/* Handle resampling for mapped interrupts if required */ vgic_irq_handle_resampling (irq, deactivated, val &ICH_LR_PENDING_BIT ); irq->on_lr = false; - -raw_spin_unlock (&irq->irq_lock); -vgic_put_irq (vcpu->kvm, irq); } +vgic_put_irq (vcpu->kvm, irq); +} + +void vgic_v3_fold_lr_state (struct kvm_vcpu *vcpu) +{ +struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; +struct vgic_v3_cpu_if *cpuif = &vgic_cpu->vgic_v3; + +DEBUG_SPINLOCK_BUG_ON (!irqs_disabled ()); + +for (int lr =0 ; lr < cpuif->used_lrs; lr++) +vgic_v3_fold_lr (vcpu, cpuif->vgic_lr[lr]); + cpuif->used_lrs =0 ; }
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 14/49 KVM: arm64: GICv3: Extract LR computing primitive
In reply to Marc ZyngierSplit vgic_v3_populate_lr() into two, so that we have another primitive that computes the LR from a vgic_irq, but doesn't update anything in the shadow structure.
--- arch/arm64/kvm/vgic/vgic-v3.c | 49 +++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 17 deletions(-)
arch/arm64/kvm/vgic/vgic-v3.c +32 -17
@@ -107,7 +107,7 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu) }/* Requires the irq to be locked already */ -void vgic_v3_populate_lr (struct kvm_vcpu *vcpu,struct vgic_irq *irq,int lr) +static u64 vgic_v3_compute_lr (struct kvm_vcpu *vcpu,struct vgic_irq *irq) {u32 model = vcpu->kvm->arch.vgic.vgic_model;u64 val = irq->intid;
@@ -154,6 +154,35 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)if (allow_pending &&irq_is_pending (irq)) { val |=ICH_LR_PENDING_BIT ; +if (is_v2_sgi) { +u32 src =ffs (irq->source); + +if (WARN_RATELIMIT (!src,"No SGI source for INTID %d\n" , + irq->intid)) +return 0 ; + + val |= (src -1 ) <<GICH_LR_PHYSID_CPUID_SHIFT ; +if (irq->source & ~BIT (src -1 )) + val |=ICH_LR_EOI ; + } + } + +if (irq->group) + val |=ICH_LR_GROUP ; + + val |= (u64 )irq->priority <<ICH_LR_PRIORITY_SHIFT ; + +return val; +} + +void vgic_v3_populate_lr (struct kvm_vcpu *vcpu,struct vgic_irq *irq,int lr) +{ +u32 model = vcpu->kvm->arch.vgic.vgic_model; +u64 val =vgic_v3_compute_lr (vcpu, irq); + + vcpu->arch.vgic_cpu.vgic_v3.vgic_lr[lr] = val; + +if (val &ICH_LR_PENDING_BIT ) {if (irq->config ==VGIC_CONFIG_EDGE ) irq->pending_latch = false;
@@ -161,16 +190,9 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) model == KVM_DEV_TYPE_ARM_VGIC_V2) {u32 src =ffs (irq->source); -if (WARN_RATELIMIT (!src,"No SGI source for INTID %d\n" , - irq->intid)) -return ; - - val |= (src -1 ) <<GICH_LR_PHYSID_CPUID_SHIFT ; - irq->source &= ~(1 << (src -1 )); -if (irq->source) { + irq->source &= ~BIT (src -1 ); +if (irq->source) irq->pending_latch = true; - val |=ICH_LR_EOI ; - } } }
@@ -183,13 +205,6 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)if (vgic_irq_is_mapped_level (irq) && (val &ICH_LR_PENDING_BIT )) irq->line_level = false; -if (irq->group) - val |=ICH_LR_GROUP ; - - val |= (u64 )irq->priority <<ICH_LR_PRIORITY_SHIFT ; - - vcpu->arch.vgic_cpu.vgic_v3.vgic_lr[lr] = val; - irq->on_lr = true; }
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 15/49 KVM: arm64: GICv2: Preserve EOIcount on exit
In reply to Marc ZyngierEOIcount is how the virtual CPU interface signals that the guest is deactivating interrupts outside of the LRs when EOImode==0.
We therefore need to preserve that information so that we can find out what actually needs deactivating, just like we already do on GICv3.
--- arch/arm64/kvm/vgic/vgic-v2.c | 6 ++++++ 1 file changed, 6 insertions(+)
arch/arm64/kvm/vgic/vgic-v2.c +6 -0
@@ -437,6 +437,12 @@ void vgic_v2_save_state(struct kvm_vcpu *vcpu)return ;if (used_lrs) { +if (vcpu->arch.vgic_cpu.vgic_v2.vgic_hcr &GICH_HCR_LRENPIE ) { +u32 val =readl_relaxed (base +GICH_HCR ); + + vcpu->arch.vgic_cpu.vgic_v2.vgic_hcr &= ~GICH_HCR_EOICOUNT ; + vcpu->arch.vgic_cpu.vgic_v2.vgic_hcr |= val &GICH_HCR_EOICOUNT ; + }save_lrs (vcpu, base);writel_relaxed (0 , base +GICH_HCR ); }
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 16/49 KVM: arm64: GICv2: Decouple GICH_HCR programming from LRs being loaded
In reply to Marc ZyngierNot programming GICH_HCR while no LRs are populated is a bit of an issue, as we otherwise don't see any maintenance interrupt when the guest interacts with the LRs.
Decouple the two and always program the control register, even when we don't have to touch the LRs.
This is very similar to what we are already doing for GICv3.
--- arch/arm64/kvm/vgic/vgic-v2.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)
arch/arm64/kvm/vgic/vgic-v2.c +14 -14
@@ -430,22 +430,25 @@ static void save_lrs(struct kvm_vcpu *vcpu, void __iomem *base)void vgic_v2_save_state (struct kvm_vcpu *vcpu) { +struct vgic_v2_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v2;void __iomem *base = kvm_vgic_global_state.vctrl_base;u64 used_lrs = vcpu->arch.vgic_cpu.vgic_v2.used_lrs;if (!base)return ; -if (used_lrs) { -if (vcpu->arch.vgic_cpu.vgic_v2.vgic_hcr &GICH_HCR_LRENPIE ) { -u32 val =readl_relaxed (base +GICH_HCR ); - vcpu->arch.vgic_cpu.vgic_v2.vgic_hcr &= ~GICH_HCR_EOICOUNT ; - vcpu->arch.vgic_cpu.vgic_v2.vgic_hcr |= val &GICH_HCR_EOICOUNT ; - } +if (used_lrs)save_lrs (vcpu, base); -writel_relaxed (0 , base +GICH_HCR ); + +if (cpu_if->vgic_hcr &GICH_HCR_LRENPIE ) { +u32 val =readl_relaxed (base +GICH_HCR ); + + cpu_if->vgic_hcr &= ~GICH_HCR_EOICOUNT ; + cpu_if->vgic_hcr |= val &GICH_HCR_EOICOUNT ; } + +writel_relaxed (0 , base +GICH_HCR ); }void vgic_v2_restore_state (struct kvm_vcpu *vcpu)
@@ -458,13 +461,10 @@ void vgic_v2_restore_state(struct kvm_vcpu *vcpu)if (!base)return ; -if (used_lrs) { -writel_relaxed (cpu_if->vgic_hcr, base +GICH_HCR ); -for (i =0 ; i < used_lrs; i++) { -writel_relaxed (cpu_if->vgic_lr[i], - base +GICH_LR0 + (i *4 )); - } - } +writel_relaxed (cpu_if->vgic_hcr, base +GICH_HCR ); + +for (i =0 ; i < used_lrs; i++) +writel_relaxed (cpu_if->vgic_lr[i], base +GICH_LR0 + (i *4 )); }void vgic_v2_load (struct kvm_vcpu *vcpu)
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 17/49 KVM: arm64: GICv2: Extract LR folding primitive
In reply to Marc ZyngierAs we are going to need to handle deactivation for interrupts that are not in the LRs, split vgic_v2_fold_lr_state() into a helper that deals with a single interrupt, and the function that loops over the used LRs.
--- arch/arm64/kvm/vgic/vgic-v2.c | 67 +++++++++++++++++------------------ 1 file changed, 32 insertions(+), 35 deletions(-)
arch/arm64/kvm/vgic/vgic-v2.c +32 -35
@@ -39,43 +39,23 @@ static bool lr_signals_eoi_mi(u32 lr_val) !(lr_val &GICH_LR_HW ); } -/* -* transfer the content of the LRs back into the corresponding ap_list: -* - active bit is transferred as is -* - pending bit is -* - transferred as is in case of edge sensitive IRQs -* - set to the line-level (resample time) for level sensitive IRQs -*/ -void vgic_v2_fold_lr_state (struct kvm_vcpu *vcpu) +static void vgic_v2_fold_lr (struct kvm_vcpu *vcpu,u32 val) { -struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; -struct vgic_v2_cpu_if *cpuif = &vgic_cpu->vgic_v2; -int lr; - -DEBUG_SPINLOCK_BUG_ON (!irqs_disabled ()); - - cpuif->vgic_hcr &= ~GICH_HCR_UIE ; +u32 cpuid, intid = val &GICH_LR_VIRTUALID ; +struct vgic_irq *irq; +bool deactivated; -for (lr =0 ; lr < vgic_cpu->vgic_v2.used_lrs; lr++) { -u32 val = cpuif->vgic_lr[lr]; -u32 cpuid, intid = val &GICH_LR_VIRTUALID ; -struct vgic_irq *irq; -bool deactivated; +/* Extract the source vCPU id from the LR */ + cpuid =FIELD_GET (GICH_LR_PHYSID_CPUID , val) &7 ; -/* Extract the source vCPU id from the LR */ - cpuid = val &GICH_LR_PHYSID_CPUID ; - cpuid >>=GICH_LR_PHYSID_CPUID_SHIFT ; - cpuid &=7 ; +/* Notify fds when the guest EOI'ed a level-triggered SPI */ +if (lr_signals_eoi_mi (val) &&vgic_valid_spi (vcpu->kvm, intid)) +kvm_notify_acked_irq (vcpu->kvm,0 , + intid -VGIC_NR_PRIVATE_IRQS ); -/* Notify fds when the guest EOI'ed a level-triggered SPI */ -if (lr_signals_eoi_mi (val) &&vgic_valid_spi (vcpu->kvm, intid)) -kvm_notify_acked_irq (vcpu->kvm,0 , - intid -VGIC_NR_PRIVATE_IRQS ); - - irq =vgic_get_vcpu_irq (vcpu, intid); - -raw_spin_lock (&irq->irq_lock); + irq =vgic_get_vcpu_irq (vcpu, intid); + scoped_guard(raw_spinlock, &irq->irq_lock) {/* Always preserve the active bit, note deactivation */ deactivated = irq->active && !(val &GICH_LR_ACTIVE_BIT ); irq->active = !!(val &GICH_LR_ACTIVE_BIT );
@@ -102,11 +82,28 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)vgic_irq_handle_resampling (irq, deactivated, val &GICH_LR_PENDING_BIT ); irq->on_lr = false; - -raw_spin_unlock (&irq->irq_lock); -vgic_put_irq (vcpu->kvm, irq); } +vgic_put_irq (vcpu->kvm, irq); +} + +/* +* transfer the content of the LRs back into the corresponding ap_list: +* - active bit is transferred as is +* - pending bit is +* - transferred as is in case of edge sensitive IRQs +* - set to the line-level (resample time) for level sensitive IRQs +*/ +void vgic_v2_fold_lr_state (struct kvm_vcpu *vcpu) +{ +struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; +struct vgic_v2_cpu_if *cpuif = &vgic_cpu->vgic_v2; + +DEBUG_SPINLOCK_BUG_ON (!irqs_disabled ()); + +for (int lr =0 ; lr < vgic_cpu->vgic_v2.used_lrs; lr++) +vgic_v2_fold_lr (vcpu, cpuif->vgic_lr[lr]); + cpuif->used_lrs =0 ; }
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 18/49 KVM: arm64: GICv2: Extract LR computing primitive
In reply to Marc ZyngierSplit vgic_v2_populate_lr() into two helpers, so that we have another primitive that computes the LR from a vgic_irq, but doesn't update anything in the shadow structure.
--- arch/arm64/kvm/vgic/vgic-v2.c | 61 ++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 22 deletions(-)
arch/arm64/kvm/vgic/vgic-v2.c +39 -22
@@ -107,18 +107,7 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu) cpuif->used_lrs =0 ; } -/* -* Populates the particular LR with the state of a given IRQ: -* - for an edge sensitive IRQ the pending state is cleared in struct vgic_irq -* - for a level sensitive IRQ the pending state value is unchanged; -* it is dictated directly by the input level -* -* If @irq describes an SGI with multiple sources, we choose the -* lowest-numbered source VCPU and clear that bit in the source bitmap. -* -* The irq_lock must be held by the caller. -*/ -void vgic_v2_populate_lr (struct kvm_vcpu *vcpu,struct vgic_irq *irq,int lr) +static u32 vgic_v2_compute_lr (struct kvm_vcpu *vcpu,struct vgic_irq *irq) {u32 val = irq->intid;bool allow_pending = true;
@@ -164,22 +153,52 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)if (allow_pending &&irq_is_pending (irq)) { val |=GICH_LR_PENDING_BIT ; -if (irq->config ==VGIC_CONFIG_EDGE ) - irq->pending_latch = false; -if (vgic_irq_is_sgi (irq->intid)) {u32 src =ffs (irq->source);if (WARN_RATELIMIT (!src,"No SGI source for INTID %d\n" , irq->intid)) -return ; +return 0 ; val |= (src -1 ) <<GICH_LR_PHYSID_CPUID_SHIFT ; - irq->source &= ~(1 << (src -1 )); -if (irq->source) { - irq->pending_latch = true; +if (irq->source & ~BIT (src -1 )) val |=GICH_LR_EOI ; - } + } + } + +/* The GICv2 LR only holds five bits of priority. */ + val |= (irq->priority >>3 ) <<GICH_LR_PRIORITY_SHIFT ; + +return val; +} + +/* +* Populates the particular LR with the state of a given IRQ: +* - for an edge sensitive IRQ the pending state is cleared in struct vgic_irq +* - for a level sensitive IRQ the pending state value is unchanged; +* it is dictated directly by the input level +* +* If @irq describes an SGI with multiple sources, we choose the +* lowest-numbered source VCPU and clear that bit in the source bitmap. +* +* The irq_lock must be held by the caller. +*/ +void vgic_v2_populate_lr (struct kvm_vcpu *vcpu,struct vgic_irq *irq,int lr) +{ +u32 val =vgic_v2_compute_lr (vcpu, irq); + + vcpu->arch.vgic_cpu.vgic_v2.vgic_lr[lr] = val; + +if (val &GICH_LR_PENDING_BIT ) { +if (irq->config ==VGIC_CONFIG_EDGE ) + irq->pending_latch = false; + +if (vgic_irq_is_sgi (irq->intid)) { +u32 src =ffs (irq->source); + + irq->source &= ~BIT (src -1 ); +if (irq->source) + irq->pending_latch = true; } }
@@ -196,8 +215,6 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) val |= (irq->priority >>3 ) <<GICH_LR_PRIORITY_SHIFT ; irq->on_lr = true; - - vcpu->arch.vgic_cpu.vgic_v2.vgic_lr[lr] = val; }void vgic_v2_clear_lr (struct kvm_vcpu *vcpu,int lr)
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 19/49 KVM: arm64: Compute vgic state irrespective of the number of interrupts
In reply to Marc ZyngierAs we are going to rely on the [G]ICH_HCR{,_EL2} register to be programmed with MI information at all times, slightly de-optimise the flush/sync code to always be called. This is rather lightweight when no interrupts are in flight.
--- arch/arm64/kvm/vgic/vgic.c | 35 ++--------------------------------- 1 file changed, 2 insertions(+), 33 deletions(-)
arch/arm64/kvm/vgic/vgic.c +2 -33
@@ -985,8 +985,6 @@ static inline void vgic_save_state(struct kvm_vcpu *vcpu)/* Sync back the hardware VGIC state into our emulation after a guest's run. */ void kvm_vgic_sync_hwstate (struct kvm_vcpu *vcpu) { -int used_lrs; -/* If nesting, emulate the HW effect from L0 to L1 */ if (vgic_state_is_nested (vcpu)) {vgic_v3_sync_nested (vcpu);
@@ -996,20 +994,10 @@ void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)if (vcpu_has_nv (vcpu))vgic_v3_nested_update_mi (vcpu); -/* An empty ap_list_head implies used_lrs == 0 */ -if (list_empty (&vcpu->arch.vgic_cpu.ap_list_head)) -return ; -if (can_access_vgic_from_kernel ())vgic_save_state (vcpu); -if (!static_branch_unlikely (&kvm_vgic_global_state.gicv3_cpuif)) - used_lrs = vcpu->arch.vgic_cpu.vgic_v2.used_lrs; -else - used_lrs = vcpu->arch.vgic_cpu.vgic_v3.used_lrs; - -if (used_lrs) -vgic_fold_lr_state (vcpu); +vgic_fold_lr_state (vcpu);vgic_prune_ap_list (vcpu); }
@@ -1053,29 +1041,10 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)if (vcpu_has_nv (vcpu))vgic_v3_nested_update_mi (vcpu); -/* -* If there are no virtual interrupts active or pending for this -* VCPU, then there is no work to do and we can bail out without -* taking any lock. There is a potential race with someone injecting -* interrupts to the VCPU, but it is a benign race as the VCPU will -* either observe the new interrupt before or after doing this check, -* and introducing additional synchronization mechanism doesn't change -* this. -* -* Note that we still need to go through the whole thing if anything -* can be directly injected (GICv4). -*/ -if (list_empty (&vcpu->arch.vgic_cpu.ap_list_head) && - !vgic_supports_direct_irqs (vcpu->kvm)) -return ; -DEBUG_SPINLOCK_BUG_ON (!irqs_disabled ()); -if (!list_empty (&vcpu->arch.vgic_cpu.ap_list_head)) { -raw_spin_lock (&vcpu->arch.vgic_cpu.ap_list_lock); + scoped_guard(raw_spinlock, &vcpu->arch.vgic_cpu.ap_list_lock)vgic_flush_lr_state (vcpu); -raw_spin_unlock (&vcpu->arch.vgic_cpu.ap_list_lock); - }if (can_access_vgic_from_kernel ())vgic_restore_state (vcpu);
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 20/49 KVM: arm64: Eagerly save VMCR on exit
In reply to Marc ZyngierWe currently save/restore the VMCR register in a pretty lazy way (on load/put, consistently with what we do with the APRs).
However, we are going to need the group-enable bits that are backed by VMCR on each entry (so that we can avoid injecting interrupts for disabled groups).
Move the synchronisation from put to sync, which results in some minor churn in the nVHE hypercalls to simplify things.
--- arch/arm64/include/asm/kvm_asm.h | 2 +- arch/arm64/include/asm/kvm_hyp.h | 2 +- arch/arm64/kvm/arm.c | 3 +-- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 7 ++++--- arch/arm64/kvm/hyp/vgic-v3-sr.c | 15 +++------------ arch/arm64/kvm/vgic/vgic-v2.c | 2 +- arch/arm64/kvm/vgic/vgic-v3-nested.c | 2 +- arch/arm64/kvm/vgic/vgic-v3.c | 2 +- 8 files changed, 13 insertions(+), 22 deletions(-)
arch/arm64/include/asm/kvm_asm.h +1 -1
@@ -79,7 +79,7 @@ enum __kvm_host_smccc_func { __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid_range, __KVM_HOST_SMCCC_FUNC___kvm_flush_cpu_context, __KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff, - __KVM_HOST_SMCCC_FUNC___vgic_v3_save_vmcr_aprs, + __KVM_HOST_SMCCC_FUNC___vgic_v3_save_aprs, __KVM_HOST_SMCCC_FUNC___vgic_v3_restore_vmcr_aprs, __KVM_HOST_SMCCC_FUNC___pkvm_reserve_vm, __KVM_HOST_SMCCC_FUNC___pkvm_unreserve_vm,
arch/arm64/include/asm/kvm_hyp.h +1 -1
@@ -82,7 +82,7 @@ void __vgic_v3_save_state(struct vgic_v3_cpu_if *cpu_if);void __vgic_v3_restore_state (struct vgic_v3_cpu_if *cpu_if);void __vgic_v3_activate_traps (struct vgic_v3_cpu_if *cpu_if);void __vgic_v3_deactivate_traps (struct vgic_v3_cpu_if *cpu_if); -void __vgic_v3_save_vmcr_aprs (struct vgic_v3_cpu_if *cpu_if); +void __vgic_v3_save_aprs (struct vgic_v3_cpu_if *cpu_if);void __vgic_v3_restore_vmcr_aprs (struct vgic_v3_cpu_if *cpu_if);int __vgic_v3_perform_cpuif_access (struct kvm_vcpu *vcpu);
arch/arm64/kvm/arm.c +1 -2
@@ -659,8 +659,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)void kvm_arch_vcpu_put (struct kvm_vcpu *vcpu) {if (is_protected_kvm_enabled ()) { -kvm_call_hyp (__vgic_v3_save_vmcr_aprs, - &vcpu->arch.vgic_cpu.vgic_v3); +kvm_call_hyp (__vgic_v3_save_aprs, &vcpu->arch.vgic_cpu.vgic_v3);kvm_call_hyp_nvhe (__pkvm_vcpu_put); }
arch/arm64/kvm/hyp/nvhe/hyp-main.c +4 -3
@@ -157,6 +157,7 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu) host_vcpu->arch.iflags = hyp_vcpu->vcpu.arch.iflags; host_cpu_if->vgic_hcr = hyp_cpu_if->vgic_hcr; + host_cpu_if->vgic_vmcr = hyp_cpu_if->vgic_vmcr;for (i =0 ; i < hyp_cpu_if->used_lrs; ++i) host_cpu_if->vgic_lr[i] = hyp_cpu_if->vgic_lr[i]; }
@@ -464,11 +465,11 @@ static void handle___vgic_v3_init_lrs(struct kvm_cpu_context *host_ctxt)__vgic_v3_init_lrs (); } -static void handle___vgic_v3_save_vmcr_aprs (struct kvm_cpu_context *host_ctxt) +static void handle___vgic_v3_save_aprs (struct kvm_cpu_context *host_ctxt) { DECLARE_REG(struct vgic_v3_cpu_if *, cpu_if, host_ctxt, 1); -__vgic_v3_save_vmcr_aprs (kern_hyp_va (cpu_if)); +__vgic_v3_save_aprs (kern_hyp_va (cpu_if)); }static void handle___vgic_v3_restore_vmcr_aprs (struct kvm_cpu_context *host_ctxt)
@@ -616,7 +617,7 @@ static const hcall_t host_hcall[] = {HANDLE_FUNC (__kvm_tlb_flush_vmid_range),HANDLE_FUNC (__kvm_flush_cpu_context),HANDLE_FUNC (__kvm_timer_set_cntvoff), -HANDLE_FUNC (__vgic_v3_save_vmcr_aprs), +HANDLE_FUNC (__vgic_v3_save_aprs),HANDLE_FUNC (__vgic_v3_restore_vmcr_aprs),HANDLE_FUNC (__pkvm_reserve_vm), HANDLE_FUNC(__pkvm_unreserve_vm),
arch/arm64/kvm/hyp/vgic-v3-sr.c +3 -12
@@ -235,6 +235,8 @@ void __vgic_v3_save_state(struct vgic_v3_cpu_if *cpu_if) } } + cpu_if->vgic_vmcr =read_gicreg (ICH_VMCR_EL2 ); +if (cpu_if->vgic_hcr &ICH_HCR_EL2_LRENPIE ) {u64 val =read_gicreg (ICH_HCR_EL2 ); cpu_if->vgic_hcr &= ~ICH_HCR_EL2_EOIcount;
@@ -332,10 +334,6 @@ void __vgic_v3_deactivate_traps(struct vgic_v3_cpu_if *cpu_if) {u64 val; -if (!cpu_if->vgic_sre) { - cpu_if->vgic_vmcr =read_gicreg (ICH_VMCR_EL2 ); - } -/* Only restore SRE if the host implements the GICv2 interface */ if (static_branch_unlikely (&vgic_v3_has_v2_compat)) { val =read_gicreg (ICC_SRE_EL2 );
@@ -357,7 +355,7 @@ void __vgic_v3_deactivate_traps(struct vgic_v3_cpu_if *cpu_if)write_gicreg (0 ,ICH_HCR_EL2 ); } -static void __vgic_v3_save_aprs (struct vgic_v3_cpu_if *cpu_if) +void __vgic_v3_save_aprs (struct vgic_v3_cpu_if *cpu_if) {u64 val;u32 nr_pre_bits;
@@ -518,13 +516,6 @@ static void __vgic_v3_write_vmcr(u32 vmcr)write_gicreg (vmcr,ICH_VMCR_EL2 ); } -void __vgic_v3_save_vmcr_aprs (struct vgic_v3_cpu_if *cpu_if) -{ -__vgic_v3_save_aprs (cpu_if); -if (cpu_if->vgic_sre) - cpu_if->vgic_vmcr =__vgic_v3_read_vmcr (); -} -void __vgic_v3_restore_vmcr_aprs (struct vgic_v3_cpu_if *cpu_if) {__vgic_v3_compat_mode_enable ();
arch/arm64/kvm/vgic/vgic-v2.c +1 -1
@@ -451,6 +451,7 @@ void vgic_v2_save_state(struct kvm_vcpu *vcpu)if (!base)return ; + cpu_if->vgic_vmcr =readl_relaxed (kvm_vgic_global_state.vctrl_base +GICH_VMCR );if (used_lrs)save_lrs (vcpu, base);
@@ -495,6 +496,5 @@ void vgic_v2_put(struct kvm_vcpu *vcpu) {struct vgic_v2_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v2; - cpu_if->vgic_vmcr =readl_relaxed (kvm_vgic_global_state.vctrl_base +GICH_VMCR ); cpu_if->vgic_apr =readl_relaxed (kvm_vgic_global_state.vctrl_base +GICH_APR ); }
arch/arm64/kvm/vgic/vgic-v3-nested.c +1 -1
@@ -340,7 +340,7 @@ void vgic_v3_put_nested(struct kvm_vcpu *vcpu)u64 val;int i; -__vgic_v3_save_vmcr_aprs (s_cpu_if); +__vgic_v3_save_aprs (s_cpu_if);__vgic_v3_deactivate_traps (s_cpu_if);__vgic_v3_save_state (s_cpu_if);
arch/arm64/kvm/vgic/vgic-v3.c +1 -1
@@ -815,7 +815,7 @@ void vgic_v3_put(struct kvm_vcpu *vcpu) }if (likely (!is_protected_kvm_enabled ())) -kvm_call_hyp (__vgic_v3_save_vmcr_aprs, cpu_if); +kvm_call_hyp (__vgic_v3_save_aprs, cpu_if);WARN_ON (vgic_v4_put (vcpu));if (has_vhe ())
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 21/49 KVM: arm64: Revamp vgic maintenance interrupt configuration
In reply to Marc ZyngierWe currently don't use the maintenance interrupt very much, apart from EOI on level interrupts, and for LR underflow in limited cases.
However, as we are moving toward a setup where active interrupts can live outside of the LRs, we need to use the MIs in a more diverse set of cases.
Add a new helper that produces a digest of the ap_list, and use that summary to set the various control bits as required.
This slightly changes the way v2 SGIs are handled, as they used to count for more than one interrupt, but not anymore.
--- arch/arm64/kvm/vgic/vgic-v2.c | 12 ++++- arch/arm64/kvm/vgic/vgic-v3.c | 18 ++++++- arch/arm64/kvm/vgic/vgic.c | 89 ++++++++++++----------------------- arch/arm64/kvm/vgic/vgic.h | 19 +++++++- 4 files changed, 73 insertions(+), 65 deletions(-)
arch/arm64/kvm/vgic/vgic-v2.c +10 -2
@@ -26,11 +26,19 @@ void vgic_v2_init_lrs(void)vgic_v2_write_lr (i,0 ); } -void vgic_v2_set_underflow (struct kvm_vcpu *vcpu) +void vgic_v2_configure_hcr (struct kvm_vcpu *vcpu, +struct ap_list_summary *als) {struct vgic_v2_cpu_if *cpuif = &vcpu->arch.vgic_cpu.vgic_v2; - cpuif->vgic_hcr |=GICH_HCR_UIE ; + cpuif->vgic_hcr =GICH_HCR_EN ; + +if (irqs_pending_outside_lrs (als)) + cpuif->vgic_hcr |=GICH_HCR_NPIE ; +if (irqs_active_outside_lrs (als)) + cpuif->vgic_hcr |=GICH_HCR_LRENPIE ; +if (irqs_outside_lrs (als)) + cpuif->vgic_hcr |=GICH_HCR_UIE ; }static bool lr_signals_eoi_mi (u32 lr_val)
arch/arm64/kvm/vgic/vgic-v3.c +16 -2
@@ -20,11 +20,25 @@ static bool common_trap;static bool dir_trap;static bool gicv4_enable; -void vgic_v3_set_underflow (struct kvm_vcpu *vcpu) +void vgic_v3_configure_hcr (struct kvm_vcpu *vcpu, +struct ap_list_summary *als) {struct vgic_v3_cpu_if *cpuif = &vcpu->arch.vgic_cpu.vgic_v3; - cpuif->vgic_hcr |=ICH_HCR_EL2_UIE ; +if (!irqchip_in_kernel (vcpu->kvm)) +return ; + + cpuif->vgic_hcr = ICH_HCR_EL2_En; + +if (irqs_pending_outside_lrs (als)) + cpuif->vgic_hcr |=ICH_HCR_EL2_NPIE ; +if (irqs_active_outside_lrs (als)) + cpuif->vgic_hcr |=ICH_HCR_EL2_LRENPIE ; +if (irqs_outside_lrs (als)) + cpuif->vgic_hcr |=ICH_HCR_EL2_UIE ; + +if (!als->nr_sgi) + cpuif->vgic_hcr |= ICH_HCR_EL2_vSGIEOICount; }static bool lr_signals_eoi_mi (u64 lr_val)
arch/arm64/kvm/vgic/vgic.c +30 -59
128 lines not on this page · raw message
arch/arm64/kvm/vgic/vgic.h +17 -2
@@ -236,6 +236,21 @@ struct its_ite {u32 event_id; }; +struct ap_list_summary { +unsigned int nr_pend;/* purely pending, not active */ +unsigned int nr_act;/* active, or active+pending */ +unsigned int nr_sgi;/* any SGI */ +}; + +#define irqs_outside_lrs (s) \ + (((s)->nr_pend + (s)->nr_act) > kvm_vgic_global_state.nr_lr) + +#define irqs_pending_outside_lrs (s) \ + ((s)->nr_pend > kvm_vgic_global_state.nr_lr) + +#define irqs_active_outside_lrs (s) \ + ((s)->nr_act && irqs_outside_lrs(s)) +int vgic_v3_parse_attr (struct kvm_device *dev,struct kvm_device_attr *attr,struct vgic_reg_attr *reg_attr); int vgic_v2_parse_attr(struct kvm_device *dev, struct kvm_device_attr *attr,
@@ -262,7 +277,7 @@ int vgic_check_iorange(struct kvm *kvm, phys_addr_t ioaddr,void vgic_v2_fold_lr_state (struct kvm_vcpu *vcpu);void vgic_v2_populate_lr (struct kvm_vcpu *vcpu,struct vgic_irq *irq,int lr);void vgic_v2_clear_lr (struct kvm_vcpu *vcpu,int lr); -void vgic_v2_set_underflow (struct kvm_vcpu *vcpu); +void vgic_v2_configure_hcr (struct kvm_vcpu *vcpu,struct ap_list_summary *als);int vgic_v2_has_attr_regs (struct kvm_device *dev,struct kvm_device_attr *attr);int vgic_v2_dist_uaccess (struct kvm_vcpu *vcpu,bool is_write,int offset,u32 *val);
@@ -302,7 +317,7 @@ static inline void vgic_get_irq_ref(struct vgic_irq *irq)void vgic_v3_fold_lr_state (struct kvm_vcpu *vcpu);void vgic_v3_populate_lr (struct kvm_vcpu *vcpu,struct vgic_irq *irq,int lr);void vgic_v3_clear_lr (struct kvm_vcpu *vcpu,int lr); -void vgic_v3_set_underflow (struct kvm_vcpu *vcpu); +void vgic_v3_configure_hcr (struct kvm_vcpu *vcpu,struct ap_list_summary *als);void vgic_v3_set_vmcr (struct kvm_vcpu *vcpu,struct vgic_vmcr *vmcr);void vgic_v3_get_vmcr (struct kvm_vcpu *vcpu,struct vgic_vmcr *vmcr);void vgic_v3_enable (struct kvm_vcpu *vcpu);
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 22/49 KVM: arm64: Turn kvm_vgic_vcpu_enable() into kvm_vgic_vcpu_reset()
In reply to Marc ZyngierNow that we always reconfigure the vgic HCR register on entry, the "enable" part of kvm_vgic_vcpu_enable() is pretty useless.
Removing the enable bits from these functions makes it plain that they are just about computing the reset state. Just rename the functions accordingly.
--- arch/arm64/kvm/vgic/vgic-init.c | 8 ++++---- arch/arm64/kvm/vgic/vgic-v2.c | 5 +---- arch/arm64/kvm/vgic/vgic-v3.c | 5 +---- arch/arm64/kvm/vgic/vgic.h | 4 ++-- 4 files changed, 8 insertions(+), 14 deletions(-)
arch/arm64/kvm/vgic/vgic-init.c +4 -4
@@ -353,12 +353,12 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)return ret; } -static void kvm_vgic_vcpu_enable (struct kvm_vcpu *vcpu) +static void kvm_vgic_vcpu_reset (struct kvm_vcpu *vcpu) {if (kvm_vgic_global_state.type ==VGIC_V2 ) -vgic_v2_enable (vcpu); +vgic_v2_reset (vcpu);else -vgic_v3_enable (vcpu); +vgic_v3_reset (vcpu); }/*
@@ -405,7 +405,7 @@ int vgic_init(struct kvm *kvm) }kvm_for_each_vcpu (idx, vcpu, kvm) -kvm_vgic_vcpu_enable (vcpu); +kvm_vgic_vcpu_reset (vcpu); ret =kvm_vgic_setup_default_irq_routing (kvm);if (ret)
arch/arm64/kvm/vgic/vgic-v2.c +1 -4
@@ -285,7 +285,7 @@ void vgic_v2_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) GICH_VMCR_PRIMASK_SHIFT) << GICV_PMR_PRIORITY_SHIFT; } -void vgic_v2_enable (struct kvm_vcpu *vcpu) +void vgic_v2_reset (struct kvm_vcpu *vcpu) {/* * By forcing VMCR to zero, the GIC will restore the binary
@@ -293,9 +293,6 @@ void vgic_v2_enable(struct kvm_vcpu *vcpu)* anyway. */ vcpu->arch.vgic_cpu.vgic_v2.vgic_vmcr =0 ; - -/* Get the show on the road... */ - vcpu->arch.vgic_cpu.vgic_v2.vgic_hcr =GICH_HCR_EN ; }/* check for overlapping regions and for regions crossing the end of memory */
arch/arm64/kvm/vgic/vgic-v3.c +1 -4
@@ -293,7 +293,7 @@ void vgic_v3_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) GIC_BASER_CACHEABILITY(GICR_PENDBASER, OUTER, SameAsInner) | \ GIC_BASER_SHAREABILITY(GICR_PENDBASER, InnerShareable)) -void vgic_v3_enable (struct kvm_vcpu *vcpu) +void vgic_v3_reset (struct kvm_vcpu *vcpu) {struct vgic_v3_cpu_if *vgic_v3 = &vcpu->arch.vgic_cpu.vgic_v3;
@@ -323,9 +323,6 @@ void vgic_v3_enable(struct kvm_vcpu *vcpu) kvm_vgic_global_state.ich_vtr_el2); vcpu->arch.vgic_cpu.num_pri_bits =FIELD_GET (ICH_VTR_EL2_PRIbits, kvm_vgic_global_state.ich_vtr_el2) +1 ; - -/* Get the show on the road... */ - vgic_v3->vgic_hcr = ICH_HCR_EL2_En; }void vcpu_set_ich_hcr (struct kvm_vcpu *vcpu)
arch/arm64/kvm/vgic/vgic.h +2 -2
@@ -285,7 +285,7 @@ int vgic_v2_cpuif_uaccess(struct kvm_vcpu *vcpu, bool is_write, int offset, u32 *val);void vgic_v2_set_vmcr (struct kvm_vcpu *vcpu,struct vgic_vmcr *vmcr);void vgic_v2_get_vmcr (struct kvm_vcpu *vcpu,struct vgic_vmcr *vmcr); -void vgic_v2_enable (struct kvm_vcpu *vcpu); +void vgic_v2_reset (struct kvm_vcpu *vcpu);int vgic_v2_probe (const struct gic_kvm_info *info);int vgic_v2_map_resources (struct kvm *kvm); int vgic_register_dist_iodev(struct kvm *kvm, gpa_t dist_base_address,
@@ -320,7 +320,7 @@ void vgic_v3_clear_lr(struct kvm_vcpu *vcpu, int lr);void vgic_v3_configure_hcr (struct kvm_vcpu *vcpu,struct ap_list_summary *als);void vgic_v3_set_vmcr (struct kvm_vcpu *vcpu,struct vgic_vmcr *vmcr);void vgic_v3_get_vmcr (struct kvm_vcpu *vcpu,struct vgic_vmcr *vmcr); -void vgic_v3_enable (struct kvm_vcpu *vcpu); +void vgic_v3_reset (struct kvm_vcpu *vcpu);int vgic_v3_probe (const struct gic_kvm_info *info);int vgic_v3_map_resources (struct kvm *kvm);int vgic_v3_lpi_sync_pending_status (struct kvm *kvm,struct vgic_irq *irq);
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 23/49 KVM: arm64: Make vgic_target_oracle() globally available
In reply to Marc ZyngierMake the internal crystal ball global, so that implementation-specific code can use it.
--- arch/arm64/kvm/vgic/vgic.c | 2 +- arch/arm64/kvm/vgic/vgic.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
arch/arm64/kvm/vgic/vgic.c +1 -1
@@ -237,7 +237,7 @@ void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active)* * Requires the IRQ lock to be held. */ -static struct kvm_vcpu *vgic_target_oracle (struct vgic_irq *irq) +struct kvm_vcpu *vgic_target_oracle (struct vgic_irq *irq) {lockdep_assert_held (&irq->irq_lock);
arch/arm64/kvm/vgic/vgic.h +1 -0
@@ -261,6 +261,7 @@ vgic_get_mmio_region(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev,struct vgic_irq *vgic_get_irq (struct kvm *kvm,u32 intid);struct vgic_irq *vgic_get_vcpu_irq (struct kvm_vcpu *vcpu,u32 intid);void vgic_put_irq (struct kvm *kvm,struct vgic_irq *irq); +struct kvm_vcpu *vgic_target_oracle (struct vgic_irq *irq);bool vgic_get_phys_line_level (struct vgic_irq *irq);void vgic_irq_set_phys_pending (struct vgic_irq *irq,bool pending);void vgic_irq_set_phys_active (struct vgic_irq *irq,bool active);
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 24/49 KVM: arm64: Invert ap_list sorting to push active interrupts out
In reply to Marc ZyngierHaving established that pending interrupts should have priority to be moved into the LRs over the active interrupts, implement this in the ap_list sorting.
--- arch/arm64/kvm/vgic/vgic.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-)
arch/arm64/kvm/vgic/vgic.c +12 -15
@@ -270,10 +270,7 @@ struct kvm_vcpu *vgic_target_oracle(struct vgic_irq *irq)* well, the first items in the list being the first things populated in the * LRs. * -* A hard rule is that active interrupts can never be pushed out of the LRs -* (and therefore take priority) since we cannot reliably trap on deactivation -* of IRQs and therefore they have to be present in the LRs. -* +* Pending, non-active interrupts must be placed at the head of the list. * Otherwise things should be sorted by the priority field and the GIC * hardware support will take care of preemption of priority groups etc. *
@@ -298,21 +295,21 @@ static int vgic_irq_cmp(void *priv, const struct list_head *a,raw_spin_lock (&irqa->irq_lock);raw_spin_lock_nested (&irqb->irq_lock,SINGLE_DEPTH_NESTING ); -if (irqa->active || irqb->active) { - ret = (int )irqb->active - (int )irqa->active; - goto out; - } + penda = irqa->enabled &&irq_is_pending (irqa) && !irqa->active; + pendb = irqb->enabled &&irq_is_pending (irqb) && !irqb->active; - penda = irqa->enabled &&irq_is_pending (irqa); - pendb = irqb->enabled &&irq_is_pending (irqb); + ret = (int )pendb - (int )penda; +if (ret) + goto out; -if (!penda || !pendb) { - ret = (int )pendb - (int )penda; +/* Both pending and enabled, sort by priority (lower number first) */ + ret = (int )irqa->priority - (int )irqb->priority; +if (ret) goto out; - } -/* Both pending and enabled, sort by priority */ - ret = irqa->priority - irqb->priority; +/* Finally, HW bit active interrupts have priority over non-HW ones */ + ret = (int )irqb->hw - (int )irqa->hw; + out:raw_spin_unlock (&irqb->irq_lock);raw_spin_unlock (&irqa->irq_lock);
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 25/49 KVM: arm64: Move undeliverable interrupts to the end of ap_list
In reply to Marc ZyngierInterrupts in the ap_list that cannot be acted upon because they are not enabled, or that their group is not enabled, shouldn't make it into the LRs if we are space-constrained.
--- arch/arm64/kvm/vgic/vgic.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)
arch/arm64/kvm/vgic/vgic.c +22 -1
@@ -265,6 +265,11 @@ struct kvm_vcpu *vgic_target_oracle(struct vgic_irq *irq)return NULL ; } +struct vgic_sort_info { +struct kvm_vcpu *vcpu; +struct vgic_vmcr vmcr; +}; +/* * The order of items in the ap_lists defines how we'll pack things in LRs as * well, the first items in the list being the first things populated in the
@@ -273,6 +278,7 @@ struct kvm_vcpu *vgic_target_oracle(struct vgic_irq *irq)* Pending, non-active interrupts must be placed at the head of the list. * Otherwise things should be sorted by the priority field and the GIC * hardware support will take care of preemption of priority groups etc. +* Interrupts that are not deliverable should be at the end of the list. * * Return negative if "a" sorts before "b", 0 to preserve order, and positive * to sort "b" before "a".
@@ -282,6 +288,8 @@ static int vgic_irq_cmp(void *priv, const struct list_head *a, { struct vgic_irq *irqa = container_of(a, struct vgic_irq, ap_list); struct vgic_irq *irqb = container_of(b, struct vgic_irq, ap_list); +struct vgic_sort_info *info = priv; +struct kvm_vcpu *vcpu = info->vcpu;bool penda, pendb;int ret;
@@ -295,6 +303,17 @@ static int vgic_irq_cmp(void *priv, const struct list_head *a,raw_spin_lock (&irqa->irq_lock);raw_spin_lock_nested (&irqb->irq_lock,SINGLE_DEPTH_NESTING ); +/* Undeliverable interrupts should be last */ + ret = (int )(vgic_target_oracle (irqb) == vcpu) - (int )(vgic_target_oracle (irqa) == vcpu); +if (ret) + goto out; + +/* Same thing for interrupts targeting a disabled group */ + ret = (int )(irqb->group ? info->vmcr.grpen1 : info->vmcr.grpen0); + ret -= (int )(irqa->group ? info->vmcr.grpen1 : info->vmcr.grpen0); +if (ret) + goto out; + penda = irqa->enabled &&irq_is_pending (irqa) && !irqa->active; pendb = irqb->enabled &&irq_is_pending (irqb) && !irqb->active;
@@ -320,10 +339,12 @@ static int vgic_irq_cmp(void *priv, const struct list_head *a,static void vgic_sort_ap_list (struct kvm_vcpu *vcpu) {struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; +struct vgic_sort_info info = { .vcpu = vcpu, };lockdep_assert_held (&vgic_cpu->ap_list_lock); -list_sort (NULL , &vgic_cpu->ap_list_head, vgic_irq_cmp); +vgic_get_vmcr (vcpu, &info.vmcr); +list_sort (&info, &vgic_cpu->ap_list_head, vgic_irq_cmp); }/*
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 26/49 KVM: arm64: Use MI to detect groups being enabled/disabled
In reply to Marc ZyngierAdd the maintenance interrupt to force an exit when the guest enables/disables individual groups, so that we can resort the ap_list accordingly.
--- arch/arm64/kvm/vgic/vgic-v2.c | 5 +++++ arch/arm64/kvm/vgic/vgic-v3.c | 5 +++++ 2 files changed, 10 insertions(+)
arch/arm64/kvm/vgic/vgic-v2.c +5 -0
@@ -39,6 +39,11 @@ void vgic_v2_configure_hcr(struct kvm_vcpu *vcpu, cpuif->vgic_hcr |=GICH_HCR_LRENPIE ;if (irqs_outside_lrs (als)) cpuif->vgic_hcr |=GICH_HCR_UIE ; + + cpuif->vgic_hcr |= (cpuif->vgic_vmcr &GICH_VMCR_ENABLE_GRP0_MASK ) ? + GICH_HCR_VGrp0DIE : GICH_HCR_VGrp0EIE; + cpuif->vgic_hcr |= (cpuif->vgic_vmcr &GICH_VMCR_ENABLE_GRP1_MASK ) ? + GICH_HCR_VGrp1DIE : GICH_HCR_VGrp1EIE; }static bool lr_signals_eoi_mi (u32 lr_val)
arch/arm64/kvm/vgic/vgic-v3.c +5 -0
@@ -39,6 +39,11 @@ void vgic_v3_configure_hcr(struct kvm_vcpu *vcpu,if (!als->nr_sgi) cpuif->vgic_hcr |= ICH_HCR_EL2_vSGIEOICount; + + cpuif->vgic_hcr |= (cpuif->vgic_vmcr &ICH_VMCR_ENG0_MASK ) ? + ICH_HCR_EL2_VGrp0DIE : ICH_HCR_EL2_VGrp0EIE; + cpuif->vgic_hcr |= (cpuif->vgic_vmcr &ICH_VMCR_ENG1_MASK ) ? + ICH_HCR_EL2_VGrp1DIE : ICH_HCR_EL2_VGrp1EIE; }static bool lr_signals_eoi_mi (u64 lr_val)
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 27/49 KVM: arm64: GICv3: Handle LR overflow when EOImode==0
In reply to Marc ZyngierNow that we can identify interrupts that have not made it into the LRs, it becomes relatively easy to use EOIcount to walk the overflow list.
What is a bit odd is that we compute a fake LR for the original state of the interrupt, clear the active bit, and feed into the existing logic for processing. In a way, this is what would have happened if the interrupt was in an LR.
--- arch/arm64/kvm/vgic/vgic-v3.c | 46 +++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
arch/arm64/kvm/vgic/vgic-v3.c +46 -0
@@ -112,16 +112,62 @@ static void vgic_v3_fold_lr(struct kvm_vcpu *vcpu, u64 val)vgic_put_irq (vcpu->kvm, irq); } +static u64 vgic_v3_compute_lr (struct kvm_vcpu *vcpu,struct vgic_irq *irq); + +static void vgic_v3_deactivate_phys (u32 intid) +{ +if (cpus_have_final_cap (ARM64_HAS_GICV5_LEGACY )) +gic_insn (intid |FIELD_PREP (GICV5_GIC_CDDI_TYPE_MASK ,1 ),CDDI ); +else +gic_write_dir (intid); +} +void vgic_v3_fold_lr_state (struct kvm_vcpu *vcpu) {struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;struct vgic_v3_cpu_if *cpuif = &vgic_cpu->vgic_v3; +u32 eoicount =FIELD_GET (ICH_HCR_EL2_EOIcount, cpuif->vgic_hcr); +struct vgic_irq *irq;DEBUG_SPINLOCK_BUG_ON (!irqs_disabled ());for (int lr =0 ; lr < cpuif->used_lrs; lr++)vgic_v3_fold_lr (vcpu, cpuif->vgic_lr[lr]); +/* +* EOIMode=0: use EOIcount to emulate deactivation. We are +* guaranteed to deactivate in reverse order of the activation, so +* just pick one active interrupt after the other in the ap_list, +* and replay the deactivation as if the CPU was doing it. We also +* rely on priority drop to have taken place, and the list to be +* sorted by priority. +*/ +list_for_each_entry (irq, &vgic_cpu->ap_list_head, ap_list) { +u64 lr; + +/* +* I would have loved to write this using a scoped_guard(), +* but using 'continue' here is a total train wreck. +*/ +if (!eoicount) { +break ; + }else { +guard (raw_spinlock)(&irq->irq_lock); + +if (!(likely (vgic_target_oracle (irq) == vcpu) && + irq->active)) +continue ; + + lr =vgic_v3_compute_lr (vcpu, irq) & ~ICH_LR_ACTIVE_BIT ; + } + +if (lr &ICH_LR_HW ) +vgic_v3_deactivate_phys (FIELD_GET (ICH_LR_PHYS_ID_MASK , lr)); + +vgic_v3_fold_lr (vcpu, lr); + eoicount--; + } + cpuif->used_lrs =0 ; }
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 28/49 KVM: arm64: GICv3: Handle deactivation via ICV_DIR_EL1 traps
In reply to Marc ZyngierDeactivation via ICV_DIR_EL1 is both relatively straightforward (we have the interrupt that needs deactivation) and really awkward.
The main issue is that the interrupt may either be in an LR on another CPU, or ourside of any LR.
In the former case, we process the deactivation is if ot was a write to GICD_CACTIVERn, which is already implemented as a big hammer IPI'ing all vcpus. In the latter case, we just perform a normal deactivation, similar to what we do for EOImode==0.
Another annoying aspect is that we need to tell the CPU owning the interrupt that its ap_list needs laudering. We use a brand new vcpu request to that effect.
Note that this doesn't address deactivation via the GICV MMIO view, which will be taken care of in a later change.
--- arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/kvm/arm.c | 4 ++ arch/arm64/kvm/hyp/vgic-v3-sr.c | 3 ++ arch/arm64/kvm/sys_regs.c | 19 ++++++- arch/arm64/kvm/vgic/vgic-v3.c | 85 +++++++++++++++++++++++++++++++ arch/arm64/kvm/vgic/vgic.c | 11 ++++ arch/arm64/kvm/vgic/vgic.h | 1 + include/kvm/arm_vgic.h | 1 + 8 files changed, 123 insertions(+), 2 deletions(-)
arch/arm64/include/asm/kvm_host.h +1 -0
@@ -54,6 +54,7 @@#define KVM_REQ_NESTED_S2_UNMAP KVM_ARCH_REQ(8)#define KVM_REQ_GUEST_HYP_IRQ_PENDING KVM_ARCH_REQ(9)#define KVM_REQ_MAP_L1_VNCR_EL2 KVM_ARCH_REQ(10) +#define KVM_REQ_VGIC_PROCESS_UPDATE KVM_ARCH_REQ(11)#define KVM_DIRTY_LOG_MANUAL_CAPS (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE | \ KVM_DIRTY_LOG_INITIALLY_SET)
arch/arm64/kvm/arm.c +4 -0
@@ -1041,6 +1041,10 @@ static int check_vcpu_requests(struct kvm_vcpu *vcpu)*/ kvm_check_request (KVM_REQ_IRQ_PENDING , vcpu); +/* Process interrupts deactivated through a trap */ +if (kvm_check_request (KVM_REQ_VGIC_PROCESS_UPDATE , vcpu)) +kvm_vgic_process_async_update (vcpu); +if (kvm_check_request (KVM_REQ_RECORD_STEAL , vcpu))kvm_update_stolen_time (vcpu);
arch/arm64/kvm/hyp/vgic-v3-sr.c +3 -0
@@ -1247,6 +1247,9 @@ int __vgic_v3_perform_cpuif_access(struct kvm_vcpu *vcpu)case SYS_ICC_DIR_EL1 :if (unlikely (is_read))return 0 ; +/* Full exit if required to handle overflow deactivation... */ +if (vcpu->arch.vgic_cpu.vgic_v3.vgic_hcr &ICH_HCR_EL2_TDIR ) +return 0 ; fn = __vgic_v3_write_dir;break ;case SYS_ICC_RPR_EL1 :
arch/arm64/kvm/sys_regs.c +17 -2
@@ -666,6 +666,21 @@ static bool access_gic_sre(struct kvm_vcpu *vcpu,return true; } +static bool access_gic_dir (struct kvm_vcpu *vcpu, +struct sys_reg_params *p, +const struct sys_reg_desc *r) +{ +if (!kvm_has_gicv3 (vcpu->kvm)) +return undef_access (vcpu, p, r); + +if (!p->is_write) +return undef_access (vcpu, p, r); + +vgic_v3_deactivate (vcpu, p->regval); + +return true; +} +static bool trap_raz_wi (struct kvm_vcpu *vcpu,struct sys_reg_params *p,const struct sys_reg_desc *r)
@@ -3370,7 +3385,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { { SYS_DESC(SYS_ICC_AP1R1_EL1), undef_access }, { SYS_DESC(SYS_ICC_AP1R2_EL1), undef_access }, { SYS_DESC(SYS_ICC_AP1R3_EL1), undef_access }, - { SYS_DESC(SYS_ICC_DIR_EL1), undef_access }, + { SYS_DESC(SYS_ICC_DIR_EL1), access_gic_dir }, { SYS_DESC(SYS_ICC_RPR_EL1), undef_access }, { SYS_DESC(SYS_ICC_SGI1R_EL1), access_gic_sgi }, { SYS_DESC(SYS_ICC_ASGI1R_EL1), access_gic_sgi },
@@ -4495,7 +4510,7 @@ static const struct sys_reg_desc cp15_regs[] = { { CP15_SYS_DESC(SYS_ICC_AP1R1_EL1), undef_access }, { CP15_SYS_DESC(SYS_ICC_AP1R2_EL1), undef_access }, { CP15_SYS_DESC(SYS_ICC_AP1R3_EL1), undef_access }, - { CP15_SYS_DESC(SYS_ICC_DIR_EL1), undef_access }, + { CP15_SYS_DESC(SYS_ICC_DIR_EL1), access_gic_dir }, { CP15_SYS_DESC(SYS_ICC_RPR_EL1), undef_access }, { CP15_SYS_DESC(SYS_ICC_IAR1_EL1), undef_access }, { CP15_SYS_DESC(SYS_ICC_EOIR1_EL1), undef_access },
arch/arm64/kvm/vgic/vgic-v3.c +85 -0
97 lines not on this page · raw message
arch/arm64/kvm/vgic/vgic.c +11 -0
@@ -990,6 +990,17 @@ void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)vgic_prune_ap_list (vcpu); } +/* Sync interrupts that were deactivated through a DIR trap */ +void kvm_vgic_process_async_update (struct kvm_vcpu *vcpu) +{ +unsigned long flags; + +/* Make sure we're in the same context as LR handling */ +local_irq_save (flags); +vgic_prune_ap_list (vcpu); +local_irq_restore (flags); +} +static inline void vgic_restore_state (struct kvm_vcpu *vcpu) {if (!static_branch_unlikely (&kvm_vgic_global_state.gicv3_cpuif))
arch/arm64/kvm/vgic/vgic.h +1 -0
@@ -318,6 +318,7 @@ static inline void vgic_get_irq_ref(struct vgic_irq *irq)void vgic_v3_fold_lr_state (struct kvm_vcpu *vcpu);void vgic_v3_populate_lr (struct kvm_vcpu *vcpu,struct vgic_irq *irq,int lr);void vgic_v3_clear_lr (struct kvm_vcpu *vcpu,int lr); +void vgic_v3_deactivate (struct kvm_vcpu *vcpu,u64 val);void vgic_v3_configure_hcr (struct kvm_vcpu *vcpu,struct ap_list_summary *als);void vgic_v3_set_vmcr (struct kvm_vcpu *vcpu,struct vgic_vmcr *vmcr);void vgic_v3_get_vmcr (struct kvm_vcpu *vcpu,struct vgic_vmcr *vmcr);
include/kvm/arm_vgic.h +1 -0
@@ -421,6 +421,7 @@ bool kvm_vcpu_has_pending_irqs(struct kvm_vcpu *vcpu);void kvm_vgic_sync_hwstate (struct kvm_vcpu *vcpu);void kvm_vgic_flush_hwstate (struct kvm_vcpu *vcpu);void kvm_vgic_reset_mapped_irq (struct kvm_vcpu *vcpu,u32 vintid); +void kvm_vgic_process_async_update (struct kvm_vcpu *vcpu);void vgic_v3_dispatch_sgi (struct kvm_vcpu *vcpu,u64 reg,bool allow_group1);
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 29/49 KVM: arm64: GICv3: Add GICv2 SGI handling to deactivation primitive
In reply to Marc ZyngierThe GICv2 SGIs require additional handling for deactivation, as they are effectively multiple interrrupts muxed into one. Make sure we check for the source CPU when deactivating.
--- arch/arm64/kvm/vgic/vgic-v3.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
arch/arm64/kvm/vgic/vgic-v3.c +19 -1
@@ -176,11 +176,20 @@ void vgic_v3_deactivate(struct kvm_vcpu *vcpu, u64 val) {struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;struct vgic_v3_cpu_if *cpuif = &vgic_cpu->vgic_v3; +u32 model = vcpu->kvm->arch.vgic.vgic_model;struct kvm_vcpu *target_vcpu =NULL ; +bool mmio = false, is_v2_sgi;struct vgic_irq *irq;unsigned long flags; -bool mmio = false;u64 lr =0 ; +u8 cpuid; + +/* Snapshot CPUID, and remove it from the INTID */ + cpuid =FIELD_GET (GENMASK_ULL (12 ,10 ), val); + val &= ~GENMASK_ULL (12 ,10 ); + + is_v2_sgi = (model ==KVM_DEV_TYPE_ARM_VGIC_V2 && + val <VGIC_NR_SGIS );/* * We only deal with DIR when EOIMode==1, and only for SGI,
@@ -216,6 +225,9 @@ void vgic_v3_deactivate(struct kvm_vcpu *vcpu, u64 val)* - Or the irq is active, but not in an LR, and we can * directly deactivate it by building a pseudo-LR, fold it, * and queue a request to prune the resulting ap_list, +* +* Special care must be taken to match the source CPUID when +* deactivating a GICv2 SGI. */ scoped_guard(raw_spinlock, &irq->irq_lock) { target_vcpu = irq->vcpu;
@@ -233,6 +245,12 @@ void vgic_v3_deactivate(struct kvm_vcpu *vcpu, u64 val) goto put; } +/* GICv2 SGI: check that the cpuid matches */ +if (is_v2_sgi && irq->active_source != cpuid) { + target_vcpu =NULL ; + goto put; + } +/* (with a Dalek voice) DEACTIVATE!!!! */ lr =vgic_v3_compute_lr (vcpu, irq) & ~ICH_LR_ACTIVE_BIT ; }
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 30/49 KVM: arm64: GICv3: Set ICH_HCR_EL2.TDIR when interrupts overflow LR capacity
In reply to Marc ZyngierNow that we are ready to handle deactivation through ICV_DIR_EL1, set the trap bit if we have active interrupts outside of the LRs.
--- arch/arm64/kvm/vgic/vgic-v3.c | 7 +++++++ 1 file changed, 7 insertions(+)
arch/arm64/kvm/vgic/vgic-v3.c +7 -0
@@ -45,6 +45,13 @@ void vgic_v3_configure_hcr(struct kvm_vcpu *vcpu, ICH_HCR_EL2_VGrp0DIE : ICH_HCR_EL2_VGrp0EIE; cpuif->vgic_hcr |= (cpuif->vgic_vmcr &ICH_VMCR_ENG1_MASK ) ? ICH_HCR_EL2_VGrp1DIE : ICH_HCR_EL2_VGrp1EIE; + +/* +* Note that we set the trap irrespective of EOIMode, as that +* can change behind our back without any warning... +*/ +if (irqs_active_outside_lrs (als)) + cpuif->vgic_hcr |=ICH_HCR_EL2_TDIR ; }static bool lr_signals_eoi_mi (u64 lr_val)
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 31/49 KVM: arm64: GICv3: Add SPI tracking to handle asymmetric deactivation
In reply to Marc ZyngierSPIs are specially annpying, as they can be activated on a CPU and deactivated on another. WHich means that when an SPI is in flight anywhere, all CPUs need to have their TDIR trap bit set.
This translates into broadcasting an IPI across all CPUs to make sure they set their trap bit, The number of in-flight SPIs is kept in an atomic variable so that CPUs can turn the trap bit off as soon as possible.
--- arch/arm64/kvm/vgic/vgic-init.c | 1 + arch/arm64/kvm/vgic/vgic-v3.c | 21 +++++++++++++++------ arch/arm64/kvm/vgic/vgic.c | 25 +++++++++++++++++++++++-- include/kvm/arm_vgic.h | 3 +++ 4 files changed, 42 insertions(+), 8 deletions(-)
arch/arm64/kvm/vgic/vgic-init.c +1 -0
@@ -188,6 +188,7 @@ static int kvm_vgic_dist_init(struct kvm *kvm, unsigned int nr_spis)struct kvm_vcpu *vcpu0 =kvm_get_vcpu (kvm,0 );int i; + dist->active_spis = (atomic_t )ATOMIC_INIT (0 ); dist->spis =kcalloc (nr_spis,sizeof (struct vgic_irq ),GFP_KERNEL_ACCOUNT );if (!dist->spis)return -ENOMEM ;
arch/arm64/kvm/vgic/vgic-v3.c +15 -6
@@ -47,10 +47,17 @@ void vgic_v3_configure_hcr(struct kvm_vcpu *vcpu, ICH_HCR_EL2_VGrp1DIE : ICH_HCR_EL2_VGrp1EIE;/* +* Dealing with EOImode=1 is a massive source of headache. Not +* only do we need to track that we have active interrupts +* outside of the LRs and force DIR to be trapped, we also +* need to deal with SPIs that can be deactivated on another +* CPU. +* * Note that we set the trap irrespective of EOIMode, as that * can change behind our back without any warning... */ -if (irqs_active_outside_lrs (als)) +if (irqs_active_outside_lrs (als) || +atomic_read (&vcpu->kvm->arch.vgic.active_spis)) cpuif->vgic_hcr |=ICH_HCR_EL2_TDIR ; }
@@ -78,11 +85,6 @@ static void vgic_v3_fold_lr(struct kvm_vcpu *vcpu, u64 val)if (!irq)/* An LPI could have been unmapped. */ return ; -/* Notify fds when the guest EOI'ed a level-triggered IRQ */ -if (lr_signals_eoi_mi (val) &&vgic_valid_spi (vcpu->kvm, intid)) -kvm_notify_acked_irq (vcpu->kvm,0 , - intid -VGIC_NR_PRIVATE_IRQS ); - scoped_guard(raw_spinlock, &irq->irq_lock) {/* Always preserve the active bit for !LPIs, note deactivation */ if (irq->intid >=VGIC_MIN_LPI )
@@ -117,6 +119,13 @@ static void vgic_v3_fold_lr(struct kvm_vcpu *vcpu, u64 val) irq->on_lr = false; } +/* Notify fds when the guest EOI'ed a level-triggered SPI, and drop the refcount */ +if (deactivated &&lr_signals_eoi_mi (val) &&vgic_valid_spi (vcpu->kvm, intid)) { +kvm_notify_acked_irq (vcpu->kvm,0 , + intid -VGIC_NR_PRIVATE_IRQS ); +atomic_dec_if_positive (&vcpu->kvm->arch.vgic.active_spis); + } +vgic_put_irq (vcpu->kvm, irq); }
arch/arm64/kvm/vgic/vgic.c +23 -2
@@ -367,6 +367,17 @@ static bool vgic_validate_injection(struct vgic_irq *irq, bool level, void *ownereturn false; } +static bool vgic_model_needs_bcst_kick (struct kvm *kvm) +{ +/* +* A GICv3 (or GICv3-like) system exposing a GICv3 to the +* guest needs a broadcast kick to set TDIR globally, even if +* the bit doesn't really exist (we still need to check for +* the shadow bit in the DIR emulation fast-path). +*/ +return (kvm->arch.vgic.vgic_model ==KVM_DEV_TYPE_ARM_VGIC_V3 ); +} +/* * Check whether an IRQ needs to (and can) be queued to a VCPU's ap list. * Do the queuing if necessary, taking the right locks in the right order.
@@ -379,6 +390,7 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq, unsigned long flags) __releases(&irq->irq_lock) {struct kvm_vcpu *vcpu; +bool bcast;lockdep_assert_held (&irq->irq_lock);
@@ -453,11 +465,20 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq,list_add_tail (&irq->ap_list, &vcpu->arch.vgic_cpu.ap_list_head); irq->vcpu = vcpu; +/* A new SPI may result in deactivation trapping on all vcpus */ + bcast = (vgic_model_needs_bcst_kick (vcpu->kvm) && +vgic_valid_spi (vcpu->kvm, irq->intid) && +atomic_fetch_inc (&vcpu->kvm->arch.vgic.active_spis) ==0 ); +raw_spin_unlock (&irq->irq_lock);raw_spin_unlock_irqrestore (&vcpu->arch.vgic_cpu.ap_list_lock, flags); -kvm_make_request (KVM_REQ_IRQ_PENDING , vcpu); -kvm_vcpu_kick (vcpu); +if (!bcast) { +kvm_make_request (KVM_REQ_IRQ_PENDING , vcpu); +kvm_vcpu_kick (vcpu); + }else { +kvm_make_all_cpus_request (vcpu->kvm,KVM_REQ_IRQ_PENDING ); + }return true; }
include/kvm/arm_vgic.h +3 -0
@@ -263,6 +263,9 @@ struct vgic_dist {/* The GIC maintenance IRQ for nested hypervisors. */ u32 mi_intid; +/* Track the number of in-flight active SPIs */ +atomic_t active_spis; +/* base addresses in guest physical address space: */ gpa_t vgic_dist_base;/* distributor */ union {
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 32/49 KVM: arm64: GICv3: Handle in-LR deactivation when possible
In reply to Marc ZyngierEven when we have either an LR overflow or SPIs in flight, it is extremely likely that the interrupt being deactivated is still in the LRs, and that going all the way back to the the generic trap handling code is a waste of time.
Instead, try and deactivate in place when possible, and only if this fails, perform a full exit.
--- arch/arm64/kvm/hyp/vgic-v3-sr.c | 38 ++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 10 deletions(-)
arch/arm64/kvm/hyp/vgic-v3-sr.c +28 -10
@@ -792,7 +792,7 @@ static void __vgic_v3_bump_eoicount(void)write_gicreg (hcr,ICH_HCR_EL2 ); } -static void __vgic_v3_write_dir (struct kvm_vcpu *vcpu,u32 vmcr,int rt) +static int ___vgic_v3_write_dir (struct kvm_vcpu *vcpu,u32 vmcr,int rt) {u32 vid =vcpu_get_reg (vcpu, rt);u64 lr_val;
@@ -800,19 +800,25 @@ static void __vgic_v3_write_dir(struct kvm_vcpu *vcpu, u32 vmcr, int rt)/* EOImode == 0, nothing to be done here */ if (!(vmcr &ICH_VMCR_EOIM_MASK )) -return ; +return 1 ;/* No deactivate to be performed on an LPI */ if (vid >=VGIC_MIN_LPI ) -return ; +return 1 ; lr =__vgic_v3_find_active_lr (vcpu, vid, &lr_val); -if (lr ==-1 ) { -__vgic_v3_bump_eoicount (); -return ; +if (lr !=-1 ) { +__vgic_v3_clear_active_lr (lr, lr_val); +return 1 ; } -__vgic_v3_clear_active_lr (lr, lr_val); +return 0 ; +} + +static void __vgic_v3_write_dir (struct kvm_vcpu *vcpu,u32 vmcr,int rt) +{ +if (!___vgic_v3_write_dir (vcpu, vmcr, rt)) +__vgic_v3_bump_eoicount (); }static void __vgic_v3_write_eoir (struct kvm_vcpu *vcpu,u32 vmcr,int rt)
@@ -1247,9 +1253,21 @@ int __vgic_v3_perform_cpuif_access(struct kvm_vcpu *vcpu)case SYS_ICC_DIR_EL1 :if (unlikely (is_read))return 0 ; -/* Full exit if required to handle overflow deactivation... */ -if (vcpu->arch.vgic_cpu.vgic_v3.vgic_hcr &ICH_HCR_EL2_TDIR ) -return 0 ; +/* +* Full exit if required to handle overflow deactivation, +* unless we can emulate it in the LRs (likely the majority +* of the cases). +*/ +if (vcpu->arch.vgic_cpu.vgic_v3.vgic_hcr &ICH_HCR_EL2_TDIR ) { +int ret; + + ret =___vgic_v3_write_dir (vcpu,__vgic_v3_read_vmcr (), +kvm_vcpu_sys_get_rt (vcpu)); +if (ret) +__kvm_skip_instr (vcpu); + +return ret; + } fn = __vgic_v3_write_dir;break ;case SYS_ICC_RPR_EL1 :
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 33/49 KVM: arm64: GICv3: Avoid broadcast kick on CPUs lacking TDIR
In reply to Marc ZyngierCPUs lacking TDIR always trap ICV_DIR_EL1, no matter what, since we have ICH_HCR_EL2.TC set permanently. For these CPUs, it is useless to use a broadcast kick on SPI injection, as the sole purpose of this is to set TDIR.
We can therefore skip this on these CPUs, which are challenged enough not to be burdened by extra IPIs. As a consequence, permanently set the TDIR bit in the shadow state to notify the fast-path emulation code of the exit reason.
--- arch/arm64/kvm/vgic/vgic-v3.c | 6 +++++- arch/arm64/kvm/vgic/vgic.c | 13 ++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-)
arch/arm64/kvm/vgic/vgic-v3.c +5 -1
@@ -53,10 +53,14 @@ void vgic_v3_configure_hcr(struct kvm_vcpu *vcpu,* need to deal with SPIs that can be deactivated on another * CPU. * +* On systems that do not implement TDIR, force the bit in the +* shadow state anyway to avoid IPI-ing on these poor sods. +* * Note that we set the trap irrespective of EOIMode, as that * can change behind our back without any warning... */ -if (irqs_active_outside_lrs (als) || +if (!cpus_have_final_cap (ARM64_HAS_ICH_HCR_EL2_TDIR ) || +irqs_active_outside_lrs (als) ||atomic_read (&vcpu->kvm->arch.vgic.active_spis)) cpuif->vgic_hcr |=ICH_HCR_EL2_TDIR ; }
arch/arm64/kvm/vgic/vgic.c +8 -5
@@ -370,12 +370,15 @@ static bool vgic_validate_injection(struct vgic_irq *irq, bool level, void *ownestatic bool vgic_model_needs_bcst_kick (struct kvm *kvm) {/* -* A GICv3 (or GICv3-like) system exposing a GICv3 to the -* guest needs a broadcast kick to set TDIR globally, even if -* the bit doesn't really exist (we still need to check for -* the shadow bit in the DIR emulation fast-path). +* A GICv3 (or GICv3-like) system exposing a GICv3 to the guest +* needs a broadcast kick to set TDIR globally. +* +* For systems that do not have TDIR (ARM's own v8.0 CPUs), the +* shadow TDIR bit is always set, and so is the register's TC bit, +* so no need to kick the CPUs. */ -return (kvm->arch.vgic.vgic_model ==KVM_DEV_TYPE_ARM_VGIC_V3 ); +return (cpus_have_final_cap (ARM64_HAS_ICH_HCR_EL2_TDIR ) && + kvm->arch.vgic.vgic_model ==KVM_DEV_TYPE_ARM_VGIC_V3 ); }/*
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 34/49 KVM: arm64: GICv3: nv: Resync LRs/VMCR/HCR early for better MI emulation
In reply to Marc ZyngierThe current approach to nested GICv3 support is to not do anything while L2 is running, wait a transition from L2 to L1 to resync LRs, VMCR and HCR, and only then evaluate the state to decide whether to generate a maintenance interrupt.
This doesn't provide a good quality of emulation, and it would be far preferable to find out early that we need to perform a switch.
Move the LRs/VMCR and HCR resync into vgic_v3_sync_nested(), so that we have most of the state available. As we turning the vgic off at this stage to avoid a screaming host MI, add a new helper vgic_v3_flush_nested() that switches the vgic on again. The MI can then be directly injected as required.
--- arch/arm64/include/asm/kvm_hyp.h | 1 + arch/arm64/kvm/hyp/vgic-v3-sr.c | 2 +- arch/arm64/kvm/vgic/vgic-v3-nested.c | 69 ++++++++++++++++------------ arch/arm64/kvm/vgic/vgic.c | 6 ++- arch/arm64/kvm/vgic/vgic.h | 1 + 5 files changed, 46 insertions(+), 33 deletions(-)
arch/arm64/include/asm/kvm_hyp.h +1 -0
@@ -77,6 +77,7 @@ DECLARE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);int __vgic_v2_perform_cpuif_access (struct kvm_vcpu *vcpu);u64 __gic_v3_get_lr (unsigned int lr); +void __gic_v3_set_lr (u64 val,int lr);void __vgic_v3_save_state (struct vgic_v3_cpu_if *cpu_if);void __vgic_v3_restore_state (struct vgic_v3_cpu_if *cpu_if);
arch/arm64/kvm/hyp/vgic-v3-sr.c +1 -1
@@ -60,7 +60,7 @@ u64 __gic_v3_get_lr(unsigned int lr)unreachable (); } -static void __gic_v3_set_lr (u64 val,int lr) +void __gic_v3_set_lr (u64 val,int lr) {switch (lr &0xf ) {case 0 :
arch/arm64/kvm/vgic/vgic-v3-nested.c +39 -30
122 lines not on this page · raw message
arch/arm64/kvm/vgic/vgic.c +4 -2
@@ -1049,8 +1049,9 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)* abort the entry procedure and inject the exception at the * beginning of the run loop. * -* - Otherwise, do exactly *NOTHING*. The guest state is -* already loaded, and we can carry on with running it. +* - Otherwise, do exactly *NOTHING* apart from enabling the virtual +* CPU interface. The guest state is already loaded, and we can +* carry on with running it. * * If we have NV, but are not in a nested state, compute the * maintenance interrupt state, as it may fire.
@@ -1059,6 +1060,7 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)if (kvm_vgic_vcpu_pending_irq (vcpu))kvm_make_request (KVM_REQ_GUEST_HYP_IRQ_PENDING , vcpu); +vgic_v3_flush_nested (vcpu);return ; }
arch/arm64/kvm/vgic/vgic.h +1 -0
@@ -445,6 +445,7 @@ static inline bool kvm_has_gicv3(struct kvm *kvm)return kvm_has_feat (kvm,ID_AA64PFR0_EL1 ,GIC ,IMP ); } +void vgic_v3_flush_nested (struct kvm_vcpu *vcpu);void vgic_v3_sync_nested (struct kvm_vcpu *vcpu);void vgic_v3_load_nested (struct kvm_vcpu *vcpu);void vgic_v3_put_nested (struct kvm_vcpu *vcpu);
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 35/49 KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
In reply to Marc ZyngierPretty much like the rest of the LR handling, deactivation of an L2 interrupt gets reflected in the L1 LRs, and therefore must be propagated into the L1 shadow state if the interrupt is HW-bound.
Instead of directly handling the active state (which looks a bit off as it ignores locking and L1->L0 HW propagation), use the new deactivation primitive to perform the deactivation and deal with the required maintenance.
--- arch/arm64/kvm/vgic/vgic-v3-nested.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
arch/arm64/kvm/vgic/vgic-v3-nested.c +9 -10
@@ -280,7 +280,6 @@ void vgic_v3_sync_nested(struct kvm_vcpu *vcpu)for_each_set_bit (i, &shadow_if->lr_map, kvm_vgic_global_state.nr_lr) {u64 val, host_lr, lr; -struct vgic_irq *irq; host_lr =__gic_v3_get_lr (lr_map_idx_to_shadow_idx (shadow_if, i));
@@ -290,7 +289,14 @@ void vgic_v3_sync_nested(struct kvm_vcpu *vcpu) val |= host_lr &ICH_LR_STATE ;__vcpu_assign_sys_reg (vcpu,ICH_LRN (i), val); -if (!(lr &ICH_LR_HW ) || !(lr &ICH_LR_STATE )) +/* +* Deactivation of a HW interrupt: the LR must have the HW +* bit set, have been in a non-invalid state before the run, +* and now be in an invalid state. If any of that doesn't +* hold, we're done with this LR. +*/ +if (!((lr &ICH_LR_HW ) && (lr &ICH_LR_STATE ) && + !(host_lr &ICH_LR_STATE )))continue ;/*
@@ -298,14 +304,7 @@ void vgic_v3_sync_nested(struct kvm_vcpu *vcpu)* need to emulate the HW effect between the guest hypervisor * and the nested guest. */ - irq =vgic_get_vcpu_irq (vcpu,FIELD_GET (ICH_LR_PHYS_ID_MASK , lr)); -if (WARN_ON (!irq))/* Shouldn't happen as we check on load */ -continue ; - -if (!(host_lr &ICH_LR_STATE )) - irq->active = false; - -vgic_put_irq (vcpu->kvm, irq); +vgic_v3_deactivate (vcpu,FIELD_GET (ICH_LR_PHYS_ID_MASK , lr)); }/* We need these to be synchronised to generate the MI */
Signature · 1 line
2.47.3
Re: [PATCH v4 35/49] KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
Hi Fuad Tabba,
I'm trying to run nested VMs on Ampere platforms after this patch
series(v6.19+) but nested VMs are not booting and triggering soft lockups on L0 and L0 hang. But just before this patch I could able to successfully boot the Nested VMs.
I bisected the failure to a single commit which is this patch which is causing the issue.
I would like to understand from you that did you observed anything like that?
Were you able to boot Nested VMs successfully after v6.19+?
LOG:
[ 164.647367] Call trace: [ 164.647368] smp_call_function_many_cond+0x334/0x7a0 (P) [ 164.647372] smp_call_function_many+0x20/0x40 [ 164.647374] kvm_make_all_cpus_request+0xec/0x1b8 [ 164.647377] vgic_queue_irq_unlock+0x1c8/0x2c8 [ 164.647380] kvm_vgic_inject_irq+0x194/0x1e0 [ 164.647381] kvm_vm_ioctl_irq_line+0x170/0x400 [ 164.647386] kvm_vm_ioctl+0x7b8/0xc88 [ 164.647389] __arm64_sys_ioctl+0xb4/0x118 [ 164.647393] invoke_syscall+0x6c/0x100 [ 164.647397] el0_svc_common.constprop.0+0x48/0xf0 [ 164.647398] do_el0_svc+0x24/0x38 [ 164.647400] el0_svc+0x3c/0x170 [ 164.647403] el0t_64_sync_handler+0xa0/0xe8 [ 164.647405] el0t_64_sync+0x1b0/0x1b8
Regards,
-Vishnu.
Quoted 59 lines · Marc Zyngier
Pretty much like the rest of the LR handling, deactivation of an L2 interrupt gets reflected in the L1 LRs, and therefore must be propagated into the L1 shadow state if the interrupt is HW-bound.
Instead of directly handling the active state (which looks a bit off as it ignores locking and
L1->L0HW propagation), use the new deactivation primitive to perform the deactivation and deal with the required maintenance.Tested-by Fuad Tabba <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/vgic/vgic-v3-nested.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)arch/arm64/kvm/vgic/vgic-v3-nested.c +9
-10@@ -280,7 +280,6 @@ void vgic_v3_sync_nested(struct kvm_vcpu *vcpu)for_each_set_bit (i, &shadow_if->lr_map, kvm_vgic_global_state.nr_lr) {u64 val, host_lr, lr; -struct vgic_irq *irq; host_lr =__gic_v3_get_lr (lr_map_idx_to_shadow_idx (shadow_if, i));@@ -290,7 +289,14 @@ void vgic_v3_sync_nested(struct kvm_vcpu *vcpu) val |= host_lr &ICH_LR_STATE ;__vcpu_assign_sys_reg (vcpu,ICH_LRN (i), val); -if (!(lr &ICH_LR_HW ) || !(lr &ICH_LR_STATE )) +/* +* Deactivation of a HW interrupt: the LR must have the HW +* bit set, have been in a non-invalid state before the run, +* and now be in an invalid state. If any of that doesn't +* hold, we're done with this LR. +*/ +if (!((lr &ICH_LR_HW ) && (lr &ICH_LR_STATE ) && + !(host_lr &ICH_LR_STATE )))continue ;/* @@ -298,14 +304,7 @@ void vgic_v3_sync_nested(struct kvm_vcpu *vcpu)* need to emulate the HW effect between the guest hypervisor * and the nested guest. */ - irq =vgic_get_vcpu_irq (vcpu,FIELD_GET (ICH_LR_PHYS_ID_MASK , lr)); -if (WARN_ON (!irq))/* Shouldn't happen as we check on load */ -continue ; - -if (!(host_lr &ICH_LR_STATE )) - irq->active = false; - -vgic_put_irq (vcpu->kvm, irq); +vgic_v3_deactivate (vcpu,FIELD_GET (ICH_LR_PHYS_ID_MASK , lr)); }/* We need these to be synchronised to generate the MI */
Marc Zyngier
maintainerRe: [PATCH v4 35/49] KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
Quoted 2 lines · Vishnu Pajjuri
Hi Fuad Tabba,
To be brutally honest, I doubt Fuad really cares about NV,
Quoted 4 lines
I'm trying to run nested VMs on Ampere platforms after this patch
series(v6.19+) but nested VMs are not booting and triggering soft lockups on L0 and L0 hang. But just before this patch I could able to successfully boot the Nested VMs.
So the host dies? There isn't much here that interacts with the host at all. Worse case, the L1 dies by not making progress.
Quoted 6 lines
I bisected the failure to a single commit which is this patch which is causing the issue.
I would like to understand from you that did you observed anything like that?
No. If I had, I wouldn't have merged the series.
Quoted 2 lines
Were you able to boot Nested VMs successfully after v6.19+?
I boot L3s every day.
Quoted 16 lines
LOG:
[ 164.647367] Call trace: [ 164.647368] smp_call_function_many_cond+0x334/0x7a0 (P) [ 164.647372] smp_call_function_many+0x20/0x40 [ 164.647374] kvm_make_all_cpus_request+0xec/0x1b8 [ 164.647377] vgic_queue_irq_unlock+0x1c8/0x2c8 [ 164.647380] kvm_vgic_inject_irq+0x194/0x1e0 [ 164.647381] kvm_vm_ioctl_irq_line+0x170/0x400 [ 164.647386] kvm_vm_ioctl+0x7b8/0xc88 [ 164.647389] __arm64_sys_ioctl+0xb4/0x118 [ 164.647393] invoke_syscall+0x6c/0x100 [ 164.647397] el0_svc_common.constprop.0+0x48/0xf0 [ 164.647398] do_el0_svc+0x24/0x38 [ 164.647400] el0_svc+0x3c/0x170 [ 164.647403] el0t_64_sync_handler+0xa0/0xe8 [ 164.647405] el0t_64_sync+0x1b0/0x1b8
This trace is about interrupt injection from userspace, not deactivation of a HW interrupt.
None of that makes much sense.
M.
Signature · 1 line
Jazz isn't dead. It just smells funny.
Re: [PATCH v4 35/49] KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
Hi Marc,
Many thanks for your reply.
Quoted 6 lines · Marc Zyngier
Quoted 2 lines · Vishnu Pajjuri
Hi Fuad Tabba,
To be brutally honest, I doubt Fuad really cares about NV,
I see Tested-by: fuad Tabba on this patch so tried to reach out him.
Quoted 8 lines
Quoted 5 lines
I'm trying to run nested VMs on Ampere platforms after this patch
series(v6.19+) but nested VMs are not booting and triggering soft lockups on L0 and L0 hang. But just before this patch I could able to successfully boot the Nested VMs.So the host dies? There isn't much here that interacts with the host at all. Worse case, the L1 dies by not making progress.
Initially L1 become unresponsive then L0 becomes unresponsive, soft lockups and L0 hang.
Quoted 8 lines
Quoted 7 lines
I bisected the failure to a single commit which is this patch which is causing the issue.
I would like to understand from you that did you observed anything like that?
Quoted 6 lines
No. If I had, I wouldn't have merged the series.
Quoted 2 lines
Were you able to boot Nested VMs successfully after v6.19+?
I boot L3s every day.
Do you mean L2s or L3s on top of L2s?
I running L1 and L2 using latest QEMU, do you use QEMU or kvmtool run L1 and L2 in your regression tests?
Quoted 18 lines
Quoted 17 lines
LOG:
[ 164.647367] Call trace: [ 164.647368] smp_call_function_many_cond+0x334/0x7a0 (P) [ 164.647372] smp_call_function_many+0x20/0x40 [ 164.647374] kvm_make_all_cpus_request+0xec/0x1b8 [ 164.647377] vgic_queue_irq_unlock+0x1c8/0x2c8 [ 164.647380] kvm_vgic_inject_irq+0x194/0x1e0 [ 164.647381] kvm_vm_ioctl_irq_line+0x170/0x400 [ 164.647386] kvm_vm_ioctl+0x7b8/0xc88 [ 164.647389] __arm64_sys_ioctl+0xb4/0x118 [ 164.647393] invoke_syscall+0x6c/0x100 [ 164.647397] el0_svc_common.constprop.0+0x48/0xf0 [ 164.647398] do_el0_svc+0x24/0x38 [ 164.647400] el0_svc+0x3c/0x170 [ 164.647403] el0t_64_sync_handler+0xa0/0xe8 [ 164.647405] el0t_64_sync+0x1b0/0x1b8
Quoted 3 lines
This trace is about interrupt injection from userspace, not deactivation of a HW interrupt.
None of that makes much sense.
Although this behavior is puzzling, it matches the trace I typically observe on L0. After reverting the patch, I was able to boot L2 guests successfully.
Regards,
-Vishnu
Quoted 3 lines
M.
Marc Zyngier
maintainerRe: [PATCH v4 35/49] KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
Quoted 5 lines · Vishnu Pajjuri
Hi Marc,
Many thanks for your reply.On 30-03-2026 17:47, Marc Zyngier wrote:
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines · Vishnu Pajjuri
Hi Fuad Tabba,
To be brutally honest, I doubt Fuad really cares about NV,
I see Tested-by: fuad Tabba on this patch so tried to reach out him.
Rather than the author of the patch?
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
I'm trying to run nested VMs on Ampere platforms after this patch
series(v6.19+) but nested VMs are not booting and triggering soft lockups on L0 and L0 hang. But just before this patch I could able to successfully boot the Nested VMs.
Quoted 5 lines
Quoted 2 lines
So the host dies? There isn't much here that interacts with the host at all. Worse case, the L1 dies by not making progress.
Initially L1 become unresponsive then L0 becomes unresponsive, soft lockups and L0 hang.
I've never seen this in the past 6 months. Doesn't mean there is no bug, but so far I haven't seen any of that.
[...]
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Were you able to boot Nested VMs successfully after v6.19+?
I boot L3s every day.
Do you mean L2s or L3s on top of L2s?
I'm running an L0 host, itself running L1 guests, themselves running L2 guests, themselves running L3 guests. That's what "running L3s" means.
Quoted 2 lines
I running L1 and L2 using latest QEMU, do you use QEMU or kvmtool run L1 and L2 in your regression tests?
Both. And that really shouldn't matter.
Quoted 20 lines
Quoted 20 lines
Quoted 19 lines
LOG:
[ 164.647367] Call trace: [ 164.647368] smp_call_function_many_cond+0x334/0x7a0 (P) [ 164.647372] smp_call_function_many+0x20/0x40 [ 164.647374] kvm_make_all_cpus_request+0xec/0x1b8 [ 164.647377] vgic_queue_irq_unlock+0x1c8/0x2c8 [ 164.647380] kvm_vgic_inject_irq+0x194/0x1e0 [ 164.647381] kvm_vm_ioctl_irq_line+0x170/0x400 [ 164.647386] kvm_vm_ioctl+0x7b8/0xc88 [ 164.647389] __arm64_sys_ioctl+0xb4/0x118 [ 164.647393] invoke_syscall+0x6c/0x100 [ 164.647397] el0_svc_common.constprop.0+0x48/0xf0 [ 164.647398] do_el0_svc+0x24/0x38 [ 164.647400] el0_svc+0x3c/0x170 [ 164.647403] el0t_64_sync_handler+0xa0/0xe8 [ 164.647405] el0t_64_sync+0x1b0/0x1b8
Quoted 7 lines
Quoted 3 lines
This trace is about interrupt injection from userspace, not deactivation of a HW interrupt.
None of that makes much sense.Although this behavior is puzzling, it matches the trace I typically observe on L0. After reverting the patch, I was able to boot L2 guests successfully.
Well, this patch fixes real bugs, so it isn't going anywhere.
The patch you are reverting addresses the deactivation of a HW interrupt, which is likely to be a timer (that's the only one we support). The stacktrace points to the userspace injection of an SPI.
If we need to broadcast IPI, that's because there is no other SPI currently in flight. But if a CPU is not responding to the IPI, what is it doing? How does this interact with the patch you are reverting?
Given that I don't know what you're running, how you are running it, that I don't have access to whatever HW you are using, and that you are providing no useful information that'd help me debug this, I will leave it up to you to debug it and come back with a detailed analysis of the problem.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerRe: [PATCH v4 35/49] KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
[+ Darren]
Quoted 39 lines · Marc Zyngier
Quoted 18 lines · Vishnu Pajjuri
Quoted 18 lines
Quoted 17 lines
LOG:
[ 164.647367] Call trace: [ 164.647368] smp_call_function_many_cond+0x334/0x7a0 (P) [ 164.647372] smp_call_function_many+0x20/0x40 [ 164.647374] kvm_make_all_cpus_request+0xec/0x1b8 [ 164.647377] vgic_queue_irq_unlock+0x1c8/0x2c8 [ 164.647380] kvm_vgic_inject_irq+0x194/0x1e0 [ 164.647381] kvm_vm_ioctl_irq_line+0x170/0x400 [ 164.647386] kvm_vm_ioctl+0x7b8/0xc88 [ 164.647389] __arm64_sys_ioctl+0xb4/0x118 [ 164.647393] invoke_syscall+0x6c/0x100 [ 164.647397] el0_svc_common.constprop.0+0x48/0xf0 [ 164.647398] do_el0_svc+0x24/0x38 [ 164.647400] el0_svc+0x3c/0x170 [ 164.647403] el0t_64_sync_handler+0xa0/0xe8 [ 164.647405] el0t_64_sync+0x1b0/0x1b8Quoted 7 lines
Quoted 3 lines
This trace is about interrupt injection from userspace, not deactivation of a HW interrupt.
None of that makes much sense.Although this behavior is puzzling, it matches the trace I typically observe on L0. After reverting the patch, I was able to boot L2 guests successfully.
Well, this patch fixes real bugs, so it isn't going anywhere.
The patch you are reverting addresses the deactivation of a HW interrupt, which is likely to be a timer (that's the only one we support). The stacktrace points to the userspace injection of an SPI.
If we need to broadcast IPI, that's because there is no other SPI currently in flight. But if a CPU is not responding to the IPI, what is it doing? How does this interact with the patch you are reverting?
Quoted 5 lines
Given that I don't know what you're running, how you are running it, that I don't have access to whatever HW you are using, and that you are providing no useful information that'd help me debug this, I will leave it up to you to debug it and come back with a detailed analysis of the problem.
Have you made progress on this? I can't reproduce it at all despite my best effort. I'm perfectly happy to help, but you need to give me *something* to go on.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Re: [PATCH v4 35/49] KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
Hi Marc,
Quoted 49 lines · Marc Zyngier
[+ Darren]
Quoted 39 lines · Marc Zyngier
Quoted 18 lines · Vishnu Pajjuri
Quoted 18 lines
Quoted 17 lines
LOG:
[ 164.647367] Call trace: [ 164.647368] smp_call_function_many_cond+0x334/0x7a0 (P) [ 164.647372] smp_call_function_many+0x20/0x40 [ 164.647374] kvm_make_all_cpus_request+0xec/0x1b8 [ 164.647377] vgic_queue_irq_unlock+0x1c8/0x2c8 [ 164.647380] kvm_vgic_inject_irq+0x194/0x1e0 [ 164.647381] kvm_vm_ioctl_irq_line+0x170/0x400 [ 164.647386] kvm_vm_ioctl+0x7b8/0xc88 [ 164.647389] __arm64_sys_ioctl+0xb4/0x118 [ 164.647393] invoke_syscall+0x6c/0x100 [ 164.647397] el0_svc_common.constprop.0+0x48/0xf0 [ 164.647398] do_el0_svc+0x24/0x38 [ 164.647400] el0_svc+0x3c/0x170 [ 164.647403] el0t_64_sync_handler+0xa0/0xe8 [ 164.647405] el0t_64_sync+0x1b0/0x1b8Quoted 7 lines
Quoted 3 lines
This trace is about interrupt injection from userspace, not deactivation of a HW interrupt.
None of that makes much sense.Although this behavior is puzzling, it matches the trace I typically observe on L0. After reverting the patch, I was able to boot L2 guests successfully.
Well, this patch fixes real bugs, so it isn't going anywhere.
The patch you are reverting addresses the deactivation of a HW interrupt, which is likely to be a timer (that's the only one we support). The stacktrace points to the userspace injection of an SPI.
If we need to broadcast IPI, that's because there is no other SPI currently in flight. But if a CPU is not responding to the IPI, what is it doing? How does this interact with the patch you are reverting?
Quoted 5 lines
Given that I don't know what you're running, how you are running it, that I don't have access to whatever HW you are using, and that you are providing no useful information that'd help me debug this, I will leave it up to you to debug it and come back with a detailed analysis of the problem.
Quoted 3 lines
Have you made progress on this? I can't reproduce it at all despite my best effort. I'm perfectly happy to help, but you need to give me *something* to go on.
Thanks for your support!!
The issue is triggered as soon as the timer interrupt (IRQ 27) is deactivated. Preventing the deactivation of IRQ 27 during nested VGIC state transitions prevents the failure from reproducing.
I am currently tracing execution paths and inspecting VGIC state to determine how disabling this interrupt leads to the observed behavior.
Regards,
-Vishnu.
Quoted 4 lines
Thanks,
M.
Marc Zyngier
maintainerRe: [PATCH v4 35/49] KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
Quoted 10 lines · Vishnu Pajjuri
Hi Marc,
Quoted 4 lines · Marc Zyngier
Have you made progress on this? I can't reproduce it at all despite my best effort. I'm perfectly happy to help, but you need to give me *something* to go on.
Quoted 5 lines
Thanks for your support!!
The issue is triggered as soon as the timer interrupt (IRQ 27) is deactivated. Preventing the deactivation of IRQ 27 during nested VGIC state transitions prevents the failure from reproducing.
Which level of deactivation? From L2 to L1? Or L1 to L0? The former should just be a an update to the irq structure, while the latter is effectively a write to ICC_DIR_EL1, and *that* is a new behaviour introduced by this patch.
I wonder if your implementation is such that ICC_DIR_EL1 is trapped by ICH_HCR_EL2.TDIR, which is allowed by the architecture, but that none of the two implementations I have actually enforce (the trap only applies to ICV_DIR_EL1). Can you try the hack below which disables the traps much earlier, and let me know if that helps?
Even if that's the case, this should result in an EL2->EL2 exception, and that should be caught as an unhandled exception in entry-common.c, so something else is afoot.
Thanks,
M.
arch/arm64/kvm/vgic/vgic-v3-nested.c +8 -8
@@ -280,6 +280,14 @@ void vgic_v3_sync_nested(struct kvm_vcpu *vcpu)struct shadow_if *shadow_if =get_shadow_if ();int i; +/* We need these to be synchronised to generate the MI */ +__vcpu_assign_sys_reg (vcpu,ICH_VMCR_EL2 ,read_sysreg_s (SYS_ICH_VMCR_EL2 )); + __vcpu_rmw_sys_reg(vcpu, ICH_HCR_EL2, &=, ~ICH_HCR_EL2_EOIcount); + __vcpu_rmw_sys_reg(vcpu, ICH_HCR_EL2, |=, read_sysreg_s(SYS_ICH_HCR_EL2) & ICH_HCR_EL2_EOIcount); + +write_sysreg_s (0 ,SYS_ICH_HCR_EL2 ); +isb (); +for_each_set_bit (i, &shadow_if->lr_map, kvm_vgic_global_state.nr_lr) {u64 val, host_lr, lr;
@@ -309,14 +317,6 @@ void vgic_v3_sync_nested(struct kvm_vcpu *vcpu)vgic_v3_deactivate (vcpu,FIELD_GET (ICH_LR_PHYS_ID_MASK , lr)); } -/* We need these to be synchronised to generate the MI */ -__vcpu_assign_sys_reg (vcpu,ICH_VMCR_EL2 ,read_sysreg_s (SYS_ICH_VMCR_EL2 )); - __vcpu_rmw_sys_reg(vcpu, ICH_HCR_EL2, &=, ~ICH_HCR_EL2_EOIcount); - __vcpu_rmw_sys_reg(vcpu, ICH_HCR_EL2, |=, read_sysreg_s(SYS_ICH_HCR_EL2) & ICH_HCR_EL2_EOIcount); - -write_sysreg_s (0 ,SYS_ICH_HCR_EL2 ); -isb (); -vgic_v3_nested_update_mi (vcpu); }
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerRe: [PATCH v4 35/49] KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
Quoted 30 lines · Marc Zyngier
Quoted 10 lines · Vishnu Pajjuri
Hi Marc,
Quoted 4 lines · Marc Zyngier
Have you made progress on this? I can't reproduce it at all despite my best effort. I'm perfectly happy to help, but you need to give me *something* to go on.
Quoted 5 lines
Thanks for your support!!
The issue is triggered as soon as the timer interrupt (IRQ 27) is deactivated. Preventing the deactivation of IRQ 27 during nested VGIC state transitions prevents the failure from reproducing.
Which level of deactivation? From L2 to L1? Or L1 to L0? The former should just be a an update to the irq structure, while the latter is effectively a write to
ICC_DIR_EL1, and *that* is a new behaviour introduced by this patch.I wonder if your implementation is such that
ICC_DIR_EL1is trapped by ICH_HCR_EL2.TDIR, which is allowed by the architecture, but that none of the two implementations I have actually enforce (the trap only applies toICV_DIR_EL1). Can you try the hack below which disables the traps much earlier, and let me know if that helps?
Quoted 3 lines
Even if that's the case, this should result in an
EL2->EL2exception, and that should be caught as an unhandled exception inentry-common.c, so something else is afoot.
Actually, this should never happen. ICH_HCR_EL2.TDIR is constructed like all the other GICv3 trap bits, in the sense that it only traps accesses from EL1, not EL2 (for sanity reasons, I'm not considering the possibility of a trap to EL3...).
Still, I'm interested in finding out if that hack helps at all.
Thanks,
M.
Signature · 1 line
Jazz isn't dead. It just smells funny.
Marc Zyngier
maintainerRe: [PATCH v4 35/49] KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
Quoted 37 lines · Marc Zyngier
Quoted 30 lines · Marc Zyngier
Quoted 10 lines · Vishnu Pajjuri
Hi Marc,
Quoted 4 lines · Marc Zyngier
Have you made progress on this? I can't reproduce it at all despite my best effort. I'm perfectly happy to help, but you need to give me *something* to go on.
Quoted 5 lines
Thanks for your support!!
The issue is triggered as soon as the timer interrupt (IRQ 27) is deactivated. Preventing the deactivation of IRQ 27 during nested VGIC state transitions prevents the failure from reproducing.
Which level of deactivation? From L2 to L1? Or L1 to L0? The former should just be a an update to the irq structure, while the latter is effectively a write to
ICC_DIR_EL1, and *that* is a new behaviour introduced by this patch.I wonder if your implementation is such that
ICC_DIR_EL1is trapped by ICH_HCR_EL2.TDIR, which is allowed by the architecture, but that none of the two implementations I have actually enforce (the trap only applies toICV_DIR_EL1). Can you try the hack below which disables the traps much earlier, and let me know if that helps?Quoted 3 lines
Even if that's the case, this should result in an
EL2->EL2exception, and that should be caught as an unhandled exception inentry-common.c, so something else is afoot.
Quoted 6 lines
Actually, this should never happen. ICH_HCR_EL2.TDIR is constructed like all the other GICv3 trap bits, in the sense that it only traps accesses from EL1, not EL2 (for sanity reasons, I'm not considering the possibility of a trap to EL3...).
Still, I'm interested in finding out if that hack helps at all.
Any update?
I've been racking my brain on this one, and I may have another potential avenue to explore. Does your CPU implement SEIS (ICC_CTLR_EL1.SEIS == 1)?
I think there is a corner case where we can end-up doing a double deactivation. On its own, that's a completely harmless thing to do. Except on an implementation with SEIS, which could deliver an SError and route that to EL3. Add quality firmware to the mix, and you could be in for a treat.
Obviously, implementing SEIS is a terrible and pointless thing to do (it's been long deprecated), but in case someone felt adventurous in the RTL department, let me know what the following hack does for you.
M.
arch/arm64/kvm/vgic/vgic-v3.c +1 -1
@@ -275,7 +275,7 @@ void vgic_v3_deactivate(struct kvm_vcpu *vcpu, u64 val) lr =vgic_v3_compute_lr (vcpu, irq) & ~ICH_LR_ACTIVE_BIT ; } -if (lr &ICH_LR_HW ) +if ((lr &ICH_LR_HW ) && !vgic_state_is_nested (vcpu))vgic_v3_deactivate_phys (FIELD_GET (ICH_LR_PHYS_ID_MASK , lr));vgic_v3_fold_lr (vcpu, lr);
Signature · 1 line
Jazz isn't dead. It just smells funny.
Re: [PATCH v4 35/49] KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
In reply to Marc ZyngierQuoted 2 lines · Marc Zyngier
[+ Darren]
Thanks for the head's up Marc.
Quoted 3 lines
Quoted 1 line · Marc Zyngier
Quoted 1 line
...
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Although this behavior is puzzling, it matches the trace I typically observe on L0. After reverting the patch, I was able to boot L2 guests successfully.
Well, this patch fixes real bugs, so it isn't going anywhere.
Understood.
Quoted 5 lines
Have you made progress on this? I can't reproduce it at all despite my best effort. I'm perfectly happy to help, but you need to give me *something* to go on.
I didn't give this the attention it deserved. I've now got Vishnu the resources he needs to help move this forward. We owe you something concrete/actionable/reproducible and we'll report back once we have it.
Signature · 2 lines
Darren Hart Ampere Computing / Linux Enabling
Marc Zyngier
maintainerPATCH v4 36/49 KVM: arm64: GICv3: Force exit to sync ICH_HCR_EL2.En
In reply to Marc ZyngierFEAT_NV2 is pretty terrible for anything that tries to enforce immediate effects, and writing to ICH_HCR_EL2 in the hope to disable a maintenance interrupt is vain. This only hits memory, and the guest hasn't cleared anything -- the MI will fire.
For example, running the vgic_irq test under NV results in about 800 maintenance interrupts being actually handled by the L1 guest, when none were expected.
As a cheap workaround, read back ICH_MISR_EL2 after writing 0 to ICH_HCR_EL2. This is very cheap on real HW, and causes a trap to the host in NV, giving it the opportunity to retire the pending MI. With this, the above test runs to completion without any MI being actually handled.
Yes, this is really poor...
--- arch/arm64/kvm/hyp/vgic-v3-sr.c | 7 +++++++ arch/arm64/kvm/vgic/vgic-v3-nested.c | 6 ++++-- 2 files changed, 11 insertions(+), 2 deletions(-)
arch/arm64/kvm/hyp/vgic-v3-sr.c +7 -0
@@ -244,6 +244,13 @@ void __vgic_v3_save_state(struct vgic_v3_cpu_if *cpu_if) }write_gicreg (0 ,ICH_HCR_EL2 ); + +/* +* Hack alert: On NV, this results in a trap so that the above write +* actually takes effect... No synchronisation is necessary, as we +* only care about the effects when this traps. +*/ +read_gicreg (ICH_MISR_EL2 ); }void __vgic_v3_restore_state (struct vgic_v3_cpu_if *cpu_if)
arch/arm64/kvm/vgic/vgic-v3-nested.c +4 -2
@@ -94,8 +94,10 @@ static int lr_map_idx_to_shadow_idx(struct shadow_if *shadow_if, int idx)* * - because most of the ICH_*_EL2 registers live in the VNCR page, the * quality of emulation is poor: L1 can setup the vgic so that an MI would -* immediately fire, and not observe anything until the next exit. Trying -* to read ICH_MISR_EL2 would do the trick, for example. +* immediately fire, and not observe anything until the next exit. +* Similarly, a pending MI is not immediately disabled by clearing +* ICH_HCR_EL2.En. Trying to read ICH_MISR_EL2 would do the trick, for +* example. * * System register emulation: *
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 37/49 KVM: arm64: GICv2: Handle LR overflow when EOImode==0
In reply to Marc ZyngierSimilarly to the GICv3 version, handle the EOIcount-driven deactivation by walking the overflow list.
--- arch/arm64/kvm/vgic/vgic-v2.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
arch/arm64/kvm/vgic/vgic-v2.c +27 -0
@@ -100,6 +100,8 @@ static void vgic_v2_fold_lr(struct kvm_vcpu *vcpu, u32 val)vgic_put_irq (vcpu->kvm, irq); } +static u32 vgic_v2_compute_lr (struct kvm_vcpu *vcpu,struct vgic_irq *irq); +/* * transfer the content of the LRs back into the corresponding ap_list: * - active bit is transferred as is
@@ -111,12 +113,37 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu) {struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;struct vgic_v2_cpu_if *cpuif = &vgic_cpu->vgic_v2; +u32 eoicount =FIELD_GET (GICH_HCR_EOICOUNT , cpuif->vgic_hcr); +struct vgic_irq *irq;DEBUG_SPINLOCK_BUG_ON (!irqs_disabled ());for (int lr =0 ; lr < vgic_cpu->vgic_v2.used_lrs; lr++)vgic_v2_fold_lr (vcpu, cpuif->vgic_lr[lr]); +/* See the GICv3 equivalent for the EOIcount handling rationale */ +list_for_each_entry (irq, &vgic_cpu->ap_list_head, ap_list) { +u32 lr; + +if (!eoicount) { +break ; + }else { +guard (raw_spinlock)(&irq->irq_lock); + +if (!(likely (vgic_target_oracle (irq) == vcpu) && + irq->active)) +continue ; + + lr =vgic_v2_compute_lr (vcpu, irq) & ~GICH_LR_ACTIVE_BIT ; + } + +if (lr &GICH_LR_HW ) +writel_relaxed (FIELD_GET (GICH_LR_PHYSID_CPUID , lr), + kvm_vgic_global_state.gicc_base +GIC_CPU_DEACTIVATE ); +vgic_v2_fold_lr (vcpu, lr); + eoicount--; + } + cpuif->used_lrs =0 ; }
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 38/49 KVM: arm64: GICv2: Handle deactivation via GICV_DIR traps
In reply to Marc ZyngierAdd the plumbing of GICv2 interrupt deactivation via GICV_DIR. This requires adding a new device so that we can easily decode the DIR address.
The deactivation itself is very similar to the GICv3 version.
--- arch/arm64/kvm/vgic/vgic-mmio-v2.c | 24 +++++++++ arch/arm64/kvm/vgic/vgic-mmio.h | 1 + arch/arm64/kvm/vgic/vgic-v2.c | 85 ++++++++++++++++++++++++++++++ arch/arm64/kvm/vgic/vgic.h | 1 + include/kvm/arm_vgic.h | 1 + 5 files changed, 112 insertions(+)
arch/arm64/kvm/vgic/vgic-mmio-v2.c +24 -0
@@ -359,6 +359,16 @@ static void vgic_mmio_write_vcpuif(struct kvm_vcpu *vcpu,vgic_set_vmcr (vcpu, &vmcr); } +static void vgic_mmio_write_dir (struct kvm_vcpu *vcpu, +gpa_t addr,unsigned int len, +unsigned long val) +{ +if (kvm_vgic_global_state.type ==VGIC_V2 ) +vgic_v2_deactivate (vcpu, val); +else +vgic_v3_deactivate (vcpu, val); +} +static unsigned long vgic_mmio_read_apr (struct kvm_vcpu *vcpu,gpa_t addr,unsigned int len) {
@@ -482,6 +492,10 @@ static const struct vgic_register_region vgic_v2_cpu_registers[] = {REGISTER_DESC_WITH_LENGTH (GIC_CPU_IDENT , vgic_mmio_read_vcpuif, vgic_mmio_write_vcpuif,4 , VGIC_ACCESS_32bit), +REGISTER_DESC_WITH_LENGTH_UACCESS (GIC_CPU_DEACTIVATE , + vgic_mmio_read_raz, vgic_mmio_write_dir, + vgic_mmio_read_raz, vgic_mmio_uaccess_write_wi, + 4, VGIC_ACCESS_32bit), };unsigned int vgic_v2_init_dist_iodev (struct vgic_io_device *dev)
@@ -494,6 +508,16 @@ unsigned int vgic_v2_init_dist_iodev(struct vgic_io_device *dev)return SZ_4K ; } +unsigned int vgic_v2_init_cpuif_iodev (struct vgic_io_device *dev) +{ + dev->regions = vgic_v2_cpu_registers; + dev->nr_regions =ARRAY_SIZE (vgic_v2_cpu_registers); + +kvm_iodevice_init (&dev->dev, &kvm_io_gic_ops); + +return KVM_VGIC_V2_CPU_SIZE ; +} +int vgic_v2_has_attr_regs (struct kvm_device *dev,struct kvm_device_attr *attr) {const struct vgic_register_region *region;
arch/arm64/kvm/vgic/vgic-mmio.h +1 -0
@@ -213,6 +213,7 @@ void vgic_write_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid, const u32 val);unsigned int vgic_v2_init_dist_iodev (struct vgic_io_device *dev); +unsigned int vgic_v2_init_cpuif_iodev (struct vgic_io_device *dev);unsigned int vgic_v3_init_dist_iodev (struct vgic_io_device *dev);
arch/arm64/kvm/vgic/vgic-v2.c +85 -0
109 lines not on this page · raw message
arch/arm64/kvm/vgic/vgic.h +1 -0
@@ -277,6 +277,7 @@ int vgic_check_iorange(struct kvm *kvm, phys_addr_t ioaddr,void vgic_v2_fold_lr_state (struct kvm_vcpu *vcpu);void vgic_v2_populate_lr (struct kvm_vcpu *vcpu,struct vgic_irq *irq,int lr); +void vgic_v2_deactivate (struct kvm_vcpu *vcpu,u32 val);void vgic_v2_clear_lr (struct kvm_vcpu *vcpu,int lr);void vgic_v2_configure_hcr (struct kvm_vcpu *vcpu,struct ap_list_summary *als);int vgic_v2_has_attr_regs (struct kvm_device *dev,struct kvm_device_attr *attr);
include/kvm/arm_vgic.h +1 -0
@@ -287,6 +287,7 @@ struct vgic_dist {struct vgic_irq *spis;struct vgic_io_device dist_iodev; +struct vgic_io_device cpuif_iodev;bool has_its;bool table_write_in_progress;
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 39/49 KVM: arm64: GICv2: Always trap GICV_DIR register
In reply to Marc ZyngierSince we can't decide to trap the DIR register on a per-vcpu basis, always trap the second page of the GIC CPU interface. Yes, this is costly. On the bright side, no sane SW should use EOImode==1 on GICv2...
--- arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c | 4 ++++ arch/arm64/kvm/vgic/vgic-v2.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-)
arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c +4 -0
@@ -63,6 +63,10 @@ int __vgic_v2_perform_cpuif_access(struct kvm_vcpu *vcpu)return -1 ; } +/* Handle deactivation as a normal exit */ +if ((fault_ipa - vgic->vgic_cpu_base) >=GIC_CPU_DEACTIVATE ) +return 0 ; + rd =kvm_vcpu_dabt_get_rd (vcpu); addr = kvm_vgic_global_state.vcpu_hyp_va; addr += fault_ipa - vgic->vgic_cpu_base;
arch/arm64/kvm/vgic/vgic-v2.c +1 -1
@@ -457,7 +457,7 @@ int vgic_v2_map_resources(struct kvm *kvm)if (!static_branch_unlikely (&vgic_v2_cpuif_trap)) { ret =kvm_phys_addr_ioremap (kvm, dist->vgic_cpu_base, kvm_vgic_global_state.vcpu_base, -KVM_VGIC_V2_CPU_SIZE , true); +KVM_VGIC_V2_CPU_SIZE -SZ_4K , true);if (ret) {kvm_err ("Unable to remap VGIC CPU to VCPU\n" );return ret;
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 40/49 KVM: arm64: selftests: gic_v3: Add irq group setting helper
In reply to Marc ZyngierBeing able to set the group of an interrupt is pretty useful. Add such a helper.
--- tools/testing/selftests/kvm/include/arm64/gic.h | 1 + tools/testing/selftests/kvm/lib/arm64/gic.c | 6 ++++++ .../testing/selftests/kvm/lib/arm64/gic_private.h | 1 + tools/testing/selftests/kvm/lib/arm64/gic_v3.c | 15 +++++++++++++++ 4 files changed, 23 insertions(+)
tools/testing/selftests/kvm/include/arm64/gic.h +1 -0
@@ -57,6 +57,7 @@ void gic_irq_set_pending(unsigned int intid);void gic_irq_clear_pending (unsigned int intid);bool gic_irq_get_pending (unsigned int intid);void gic_irq_set_config (unsigned int intid,bool is_edge); +void gic_irq_set_group (unsigned int intid,bool group);void gic_rdist_enable_lpis (vm_paddr_t cfg_table,size_t cfg_table_size,vm_paddr_t pend_table);
tools/testing/selftests/kvm/lib/arm64/gic.c +6 -0
@@ -155,3 +155,9 @@ void gic_irq_set_config(unsigned int intid, bool is_edge)GUEST_ASSERT (gic_common_ops); gic_common_ops->gic_irq_set_config (intid, is_edge); } + +void gic_irq_set_group (unsigned int intid,bool group) +{ +GUEST_ASSERT (gic_common_ops); + gic_common_ops->gic_irq_set_group (intid, group); +}
tools/testing/selftests/kvm/lib/arm64/gic_private.h +1 -0
@@ -25,6 +25,7 @@ struct gic_common_ops {void (*gic_irq_clear_pending)(uint32_t intid);bool (*gic_irq_get_pending)(uint32_t intid);void (*gic_irq_set_config)(uint32_t intid,bool is_edge); +void (*gic_irq_set_group)(uint32_t intid,bool group); };extern const struct gic_common_ops gicv3_ops;
tools/testing/selftests/kvm/lib/arm64/gic_v3.c +15 -0
@@ -293,6 +293,20 @@ static void gicv3_enable_redist(volatile void *redist_base) } } +static void gicv3_set_group (uint32_t intid,bool grp) +{ +uint32_t cpu_or_dist; +uint32_t val; + + cpu_or_dist = (get_intid_range (intid) ==SPI_RANGE ) ?DIST_BIT :guest_get_vcpuid (); + val =gicv3_reg_readl (cpu_or_dist,GICD_IGROUPR + (intid /32 ) *4 ); +if (grp) + val |=BIT (intid %32 ); +else + val &= ~BIT (intid %32 ); +gicv3_reg_writel (cpu_or_dist,GICD_IGROUPR + (intid /32 ) *4 , val); +} +static void gicv3_cpu_init (unsigned int cpu) {volatile void *sgi_base;
@@ -400,6 +414,7 @@ const struct gic_common_ops gicv3_ops = { .gic_irq_clear_pending = gicv3_irq_clear_pending, .gic_irq_get_pending = gicv3_irq_get_pending, .gic_irq_set_config = gicv3_irq_set_config, + .gic_irq_set_group = gicv3_set_group, }; void gic_rdist_enable_lpis(vm_paddr_t cfg_table, size_t cfg_table_size,
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 41/49 KVM: arm64: selftests: gic_v3: Disable Group-0 interrupts by default
In reply to Marc ZyngierMake sure G0 is disabled at the point of initialising the GIC.
--- tools/testing/selftests/kvm/lib/arm64/gic_v3.c | 2 ++ 1 file changed, 2 insertions(+)
tools/testing/selftests/kvm/lib/arm64/gic_v3.c +2 -0
@@ -342,6 +342,8 @@ static void gicv3_cpu_init(unsigned int cpu)/* Set a default priority threshold */ write_sysreg_s (ICC_PMR_DEF_PRIO ,SYS_ICC_PMR_EL1 ); +/* Disable Group-0 interrupts */ +write_sysreg_s (ICC_IGRPEN0_EL1_MASK ,SYS_ICC_IGRPEN1_EL1 );/* Enable non-secure Group-1 interrupts */ write_sysreg_s (ICC_IGRPEN1_EL1_MASK ,SYS_ICC_IGRPEN1_EL1 ); }
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 42/49 KVM: arm64: selftests: vgic_irq: Fix GUEST_ASSERT_IAR_EMPTY() helper
In reply to Marc ZyngierNo, 0 is not a spurious INTID. Never been, never was.
--- tools/testing/selftests/kvm/arm64/vgic_irq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/testing/selftests/kvm/arm64/vgic_irq.c +1 -1
@@ -205,7 +205,7 @@ static void kvm_inject_call(kvm_inject_cmd cmd, uint32_t first_intid,do { \uint32_t _intid; \ _intid =gic_get_and_ack_irq (); \ -GUEST_ASSERT (_intid ==0 || _intid ==IAR_SPURIOUS ); \ +GUEST_ASSERT (_intid ==IAR_SPURIOUS ); \ } while (0)#define CAT_HELPER (a, b) a ## b
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 43/49 KVM: arm64: selftests: vgic_irq: Change configuration before enabling interrupt
In reply to Marc ZyngierThe architecture is pretty clear that changing the configuration of an enable interrupt is not OK. It doesn't really matter here, but doing the right thing is not more expensive.
--- tools/testing/selftests/kvm/arm64/vgic_irq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
tools/testing/selftests/kvm/arm64/vgic_irq.c +3 -3
@@ -473,12 +473,12 @@ static void guest_code(struct test_args *args)gic_init (GIC_V3 ,1 ); -for (i =0 ; i < nr_irqs; i++) -gic_irq_enable (i); -for (i =MIN_SPI ; i < nr_irqs; i++)gic_irq_set_config (i, !level_sensitive); +for (i =0 ; i < nr_irqs; i++) +gic_irq_enable (i); +gic_set_eoi_split (args->eoi_split);reset_priorities (args);
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 44/49 KVM: arm64: selftests: vgic_irq: Exclude timer-controlled interrupts
In reply to Marc ZyngierThe PPI injection API is clear that you can't inject the timer PPIs from userspace, since they are controlled by the timers themselves.
Add an exclusion list for this purpose.
--- tools/testing/selftests/kvm/arm64/vgic_irq.c | 31 ++++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-)
tools/testing/selftests/kvm/arm64/vgic_irq.c +25 -6
@@ -359,8 +359,9 @@ static uint32_t wait_for_and_activate_irq(void)* interrupts for the whole test. */ static void test_inject_preemption (struct test_args *args, -uint32_t first_intid,int num, -kvm_inject_cmd cmd) +uint32_t first_intid,int num, +const unsigned long *exclude, +kvm_inject_cmd cmd) {uint32_t intid, prio, step =KVM_PRIO_STEPS ;int i;
@@ -379,6 +380,10 @@ static void test_inject_preemption(struct test_args *args,for (i =0 ; i < num; i++) {uint32_t tmp; intid = i + first_intid; + +if (exclude &&test_bit (i, exclude)) +continue ; +KVM_INJECT (cmd, intid);/* Each successive IRQ will preempt the previous one. */ tmp =wait_for_and_activate_irq ();
@@ -390,6 +395,10 @@ static void test_inject_preemption(struct test_args *args,/* finish handling the IRQs starting with the highest priority one. */ for (i =0 ; i < num; i++) { intid = num - i -1 + first_intid; + +if (exclude &&test_bit (intid - first_intid, exclude)) +continue ; +gic_set_eoi (intid);if (args->eoi_split)gic_set_dir (intid);
@@ -397,8 +406,12 @@ static void test_inject_preemption(struct test_args *args,local_irq_enable (); -for (i =0 ; i < num; i++) +for (i =0 ; i < num; i++) { +if (exclude &&test_bit (i, exclude)) +continue ; +GUEST_ASSERT (!gic_irq_get_active (i + first_intid)); + }GUEST_ASSERT_EQ (gic_read_ap1r0 (),0 );GUEST_ASSERT_IAR_EMPTY ();
@@ -442,14 +455,20 @@ static void test_preemption(struct test_args *args, struct kvm_inject_desc *f)* number of concurrently active IRQs. The number of LRs implemented is * IMPLEMENTATION DEFINED, however, it seems that most implement 4. */ +/* Timer PPIs cannot be injected from userspace */ +static const unsigned long ppi_exclude = (BIT (27 -MIN_PPI ) | +BIT (30 -MIN_PPI ) | +BIT (28 -MIN_PPI ) | +BIT (26 -MIN_PPI )); +if (f->sgi) -test_inject_preemption (args,MIN_SGI ,4 , f->cmd); +test_inject_preemption (args,MIN_SGI ,4 ,NULL , f->cmd);if (f->ppi) -test_inject_preemption (args,MIN_PPI ,4 , f->cmd); +test_inject_preemption (args,MIN_PPI ,4 , &ppi_exclude, f->cmd);if (f->spi) -test_inject_preemption (args,MIN_SPI ,4 , f->cmd); +test_inject_preemption (args,MIN_SPI ,4 ,NULL , f->cmd); }static void test_restore_active (struct test_args *args,struct kvm_inject_desc *f)
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 45/49 KVM: arm64: selftests: vgic_irq: Remove LR-bound limitation
In reply to Marc ZyngierGood news: our GIC emulation is not completely broken, and we can activate as many interrupts as we want.
Bump the test to cover all the SGIs, all the allowed PPIs, and 31 SPIs. Yes, 31, because we have 31 available priorities, and the test is not happy with having two interrupts with the same priority.
--- tools/testing/selftests/kvm/arm64/vgic_irq.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-)
tools/testing/selftests/kvm/arm64/vgic_irq.c +6 -13
@@ -449,12 +449,6 @@ static void test_injection_failure(struct test_args *args,static void test_preemption (struct test_args *args,struct kvm_inject_desc *f) { -/* -* Test up to 4 levels of preemption. The reason is that KVM doesn't -* currently implement the ability to have more than the number-of-LRs -* number of concurrently active IRQs. The number of LRs implemented is -* IMPLEMENTATION DEFINED, however, it seems that most implement 4. -*/ /* Timer PPIs cannot be injected from userspace */ static const unsigned long ppi_exclude = (BIT(27 - MIN_PPI) | BIT(30 - MIN_PPI) |
@@ -462,26 +456,25 @@ static void test_preemption(struct test_args *args, struct kvm_inject_desc *f) BIT(26 - MIN_PPI));if (f->sgi) -test_inject_preemption (args,MIN_SGI ,4 ,NULL , f->cmd); +test_inject_preemption (args,MIN_SGI ,16 ,NULL , f->cmd);if (f->ppi) -test_inject_preemption (args,MIN_PPI ,4 , &ppi_exclude, f->cmd); +test_inject_preemption (args,MIN_PPI ,16 , &ppi_exclude, f->cmd);if (f->spi) -test_inject_preemption (args,MIN_SPI ,4 ,NULL , f->cmd); +test_inject_preemption (args,MIN_SPI ,31 ,NULL , f->cmd); }static void test_restore_active (struct test_args *args,struct kvm_inject_desc *f) { -/* Test up to 4 active IRQs. Same reason as in test_preemption. */ if (f->sgi) -guest_restore_active (args,MIN_SGI ,4 , f->cmd); +guest_restore_active (args,MIN_SGI ,16 , f->cmd);if (f->ppi) -guest_restore_active (args,MIN_PPI ,4 , f->cmd); +guest_restore_active (args,MIN_PPI ,16 , f->cmd);if (f->spi) -guest_restore_active (args,MIN_SPI ,4 , f->cmd); +guest_restore_active (args,MIN_SPI ,31 , f->cmd); }static void guest_code (struct test_args *args)
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 46/49 KVM: arm64: selftests: vgic_irq: Perform EOImode==1 deactivation in ack order
In reply to Marc ZyngierWhen EOImode==1, perform the deactivation in the order of activation, just to make things a bit worse for KVM. Yes, I'm nasty.
--- tools/testing/selftests/kvm/arm64/vgic_irq.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
tools/testing/selftests/kvm/arm64/vgic_irq.c +12 -2
@@ -400,8 +400,18 @@ static void test_inject_preemption(struct test_args *args,continue ;gic_set_eoi (intid); -if (args->eoi_split) -gic_set_dir (intid); + } + +if (args->eoi_split) { +for (i =0 ; i < num; i++) { + intid = i + first_intid; + +if (exclude &&test_bit (i, exclude)) +continue ; + +if (args->eoi_split) +gic_set_dir (intid); + } }local_irq_enable ();
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 47/49 KVM: arm64: selftests: vgic_irq: Add asymmetric SPI deaectivation test
In reply to Marc ZyngierAdd a new test case that makes an interrupt pending on a vcpu, activates it, do the priority drop, and then get *another* vcpu to do the deactivation.
Special care is taken not to trigger an exit in the process, so that we are sure that the active interrupt is in an LR. Joy.
--- tools/testing/selftests/kvm/arm64/vgic_irq.c | 105 +++++++++++++++++++ 1 file changed, 105 insertions(+)
tools/testing/selftests/kvm/arm64/vgic_irq.c +105 -0
@@ -29,6 +29,7 @@ struct test_args {bool level_sensitive;/* 1 is level, 0 is edge */ int kvm_max_routes;/* output of KVM_CAP_IRQ_ROUTING */ bool kvm_supports_irqfd;/* output of KVM_CAP_IRQFD */ +uint32_t shared_data; };/*
@@ -801,6 +802,109 @@ static void test_vgic(uint32_t nr_irqs, bool level_sensitive, bool eoi_split)kvm_vm_free (vm); } +static void guest_code_asym_dir (struct test_args *args,int cpuid) +{ +gic_init (GIC_V3 ,2 ); + +gic_set_eoi_split (1 ); +gic_set_priority_mask (CPU_PRIO_MASK ); + +if (cpuid ==0 ) { +uint32_t intid; + +local_irq_disable (); + +gic_set_priority (MIN_PPI ,IRQ_DEFAULT_PRIO ); +gic_irq_enable (MIN_SPI ); +gic_irq_set_pending (MIN_SPI ); + + intid =wait_for_and_activate_irq (); +GUEST_ASSERT_EQ (intid,MIN_SPI ); + +gic_set_eoi (intid); +isb (); + +WRITE_ONCE (args->shared_data,MIN_SPI ); +dsb (ishst); + +do { +dsb (ishld); + }while (READ_ONCE (args->shared_data) ==MIN_SPI ); +GUEST_ASSERT (!gic_irq_get_active (MIN_SPI )); + }else { +do { +dsb (ishld); + }while (READ_ONCE (args->shared_data) !=MIN_SPI ); + +gic_set_dir (MIN_SPI ); +isb (); + +WRITE_ONCE (args->shared_data,0 ); +dsb (ishst); + } + +GUEST_DONE (); +} + +static void *test_vcpu_run (void *arg) +{ +struct kvm_vcpu *vcpu = arg; +struct ucall uc; + +while (1 ) { +vcpu_run (vcpu); + +switch (get_ucall (vcpu, &uc)) { +case UCALL_ABORT : +REPORT_GUEST_ASSERT (uc); +break ; +case UCALL_DONE : +return NULL ; +default : +TEST_FAIL ("Unknown ucall %lu" , uc.cmd); + } + } + +return NULL ; +} + +static void test_vgic_two_cpus (void *gcode) +{ +pthread_t thr[2 ]; +struct kvm_vcpu *vcpus[2 ]; +struct test_args args = {}; +struct kvm_vm *vm; +vm_vaddr_t args_gva; +int gic_fd, ret; + + vm =vm_create_with_vcpus (2 , gcode, vcpus); + +vm_init_descriptor_tables (vm); +vcpu_init_descriptor_tables (vcpus[0 ]); +vcpu_init_descriptor_tables (vcpus[1 ]); + +/* Setup the guest args page (so it gets the args). */ + args_gva =vm_vaddr_alloc_page (vm); +memcpy (addr_gva2hva (vm, args_gva), &args,sizeof (args)); +vcpu_args_set (vcpus[0 ],2 , args_gva,0 ); +vcpu_args_set (vcpus[1 ],2 , args_gva,1 ); + + gic_fd =vgic_v3_setup (vm,2 ,64 ); + + ret =pthread_create (&thr[0 ],NULL , test_vcpu_run, vcpus[0 ]); +if (ret) +TEST_FAIL ("Can't create thread for vcpu 0 (%d)\n" , ret); + ret =pthread_create (&thr[1 ],NULL , test_vcpu_run, vcpus[1 ]); +if (ret) +TEST_FAIL ("Can't create thread for vcpu 1 (%d)\n" , ret); + +pthread_join (thr[0 ],NULL ); +pthread_join (thr[1 ],NULL ); + +close (gic_fd); +kvm_vm_free (vm); +} +static void help (const char *name) { printf(
@@ -857,6 +961,7 @@ int main(int argc, char **argv)test_vgic (nr_irqs, false/* level */ , true/* eoi_split */ );test_vgic (nr_irqs, true/* level */ , false/* eoi_split */ );test_vgic (nr_irqs, true/* level */ , true/* eoi_split */ ); +test_vgic_two_cpus (guest_code_asym_dir); }else {test_vgic (nr_irqs, level_sensitive, eoi_split); }
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 48/49 KVM: arm64: selftests: vgic_irq: Add Group-0 enable test
In reply to Marc ZyngierAdd a new test case that inject a Group-0 interrupt together with a bunch of Group-1 interrupts, Ack/EOI the G1 interrupts, and only then enable G0, expecting to get the G0 interrupt.
--- tools/testing/selftests/kvm/arm64/vgic_irq.c | 49 ++++++++++++++++++++ 1 file changed, 49 insertions(+)
tools/testing/selftests/kvm/arm64/vgic_irq.c +49 -0
@@ -846,6 +846,54 @@ static void guest_code_asym_dir(struct test_args *args, int cpuid)GUEST_DONE (); } +static void guest_code_group_en (struct test_args *args,int cpuid) +{ +uint32_t intid; + +gic_init (GIC_V3 ,2 ); + +gic_set_eoi_split (0 ); +gic_set_priority_mask (CPU_PRIO_MASK ); +/* SGI0 is G0, which is disabled */ +gic_irq_set_group (0 ,0 ); + +/* Configure all SGIs with decreasing priority */ +for (intid =0 ; intid <MIN_PPI ; intid++) { +gic_set_priority (intid, (intid +1 ) *8 ); +gic_irq_enable (intid); +gic_irq_set_pending (intid); + } + +/* Ack and EOI all G1 interrupts */ +for (int i =1 ; i <MIN_PPI ; i++) { + intid =wait_for_and_activate_irq (); + +GUEST_ASSERT (intid <MIN_PPI ); +gic_set_eoi (intid); +isb (); + } + +/* +* Check that SGI0 is still pending, inactive, and that we cannot +* ack anything. +*/ +GUEST_ASSERT (gic_irq_get_pending (0 )); +GUEST_ASSERT (!gic_irq_get_active (0 )); +GUEST_ASSERT_IAR_EMPTY (); +GUEST_ASSERT (read_sysreg_s (SYS_ICC_IAR0_EL1 ) ==IAR_SPURIOUS ); + +/* Open the G0 gates, and verify we can ack SGI0 */ +write_sysreg_s (1 ,SYS_ICC_IGRPEN0_EL1 ); +isb (); + +do { + intid =read_sysreg_s (SYS_ICC_IAR0_EL1 ); + }while (intid ==IAR_SPURIOUS ); + +GUEST_ASSERT (intid ==0 ); +GUEST_DONE (); +} +static void *test_vcpu_run (void *arg) {struct kvm_vcpu *vcpu = arg;
@@ -962,6 +1010,7 @@ int main(int argc, char **argv)test_vgic (nr_irqs, true/* level */ , false/* eoi_split */ );test_vgic (nr_irqs, true/* level */ , true/* eoi_split */ );test_vgic_two_cpus (guest_code_asym_dir); +test_vgic_two_cpus (guest_code_group_en); }else {test_vgic (nr_irqs, level_sensitive, eoi_split); }
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH v4 49/49 KVM: arm64: selftests: vgic_irq: Add timer deactivation test
In reply to Marc ZyngierAdd a new test case that triggers the HW deactivation emulation path when trapping ICV_DIR_EL1. This is obviously tied to the way KVM works now, but the test follows the expected architectural behaviour.
--- tools/testing/selftests/kvm/arm64/vgic_irq.c | 65 ++++++++++++++++++++ 1 file changed, 65 insertions(+)
tools/testing/selftests/kvm/arm64/vgic_irq.c +65 -0
@@ -894,6 +894,70 @@ static void guest_code_group_en(struct test_args *args, int cpuid)GUEST_DONE (); } +static void guest_code_timer_spi (struct test_args *args,int cpuid) +{ +uint32_t intid; +u64 val; + +gic_init (GIC_V3 ,2 ); + +gic_set_eoi_split (1 ); +gic_set_priority_mask (CPU_PRIO_MASK ); + +/* Add a pending SPI so that KVM starts trapping DIR */ +gic_set_priority (MIN_SPI + cpuid,IRQ_DEFAULT_PRIO ); +gic_irq_set_pending (MIN_SPI + cpuid); + +/* Configure the timer with a higher priority, make it pending */ +gic_set_priority (27 ,IRQ_DEFAULT_PRIO -8 ); + +isb (); + val =read_sysreg (cntvct_el0); +write_sysreg (val, cntv_cval_el0); +write_sysreg (1 , cntv_ctl_el0); +isb (); + +GUEST_ASSERT (gic_irq_get_pending (27 )); + +/* Enable both interrupts */ +gic_irq_enable (MIN_SPI + cpuid); +gic_irq_enable (27 ); + +/* The timer must fire */ + intid =wait_for_and_activate_irq (); +GUEST_ASSERT (intid ==27 ); + +/* Check that we can deassert it */ +write_sysreg (0 , cntv_ctl_el0); +isb (); + +GUEST_ASSERT (!gic_irq_get_pending (27 )); + +/* +* Priority drop, deactivation -- we expect that the host +* deactivation will have been effective +*/ +gic_set_eoi (27 ); +gic_set_dir (27 ); + +GUEST_ASSERT (!gic_irq_get_active (27 )); + +/* Do it one more time */ +isb (); + val =read_sysreg (cntvct_el0); +write_sysreg (val, cntv_cval_el0); +write_sysreg (1 , cntv_ctl_el0); +isb (); + +GUEST_ASSERT (gic_irq_get_pending (27 )); + +/* The timer must fire again */ + intid =wait_for_and_activate_irq (); +GUEST_ASSERT (intid ==27 ); + +GUEST_DONE (); +} +static void *test_vcpu_run (void *arg) {struct kvm_vcpu *vcpu = arg;
@@ -1011,6 +1075,7 @@ int main(int argc, char **argv)test_vgic (nr_irqs, true/* level */ , true/* eoi_split */ );test_vgic_two_cpus (guest_code_asym_dir);test_vgic_two_cpus (guest_code_group_en); +test_vgic_two_cpus (guest_code_timer_spi); }else {test_vgic (nr_irqs, level_sensitive, eoi_split); }
Signature · 1 line
2.47.3
Quoted 7 lines · Marc Zyngier
As $SUBJECT says, I really hope this is the last dance for this particular series -- I'm done with it! It was supposed to be a 5 patch job, and we're close to 50. Something went really wrong...
Most of the fixes have now been squashed back into the base patches, and the only new patch is plugging the deactivation helper into the NV code, making it more correct.
I threw this at my CI and it seemed happy:
Quoted 9 lines · Marc Zyngier
As $SUBJECT says, I really hope this is the last dance for this particular series -- I'm done with it! It was supposed to be a 5 patch job, and we're close to 50. Something went really wrong...
Most of the fixes have now been squashed back into the base patches, and the only new patch is plugging the deactivation helper into the NV code, making it more correct.
[...]
Applied to next, thanks!
[01/49] irqchip/gic: Add missing GICH_HCR control bits
https://git.kernel.org/kvmarm/kvmarm/c/8cb4ecec5e36 [02/49] irqchip/gic: Expose CPU interface VA to KVM https://git.kernel.org/kvmarm/kvmarm/c/fa8f11e8e183 [03/49] irqchip/apple-aic: Spit out ICH_MISR_EL2 value on spurious vGIC MI https://git.kernel.org/kvmarm/kvmarm/c/08f4f41c1e95 [04/49] KVM: arm64: Turn vgic-v3 errata traps into a patched-in constant https://git.kernel.org/kvmarm/kvmarm/c/8d3dfab1d305 [05/49] KVM: arm64: vgic-v3: Fix GICv3 trapping in protected mode https://git.kernel.org/kvmarm/kvmarm/c/567ebfedb5bd [06/49] KVM: arm64: GICv3: Detect and work around the lack of ICV_DIR_EL1 trapping https://git.kernel.org/kvmarm/kvmarm/c/2a28810cbb8b [07/49] KVM: arm64: Repack struct vgic_irq fields https://git.kernel.org/kvmarm/kvmarm/c/a4413a7c31cf [08/49] KVM: arm64: Add tracking of vgic_irq being present in a LR https://git.kernel.org/kvmarm/kvmarm/c/879a7fd4fd64 [09/49] KVM: arm64: Add LR overflow handling documentation https://git.kernel.org/kvmarm/kvmarm/c/0dc433e79ad0 [10/49] KVM: arm64: GICv3: Drop LPI active state when folding LRs https://git.kernel.org/kvmarm/kvmarm/c/73c9726975af [11/49] KVM: arm64: GICv3: Preserve EOIcount on exit https://git.kernel.org/kvmarm/kvmarm/c/f4ded7b0848e [12/49] KVM: arm64: GICv3: Decouple ICH_HCR_EL2 programming from LRs https://git.kernel.org/kvmarm/kvmarm/c/00c6d0d4a805 [13/49] KVM: arm64: GICv3: Extract LR folding primitive https://git.kernel.org/kvmarm/kvmarm/c/438e47b697f7 [14/49] KVM: arm64: GICv3: Extract LR computing primitive https://git.kernel.org/kvmarm/kvmarm/c/1ae0448ca797 [15/49] KVM: arm64: GICv2: Preserve EOIcount on exit https://git.kernel.org/kvmarm/kvmarm/c/5ceb3dac8022 [16/49] KVM: arm64: GICv2: Decouple GICH_HCR programming from LRs being loaded https://git.kernel.org/kvmarm/kvmarm/c/a00c88ac1f90 [17/49] KVM: arm64: GICv2: Extract LR folding primitive https://git.kernel.org/kvmarm/kvmarm/c/3aa9a50c2007 [18/49] KVM: arm64: GICv2: Extract LR computing primitive https://git.kernel.org/kvmarm/kvmarm/c/0660bc4a2b70 [19/49] KVM: arm64: Compute vgic state irrespective of the number of interrupts https://git.kernel.org/kvmarm/kvmarm/c/dd598fc1139f [20/49] KVM: arm64: Eagerly save VMCR on exit https://git.kernel.org/kvmarm/kvmarm/c/cf72ee637119 [21/49] KVM: arm64: Revamp vgic maintenance interrupt configuration https://git.kernel.org/kvmarm/kvmarm/c/6780a756044c [22/49] KVM: arm64: Turn kvm_vgic_vcpu_enable() into kvm_vgic_vcpu_reset() https://git.kernel.org/kvmarm/kvmarm/c/f04b8a5a83db [23/49] KVM: arm64: Make vgic_target_oracle() globally available https://git.kernel.org/kvmarm/kvmarm/c/76b2eda65ccc [24/49] KVM: arm64: Invert ap_list sorting to push active interrupts out https://git.kernel.org/kvmarm/kvmarm/c/05984ba67eb6 [25/49] KVM: arm64: Move undeliverable interrupts to the end of ap_list https://git.kernel.org/kvmarm/kvmarm/c/33c1f60b3213 [26/49] KVM: arm64: Use MI to detect groups being enabled/disabled https://git.kernel.org/kvmarm/kvmarm/c/a69e2d6f8934 [27/49] KVM: arm64: GICv3: Handle LR overflow when EOImode==0 https://git.kernel.org/kvmarm/kvmarm/c/3cfd59f81e0f [28/49] KVM: arm64: GICv3: Handle deactivation via ICV_DIR_EL1 traps https://git.kernel.org/kvmarm/kvmarm/c/cd4f6ee99b28 [29/49] KVM: arm64: GICv3: Add GICv2 SGI handling to deactivation primitive https://git.kernel.org/kvmarm/kvmarm/c/295b69216558 [30/49] KVM: arm64: GICv3: Set ICH_HCR_EL2.TDIR when interrupts overflow LR capacity https://git.kernel.org/kvmarm/kvmarm/c/70fd60bdedc9 [31/49] KVM: arm64: GICv3: Add SPI tracking to handle asymmetric deactivation https://git.kernel.org/kvmarm/kvmarm/c/1c3b3cadcd69 [32/49] KVM: arm64: GICv3: Handle in-LR deactivation when possible https://git.kernel.org/kvmarm/kvmarm/c/ca3c34da3644 [33/49] KVM: arm64: GICv3: Avoid broadcast kick on CPUs lacking TDIR https://git.kernel.org/kvmarm/kvmarm/c/84792050e039 [34/49] KVM: arm64: GICv3: nv: Resync LRs/VMCR/HCR early for better MI emulation https://git.kernel.org/kvmarm/kvmarm/c/eb33ffa2bd3f [35/49] KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive https://git.kernel.org/kvmarm/kvmarm/c/6dd333c8942b [36/49] KVM: arm64: GICv3: Force exit to sync ICH_HCR_EL2.En https://git.kernel.org/kvmarm/kvmarm/c/78ffc28456f5 [37/49] KVM: arm64: GICv2: Handle LR overflow when EOImode==0 https://git.kernel.org/kvmarm/kvmarm/c/281c6c06e2a7 [38/49] KVM: arm64: GICv2: Handle deactivation via GICV_DIR traps https://git.kernel.org/kvmarm/kvmarm/c/255de897e7fb [39/49] KVM: arm64: GICv2: Always trap GICV_DIR register https://git.kernel.org/kvmarm/kvmarm/c/07bb1c5622a5 [40/49] KVM: arm64: selftests: gic_v3: Add irq group setting helper https://git.kernel.org/kvmarm/kvmarm/c/a1650de7c160 [41/49] KVM: arm64: selftests: gic_v3: Disable Group-0 interrupts by default https://git.kernel.org/kvmarm/kvmarm/c/2366295c76c2 [42/49] KVM: arm64: selftests: vgic_irq: Fix GUEST_ASSERT_IAR_EMPTY() helper https://git.kernel.org/kvmarm/kvmarm/c/27392612c882 [43/49] KVM: arm64: selftests: vgic_irq: Change configuration before enabling interrupt https://git.kernel.org/kvmarm/kvmarm/c/8b7888c5114d [44/49] KVM: arm64: selftests: vgic_irq: Exclude timer-controlled interrupts https://git.kernel.org/kvmarm/kvmarm/c/5053c2ab92a1 [45/49] KVM: arm64: selftests: vgic_irq: Remove LR-bound limitation https://git.kernel.org/kvmarm/kvmarm/c/fd5fa1c8d09a [46/49] KVM: arm64: selftests: vgic_irq: Perform EOImode==1 deactivation in ack order https://git.kernel.org/kvmarm/kvmarm/c/b6c68612ab41 [47/49] KVM: arm64: selftests: vgic_irq: Add asymmetric SPI deaectivation test https://git.kernel.org/kvmarm/kvmarm/c/d2dee2e84983 [48/49] KVM: arm64: selftests: vgic_irq: Add Group-0 enable test https://git.kernel.org/kvmarm/kvmarm/c/1c9c71ac1b9f [49/49] KVM: arm64: selftests: vgic_irq: Add timer deactivation test https://git.kernel.org/kvmarm/kvmarm/c/de8842327728
--
Best,
Oliver