Re: [PATCH 14/21] KVM: arm64: Apply dynamic guest counter reservations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Data race and state corruption of the global `arm_pmu->cntr_mask` when dynamically reserving guest PMU counters.
- [High] Lockless access to `cpuc->events[idx]` in `kvm_pmu_set_guest_counters()` can cause a NULL pointer dereference (kernel oops) if interrupted by a concurrent perf IPI.
--

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

KVM: arm64: Apply dynamic guest counter reservations

Apply dynamic guest counter reservations by checking if the requested
guest mask collides with any events the host has scheduled and calling
pmu_perf_resched_update() with a hook that updates the mask of
available counters in between schedule out and schedule in.

> diff --git a/arch/arm64/kvm/pmu-direct.c b/arch/arm64/kvm/pmu-direct.c
> index 49f1feb5d280c..044f011c9c84b 100644
> --- a/arch/arm64/kvm/pmu-direct.c
> +++ b/arch/arm64/kvm/pmu-direct.c
> @@ -87,6 +87,73 @@ u64 kvm_pmu_direct_pmcr_read(struct kvm_vcpu *vcpu)
>  		ARMV8_PMU_PMCR_N);
>  }
>  
> +/* Callback to update counter mask between perf scheduling */
> +static void kvm_pmu_update_mask(struct pmu *pmu, void *data)
> +{
> +	struct arm_pmu *arm_pmu = to_arm_pmu(pmu);
> +	unsigned long *new_mask = data;
> +
> +	bitmap_copy(arm_pmu->cntr_mask, new_mask, ARMPMU_MAX_HWEVENTS);

[Severity: Critical]
Does this locklessly overwrite the shared arm_pmu->cntr_mask?

Since struct arm_pmu and its cntr_mask are shared across all CPUs in a PMU
cluster, altering it dynamically from a per-CPU context without cross-CPU
locking can cause torn writes and clobber reservations for other CPUs sharing
the PMU.

> +}
> +
> +/**
> + * kvm_pmu_set_guest_counters() - Handle dynamic counter reservations
> + * @cpu_pmu: struct arm_pmu to potentially modify
> + * @guest_mask: new guest mask for the pmu

[ ... ]

> +	if (guest_mask) {
> +		/* Subtract guest counters from available host mask */
> +		bitmap_andnot(new_mask, new_mask, guest_bitmap, ARMPMU_MAX_HWEVENTS);
> +
> +		/* Did we collide with an active host event? */
> +		if (bitmap_intersects(cpuc->used_mask, guest_bitmap, ARMPMU_MAX_HWEVENTS)) {
> +			int idx;
> +
> +			need_resched = true;
> +			cpuc->host_squeezed = true;
> +
> +			/* Look for pinned events that are about to be preempted */
> +			for_each_set_bit(idx, guest_bitmap, ARMPMU_MAX_HWEVENTS) {
> +				if (test_bit(idx, cpuc->used_mask) && cpuc->events[idx] &&
> +				    cpuc->events[idx]->attr.pinned) {

[Severity: High]
Can this result in a NULL pointer dereference?

The function iterates over cpuc->events[idx] with preemption disabled but
local IRQs enabled. If a concurrent perf IPI interrupts KVM between the two
compiler-generated loads of cpuc->events[idx] and frees the event slot
(setting it to NULL), the subsequent dereference of ->attr.pinned will cause a
fatal kernel oops.

Using READ_ONCE() for the load would ensure the compiler reads it only once.

> +					pr_warn_once("perf: Pinned host event squeezed out by KVM guest PMU partition\n");
> +					break;
> +				}
> +			}
> +		}
> +	} else {

[ ... ]

> +	if (need_resched) {
> +		/* Collision: run full perf reschedule */
> +		perf_pmu_resched_update(&cpu_pmu->pmu, kvm_pmu_update_mask, new_mask);
> +	} else {
> +		/* Host was never using guest counters anyway */
> +		bitmap_copy(cpu_pmu->cntr_mask, new_mask, ARMPMU_MAX_HWEVENTS);

[Severity: Critical]
Does this restoration also silently destroy active reservations for other
CPUs in the cluster?

When one CPU exits a guest (kvm_pmu_put()), this unconditionally restores
cntr_mask to hw_cntr_impl, which would remove any active PMU counter
reservations for guests concurrently running on other CPUs. This might allow
the host perf subsystem to allocate those guest-reserved counters, leading to
PMU register corruption and interrupt storms.

> +	}
> +}

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