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".
+#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 &&} @@ -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.
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