Re: [PATCH v2 4/8] KVM: Initialize a vCPU's index to '-1' while it's being created

[Sean Christopherson <seanjc@google.com>]

On Sat, Jun 13, 2026, David Woodhouse wrote:
> On Fri, 2026-06-12 at 16:40 -0700, Sean Christopherson wrote:
> > > 
> > > arch/x86/kvm/xen.c:kvm_xen_init_vcpu() {
> > >     ...
> > >     vcpu->arch.xen.vcpu_id = vcpu->vcpu_idx;
> > 
> > I don't see how the existing code can be correct.  David/Paul, is this supposed
> > to be vcpu->vcpu_id?
> 
> No, vcpu->vcpu_id is the APIC ID. 
> 
> But xen.vcpu_id is the Xen/ACPI ID. The kernel needs to know it in
> order to accelerate timer hypercalls. The vcpu->vcpu_idx is a
> reasonable default for that.

Right, but isn't vcpu_id also a reasonable default?  Ugh, never mind.  Even if
it's conceptually a reasonable default, in practice using vcpu_id would lead to
creating an illegal xen.vcpu_id since KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID restricts
the ID to KVM_MAX_VCPUS, not KVM_MAX_VCPU_IDS (which is KVM_MAX_VCPUS * 4).

	case KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID:
		if (data->u.vcpu_id >= KVM_MAX_VCPUS)
			r = -EINVAL;
		else {
			vcpu->arch.xen.vcpu_id = data->u.vcpu_id;
			r = 0;
		}
		break;

> (As if this wasn't confusing enough, note ACPI != APIC and neither of
> those are typos)
> 
> > >     ...
> > > }
> > > 
> > > Since the actual vcpu_idx isn't assigned until later in
> > > kvm_vm_ioctl_create_vcpu(), this leaves vcpu->arch.xen.vcpu_id as -1 instead
> > > of its previous default of 0.
> > > 
> > > Could this cause guest hypercalls, such as VCPUOP_set_singleshot_timer,
> > > to fail with -EINVAL if a VMM relies on the previous default of 0 for vCPU 0
> > > and doesn't explicitly invoke KVM_XEN_VCPU_SET_ATTR?
> 
> Yeah, but it would be tolerable to just return !handled for the -1 case
> instead of "I handled that and it got -EINVAL". Then the hypercall gets
> punted to userspace.

Are you thinking this?

diff --git arch/x86/kvm/xen.c arch/x86/kvm/xen.c
index 694b31c1fcc9..7a94b84f6492 100644
--- arch/x86/kvm/xen.c
+++ arch/x86/kvm/xen.c
@@ -1607,7 +1607,7 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
        struct vcpu_set_singleshot_timer oneshot;
        struct x86_exception e;
 
-       if (!kvm_xen_timer_enabled(vcpu))
+       if (!kvm_xen_timer_enabled(vcpu) || vcpu->arch.xen.vcpu_id < 0)
                return false;
 
        switch (cmd) {

> I'd slightly prefer a deferred initialization, but I believe all known
> userspace explicitly *sets* the ID via KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID

Yeah, userspace pretty much has to, otherwise only vCPU0 could do
VCPUOP_set_singleshot_timer.  If we were implementing this for the first time,
I would be 100% in favor of deferred initialization, but since this will be an
ABI change, explicitly initializing xen.vcpu_id to -1 feels "safer", because at
least then there will be a fairly noisy failure (exit to userspace), versus the
guest silently getting different behavior.