Re: [PATCH 7/8] KVM: x86/xen: Use 32-bit locked bts for vcpu_info evtchn_pending_sel

[Sean Christopherson <seanjc@google.com>]

On Fri, Jun 05, 2026, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> Replace test_and_set_bit() on vcpu_info->evtchn_pending_sel with an
> explicit 'lock btsl' in kvm_xen_set_evtchn_fast(). The generic
> test_and_set_bit() uses a 64-bit locked operation ('lock btsq') on
> x86-64,

  x86-64, and the address of the per-vCPU info is guest-controlled and
  only required to be 32-bit aligned.

I would also add somewhere:

  Note, KVM reuses the local gpc.  The atomic accesses to pending_bits is
  on the page-aligned per-VM shared_info structure, i.e. doesn't need the
  same treatment as the access is guarantee to be 64-bit aligned.

Because without that knowledge it's very difficult to understand why the
test_and_set_bit(xe->port, pending_bits) is fine but the one the event channel
is not.

> which requires 8-byte alignment to avoid split-lock #AC
> exceptions.
> 
> Since evtchn_pending_sel is at most 64 bits wide and port_word_bit
> ranges 0-63, a 32-bit 'lock btsl' suffices for both native and compat
> vcpu_info layouts, and only requires the 4-byte alignment that is
> already guaranteed by the registration path.
> 
> This also eliminates the bogus cast of compat_vcpu_info's 32-bit
> evtchn_pending_sel to 'unsigned long *' which was the original source
> of the split-lock hazard.
> 
> Fixes: 14243b387137 ("KVM: x86/xen: Add KVM_IRQ_ROUTING_XEN_EVTCHN and event channel delivery")
> Reported-by: sashiko-bot@kernel.org

Please add the Closes: link for publicly reported issues, especially for flaws
reported by review bots, as such "bugs" are generally unproven.

Closes: https://lore.kernel.org/all/20260604193554.1BA311F00893@smtp.kernel.org

> Assisted-by: Kiro:claude-opus-4.6-1m
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  arch/x86/kvm/xen.c | 42 ++++++++++++++++++++++++++++--------------
>  1 file changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index ae0713718367..24e939ef5d64 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -1811,7 +1811,7 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
>  {
>  	struct gfn_to_pfn_cache *gpc = &kvm->arch.xen.shinfo_cache;
>  	bool has_64bit_shinfo = kvm_xen_has_64bit_shinfo(kvm);
> -	unsigned long *pending_bits, *mask_bits;
> +	unsigned long *pending_bits, *mask_bits, vi_pending_sel_ofs;
>  	struct kvm_vcpu *vcpu;
>  	unsigned long flags;
>  	int port_word_bit;
> @@ -1844,11 +1844,18 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
>  		pending_bits = (unsigned long *)&shinfo->evtchn_pending;
>  		mask_bits = (unsigned long *)&shinfo->evtchn_mask;
>  		port_word_bit = xe->port / 64;
> +
> +		vi_pending_sel_ofs = offsetof(struct vcpu_info, evtchn_pending_sel);
>  	} else {
>  		struct compat_shared_info *shinfo = gpc->khva;
>  		pending_bits = (unsigned long *)&shinfo->evtchn_pending;
>  		mask_bits = (unsigned long *)&shinfo->evtchn_mask;
>  		port_word_bit = xe->port / 32;
> +
> +		vi_pending_sel_ofs = offsetof(struct compat_vcpu_info, evtchn_pending_sel);
> +
> +		/* test_and_set_bit() needs 64-bit alignment, but that's OK */
> +		BUILD_BUG_ON(offsetof(struct compat_shared_info, evtchn_pending) & 7);

		BUILD_BUG_ON(!IS_ALIGNED(offsetof(struct compat_shared_info, evtchn_pending),
					 sizeof(unsigned long)));

>  	}
>  
>  	/*
> @@ -1864,6 +1871,8 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
>  		rc = -ENOTCONN; /* Masked */
>  		kvm_xen_check_poller(vcpu, xe->port);
>  	} else {
> +		bool old;
> +
>  		rc = 1; /* Delivered to the bitmap in shared_info. */
>  		/* Now switch to the vCPU's vcpu_info to set the index and pending_sel */
>  		read_unlock_irqrestore(&gpc->lock, flags);
> @@ -1880,19 +1889,24 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
>  			goto out_rcu;
>  		}
>  
> -		if (has_64bit_shinfo) {
> -			struct vcpu_info *vcpu_info = gpc->khva;
> -			if (!test_and_set_bit(port_word_bit, &vcpu_info->evtchn_pending_sel)) {
> -				WRITE_ONCE(vcpu_info->evtchn_upcall_pending, 1);
> -				kick_vcpu = true;
> -			}
> -		} else {
> -			struct compat_vcpu_info *vcpu_info = gpc->khva;
> -			if (!test_and_set_bit(port_word_bit,
> -					      (unsigned long *)&vcpu_info->evtchn_pending_sel)) {
> -				WRITE_ONCE(vcpu_info->evtchn_upcall_pending, 1);
> -				kick_vcpu = true;
> -			}
> +		/*
> +		 * Explicitly use btsl instead of test_and_set_bit() because
> +		 * a 32-bit guest is not required to align to 64 bits, and
> +		 * that might cause splitlock exceptions.
> +		 */
> +		asm volatile(LOCK_PREFIX "btsl %[bit], %[sel]"
> +			     : [sel] "+m" (*(u32 *)(gpc->khva + vi_pending_sel_ofs)),
> +			       "=@ccc" (old)
> +			     : [bit] "Ir" (port_word_bit) : "memory");

Use GEN_BINARY_RMWcc() to make this less ugly?  It'll still be ugly, but slightly
less so.  Completely untested, but I think it's this?

		old = GEN_BINARY_RMWcc(LOCK_PREFIX "btsl",
				       *(u32 *)(gpc->khva + vi_pending_sel_ofs),
				       c, "Ir", port_word_bit);

> +		if (!old) {
> +			struct vcpu_info *vi = gpc->khva;
> +
> +			/* No need for compat handling */
> +			BUILD_BUG_ON(offsetof(struct vcpu_info, evtchn_upcall_pending) !=
> +				     offsetof(struct compat_vcpu_info, evtchn_upcall_pending));
> +
> +			WRITE_ONCE(vi->evtchn_upcall_pending, 1);
> +			kick_vcpu = true;
>  		}
>  
>  		/* For the per-vCPU lapic vector, deliver it as MSI. */
> -- 
> 2.54.0
>