Re: [PATCH v9 20/22] KVM: arm64: Add vCPU device attr to partition the PMU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: Missing FEAT_HPMN0 validation allows PMU partition mismatch breaking host profiling
--

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

KVM: arm64: Add vCPU device attr to partition the PMU

This commit introduces the KVM_ARM_VCPU_PMU_V3_ENABLE_PARTITION attribute 
to enable partitioned PMU support for a VM. When partitioning is enabled,
userspace must configure the number of guest event counters strictly less 
than the maximum implemented by the PMU, reserving at least one for host 
profiling.

> diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> index 4a3c6600b2678..31e46a5e937c7 100644
> --- a/arch/arm64/kvm/pmu.c
> +++ b/arch/arm64/kvm/pmu.c
[ ... ]
> @@ -667,14 +694,26 @@ static int kvm_arm_pmu_v3_set_nr_counters(struct kvm_vcpu *vcpu, unsigned int n)
>  {
>  	struct kvm *kvm = vcpu->kvm;
> +	unsigned int max_counters;
> +
> +	if (kvm_vm_has_ran_once(kvm) ||
> +	    (kvm_arm_pmu_any_vcpu_created(kvm) &&
> +	     (!kvm->arch.pmu_nr_counters_specified ||
> +	      kvm->arch.nr_pmu_counters != n)))
> +		return -EBUSY;
>  
>  	if (!kvm->arch.arm_pmu)
>  		return -EINVAL;
>  
> -	if (n > kvm_arm_pmu_get_max_counters(kvm))
> +	max_counters = kvm_arm_pmu_get_max_counters(kvm);
> +	if (n > max_counters)
> +		return -EINVAL;
> +
> +	if (kvm_pmu_is_partitioned(kvm) && n >= max_counters)
>  		return -EINVAL;
>  
>  	kvm_arm_set_nr_counters(kvm, n);

[Severity: Medium]
The commit message states that the number of guest event counters must be 
configured "with a value strictly less than the maximum... or 0 only when 
FEAT_HPMN0 is supported". 

Is the FEAT_HPMN0 check missing from kvm_arm_pmu_v3_set_nr_counters() here?

If n is set to 0 on hardware without ARM64_HAS_HPMN0, the hardware fallback
in debug.c will prevent a panic by leaving HPMN set to max_counters. However,
kvm_pmu_host_counter_mask() will still assume all counters belong to the 
host. 

Because the hardware assigns all counters to the guest partition in this 
state, does this mismatch break host-side performance profiling?

[ ... ]
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 20c46ef705d30..a4779593c4f8d 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
[ ... ]
> @@ -1756,7 +1756,10 @@ static int set_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
>  	if (!kvm_vm_has_ran_once(kvm) &&
>  	    !vcpu_has_nv(vcpu)	      &&
>  	    !kvm_vcpu_has_pmuv3_strict(vcpu) &&
> -	    new_n <= kvm_arm_pmu_get_max_counters(kvm))
> +	    !kvm->arch.pmu_nr_counters_specified &&
> +	    new_n <= kvm_arm_pmu_get_max_counters(kvm) &&
> +	    (!kvm_pmu_is_partitioned(kvm) ||
> +	     new_n < kvm_arm_pmu_get_max_counters(kvm)))
>  		kvm->arch.nr_pmu_counters = new_n;

[Severity: Medium]
Can userspace also bypass the FEAT_HPMN0 validation by writing a 0 to 
PMCR_EL0.N via SET_ONE_REG?

Without a check for FEAT_HPMN0 in set_pmcr(), it appears nr_pmu_counters 
could be set to 0, which would lead to the same host profiling breakage 
described above.

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