Re: [PATCH v9 11/22] KVM: arm64: Set up MDCR_EL2 to handle a Partitioned PMU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] KVM: arm64: Missing validation allows 0 PMU counters on CPUs lacking FEAT_HPMN0, breaking host PMU profiling
- [High] KVM: arm64: local_irq_save() fails to mask pseudo-NMIs, causing TOCTOU race on mdcr_el2
--

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

KVM: arm64: Set up MDCR_EL2 to handle a Partitioned PMU

This commit sets up MDCR_EL2 to handle partitioned PMUs, configuring the
HPME, HPMD, and HCCD bits when a partitioned PMU is used. It also unsets
coarse grain PMU traps if fine grain traps are available and protects
mdcr_el2 updates using local_irq_save() to prevent races with the PMU NMI
handler.

> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index f4d7b12045e8f..a722fd4594e09 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -37,14 +37,16 @@ static int cpu_has_spe(u64 dfr0)
>   */
>  static void kvm_arm_setup_mdcr_el2(struct kvm_vcpu *vcpu)
>  {
> -	preempt_disable();
> +	unsigned long flags;
> +
> +	local_irq_save(flags);
        ^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Does this lock adequately protect against PMU interrupts?

On ARM64 systems with GICv3, the perf subsystem can use pseudo-NMIs for PMU
interrupts, and local_irq_save() does not mask NMIs. This leaves a structurally
open race window where a PMU NMI could fire during the read-modify-write phase
of vcpu->arch.mdcr_el2 inside kvm_arm_setup_mdcr_el2().

[ ... ]

> +		/*
> +		 * Take out the coarse grain traps if we are using
> +		 * fine grain traps and enforce counter access with
> +		 * HPMN.
> +		 */
> +		if (!vcpu_on_unsupported_cpu(vcpu) &&
> +		    (cpus_have_final_cap(ARM64_HAS_HPMN0) || nr_guest_cntr > 0)) {
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Can host userspace break host PMU profiling by configuring 0 guest counters?

If host userspace configures a partitioned PMU with 0 guest counters via
KVM_ARM_VCPU_PMU_V3_SET_NR_COUNTERS on a CPU where FEAT_HPMN0 is unsupported,
this condition evaluates to false and skips configuring HPMN.

Because KVM skips HPMN configuration when nr_guest_cntr == 0 on these older CPUs,
HPMN defaults to N. This transfers all host counters to PMCR_EL0.E control and
renders MDCR_EL2.HPME entirely ineffective for the host, which loses the ability
to stop host counters. This continuous hardware counting could lead to NMI storms.

Should KVM reject such invalid configurations on CPUs lacking FEAT_HPMN0 instead?

> +			vcpu->arch.mdcr_el2 &= ~MDCR_EL2_HPMN;
> +			vcpu->arch.mdcr_el2 |= FIELD_PREP(MDCR_EL2_HPMN, nr_guest_cntr);
> +
> +			if (cpus_have_final_cap(ARM64_HAS_FGT))
> +				vcpu->arch.mdcr_el2 &= ~(MDCR_EL2_TPM | MDCR_EL2_TPMCR);
> +		}
> +	}
> +

[ ... ]

>  	/* Write MDCR_EL2 directly if we're already at EL2 */
>  	if (has_vhe())
>  		write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
If a PMU pseudo-NMI fires during the critical section and calls
kvm_pmu_host_stop() to clear HPME, will this write clobber that update?

Because local_irq_save() fails to mask NMIs, this line writes the potentially
stale, cached vcpu->arch.mdcr_el2 state back to hardware, discarding any
intermediate HPME updates from the NMI handler and leaving PMU counters in the
wrong hardware state.

>  
> -	preempt_enable();
> +	local_irq_restore(flags);
>  }

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