Re: [PATCH v5 19/24] KVM: arm64: Implement lazy PMU context swaps
From: Oliver Upton <oupton@kernel.org>
Date: 2025-12-09 22:06:55
Also in:
kvm, kvmarm, linux-doc, linux-kselftest, linux-perf-users, lkml
On Tue, Dec 09, 2025 at 08:51:16PM +0000, Colton Lewis wrote:
+enum vcpu_pmu_register_access {
+ VCPU_PMU_ACCESS_UNSET,
+ VCPU_PMU_ACCESS_VIRTUAL,
+ VCPU_PMU_ACCESS_PHYSICAL,
+};This is confusing. Even when the guest is accessing registers directly on the CPU I'd still call that "hardware assisted virtualization" and not "physical".
quoted hunk ↗ jump to hunk
+#endif /* _ASM_ARM64_KVM_TYPES_H */diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c index 0ab89c91e19cb..c2cf6b308ec60 100644 --- a/arch/arm64/kvm/debug.c +++ b/arch/arm64/kvm/debug.c@@ -34,7 +34,7 @@ static int cpu_has_spe(u64 dfr0) * - Self-hosted Trace Filter controls (MDCR_EL2_TTRF) * - Self-hosted Trace (MDCR_EL2_TTRF/MDCR_EL2_E2TB) */ -static void kvm_arm_setup_mdcr_el2(struct kvm_vcpu *vcpu) +void kvm_arm_setup_mdcr_el2(struct kvm_vcpu *vcpu) { int hpmn = kvm_pmu_hpmn(vcpu);diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h index bde79ec1a1836..ea288a712bb5d 100644 --- a/arch/arm64/kvm/hyp/include/hyp/switch.h +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h@@ -963,6 +963,8 @@ static bool kvm_hyp_handle_pmu_regs(struct kvm_vcpu *vcpu) if (ret) __kvm_skip_instr(vcpu); + kvm_pmu_set_physical_access(vcpu); + return ret; }diff --git a/arch/arm64/kvm/pmu-direct.c b/arch/arm64/kvm/pmu-direct.c index 8d0d6d1a0d851..c5767e2ebc651 100644 --- a/arch/arm64/kvm/pmu-direct.c +++ b/arch/arm64/kvm/pmu-direct.c@@ -73,6 +73,7 @@ bool kvm_vcpu_pmu_use_fgt(struct kvm_vcpu *vcpu) u8 hpmn = vcpu->kvm->arch.nr_pmu_counters; return kvm_vcpu_pmu_is_partitioned(vcpu) && + vcpu->arch.pmu.access == VCPU_PMU_ACCESS_PHYSICAL && cpus_have_final_cap(ARM64_HAS_FGT) && (hpmn != 0 || cpus_have_final_cap(ARM64_HAS_HPMN0)); }@@ -92,6 +93,26 @@ u64 kvm_pmu_fgt2_bits(void) | HDFGRTR2_EL2_nPMICNTR_EL0; } +/** + * kvm_pmu_set_physical_access() + * @vcpu: Pointer to vcpu struct + * + * Reconfigure the guest for physical access of PMU hardware if + * allowed. This means reconfiguring mdcr_el2 and loading the vCPU + * state onto hardware. + * + */ + +void kvm_pmu_set_physical_access(struct kvm_vcpu *vcpu) +{ + if (kvm_vcpu_pmu_is_partitioned(vcpu) + && vcpu->arch.pmu.access == VCPU_PMU_ACCESS_VIRTUAL) { + vcpu->arch.pmu.access = VCPU_PMU_ACCESS_PHYSICAL; + kvm_arm_setup_mdcr_el2(vcpu); + kvm_pmu_load(vcpu); + }
It isn't immediately obvious how this guards against preemption. Also, the general approach for these context-loading situations is to do a full load/put on the vCPU rather than a directed load.
+static void kvm_pmu_register_init(struct kvm_vcpu *vcpu)
+{
+ if (vcpu->arch.pmu.access == VCPU_PMU_ACCESS_UNSET)
+ vcpu->arch.pmu.access = VCPU_PMU_ACCESS_VIRTUAL;This is confusing. The zero value of the enum should be consistent with the "unloaded" state.
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index f2ae761625a66..d73218706b834 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c@@ -1197,6 +1197,8 @@ static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, struct sys_reg_params *p, p->regval = __vcpu_sys_reg(vcpu, reg); } + kvm_pmu_set_physical_access(vcpu); + return true; }@@ -1302,6 +1304,8 @@ static bool access_pmovs(struct kvm_vcpu *vcpu, struct sys_reg_params *p, p->regval = __vcpu_sys_reg(vcpu, PMOVSSET_EL0); } + kvm_pmu_set_physical_access(vcpu); + return true; }
Aren't there a ton of other registers the guest may access before these two? Having generic PMU register accessors would allow you to manage residence of PMU registers from a single spot. Thanks, Oliver