Re: [PATCH 1/8] KVM: x86/pmu: Add mediated PMU capability flags

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Transient failure in `perf_create_mediated_pmu` permanently clears `mediated_pmu_caps`, corrupting VM capability state on retry.
- [Medium] `kvm_vcpu_has_mediated_pmu_caps` evaluates to true if *any* capability matches, rather than *all*.
--

--- Patch [1]: [PATCH 1/8] KVM: x86/pmu: Add mediated PMU capability flags ---
commit b775f5357bd23699130ac68abbdd7cd26849e026
Author: Sandipan Das <[email protected]>

KVM: x86/pmu: Add mediated PMU capability flags

This commit introduces VM-scoped mediated PMU capability flags to help KVM
determine available hardware assists for saving and restoring guest PMU
state. It defines flags for hardware-switched and hardware-filtered PMU
states, and manages these capabilities during VM initialization.

> diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
> index a5821d7c87f93..f60856c48462b 100644
> --- a/arch/x86/kvm/pmu.h
> +++ b/arch/x86/kvm/pmu.h
[ ... ]
> @@ -88,6 +88,26 @@ static inline bool kvm_vcpu_has_mediated_pmu(struct kvm_vcpu *vcpu)
>  	return enable_mediated_pmu && vcpu_to_pmu(vcpu)->version;
>  }
>  
> +static inline bool kvm_vcpu_has_mediated_pmu_caps(struct kvm_vcpu *vcpu, u32 caps)
> +{
> +	return kvm_vcpu_has_mediated_pmu(vcpu) &&
> +	       !!(vcpu->kvm->arch.mediated_pmu_caps & caps);

[Severity: Medium]
When a caller checks for multiple capabilities simultaneously, will this
return true if only one of the bits matches, rather than all requested bits?

If a future caller checks kvm_vcpu_has_mediated_pmu_caps() with multiple
flags, it appears it would return true even if only a subset of those flags
are present in mediated_pmu_caps.

[ ... ]
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index afcac1042947a..9f0e072ff466d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
[ ... ]
> @@ -12759,6 +12759,7 @@ int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int id)
>  		if (irqchip_in_kernel(kvm)) {
>  			r = perf_create_mediated_pmu();
>  			if (r) {
> +				kvm->arch.mediated_pmu_caps = 0;

[Severity: High]
If perf_create_mediated_pmu() returns a transient error like -EBUSY, is it
safe to permanently clear the VM-wide mediated_pmu_caps here?

If userspace receives the error, resolves the host perf conflict, and
retries the KVM_CREATE_VCPU ioctl, the retry might succeed but the VM's
capabilities will have been irreversibly wiped. Does this lead to a silent
downgrade to software switching for the VM on a successful retry?

>  				pr_warn_ratelimited(PERF_MEDIATED_PMU_MSG);
>  				return r;
>  			}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1