All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: David Woodhouse <dwmw2@infradead.org>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,  Paul Durrant <paul@xen.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	 syzbot+5b32c49cd8f005e65654@syzkaller.appspotmail.com,
	 syzbot+5d2b94b77112148d1744@syzkaller.appspotmail.com
Subject: Re: [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set
Date: Fri, 26 Jun 2026 07:27:47 -0700	[thread overview]
Message-ID: <aj6MY_9wx_uaDvHN@google.com> (raw)
In-Reply-To: <123bd3932db959a9c26b9ede46632e482dbdc4ab.camel@infradead.org>

On Fri, Jun 26, 2026, David Woodhouse wrote:
> On Thu, 2026-06-25 at 15:36 -0700, Sean Christopherson wrote:
> > Explicitly invalidate KVM's internal Xen vCPU ID during vCPU creation
> > instead of *trying* to set the Xen ID to the vCPU index by default, and
> > forward singleshot timer hypercalls to userspace if the VMM hasn't set the
> > Xen ID via KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID.  Using the vCPU's index as its
> > default Xen ID is reasonable in concept, but in practice is horribly flawed
> > as the index is left as '0' until after vCPU initialization completes, i.e.
> > every vCPU gets a Xen ID of '0' by default.
> > 
> > Forward hypercalls to userspace instead of trying to salvage any kind of
> > default behavior, as all userspace implementations that support multiple
> > vCPUs either don't enable the timer, are guaranteed to set Xen ID, or work
> > only because *all* guests also screw up the singleshot timer hypercalls.
> > The last scenarios is extremely unlikely given that Linux-as-a-guest uses
> > the actual Xen vCPU ID when making timer hypercalls.  In other words, for
> > all intents and purposes, KVM's ABI is already that userspace must set the
> > Xen vCPU ID, so just commit to that ABI.
> > 
> > Note, KVM's handling of KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID restricts the ID to
> > KVM_MAX_VCPUS, so there's no chance of a valid ID colliding with -1u.
> 
> Nit: XEN_VCPU_ID_INVALID isn't -1u; it's U32_MAX.
> 
> I'd have been slightly happier if you'd used an explicit -1U, perhaps
> with a name of its own (although I guess we wouldn't want to call it
> KVM_VCPU_IDX_INVALID in the *Xen* part).

Ya, I was 50/50 on having KVM define its own INVALID macro versus reusing what
the Xen-the-guest has.  I didn't want to open code the literal, because that
would require open coding a somewhat magical value in multiple locations.  And
having XEN_VCPU_ID_INVALID and KVM_XEN_VCPU_ID_INVALID, with the same value,
seemed even worse than reusing a macro that isn't strictly owned by KVM.

What if I also add a compile-timer assertion?  In the end, KVM doesn't care what
value XEN_VCPU_ID_INVALID uses, so long as it can't colled with what is allowed
by KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID.

diff --git arch/x86/kvm/xen.c arch/x86/kvm/xen.c
index 3ed6686e0a1a..5c60ed9ef2cc 100644
--- arch/x86/kvm/xen.c
+++ arch/x86/kvm/xen.c
@@ -1103,6 +1103,8 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
                break;
 
        case KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID:
+               BUILD_BUG_ON(XEN_VCPU_ID_INVALID < KVM_MAX_VCPUS);
+
                if (data->u.vcpu_id >= KVM_MAX_VCPUS)
                        r = -EINVAL;
                else {

  reply	other threads:[~2026-06-26 14:27 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-25 22:36 [PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 01/10] KVM: x86/hyperv: Get target FIFO in hv_tlb_flush_enqueue(), not caller Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 02/10] KVM: x86/hyperv: Check for NULL vCPU Hyper-V object in kvm_hv_get_tlb_flush_fifo() Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 03/10] KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set Sean Christopherson
2026-06-25 22:50   ` sashiko-bot
2026-06-26  8:05   ` David Woodhouse
2026-06-26 14:27     ` Sean Christopherson [this message]
2026-06-26 15:19       ` David Woodhouse
2026-06-25 22:36 ` [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls Sean Christopherson
2026-06-25 22:43   ` sashiko-bot
2026-06-25 23:30     ` Sean Christopherson
2026-06-26  8:11   ` David Woodhouse
2026-06-26 14:19     ` Sean Christopherson
2026-06-26 15:32       ` David Woodhouse
2026-06-26 18:12         ` Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 06/10] KVM: Initialize a vCPU's index to '-1' while it's being created Sean Christopherson
2026-06-25 22:57   ` sashiko-bot
2026-06-25 23:31     ` Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 07/10] KVM: Move nVMX's lockdep logic for vcpu->mutex to a common helper Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 08/10] KVM: x86: Treat a vCPU as unreachable if its index is invalid Sean Christopherson
2026-06-25 22:50   ` sashiko-bot
2026-06-25 22:36 ` [PATCH v3 09/10] KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu() Sean Christopherson
2026-06-25 22:50   ` sashiko-bot
2026-06-25 22:36 ` [PATCH v3 10/10] KVM: x86/hyperv: Use {READ,WRITE}_ONCE for cross-task synic->active accesses Sean Christopherson
2026-06-26  7:06 ` [syzbot ci] Re: KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv syzbot ci
2026-06-26 13:24   ` Sean Christopherson

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=aj6MY_9wx_uaDvHN@google.com \
    --to=seanjc@google.com \
    --cc=dwmw2@infradead.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paul@xen.org \
    --cc=pbonzini@redhat.com \
    --cc=syzbot+5b32c49cd8f005e65654@syzkaller.appspotmail.com \
    --cc=syzbot+5d2b94b77112148d1744@syzkaller.appspotmail.com \
    --cc=vkuznets@redhat.com \
    /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.