All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Congkai Tan" <congkai@amazon.com>
Cc: kvmarm@lists.linux.dev, "Oliver Upton" <oupton@kernel.org>,
	kvm@vger.kernel.org, "Marc Zyngier" <maz@kernel.org>
Subject: Re: [PATCH v3 1/4] KVM: arm64: Expose PMMIR_EL1.SLOTS under strict PMUv3 UAPI
Date: Wed, 22 Jul 2026 20:47:58 +0000	[thread overview]
Message-ID: <20260722204759.1582A1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260722202702.4165917-2-congkai@amazon.com>

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.
--

--- Patch [1]: [PATCH v3 1/4] KVM: arm64: Expose PMMIR_EL1.SLOTS under strict PMUv3 UAPI ---
commit 5883b44965f93aed147ca4ed5fb390ae481daa37
Author: Congkai Tan <congkai@amazon.com>

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/20260722202702.4165917-1-congkai@amazon.com?part=1

  reply	other threads:[~2026-07-22 20:47 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22 20:26 [PATCH v3 0/4] KVM: arm64: Expose PMMIR_EL1.SLOTS to guests Congkai Tan
2026-07-22 20:26 ` [PATCH v3 1/4] KVM: arm64: Expose PMMIR_EL1.SLOTS under strict PMUv3 UAPI Congkai Tan
2026-07-22 20:47   ` sashiko-bot [this message]
2026-07-22 20:27 ` [PATCH v3 2/4] KVM: arm64: Advertise STALL_SLOT* in PMCEID1 " Congkai Tan
2026-07-22 20:27 ` [PATCH v3 3/4] KVM: arm64: Ignore writes to PMCR_EL0.N when using strict UAPI Congkai Tan
2026-07-22 20:27 ` [PATCH v3 4/4] KVM: arm64: Add KVM_ARM_VCPU_PMU_V3_STRICT vCPU feature Congkai Tan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260722204759.1582A1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=congkai@amazon.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=maz@kernel.org \
    --cc=oupton@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.