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

[Dmytro Maluka <dmaluka@chromium.org>]

On Thu, Jul 30, 2026 at 06:52:43PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The selftest changes assert that setting `bsp_vcpu_id` equal to `max_vcpu_ids` (the exclusive upper bound for vcpu IDs) will be rejected with `-EINVAL`, but the KVM kernel code was not fixed to enforce this and still returns success, causing the tests to fail.
> --
> 
> --- Patch [1]: [PATCH 1/3] KVM: selftests: Fix treating KVM_CAP_MAX_VCPU_ID as inclusive ---
> commit 985e68429c6e20e59d983752bd84f5dc599b83f0
> Author: Dmytro Maluka <dmaluka@chromium.org>
> 
> KVM: selftests: Fix treating KVM_CAP_MAX_VCPU_ID as inclusive
> 
> This commit updates KVM selftests to treat KVM_CAP_MAX_VCPU_ID as
> exclusive rather than inclusive. It modifies the tests to assert that
> setting a boot CPU ID or vCPU ID equal to or greater than the maximum
> capacity will fail.
> 
> > 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..43f16c052de4a 100644
> > --- a/tools/testing/selftests/kvm/x86/max_vcpuid_cap_test.c
> > +++ b/tools/testing/selftests/kvm/x86/max_vcpuid_cap_test.c
> > @@ -30,22 +30,25 @@ int main(int argc, char *argv[])
> >  	if (kvm_has_cap(KVM_CAP_SET_BOOT_CPU_ID)) {
> >  		vm_ioctl(vm, KVM_SET_BOOT_CPU_ID, (void *)MAX_VCPU_ID);
> >  
> > -		/* Try setting KVM_CAP_MAX_VCPU_ID below BOOT_CPU_ID */
> > -		ret = __vm_enable_cap(vm, KVM_CAP_MAX_VCPU_ID, MAX_VCPU_ID - 1);
> > +		/* Try setting KVM_CAP_MAX_VCPU_ID below or equal to BOOT_CPU_ID */
> > +		ret = __vm_enable_cap(vm, KVM_CAP_MAX_VCPU_ID, MAX_VCPU_ID);
> >  		TEST_ASSERT(ret < 0,
> > -			    "Setting KVM_CAP_MAX_VCPU_ID below BOOT_CPU_ID should fail");
> > +			    "Setting KVM_CAP_MAX_VCPU_ID <= BOOT_CPU_ID should fail");
> 
> [Severity: Medium]
> Will this assertion now fail because the kernel code hasn't been updated to
> enforce this behavior?
> 
> Looking at how KVM_CAP_MAX_VCPU_ID is handled in the kernel, it checks:
> 
>     if (kvm->arch.bsp_vcpu_id > cap->args[0]) {
>         r = -EINVAL;
>         break;
>     }
> 
> If both kvm->arch.bsp_vcpu_id and cap->args[0] are set to MAX_VCPU_ID, the
> greater-than condition evaluates to false. Will the ioctl return success
> instead of -EINVAL, causing this test to crash?
> 
> [ ... ]
> 
> > diff --git a/tools/testing/selftests/kvm/x86/set_boot_cpu_id.c b/tools/testing/selftests/kvm/x86/set_boot_cpu_id.c
> > index 8e3898646c69d..1c4ebdf8e82c5 100644
> > --- a/tools/testing/selftests/kvm/x86/set_boot_cpu_id.c
> > +++ b/tools/testing/selftests/kvm/x86/set_boot_cpu_id.c
> > @@ -39,8 +39,8 @@ static void test_set_invalid_bsp(struct kvm_vm *vm)
> >  	int r;
> >  
> >  	if (max_vcpu_id) {
> > -		r = __vm_ioctl(vm, KVM_SET_BOOT_CPU_ID, (void *)(max_vcpu_id + 1));
> > -		TEST_ASSERT(r == -1 && errno == EINVAL, "BSP with ID > MAX should fail");
> > +		r = __vm_ioctl(vm, KVM_SET_BOOT_CPU_ID, (void *)max_vcpu_id);
> > +		TEST_ASSERT(r == -1 && errno == EINVAL, "BSP with ID >= MAX should fail");
> 
> [Severity: Medium]
> Does this change also cause the test to fail against the current kernel?
> 
> Looking at the KVM_SET_BOOT_CPU_ID handling in arch/x86/kvm/x86.c:kvm_vm_ioctl():
> 
>     else if (arg > KVM_MAX_VCPU_IDS ||
>              (kvm->arch.max_vcpu_ids && arg > kvm->arch.max_vcpu_ids))
>         r = -EINVAL;
> 
> Since the kernel checks if arg is greater than max_vcpu_ids instead of
> greater than or equal to, will setting arg to max_vcpu_id return success
> here and trigger the TEST_ASSERT failure?

Yeah, I should split this selftest patch into two: the changes which
prevent the KVM fix from breaking the test (and thus should go before
the KVM fix) and the changes which break the test unless the KVM fix is
applied (and thus should go after).