Re: [PATCH v3 1/4] KVM: arm64: Expose PMMIR_EL1.SLOTS under strict PMUv3 UAPI

On Wed, 22 Jul 2026 at 21:50, <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The implementation of `KVM_ARM_VCPU_PMU_V3_STRICT` introduces two logic errors that completely break the new feature and its API contract:
> 1. `KVM_VCPU_MAX_FEATURES` was not increased from 9 to 10 to accommodate the new flag (which is defined as 9). As a result, `KVM_VCPU_VALID_FEATURES` excludes bit 9, causing `kvm_vcpu_init_check_features()` to unconditionally reject `KVM_ARM_VCPU_INIT` ioctls with `-ENOENT` when this feature is requested.
> 2. Even if the feature validation were fixed, the code fails to enforce the documented 'No default PMU creation' behavior. `kvm_setup_vcpu()` unconditionally calls `kvm_arm_set_default_pmu()` if `kvm_vcpu_has_pmu()` is true, without checking if the strict flag is set. Thus, KVM will always assign a default PMU on initialization, defeating the stated purpose of the flag.

Both are done in patch 4: it bumps KVM_VCPU_MAX_FEATURES to 10 and
adds the !kvm_vcpu_has_pmuv3_strict() check in kvm_setup_vcpu(). Until
then the flag stays unsettable by design (INIT rejects bit 9 with
-ENOENT), so patches 1-3 are inert and there is no half-enabled state.

/fuad

> --
>
> --- Patch [1]: [PATCH v3 1/4] KVM: arm64: Expose PMMIR_EL1.SLOTS under strict PMUv3 UAPI ---
> commit 5883b44965f93aed147ca4ed5fb390ae481daa37
> Author: Congkai Tan <[email protected]>
>
> KVM: arm64: Expose PMMIR_EL1.SLOTS under strict PMUv3 UAPI
>
> This commit introduces a new VCPU feature flag KVM_ARM_VCPU_PMU_V3_STRICT
> to prevent KVM from automatically creating a default PMU, and exposes
> the PMMIR_EL1.SLOTS value to the guest when a PMU is explicitly selected.
>
> > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> > index 1c13bfa2d38aa..019e5e3d892e6 100644
> > --- a/arch/arm64/include/uapi/asm/kvm.h
> > +++ b/arch/arm64/include/uapi/asm/kvm.h
> > @@ -106,6 +106,7 @@ struct kvm_regs {
> >  #define KVM_ARM_VCPU_PTRAUTH_GENERIC 6 /* VCPU uses generic authentication */
> >  #define KVM_ARM_VCPU_HAS_EL2         7 /* Support nested virtualization */
> >  #define KVM_ARM_VCPU_HAS_EL2_E2H0    8 /* Limit NV support to E2H RES0 */
> > +#define KVM_ARM_VCPU_PMU_V3_STRICT   9 /* No default PMU creation */
>
> [Severity: High]
> Does KVM_VCPU_MAX_FEATURES need to be updated to 10 to support this new flag?
>
> Currently, KVM_VCPU_MAX_FEATURES in arch/arm64/include/asm/kvm_host.h is 9,
> so KVM_VCPU_VALID_FEATURES (BIT(KVM_VCPU_MAX_FEATURES) - 1) only covers bits
> 0 through 8. When userspace attempts to use this new flag (bit 9),
> kvm_vcpu_init_check_features() will reject it with -ENOENT.
>
> Additionally, does this patch actually prevent default PMU creation?
>
> Looking at kvm_setup_vcpu() in arch/arm64/kvm/arm.c:
>
>         if (kvm_vcpu_has_pmu(vcpu) && !kvm->arch.arm_pmu)
>                 ret = kvm_arm_set_default_pmu(kvm);
>
> It appears that kvm_arm_set_default_pmu() will still be called on
> initialization even if the strict flag is set, since there is no check for
> kvm_vcpu_has_pmuv3_strict(vcpu) here.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
>