Hi Sean,

On Mon, Sep 8, 2025 at 4:30 PM Sean Christopherson seanjc@google.com wrote:

On Sat, Sep 06, 2025, Sukrut Heroorkar wrote:

...

Replace the hardcoded 0xff in test_icr() with the actual number of vcpus created for the vm. This address the existing TODO and keeps the test correct if it is ever run with multiple vcpus.

The TODO is stale, it was resolved by commit 376bc1b458c9 ("KVM: selftests: Don't assume vcpu->id is '0' in xAPIC state test"), I/we just forgot to delete the comment.

...

Signed-off-by: Sukrut Heroorkar hsukrut3@gmail.com

tools/testing/selftests/kvm/x86/xapic_state_test.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/x86/xapic_state_test.c b/tools/testing/selftests/kvm/x86/xapic_state_test.c index fdebff1165c7..4af36682503e 100644 --- a/tools/testing/selftests/kvm/x86/xapic_state_test.c +++ b/tools/testing/selftests/kvm/x86/xapic_state_test.c @@ -56,6 +56,17 @@ static void x2apic_guest_code(void) } while (1); }

+static unsigned int vm_nr_vcpus(struct kvm_vm *vm) +{

• struct kvm_vcpu *vcpu;
  

• unsigned int count = 0;
  

• list_for_each_entry(vcpu, &vm->vcpus, list)
  

•         count++;
  

• return count;
  

+}

static void ____test_icr(struct xapic_vcpu *x, uint64_t val) { struct kvm_vcpu *vcpu = x->vcpu; @@ -124,7 +135,7 @@ static void test_icr(struct xapic_vcpu *x) * vCPUs, not vcpu.id + 1. Arbitrarily use vector 0xff. */ icr = APIC_INT_ASSERT | 0xff;

• for (i = 0; i < 0xff; i++) {
  

• for (i = 0; i < vm_nr_vcpus(vcpu->vm); i++) {
  

This is wrong/undesirable. The original code was:

    for (i = vcpu->id + 1; i < 0xff; i++) {
            for (j = 0; j < 8; j++)
                    __test_icr(vm, vcpu, i << (32 + 24) | APIC_INT_ASSERT | (j << 8));
    }

I.e. the _lower_ bound was nr_vcpus+1. Regardless, as fixed by the aformentioned commit, using the number of vCPUs in any capacity is simply wrong. The stale comment just needs to be deleted.:

Thanks for the clarification. I will send a patch removing the stale TODO comment.

...

          if (i == vcpu->id)
                  continue;
          for (j = 0; j < 8; j++)

-- 2.43.0