Re: [PATCH v2 1/4] KVM: selftests: Fix treating KVM_CAP_MAX_VCPU_ID as inclusive

On Fri, Jul 31, 2026 at 03:24:07PM +0000, [email protected] 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 <[email protected]>
> 
> 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.