Re: [PATCH 12/21] KVM: arm64: Enforce PMU event filter at vcpu_load()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Missing Context Synchronization Event (isb) after writing PMU event configuration registers before enabling counters.
- [High] Guest PMU counters erroneously count guest EL2 execution when the guest hypervisor requested NOT to count at EL2.

Pre-existing issues:
- [High] Guest writes to PMEVTYPERn_EL0 and PMCCFILTR_EL0 under a partitioned PMU are trapped but not synchronously propagated to the hardware PMU, breaking dynamic PMU configuration.
--

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

KVM: arm64: Enforce PMU event filter at vcpu_load()

This commit enforces the KVM PMU event filter by rechecking and applying
the filter on every vcpu load. It excludes counting at all exception levels
if the event is filtered before writing to the hardware.

diff --git a/arch/arm64/kvm/pmu-direct.c b/arch/arm64/kvm/pmu-direct.c
--- a/arch/arm64/kvm/pmu-direct.c
+++ b/arch/arm64/kvm/pmu-direct.c
@@ -131,6 +131,57 @@ u64 kvm_pmu_guest_counter_mask(struct arm_pmu *pmu)

[ ... ]

> +	for_each_set_bit(i, &guest_counters, ARMPMU_MAX_HWEVENTS) {
> +		if (i == ARMV8_PMU_CYCLE_IDX) {
> +			val = __vcpu_sys_reg(vcpu, PMCCFILTR_EL0);
> +			evsel = ARMV8_PMUV3_PERFCTR_CPU_CYCLES;
> +		} else {
> +			val = __vcpu_sys_reg(vcpu, PMEVTYPER0_EL0 + i);
> +			evsel = val & kvm_pmu_event_mask(vcpu->kvm);
> +		}
> +
> +		guest_include_el2 = (val & ARMV8_PMU_INCLUDE_EL2);
> +		val &= ~evtyper_clr;
> +
> +		if (unlikely(is_hyp_ctxt(vcpu)) && guest_include_el2)
> +			val &= ~ARMV8_PMU_EXCLUDE_EL1;

[Severity: High]
When the guest is executing at guest EL2 (which runs at host EL1 under
nested virtualization), does this correctly handle the case where the guest
requested not to count at guest EL2?

If guest_include_el2 is false, but the guest's configuration originally had
ARMV8_PMU_EXCLUDE_EL1 cleared (for example, to profile guest EL1), this code
leaves it cleared. Will this cause the hardware PMU to erroneously count
guest EL2 execution at host EL1?

> +
> +		if (vcpu->kvm->arch.pmu_filter &&
> +		    !test_bit(evsel, vcpu->kvm->arch.pmu_filter))
> +			val |= evtyper_set;
> +
> +		if (i == ARMV8_PMU_CYCLE_IDX)
> +			write_pmccfiltr(val);
> +		else
> +			write_pmevtypern(i, val);
> +	}
> +}

[Severity: High]
Does this sequence require a context synchronization event?

According to the ARM64 architecture rules, writes to control-plane system
registers must be followed by an isb() as the very next instruction. This
code loops over the counters and writes the registers without issuing an
isb().

Later, kvm_pmu_load() writes to PMCNTENSET_EL0 to enable the counters:

kvm_pmu_load() {
        ...
        kvm_pmu_apply_event_filter(vcpu);
        ...
        val = __vcpu_sys_reg(vcpu, PMCNTENSET_EL0);
        write_sysreg(val & mask, pmcntenset_el0);
}

Without an intervening isb(), could the PMU hardware start counting before
the pipeline resolves the new event configurations, leading to spurious
event counts or PMU filter bypass?

[Severity: High]
This is a pre-existing issue, but does applying the event filter only during
vcpu_load() break dynamic PMU configuration when the PMU is partitioned?

When the partitioned PMU is enabled, KVM traps guest writes to PMEVTYPERn_EL0
in pmu_reg_write() in arch/arm64/kvm/sys_regs.c:

pmu_reg_write() {
        ...
        if (kvm_pmu_is_partitioned(vcpu->kvm)) {
                mask = kvm_pmu_evtyper_mask(vcpu->kvm);
                __vcpu_assign_sys_reg(vcpu, reg, val & mask);
        }
        ...
}

This updates the in-memory sys_regs array but fails to push the new value to
the physical hardware registers or request a KVM_REQ_RELOAD_PMU.

If the guest subsequently enables the counter (which writes directly to the
physical PMCNTENSET_EL0 without a trap delay), the hardware PMU will use the
stale event type. The new configuration and the event filter applied here in
kvm_pmu_apply_event_filter() are delayed until the next VCPU reschedule.

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