Re: [PATCH v7 15/20] perf: arm_pmuv3: Handle IRQs for Partitioned PMU guest counters

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Non-atomic read-modify-write race on `PMOVSSET_EL0` corrupts guest PMU state.
- [High] Guest PMU interrupts are permanently dropped if they fire while the vCPU is scheduled out.
- [High] Missing KVM request fails to inject virtual PMU interrupts into the guest.
- [High] Pending or polled guest PMU overflow flags are silently lost during vCPU context switches.
--

commit a0dad1aa1988f327f4127d17e8307aeadf9dd34d
Author: Colton Lewis <coltonlewis@google.com>

perf: arm_pmuv3: Handle IRQs for Partitioned PMU guest counters

Because ARM hardware is not yet capable of direct PPI injection into
guests, guest counters will still trigger interrupts that need to be
handled by the host PMU interrupt handler. Clear the overflow flags in
hardware to handle the interrupt as normal, but the virtual overflow
register for later injecting the interrupt into the guest.

[ ... ]

> diff --git a/arch/arm64/kvm/pmu-direct.c b/arch/arm64/kvm/pmu-direct.c
> index 881cea5117515..535b4c492ff80 100644
> --- a/arch/arm64/kvm/pmu-direct.c
> +++ b/arch/arm64/kvm/pmu-direct.c
> @@ -411,3 +411,25 @@ void kvm_pmu_put(struct kvm_vcpu *vcpu)
>  	kvm_pmu_set_guest_counters(pmu, 0);
>  	preempt_enable();
>  }

Does this code need to save the guest's hardware overflow flags from
pmovsclr_el0 into the vCPU memory context when it is scheduled out?

If a guest counter overflows but the vCPU is preempted before the PMU IRQ
is delivered, or if the guest is intentionally polling the counters with
interrupts disabled, the hardware overflow bit remains set. Because this
function doesn't save the active hardware state, could the overflow be
lost across the context switch?

Also, should this function clear the hardware registers (pmcntenclr_el0
and pmintenclr_el1) to stop the guest counters? If they continue running
while the physical CPU executes host tasks, they can overflow and trigger
a physical PMU interrupt while the vCPU is scheduled out.

> +
> +/**
> + * kvm_pmu_handle_guest_irq() - Record IRQs in guest counters
> + * @pmu: PMU to check for overflows
> + * @pmovsr: Overflow flags reported by driver
> + *
> + * Set overflow flags in guest-reserved counters in the VCPU register
> + * for the guest to clear later.
> + */
> +void kvm_pmu_handle_guest_irq(struct arm_pmu *pmu, u64 pmovsr)
> +{
> +	struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
> +	u64 mask = kvm_pmu_guest_counter_mask(pmu);
> +	u64 govf = pmovsr & mask;
> +
> +	write_pmovsclr(govf);
> +
> +	if (!vcpu)
> +		return;

If the counters continue running while the vCPU is scheduled out, can
a physical PMU interrupt fire when kvm_get_running_vcpu() is NULL?

Since the handler clears the physical overflow flag via write_pmovsclr()
but immediately returns here without saving the overflow state to the
vCPU memory backing store, are guest PMU interrupts permanently dropped
in this scenario?

> +
> +	__vcpu_rmw_sys_reg(vcpu, PMOVSSET_EL0, |=, govf);

Could this non-atomic read-modify-write corrupt the guest PMU state?

kvm_pmu_handle_guest_irq() executes in hardirq context, while KVM FGT trap
handlers execute in KVM process context with local IRQs enabled. If a
physical PMU interrupt preempts the vCPU thread while it is in the middle
of a process-context read-modify-write on PMOVSSET_EL0, the hardirq
handler's modification could be completely overwritten and lost when the
interrupted trap handler writes back its stale value.

Additionally, since this executes unconditionally even if govf == 0,
could it create a non-atomic read/write cycle that blindly clobbers the
trap handler's concurrent modifications?

Finally, how does KVM know to inject the virtual PMU interrupt? Should
there be a call to kvm_make_request(KVM_REQ_PMU, vcpu) here? Without a
vCPU request to evaluate and route virtual interrupts to the VGIC, the
KVM run loop will re-enter the guest without asserting the vPPI, and the
guest will remain unaware of the overflow until an unrelated VM exit
occurs.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260504211813.1804997-1-coltonlewis@google.com?part=15