All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmytro Maluka <dmaluka@chromium.org>
To: sashiko-reviews@lists.linux.dev
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH v2 1/4] KVM: selftests: Fix treating KVM_CAP_MAX_VCPU_ID as inclusive
Date: Fri, 31 Jul 2026 17:44:37 +0200	[thread overview]
Message-ID: <amzC5dDVAspfK16u@google.com> (raw)
In-Reply-To: <20260731152407.E3EAD1F000E9@smtp.kernel.org>

On Fri, Jul 31, 2026 at 03:24:07PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch modifies a KVM selftest to avoid an invalid configuration but sweeps an underlying kernel bug under the rug by failing to fix the kernel and masking the issue in the test suite.
> --
> 
> --- Patch [1]: [PATCH v2 1/4] KVM: selftests: Fix treating KVM_CAP_MAX_VCPU_ID as inclusive ---
> commit 5e2afeaca4e83d31005f9aa5caf94dbbc9f20b8c
> Author: Dmytro Maluka <dmaluka@chromium.org>
> 
> KVM: selftests: Fix treating KVM_CAP_MAX_VCPU_ID as inclusive
> 
> This patch modifies the max_vcpuid_cap_test selftest to account for
> KVM_CAP_MAX_VCPU_ID being exclusive (maximum allowed vcpu_id plus one).
> It changes the test to use MAX_VCPU_ID + 1 when enabling the capability,
> avoiding the boundary condition.
> 
> > diff --git a/tools/testing/selftests/kvm/x86/max_vcpuid_cap_test.c b/tools/testing/selftests/kvm/x86/max_vcpuid_cap_test.c
> > index 7e2bfb3c3f3ba..47ee14967873f 100644
> > --- a/tools/testing/selftests/kvm/x86/max_vcpuid_cap_test.c
> > +++ b/tools/testing/selftests/kvm/x86/max_vcpuid_cap_test.c
> [ ... ]
> > @@ -36,16 +36,19 @@ int main(int argc, char *argv[])
> >  			    "Setting KVM_CAP_MAX_VCPU_ID below BOOT_CPU_ID should fail");
> >  	}
> >  
> > -	/* Set KVM_CAP_MAX_VCPU_ID */
> > -	vm_enable_cap(vm, KVM_CAP_MAX_VCPU_ID, MAX_VCPU_ID);
> > +	/*
> > +	 * Set KVM_CAP_MAX_VCPU_ID. Note: KVM_CAP_MAX_VCPU_ID is a misnomer,
> > +	 * it actually represents maximum vcpu_id plus one.
> > +	 */
> > +	vm_enable_cap(vm, KVM_CAP_MAX_VCPU_ID, MAX_VCPU_ID + 1);
> 
> [Severity: Medium]
> Does this patch mask an underlying kernel bug rather than fixing it?
> 
> The commit message notes that setting bsp_vcpu_id equal to KVM_CAP_MAX_VCPU_ID
> is invalid. However, by changing the test to avoid this exact boundary
> condition, it seems we are leaving the kernel's off-by-one bug unfixed.
> 
> Looking at the kernel side in arch/x86/kvm/x86.c:kvm_vm_ioctl_enable_cap():
> 
>     if (kvm->arch.bsp_vcpu_id > cap->args[0]) {
> 
> This bounds check uses ">" instead of ">=", which appears to incorrectly allow
> bsp_vcpu_id to equal max_vcpu_ids. 
> 
> Should the kernel be fixed to reject this invalid configuration,

Yes, it is fixed in patch 2 in this series.

The reason for this selftest patch is not to "mask" the KVM bug but to
prevent the test from incorrectly failing once the KVM bug is fixed.

> and the
> selftest be updated with a negative test to ensure it actually fails?

That is done in patch 3 in this series.

  reply	other threads:[~2026-07-31 15:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-31 15:08 [PATCH v2 0/4] KVM: x86: Fix off-by-one in max_vcpu_ids bounds checks Dmytro Maluka
2026-07-31 15:08 ` [PATCH v2 1/4] KVM: selftests: Fix treating KVM_CAP_MAX_VCPU_ID as inclusive Dmytro Maluka
2026-07-31 15:24   ` sashiko-bot
2026-07-31 15:44     ` Dmytro Maluka [this message]
2026-07-31 15:08 ` [PATCH v2 2/4] KVM: x86: Fix off-by-one in max_vcpu_ids bounds checks Dmytro Maluka
2026-07-31 15:08 ` [PATCH v2 3/4] KVM: selftests: Improve bsp_vcpu_id and max_vcpu_ids out-of-bound tests Dmytro Maluka
2026-07-31 15:08 ` [PATCH v2 4/4] Documentation: KVM: Clarify that KVM_CAP_MAX_VCPU_ID is exclusive Dmytro Maluka

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=amzC5dDVAspfK16u@google.com \
    --to=dmaluka@chromium.org \
    --cc=kvm@vger.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.