On 02/02/2024 22:00, Oliver Upton wrote:
On Thu, Jan 04, 2024 at 04:27:02PM +0000, James Clark wrote:
[...]
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c index c50f8459e4fc..89147a9dc38c 100644 --- a/arch/arm64/kvm/hyp/nvhe/switch.c +++ b/arch/arm64/kvm/hyp/nvhe/switch.c @@ -130,13 +130,18 @@ static void __hyp_vgic_restore_state(struct kvm_vcpu *vcpu) } } +static struct kvm_pmu_events *kvm_nvhe_get_pmu_events(struct kvm_vcpu *vcpu) +{
- return &kvm_host_global_state[vcpu->cpu].pmu_events;
+}
/*
- Disable host events, enable guest events
*/ #ifdef CONFIG_HW_PERF_EVENTS static bool __pmu_switch_to_guest(struct kvm_vcpu *vcpu) {
- struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
- struct kvm_pmu_events *pmu = kvm_nvhe_get_pmu_events(vcpu);
if (pmu->events_host) write_sysreg(pmu->events_host, pmcntenclr_el0); @@ -152,7 +157,7 @@ static bool __pmu_switch_to_guest(struct kvm_vcpu *vcpu) */ static void __pmu_switch_to_host(struct kvm_vcpu *vcpu) {
- struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
- struct kvm_pmu_events *pmu = kvm_nvhe_get_pmu_events(vcpu);
if (pmu->events_guest) write_sysreg(pmu->events_guest, pmcntenclr_el0);
This now allows the host to program event counters for a protected guest. That _might_ be a useful feature behind some debug option, but is most definitely *not* something we want to do for pVMs generally.
Unless I'm missing something, using PMUs on protected guests was added by 722625c6f4c5b ("KVM: arm64: Reenable pmu in Protected Mode"). This change is just a refactor that will allow us to add the same behavior for a similar feature (tracing) without adding yet another copy of some state before the guest switch.
Do we even need to make this shared data work at all for pKVM? The rest of the shared data between pKVM and the kernel is system information, which (importantly) doesn't have any guest context in it.
Probably not, Marc actually mentioned on one of the first versions of that this could be hidden behind a debug flag. To be honest one of the reasons I didn't do that was because I wasn't sure what the appropriate debug setting was. NVHE_EL2_DEBUG didn't seem quite right. DEBUG_KERNEL maybe? Or a new one?
And then I suppose I got distracted by trying to make it have feature parity with PMUs and forgot about the debug only thing.
I'm perfectly happy leaving these sorts of features broken for pKVM and using the 'normal' way of getting percpu data to the nVHE hypervisor otherwise.
I can do that. But do I also disable PMU at the same time in a new commit? Now that both PMU and tracing is working maybe it would be a waste to throw that away and hiding it behind an option is better. Or I can leave the PMU as it is and just keep tracing disabled in pKVM.
I don't mind either way, my main goal was to get exclude/include guest tracing working for normal VMs. For pKVM I don't have a strong opinion.
-
James Clark