Re: [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set

[David Woodhouse]

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).

But this works.

Reviewed-by: David Woodhouse <dwmw@xxxxxxxxxxxx>

I'd have put this *after* your patch 5 though (qv).

> Link: https://lore.kernel.org/all/20260612233017.1F9771F000E9@xxxxxxxxxxxxxxx
> Suggested-by: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>  arch/x86/kvm/xen.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 694b31c1fcc9..27c1aeeab8af 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -22,6 +22,7 @@
>  #include <xen/interface/version.h>
>  #include <xen/interface/event_channel.h>
>  #include <xen/interface/sched.h>
> +#include <xen/xen-ops.h>
>  
>  #include <asm/xen/cpuid.h>
>  #include <asm/pvclock.h>
> @@ -1610,6 +1611,9 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
>  	if (!kvm_xen_timer_enabled(vcpu))
>  		return false;
>  
> +	if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
> +		return false;
> +
>  	switch (cmd) {
>  	case VCPUOP_set_singleshot_timer:
>  		if (vcpu->arch.xen.vcpu_id != vcpu_id) {
> @@ -2299,7 +2303,7 @@ static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r)
>  
>  void kvm_xen_init_vcpu(struct kvm_vcpu *vcpu)
>  {
> -	vcpu->arch.xen.vcpu_id = vcpu->vcpu_idx;
> +	vcpu->arch.xen.vcpu_id = XEN_VCPU_ID_INVALID;
>  	vcpu->arch.xen.poll_evtchn = 0;
>  
>  	timer_setup(&vcpu->arch.xen.poll_timer, cancel_evtchn_poll, 0);

Attachment: smime.p7s
Description: S/MIME cryptographic signature