* [PATCH 1/8] KVM: x86/xen: Rename 'longmode' to 'is_64bit' in hypercall handling
2026-06-05 14:17 [PATCH 0/8] KVM: x86/xen: Clean up 32-bit vs. 64-bit shared info mode handling David Woodhouse
@ 2026-06-05 14:17 ` David Woodhouse
2026-06-05 14:17 ` [PATCH 2/8] KVM: x86/xen: Introduce kvm_xen_has_64bit_shinfo() macro David Woodhouse
` (6 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2026-06-05 14:17 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paul Durrant, Hyunwoo Kim, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-kernel
From: David Woodhouse <dwmw@amazon.co.uk>
Rename the local 'longmode' variable and function parameter to
'is_64bit' throughout the Xen hypercall handling code. This
distinguishes it from the VM-wide kvm->arch.xen.long_mode which
represents the Xen shared_info layout mode.
The 'is_64bit' parameter indicates whether the vCPU was in 64-bit
mode when it made the hypercall, which determines how to parse the
hypercall arguments. The UAPI field name (vcpu->run->xen.u.hcall.longmode)
is unchanged.
Assisted-by: Kiro:claude-opus-4.6-1m
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
arch/x86/kvm/xen.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 91fd3673c09a..a7ab19f38b59 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -1468,7 +1468,7 @@ static bool wait_pending_event(struct kvm_vcpu *vcpu, int nr_ports,
return ret;
}
-static bool kvm_xen_schedop_poll(struct kvm_vcpu *vcpu, bool longmode,
+static bool kvm_xen_schedop_poll(struct kvm_vcpu *vcpu, bool is_64bit,
u64 param, u64 *r)
{
struct sched_poll sched_poll;
@@ -1480,7 +1480,7 @@ static bool kvm_xen_schedop_poll(struct kvm_vcpu *vcpu, bool longmode,
!(vcpu->kvm->arch.xen.hvm_config.flags & KVM_XEN_HVM_CONFIG_EVTCHN_SEND))
return false;
- if (IS_ENABLED(CONFIG_64BIT) && !longmode) {
+ if (IS_ENABLED(CONFIG_64BIT) && !is_64bit) {
struct compat_sched_poll sp32;
/* Sanity check that the compat struct definition is correct */
@@ -1577,12 +1577,12 @@ static void cancel_evtchn_poll(struct timer_list *t)
kvm_vcpu_kick(vcpu);
}
-static bool kvm_xen_hcall_sched_op(struct kvm_vcpu *vcpu, bool longmode,
+static bool kvm_xen_hcall_sched_op(struct kvm_vcpu *vcpu, bool is_64bit,
int cmd, u64 param, u64 *r)
{
switch (cmd) {
case SCHEDOP_poll:
- if (kvm_xen_schedop_poll(vcpu, longmode, param, r))
+ if (kvm_xen_schedop_poll(vcpu, is_64bit, param, r))
return true;
fallthrough;
case SCHEDOP_yield:
@@ -1601,7 +1601,7 @@ struct compat_vcpu_set_singleshot_timer {
uint32_t flags;
} __attribute__((packed));
-static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
+static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool is_64bit, int cmd,
int vcpu_id, u64 param, u64 *r)
{
struct vcpu_set_singleshot_timer oneshot;
@@ -1633,7 +1633,7 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
BUILD_BUG_ON(sizeof_field(struct compat_vcpu_set_singleshot_timer, flags) !=
sizeof_field(struct vcpu_set_singleshot_timer, flags));
- if (kvm_read_guest_virt(vcpu, param, &oneshot, longmode ? sizeof(oneshot) :
+ if (kvm_read_guest_virt(vcpu, param, &oneshot, is_64bit ? sizeof(oneshot) :
sizeof(struct compat_vcpu_set_singleshot_timer), &e)) {
*r = -EFAULT;
return true;
@@ -1673,7 +1673,7 @@ static bool kvm_xen_hcall_set_timer_op(struct kvm_vcpu *vcpu, uint64_t timeout,
int kvm_xen_hypercall(struct kvm_vcpu *vcpu)
{
- bool longmode;
+ bool is_64bit;
u64 input, params[6], r = -ENOSYS;
bool handled = false;
u8 cpl;
@@ -1685,8 +1685,8 @@ int kvm_xen_hypercall(struct kvm_vcpu *vcpu)
kvm_hv_hypercall_enabled(vcpu))
return kvm_hv_hypercall(vcpu);
- longmode = is_64_bit_hypercall(vcpu);
- if (!longmode) {
+ is_64bit = is_64_bit_hypercall(vcpu);
+ if (!is_64bit) {
params[0] = (u32)kvm_rbx_read(vcpu);
params[1] = (u32)kvm_rcx_read(vcpu);
params[2] = (u32)kvm_rdx_read(vcpu);
@@ -1727,17 +1727,17 @@ int kvm_xen_hypercall(struct kvm_vcpu *vcpu)
handled = kvm_xen_hcall_evtchn_send(vcpu, params[1], &r);
break;
case __HYPERVISOR_sched_op:
- handled = kvm_xen_hcall_sched_op(vcpu, longmode, params[0],
+ handled = kvm_xen_hcall_sched_op(vcpu, is_64bit, params[0],
params[1], &r);
break;
case __HYPERVISOR_vcpu_op:
- handled = kvm_xen_hcall_vcpu_op(vcpu, longmode, params[0], params[1],
+ handled = kvm_xen_hcall_vcpu_op(vcpu, is_64bit, params[0], params[1],
params[2], &r);
break;
case __HYPERVISOR_set_timer_op: {
u64 timeout = params[0];
/* In 32-bit mode, the 64-bit timeout is in two 32-bit params. */
- if (!longmode)
+ if (!is_64bit)
timeout |= params[1] << 32;
handled = kvm_xen_hcall_set_timer_op(vcpu, timeout, &r);
break;
@@ -1752,7 +1752,7 @@ int kvm_xen_hypercall(struct kvm_vcpu *vcpu)
handle_in_userspace:
vcpu->run->exit_reason = KVM_EXIT_XEN;
vcpu->run->xen.type = KVM_EXIT_XEN_HCALL;
- vcpu->run->xen.u.hcall.longmode = longmode;
+ vcpu->run->xen.u.hcall.longmode = is_64bit;
vcpu->run->xen.u.hcall.cpl = cpl;
vcpu->run->xen.u.hcall.input = input;
vcpu->run->xen.u.hcall.params[0] = params[0];
--
2.54.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/8] KVM: x86/xen: Introduce kvm_xen_has_64bit_shinfo() macro
2026-06-05 14:17 [PATCH 0/8] KVM: x86/xen: Clean up 32-bit vs. 64-bit shared info mode handling David Woodhouse
2026-06-05 14:17 ` [PATCH 1/8] KVM: x86/xen: Rename 'longmode' to 'is_64bit' in hypercall handling David Woodhouse
@ 2026-06-05 14:17 ` David Woodhouse
2026-06-06 9:30 ` David Laight
2026-06-05 14:17 ` [PATCH 3/8] KVM: x86/xen: Rename max_evtchn_port() to kvm_max_evtchn_port() David Woodhouse
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: David Woodhouse @ 2026-06-05 14:17 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paul Durrant, Hyunwoo Kim, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-kernel
From: David Woodhouse <dwmw@amazon.co.uk>
Add a kvm_xen_has_64bit_shinfo() helper macro to replace the repeated
pattern of 'IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode'
throughout the Xen emulation code. The macro uses READ_ONCE() to
ensure a consistent snapshot of the flag, which can be changed by
another vCPU at any time.
This is the KVM equivalent of Xen's !has_32bit_shinfo().
Assisted-by: Kiro:claude-opus-4.6-1m
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
arch/x86/kvm/xen.c | 16 ++++++++--------
arch/x86/kvm/xen.h | 5 +++++
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index a7ab19f38b59..acd3cd87dd2f 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -72,7 +72,7 @@ static int kvm_xen_shared_info_init(struct kvm *kvm)
BUILD_BUG_ON(offsetof(struct shared_info, wc) != 0xc00);
BUILD_BUG_ON(offsetof(struct shared_info, wc_sec_hi) != 0xc0c);
- if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode) {
+ if (kvm_xen_has_64bit_shinfo(kvm)) {
struct shared_info *shinfo = gpc->khva;
wc_sec_hi = &shinfo->wc_sec_hi;
@@ -390,7 +390,7 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
BUILD_BUG_ON(sizeof_field(struct vcpu_runstate_info, time) !=
sizeof(vx->runstate_times));
- if (IS_ENABLED(CONFIG_64BIT) && v->kvm->arch.xen.long_mode) {
+ if (kvm_xen_has_64bit_shinfo(v->kvm)) {
user_len = sizeof(struct vcpu_runstate_info);
times_ofs = offsetof(struct vcpu_runstate_info,
state_entry_time);
@@ -661,7 +661,7 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
}
/* Now gpc->khva is a valid kernel address for the vcpu_info */
- if (IS_ENABLED(CONFIG_64BIT) && v->kvm->arch.xen.long_mode) {
+ if (kvm_xen_has_64bit_shinfo(v->kvm)) {
struct vcpu_info *vi = gpc->khva;
asm volatile(LOCK_PREFIX "orq %0, %1\n"
@@ -978,7 +978,7 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
* address, that's actually OK. kvm_xen_update_runstate_guest()
* will cope.
*/
- if (IS_ENABLED(CONFIG_64BIT) && vcpu->kvm->arch.xen.long_mode)
+ if (kvm_xen_has_64bit_shinfo(vcpu->kvm))
sz = sizeof(struct vcpu_runstate_info);
else
sz = sizeof(struct compat_vcpu_runstate_info);
@@ -1424,7 +1424,7 @@ static int kvm_xen_hypercall_complete_userspace(struct kvm_vcpu *vcpu)
static inline int max_evtchn_port(struct kvm *kvm)
{
- if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode)
+ if (kvm_xen_has_64bit_shinfo(kvm))
return EVTCHN_2L_NR_CHANNELS;
else
return COMPAT_EVTCHN_2L_NR_CHANNELS;
@@ -1446,7 +1446,7 @@ static bool wait_pending_event(struct kvm_vcpu *vcpu, int nr_ports,
goto out_rcu;
ret = false;
- if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode) {
+ if (kvm_xen_has_64bit_shinfo(kvm)) {
struct shared_info *shinfo = gpc->khva;
pending_bits = (unsigned long *)&shinfo->evtchn_pending;
} else {
@@ -1820,7 +1820,7 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
if (!kvm_gpc_check(gpc, PAGE_SIZE))
goto out_rcu;
- if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode) {
+ if (kvm_xen_has_64bit_shinfo(kvm)) {
struct shared_info *shinfo = gpc->khva;
pending_bits = (unsigned long *)&shinfo->evtchn_pending;
mask_bits = (unsigned long *)&shinfo->evtchn_mask;
@@ -1861,7 +1861,7 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
goto out_rcu;
}
- if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode) {
+ if (kvm_xen_has_64bit_shinfo(kvm)) {
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);
diff --git a/arch/x86/kvm/xen.h b/arch/x86/kvm/xen.h
index 59e6128a7bd3..3e8c9306eb89 100644
--- a/arch/x86/kvm/xen.h
+++ b/arch/x86/kvm/xen.h
@@ -248,6 +248,11 @@ struct compat_shared_info {
#define COMPAT_EVTCHN_2L_NR_CHANNELS (8 * \
sizeof_field(struct compat_shared_info, \
evtchn_pending))
+
+/* Latched VM-wide mode; the KVM equivalent of Xen's !has_32bit_shinfo(). */
+#define kvm_xen_has_64bit_shinfo(kvm) \
+ (IS_ENABLED(CONFIG_64BIT) && READ_ONCE((kvm)->arch.xen.long_mode))
+
struct compat_vcpu_runstate_info {
int state;
uint64_t state_entry_time;
--
2.54.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/8] KVM: x86/xen: Introduce kvm_xen_has_64bit_shinfo() macro
2026-06-05 14:17 ` [PATCH 2/8] KVM: x86/xen: Introduce kvm_xen_has_64bit_shinfo() macro David Woodhouse
@ 2026-06-06 9:30 ` David Laight
2026-06-06 9:35 ` David Woodhouse
0 siblings, 1 reply; 16+ messages in thread
From: David Laight @ 2026-06-06 9:30 UTC (permalink / raw)
To: David Woodhouse
Cc: Sean Christopherson, Paul Durrant, Hyunwoo Kim, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-kernel
On Fri, 5 Jun 2026 15:17:27 +0100
David Woodhouse <dwmw2@infradead.org> wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> Add a kvm_xen_has_64bit_shinfo() helper macro to replace the repeated
> pattern of 'IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode'
> throughout the Xen emulation code. The macro uses READ_ONCE() to
> ensure a consistent snapshot of the flag, which can be changed by
> another vCPU at any time.
If another vCPU changes the flag then isn't it all broken?
The code seems to need the value to be stable.
-- David
>
> This is the KVM equivalent of Xen's !has_32bit_shinfo().
>
> Assisted-by: Kiro:claude-opus-4.6-1m
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
> arch/x86/kvm/xen.c | 16 ++++++++--------
> arch/x86/kvm/xen.h | 5 +++++
> 2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index a7ab19f38b59..acd3cd87dd2f 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -72,7 +72,7 @@ static int kvm_xen_shared_info_init(struct kvm *kvm)
> BUILD_BUG_ON(offsetof(struct shared_info, wc) != 0xc00);
> BUILD_BUG_ON(offsetof(struct shared_info, wc_sec_hi) != 0xc0c);
>
> - if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode) {
> + if (kvm_xen_has_64bit_shinfo(kvm)) {
> struct shared_info *shinfo = gpc->khva;
>
> wc_sec_hi = &shinfo->wc_sec_hi;
> @@ -390,7 +390,7 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
> BUILD_BUG_ON(sizeof_field(struct vcpu_runstate_info, time) !=
> sizeof(vx->runstate_times));
>
> - if (IS_ENABLED(CONFIG_64BIT) && v->kvm->arch.xen.long_mode) {
> + if (kvm_xen_has_64bit_shinfo(v->kvm)) {
> user_len = sizeof(struct vcpu_runstate_info);
> times_ofs = offsetof(struct vcpu_runstate_info,
> state_entry_time);
> @@ -661,7 +661,7 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
> }
>
> /* Now gpc->khva is a valid kernel address for the vcpu_info */
> - if (IS_ENABLED(CONFIG_64BIT) && v->kvm->arch.xen.long_mode) {
> + if (kvm_xen_has_64bit_shinfo(v->kvm)) {
> struct vcpu_info *vi = gpc->khva;
>
> asm volatile(LOCK_PREFIX "orq %0, %1\n"
> @@ -978,7 +978,7 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
> * address, that's actually OK. kvm_xen_update_runstate_guest()
> * will cope.
> */
> - if (IS_ENABLED(CONFIG_64BIT) && vcpu->kvm->arch.xen.long_mode)
> + if (kvm_xen_has_64bit_shinfo(vcpu->kvm))
> sz = sizeof(struct vcpu_runstate_info);
> else
> sz = sizeof(struct compat_vcpu_runstate_info);
> @@ -1424,7 +1424,7 @@ static int kvm_xen_hypercall_complete_userspace(struct kvm_vcpu *vcpu)
>
> static inline int max_evtchn_port(struct kvm *kvm)
> {
> - if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode)
> + if (kvm_xen_has_64bit_shinfo(kvm))
> return EVTCHN_2L_NR_CHANNELS;
> else
> return COMPAT_EVTCHN_2L_NR_CHANNELS;
> @@ -1446,7 +1446,7 @@ static bool wait_pending_event(struct kvm_vcpu *vcpu, int nr_ports,
> goto out_rcu;
>
> ret = false;
> - if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode) {
> + if (kvm_xen_has_64bit_shinfo(kvm)) {
> struct shared_info *shinfo = gpc->khva;
> pending_bits = (unsigned long *)&shinfo->evtchn_pending;
> } else {
> @@ -1820,7 +1820,7 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
> if (!kvm_gpc_check(gpc, PAGE_SIZE))
> goto out_rcu;
>
> - if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode) {
> + if (kvm_xen_has_64bit_shinfo(kvm)) {
> struct shared_info *shinfo = gpc->khva;
> pending_bits = (unsigned long *)&shinfo->evtchn_pending;
> mask_bits = (unsigned long *)&shinfo->evtchn_mask;
> @@ -1861,7 +1861,7 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
> goto out_rcu;
> }
>
> - if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode) {
> + if (kvm_xen_has_64bit_shinfo(kvm)) {
> 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);
> diff --git a/arch/x86/kvm/xen.h b/arch/x86/kvm/xen.h
> index 59e6128a7bd3..3e8c9306eb89 100644
> --- a/arch/x86/kvm/xen.h
> +++ b/arch/x86/kvm/xen.h
> @@ -248,6 +248,11 @@ struct compat_shared_info {
> #define COMPAT_EVTCHN_2L_NR_CHANNELS (8 * \
> sizeof_field(struct compat_shared_info, \
> evtchn_pending))
> +
> +/* Latched VM-wide mode; the KVM equivalent of Xen's !has_32bit_shinfo(). */
> +#define kvm_xen_has_64bit_shinfo(kvm) \
> + (IS_ENABLED(CONFIG_64BIT) && READ_ONCE((kvm)->arch.xen.long_mode))
> +
> struct compat_vcpu_runstate_info {
> int state;
> uint64_t state_entry_time;
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/8] KVM: x86/xen: Introduce kvm_xen_has_64bit_shinfo() macro
2026-06-06 9:30 ` David Laight
@ 2026-06-06 9:35 ` David Woodhouse
2026-06-06 11:11 ` David Laight
0 siblings, 1 reply; 16+ messages in thread
From: David Woodhouse @ 2026-06-06 9:35 UTC (permalink / raw)
To: David Laight
Cc: Sean Christopherson, Paul Durrant, Hyunwoo Kim, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 698 bytes --]
On Sat, 2026-06-06 at 10:30 +0100, David Laight wrote:
> On Fri, 5 Jun 2026 15:17:27 +0100
> David Woodhouse <dwmw2@infradead.org> wrote:
>
> > From: David Woodhouse <dwmw@amazon.co.uk>
> >
> > Add a kvm_xen_has_64bit_shinfo() helper macro to replace the repeated
> > pattern of 'IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode'
> > throughout the Xen emulation code. The macro uses READ_ONCE() to
> > ensure a consistent snapshot of the flag, which can be changed by
> > another vCPU at any time.
>
> If another vCPU changes the flag then isn't it all broken?
> The code seems to need the value to be stable.
Do you believe the explanation in patch 4 to be insufficient?
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/8] KVM: x86/xen: Introduce kvm_xen_has_64bit_shinfo() macro
2026-06-06 9:35 ` David Woodhouse
@ 2026-06-06 11:11 ` David Laight
2026-06-06 11:22 ` David Woodhouse
0 siblings, 1 reply; 16+ messages in thread
From: David Laight @ 2026-06-06 11:11 UTC (permalink / raw)
To: David Woodhouse
Cc: Sean Christopherson, Paul Durrant, Hyunwoo Kim, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-kernel
On Sat, 06 Jun 2026 10:35:45 +0100
David Woodhouse <dwmw2@infradead.org> wrote:
> On Sat, 2026-06-06 at 10:30 +0100, David Laight wrote:
> > On Fri, 5 Jun 2026 15:17:27 +0100
> > David Woodhouse <dwmw2@infradead.org> wrote:
> >
> > > From: David Woodhouse <dwmw@amazon.co.uk>
> > >
> > > Add a kvm_xen_has_64bit_shinfo() helper macro to replace the repeated
> > > pattern of 'IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode'
> > > throughout the Xen emulation code. The macro uses READ_ONCE() to
> > > ensure a consistent snapshot of the flag, which can be changed by
> > > another vCPU at any time.
> >
> > If another vCPU changes the flag then isn't it all broken?
> > The code seems to need the value to be stable.
>
> Do you believe the explanation in patch 4 to be insufficient?
(I read patch 2 first...)
Changing the same lines twice is s bit excessive.
There is this bit in #4:
Even with this fix, the same corruption can occur if 64-bit mode is
latched and the guest switches to 32-bit mode immediately afterward.
which, I think, means it is all hopeless really.
If the structure layout doesn't depend on 32/64 bit then why check
(or compile-time check).
But if the layout differs and the the mode can change dynamically
then you need some kind of locking (or similar).
-- David
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/8] KVM: x86/xen: Introduce kvm_xen_has_64bit_shinfo() macro
2026-06-06 11:11 ` David Laight
@ 2026-06-06 11:22 ` David Woodhouse
0 siblings, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2026-06-06 11:22 UTC (permalink / raw)
To: David Laight
Cc: Sean Christopherson, Paul Durrant, Hyunwoo Kim, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]
On Sat, 2026-06-06 at 12:11 +0100, David Laight wrote:
> On Sat, 06 Jun 2026 10:35:45 +0100
> David Woodhouse <dwmw2@infradead.org> wrote:
>
> > On Sat, 2026-06-06 at 10:30 +0100, David Laight wrote:
> > > On Fri, 5 Jun 2026 15:17:27 +0100
> > > David Woodhouse <dwmw2@infradead.org> wrote:
> > >
> > > > From: David Woodhouse <dwmw@amazon.co.uk>
> > > >
> > > > Add a kvm_xen_has_64bit_shinfo() helper macro to replace the repeated
> > > > pattern of 'IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode'
> > > > throughout the Xen emulation code. The macro uses READ_ONCE() to
> > > > ensure a consistent snapshot of the flag, which can be changed by
> > > > another vCPU at any time.
> > >
> > > If another vCPU changes the flag then isn't it all broken?
> > > The code seems to need the value to be stable.
> >
> > Do you believe the explanation in patch 4 to be insufficient?
>
> (I read patch 2 first...)
> Changing the same lines twice is s bit excessive.
You mean a "no functional change" cleanup followed by the actual
change? No, that's normal.
>
> There is this bit in #4:
>
> Even with this fix, the same corruption can occur if 64-bit mode is
> latched and the guest switches to 32-bit mode immediately afterward.
>
> which, I think, means it is all hopeless really.
I'm glad you now think that. I had explained it twice, both in the
cover letter and in the commit message of patch 4 that I pointed you
at.
If a guest switches an *active* shared info page between 32 and 64 bit
then we make almost no attempt to convert the contents.
All we're doing here is making things prettier by at least being
consistent about whether we're treating it as 32-bit or 64-bit for the
duration of each hypercall or event.
Which is why it says, "This fix is for internal consistency rather than
correcting any observable bug."
Did you have a remaining question?
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/8] KVM: x86/xen: Rename max_evtchn_port() to kvm_max_evtchn_port()
2026-06-05 14:17 [PATCH 0/8] KVM: x86/xen: Clean up 32-bit vs. 64-bit shared info mode handling David Woodhouse
2026-06-05 14:17 ` [PATCH 1/8] KVM: x86/xen: Rename 'longmode' to 'is_64bit' in hypercall handling David Woodhouse
2026-06-05 14:17 ` [PATCH 2/8] KVM: x86/xen: Introduce kvm_xen_has_64bit_shinfo() macro David Woodhouse
@ 2026-06-05 14:17 ` David Woodhouse
2026-06-05 14:17 ` [PATCH 4/8] KVM: x86/xen: Latch shinfo mode in kvm_xen_set_evtchn_fast() David Woodhouse
` (4 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2026-06-05 14:17 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paul Durrant, Hyunwoo Kim, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-kernel
From: David Woodhouse <dwmw@amazon.co.uk>
Rename in preparation for adding a variant that takes a latched bool
argument for use in paths that need a consistent snapshot of the
shinfo mode.
No functional change.
Assisted-by: Kiro:claude-opus-4.6-1m
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
arch/x86/kvm/xen.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index acd3cd87dd2f..2c432fcfab15 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -1422,7 +1422,7 @@ static int kvm_xen_hypercall_complete_userspace(struct kvm_vcpu *vcpu)
return kvm_xen_hypercall_set_result(vcpu, run->xen.u.hcall.result);
}
-static inline int max_evtchn_port(struct kvm *kvm)
+static inline int kvm_max_evtchn_port(struct kvm *kvm)
{
if (kvm_xen_has_64bit_shinfo(kvm))
return EVTCHN_2L_NR_CHANNELS;
@@ -1529,7 +1529,7 @@ static bool kvm_xen_schedop_poll(struct kvm_vcpu *vcpu, bool is_64bit,
}
for (i = 0; i < sched_poll.nr_ports; i++) {
- if (ports[i] >= max_evtchn_port(vcpu->kvm)) {
+ if (ports[i] >= kvm_max_evtchn_port(vcpu->kvm)) {
*r = -EINVAL;
goto out;
}
@@ -1809,7 +1809,7 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
WRITE_ONCE(xe->vcpu_idx, vcpu->vcpu_idx);
}
- if (xe->port >= max_evtchn_port(kvm))
+ if (xe->port >= kvm_max_evtchn_port(kvm))
return -EINVAL;
rc = -EWOULDBLOCK;
@@ -1971,7 +1971,7 @@ int kvm_xen_setup_evtchn(struct kvm *kvm,
struct kvm_vcpu *vcpu;
/*
- * Don't check for the port being within range of max_evtchn_port().
+ * Don't check for the port being within range of kvm_max_evtchn_port().
* Userspace can configure what ever targets it likes; events just won't
* be delivered if/while the target is invalid, just like userspace can
* configure MSIs which target non-existent APICs.
@@ -1980,8 +1980,8 @@ int kvm_xen_setup_evtchn(struct kvm *kvm,
* can be restored *independently* of other things like creating vCPUs,
* without imposing an ordering dependency on userspace. In this
* particular case, the problematic ordering would be with setting the
- * Xen 'long mode' flag, which changes max_evtchn_port() to allow 4096
- * instead of 1024 event channels.
+ * Xen 'long mode' flag, which changes kvm_max_evtchn_port() to allow
+ * 4096 instead of 1024 event channels.
*/
/* We only support 2 level event channels for now */
@@ -2018,7 +2018,7 @@ int kvm_xen_hvm_evtchn_send(struct kvm *kvm, struct kvm_irq_routing_xen_evtchn *
struct kvm_xen_evtchn e;
int ret;
- if (!uxe->port || uxe->port >= max_evtchn_port(kvm))
+ if (!uxe->port || uxe->port >= kvm_max_evtchn_port(kvm))
return -EINVAL;
/* We only support 2 level event channels for now */
@@ -2128,7 +2128,7 @@ static int kvm_xen_eventfd_assign(struct kvm *kvm,
case EVTCHNSTAT_interdomain:
if (data->u.evtchn.deliver.port.port) {
- if (data->u.evtchn.deliver.port.port >= max_evtchn_port(kvm))
+ if (data->u.evtchn.deliver.port.port >= kvm_max_evtchn_port(kvm))
goto out_noeventfd; /* -EINVAL */
} else {
eventfd = eventfd_ctx_fdget(data->u.evtchn.deliver.eventfd.fd);
@@ -2246,7 +2246,7 @@ static int kvm_xen_setattr_evtchn(struct kvm *kvm, struct kvm_xen_hvm_attr *data
if (data->u.evtchn.flags == KVM_XEN_EVTCHN_RESET)
return kvm_xen_eventfd_reset(kvm);
- if (!port || port >= max_evtchn_port(kvm))
+ if (!port || port >= kvm_max_evtchn_port(kvm))
return -EINVAL;
if (data->u.evtchn.flags == KVM_XEN_EVTCHN_DEASSIGN)
--
2.54.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 4/8] KVM: x86/xen: Latch shinfo mode in kvm_xen_set_evtchn_fast()
2026-06-05 14:17 [PATCH 0/8] KVM: x86/xen: Clean up 32-bit vs. 64-bit shared info mode handling David Woodhouse
` (2 preceding siblings ...)
2026-06-05 14:17 ` [PATCH 3/8] KVM: x86/xen: Rename max_evtchn_port() to kvm_max_evtchn_port() David Woodhouse
@ 2026-06-05 14:17 ` David Woodhouse
2026-06-05 14:17 ` [PATCH 5/8] KVM: x86/xen: Latch shinfo mode in kvm_xen_schedop_poll() David Woodhouse
` (3 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2026-06-05 14:17 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paul Durrant, Hyunwoo Kim, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-kernel
From: Hyunwoo Kim <imv4bel@gmail.com>
kvm_xen_set_evtchn_fast() assumes the port range check in
max_evtchn_port() and the bitmap layout selection observe the same
shinfo mode, but each calls kvm_xen_has_64bit_shinfo() separately.
If the guest changes the mode in between, a port accepted by the
64-bit range check is handled with the 32-bit layout, and
port_word_bit falls outside evtchn_pending_sel.
Latch kvm_xen_has_64bit_shinfo() once on entry so the range check
and both layout computations use the same value.
In practice this is harmless: the evtchn_pending bitmap is at the same
offset in both native and compat shared_info layouts, so a stale mode
just results in setting a bit in what the guest (in its new compat mode)
considers the evtchn_mask, wallclock, or the arch_shared_info fields
which follow it — all of which are in the guest's own page. Even with
this fix, the same corruption can occur if 64-bit mode is latched and
the guest switches to 32-bit mode immediately afterward. Like Xen, KVM
makes no attempt to *convert* when shinfo mode is changed. Only the
wallclock field is updated in the new location.
This fix is for internal consistency rather than correcting any
observable bug.
Fixes: 14243b387137 ("KVM: x86/xen: Add KVM_IRQ_ROUTING_XEN_EVTCHN and event channel delivery")
Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
[dwmw2: Rework on top of long_mode/has_64bit_shinfo cleanups]
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
arch/x86/kvm/xen.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 2c432fcfab15..60bdf65216c2 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -1422,14 +1422,19 @@ static int kvm_xen_hypercall_complete_userspace(struct kvm_vcpu *vcpu)
return kvm_xen_hypercall_set_result(vcpu, run->xen.u.hcall.result);
}
-static inline int kvm_max_evtchn_port(struct kvm *kvm)
+static inline int max_evtchn_port(bool has_64bit_shinfo)
{
- if (kvm_xen_has_64bit_shinfo(kvm))
+ if (has_64bit_shinfo)
return EVTCHN_2L_NR_CHANNELS;
else
return COMPAT_EVTCHN_2L_NR_CHANNELS;
}
+static inline int kvm_max_evtchn_port(struct kvm *kvm)
+{
+ return max_evtchn_port(kvm_xen_has_64bit_shinfo(kvm));
+}
+
static bool wait_pending_event(struct kvm_vcpu *vcpu, int nr_ports,
evtchn_port_t *ports)
{
@@ -1792,8 +1797,9 @@ static void kvm_xen_check_poller(struct kvm_vcpu *vcpu, int port)
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;
- struct kvm_vcpu *vcpu;
+ bool has_64bit_shinfo = kvm_xen_has_64bit_shinfo(kvm);
unsigned long *pending_bits, *mask_bits;
+ struct kvm_vcpu *vcpu;
unsigned long flags;
int port_word_bit;
bool kick_vcpu = false;
@@ -1809,7 +1815,7 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
WRITE_ONCE(xe->vcpu_idx, vcpu->vcpu_idx);
}
- if (xe->port >= kvm_max_evtchn_port(kvm))
+ if (xe->port >= max_evtchn_port(has_64bit_shinfo))
return -EINVAL;
rc = -EWOULDBLOCK;
@@ -1820,7 +1826,7 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
if (!kvm_gpc_check(gpc, PAGE_SIZE))
goto out_rcu;
- if (kvm_xen_has_64bit_shinfo(kvm)) {
+ if (has_64bit_shinfo) {
struct shared_info *shinfo = gpc->khva;
pending_bits = (unsigned long *)&shinfo->evtchn_pending;
mask_bits = (unsigned long *)&shinfo->evtchn_mask;
@@ -1861,7 +1867,7 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
goto out_rcu;
}
- if (kvm_xen_has_64bit_shinfo(kvm)) {
+ 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);
--
2.54.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 5/8] KVM: x86/xen: Latch shinfo mode in kvm_xen_schedop_poll()
2026-06-05 14:17 [PATCH 0/8] KVM: x86/xen: Clean up 32-bit vs. 64-bit shared info mode handling David Woodhouse
` (3 preceding siblings ...)
2026-06-05 14:17 ` [PATCH 4/8] KVM: x86/xen: Latch shinfo mode in kvm_xen_set_evtchn_fast() David Woodhouse
@ 2026-06-05 14:17 ` David Woodhouse
2026-06-05 14:17 ` [PATCH 6/8] KVM: x86/xen: Enforce alignment of vcpu_info registration David Woodhouse
` (2 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2026-06-05 14:17 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paul Durrant, Hyunwoo Kim, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-kernel
From: David Woodhouse <dwmw@amazon.co.uk>
kvm_xen_schedop_poll() validates port numbers against
kvm_max_evtchn_port() and then calls wait_pending_event() which reads
the shinfo mode again to select the bitmap layout.
Latch kvm_xen_has_64bit_shinfo() once and pass it to both
max_evtchn_port() and wait_pending_event().
As with the previous fix to kvm_xen_set_evtchn_fast(), this is
harmless in practice for the same reasons: the inconsistency can only
corrupt fields in the guest's own shared_info page, and the same
corruption can occur anyway if the mode changes immediately after the
latch.
Fixes: d518b9d0fc80 ("KVM: x86/xen: handle PV spinlocks slowpath")
Assisted-by: Kiro:claude-opus-4.6-1m
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
arch/x86/kvm/xen.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 60bdf65216c2..f9085612e7df 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -1435,8 +1435,8 @@ static inline int kvm_max_evtchn_port(struct kvm *kvm)
return max_evtchn_port(kvm_xen_has_64bit_shinfo(kvm));
}
-static bool wait_pending_event(struct kvm_vcpu *vcpu, int nr_ports,
- evtchn_port_t *ports)
+static bool wait_pending_event(struct kvm_vcpu *vcpu, bool has_64bit_shinfo,
+ int nr_ports, evtchn_port_t *ports)
{
struct kvm *kvm = vcpu->kvm;
struct gfn_to_pfn_cache *gpc = &kvm->arch.xen.shinfo_cache;
@@ -1451,7 +1451,7 @@ static bool wait_pending_event(struct kvm_vcpu *vcpu, int nr_ports,
goto out_rcu;
ret = false;
- if (kvm_xen_has_64bit_shinfo(kvm)) {
+ if (has_64bit_shinfo) {
struct shared_info *shinfo = gpc->khva;
pending_bits = (unsigned long *)&shinfo->evtchn_pending;
} else {
@@ -1476,6 +1476,7 @@ static bool wait_pending_event(struct kvm_vcpu *vcpu, int nr_ports,
static bool kvm_xen_schedop_poll(struct kvm_vcpu *vcpu, bool is_64bit,
u64 param, u64 *r)
{
+ bool has_64bit_shinfo = kvm_xen_has_64bit_shinfo(vcpu->kvm);
struct sched_poll sched_poll;
evtchn_port_t port, *ports;
struct x86_exception e;
@@ -1534,7 +1535,7 @@ static bool kvm_xen_schedop_poll(struct kvm_vcpu *vcpu, bool is_64bit,
}
for (i = 0; i < sched_poll.nr_ports; i++) {
- if (ports[i] >= kvm_max_evtchn_port(vcpu->kvm)) {
+ if (ports[i] >= max_evtchn_port(has_64bit_shinfo)) {
*r = -EINVAL;
goto out;
}
@@ -1547,7 +1548,7 @@ static bool kvm_xen_schedop_poll(struct kvm_vcpu *vcpu, bool is_64bit,
set_bit(vcpu->vcpu_idx, vcpu->kvm->arch.xen.poll_mask);
- if (!wait_pending_event(vcpu, sched_poll.nr_ports, ports)) {
+ if (!wait_pending_event(vcpu, has_64bit_shinfo, sched_poll.nr_ports, ports)) {
kvm_set_mp_state(vcpu, KVM_MP_STATE_HALTED);
if (sched_poll.timeout)
--
2.54.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 6/8] KVM: x86/xen: Enforce alignment of vcpu_info registration
2026-06-05 14:17 [PATCH 0/8] KVM: x86/xen: Clean up 32-bit vs. 64-bit shared info mode handling David Woodhouse
` (4 preceding siblings ...)
2026-06-05 14:17 ` [PATCH 5/8] KVM: x86/xen: Latch shinfo mode in kvm_xen_schedop_poll() David Woodhouse
@ 2026-06-05 14:17 ` David Woodhouse
2026-06-05 14:17 ` [PATCH 7/8] KVM: x86/xen: Use 32-bit locked bts for vcpu_info evtchn_pending_sel David Woodhouse
2026-06-05 14:17 ` [PATCH 8/8] KVM: x86/xen: Use 32-bit locked ops in kvm_xen_inject_pending_events() David Woodhouse
7 siblings, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2026-06-05 14:17 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paul Durrant, Hyunwoo Kim, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-kernel
From: David Woodhouse <dwmw@amazon.co.uk>
Xen enforces that the vcpu_info GPA is aligned to sizeof(xen_ulong_t)
(8 bytes in 64-bit mode, 4 bytes in 32-bit mode) in map_guest_area().
KVM has no such check, allowing a guest to register a vcpu_info at an
arbitrary byte alignment. This can cause split-lock #AC exceptions on
hosts with split_lock_detect=fatal, since the event channel delivery
paths use locked atomic operations on evtchn_pending_sel within the
vcpu_info.
Add the same alignment check that Xen performs at vcpu_info
registration time, based on the current long_mode state. Return -ENXIO
on failure, matching Xen's map_guest_area() behaviour for unaligned
requests.
Like Xen, this allows a guest to register vcpu_info at a 4-byte (but
not 8-byte) aligned address while in 32-bit mode and then switch to
64-bit mode. Xen copes with this because it only uses 32-bit atomics
on vcpu_info fields, which is what subsequent commits will do where
necessary for KVM too.
Originally observed in review of an unrelated patch:
https://lore.kernel.org/all/20260604193554.1BA311F00893@smtp.kernel.org/
Cc: stable@vger.kernel.org
Fixes: 73e69a86347a ("KVM: x86/xen: register vcpu info")
Reported-by: sashiko-bot@kernel.org
Assisted-by: Kiro:claude-opus-4.6-1m
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
arch/x86/kvm/xen.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index f9085612e7df..ae0713718367 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -926,6 +926,12 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
break;
}
+ r = -ENXIO;
+ if (!IS_ALIGNED(data->u.gpa,
+ kvm_xen_has_64bit_shinfo(vcpu->kvm) ?
+ sizeof(unsigned long) : sizeof(u32)))
+ break;
+
r = kvm_gpc_activate(&vcpu->arch.xen.vcpu_info_cache,
data->u.gpa, sizeof(struct vcpu_info));
} else {
@@ -935,6 +941,12 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
break;
}
+ r = -ENXIO;
+ if (!IS_ALIGNED(data->u.hva,
+ kvm_xen_has_64bit_shinfo(vcpu->kvm) ?
+ sizeof(unsigned long) : sizeof(u32)))
+ break;
+
r = kvm_gpc_activate_hva(&vcpu->arch.xen.vcpu_info_cache,
data->u.hva, sizeof(struct vcpu_info));
}
--
2.54.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 7/8] KVM: x86/xen: Use 32-bit locked bts for vcpu_info evtchn_pending_sel
2026-06-05 14:17 [PATCH 0/8] KVM: x86/xen: Clean up 32-bit vs. 64-bit shared info mode handling David Woodhouse
` (5 preceding siblings ...)
2026-06-05 14:17 ` [PATCH 6/8] KVM: x86/xen: Enforce alignment of vcpu_info registration David Woodhouse
@ 2026-06-05 14:17 ` David Woodhouse
2026-07-08 15:48 ` Sean Christopherson
2026-06-05 14:17 ` [PATCH 8/8] KVM: x86/xen: Use 32-bit locked ops in kvm_xen_inject_pending_events() David Woodhouse
7 siblings, 1 reply; 16+ messages in thread
From: David Woodhouse @ 2026-06-05 14:17 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paul Durrant, Hyunwoo Kim, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-kernel
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, 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
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);
}
/*
@@ -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");
+ 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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 7/8] KVM: x86/xen: Use 32-bit locked bts for vcpu_info evtchn_pending_sel
2026-06-05 14:17 ` [PATCH 7/8] KVM: x86/xen: Use 32-bit locked bts for vcpu_info evtchn_pending_sel David Woodhouse
@ 2026-07-08 15:48 ` Sean Christopherson
0 siblings, 0 replies; 16+ messages in thread
From: Sean Christopherson @ 2026-07-08 15:48 UTC (permalink / raw)
To: David Woodhouse
Cc: Paul Durrant, Hyunwoo Kim, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-kernel
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 8/8] KVM: x86/xen: Use 32-bit locked ops in kvm_xen_inject_pending_events()
2026-06-05 14:17 [PATCH 0/8] KVM: x86/xen: Clean up 32-bit vs. 64-bit shared info mode handling David Woodhouse
` (6 preceding siblings ...)
2026-06-05 14:17 ` [PATCH 7/8] KVM: x86/xen: Use 32-bit locked bts for vcpu_info evtchn_pending_sel David Woodhouse
@ 2026-06-05 14:17 ` David Woodhouse
2026-07-08 15:23 ` Sean Christopherson
7 siblings, 1 reply; 16+ messages in thread
From: David Woodhouse @ 2026-06-05 14:17 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paul Durrant, Hyunwoo Kim, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-kernel
From: David Woodhouse <dwmw@amazon.co.uk>
The 64-bit path in kvm_xen_inject_pending_events() uses 'lock orq' and
'lock andq' on vcpu_info->evtchn_pending_sel. If the vcpu_info was
registered with only 4-byte alignment (valid for a 32-bit guest that
later switches to 64-bit mode), this 8-byte locked operation can cause
split-lock #AC exceptions on hosts with split_lock_detect=fatal.
Use the original 64-bit atomics when the vcpu_info is 8-byte aligned
(the common case). Fall back to a 32-bit loop for the rare case where
vcpu_info was registered at only 4-byte alignment. For compat guests
(32-bit evtchn_pending_sel) the loop executes once. For native guests
it executes a second iteration only if the high half has bits to
deliver.
Fixes: 14243b387137 ("KVM: x86/xen: Add KVM_IRQ_ROUTING_XEN_EVTCHN and event channel delivery")
Reported-by: sashiko-bot@kernel.org
Assisted-by: Kiro:claude-opus-4.6-1m
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
arch/x86/kvm/xen.c | 63 ++++++++++++++++++++++++++++++++--------------
1 file changed, 44 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 24e939ef5d64..e7b0263d5143 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -638,11 +638,17 @@ void kvm_xen_inject_vcpu_vector(struct kvm_vcpu *v)
*/
void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
{
- unsigned long evtchn_pending_sel = READ_ONCE(v->arch.xen.evtchn_pending_sel);
struct gfn_to_pfn_cache *gpc = &v->arch.xen.vcpu_info_cache;
+ bool has_64bit_shinfo = kvm_xen_has_64bit_shinfo(v->kvm);
+ union evtchn_pending_sel {
+ u64 sel64;
+ u32 sel32[2];
+ } pending, *sel_addr;
+ struct vcpu_info *vi;
unsigned long flags;
- if (!evtchn_pending_sel)
+ pending.sel64 = READ_ONCE(v->arch.xen.evtchn_pending_sel);
+ if (!pending.sel64)
return;
/*
@@ -661,31 +667,50 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
}
/* Now gpc->khva is a valid kernel address for the vcpu_info */
- if (kvm_xen_has_64bit_shinfo(v->kvm)) {
- struct vcpu_info *vi = gpc->khva;
+ vi = gpc->khva;
+ sel_addr = gpc->khva + (has_64bit_shinfo ?
+ offsetof(struct vcpu_info, evtchn_pending_sel) :
+ offsetof(struct compat_vcpu_info, evtchn_pending_sel));
+ if (has_64bit_shinfo && IS_ALIGNED((unsigned long)sel_addr, sizeof(u64))) {
+ /*
+ * 64-bit shinfo with 8-byte aligned vcpu_info (the common
+ * case): use a single 64-bit atomic.
+ */
asm volatile(LOCK_PREFIX "orq %0, %1\n"
"notq %0\n"
LOCK_PREFIX "andq %0, %2\n"
- : "=r" (evtchn_pending_sel),
- "+m" (vi->evtchn_pending_sel),
+ : "=r" (pending.sel64),
+ "+m" (sel_addr->sel64),
"+m" (v->arch.xen.evtchn_pending_sel)
- : "0" (evtchn_pending_sel));
- WRITE_ONCE(vi->evtchn_upcall_pending, 1);
+ : "0" (pending.sel64));
} else {
- u32 evtchn_pending_sel32 = evtchn_pending_sel;
- struct compat_vcpu_info *vi = gpc->khva;
-
- asm volatile(LOCK_PREFIX "orl %0, %1\n"
- "notl %0\n"
- LOCK_PREFIX "andl %0, %2\n"
- : "=r" (evtchn_pending_sel32),
- "+m" (vi->evtchn_pending_sel),
- "+m" (v->arch.xen.evtchn_pending_sel)
- : "0" (evtchn_pending_sel32));
- WRITE_ONCE(vi->evtchn_upcall_pending, 1);
+ /*
+ * Use 32-bit operations to avoid splitlock on a vcpu_info
+ * that is only 4-byte aligned (registered in 32-bit mode).
+ * The loop copes with the extremely rare case that the
+ * vcpu_info was registered in 32-bit mode and only enforced
+ * 4-byte alignment, and then the VM was latched to 64-bit
+ * mode afterwards. Which Xen tolerates, so so should KVM.
+ */
+ int i = 0;
+ do {
+ asm volatile(LOCK_PREFIX "orl %0, %1\n"
+ "notl %0\n"
+ LOCK_PREFIX "andl %0, %2\n"
+ : "=r" (pending.sel32[i]),
+ "+m" (sel_addr->sel32[i]),
+ "+m" (((u32 *)&v->arch.xen.evtchn_pending_sel)[i])
+ : "0" (pending.sel32[i]));
+ i++;
+ } while (has_64bit_shinfo && i < 2 && pending.sel32[i]);
}
+ /* Assert that there is no need for compat for evtchn_upcall_pending */
+ 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);
+
kvm_gpc_mark_dirty_in_slot(gpc);
read_unlock_irqrestore(&gpc->lock, flags);
--
2.54.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 8/8] KVM: x86/xen: Use 32-bit locked ops in kvm_xen_inject_pending_events()
2026-06-05 14:17 ` [PATCH 8/8] KVM: x86/xen: Use 32-bit locked ops in kvm_xen_inject_pending_events() David Woodhouse
@ 2026-07-08 15:23 ` Sean Christopherson
2026-07-08 15:41 ` David Woodhouse
0 siblings, 1 reply; 16+ messages in thread
From: Sean Christopherson @ 2026-07-08 15:23 UTC (permalink / raw)
To: David Woodhouse
Cc: Paul Durrant, Hyunwoo Kim, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-kernel
On Fri, Jun 05, 2026, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> The 64-bit path in kvm_xen_inject_pending_events() uses 'lock orq' and
> 'lock andq' on vcpu_info->evtchn_pending_sel. If the vcpu_info was
> registered with only 4-byte alignment (valid for a 32-bit guest that
> later switches to 64-bit mode), this 8-byte locked operation can cause
> split-lock #AC exceptions on hosts with split_lock_detect=fatal.
>
> Use the original 64-bit atomics when the vcpu_info is 8-byte aligned
> (the common case). Fall back to a 32-bit loop for the rare case where
> vcpu_info was registered at only 4-byte alignment. For compat guests
> (32-bit evtchn_pending_sel) the loop executes once. For native guests
> it executes a second iteration only if the high half has bits to
> deliver.
>
> Fixes: 14243b387137 ("KVM: x86/xen: Add KVM_IRQ_ROUTING_XEN_EVTCHN and event channel delivery")
> Reported-by: sashiko-bot@kernel.org
> Assisted-by: Kiro:claude-opus-4.6-1m
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
> arch/x86/kvm/xen.c | 63 ++++++++++++++++++++++++++++++++--------------
> 1 file changed, 44 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 24e939ef5d64..e7b0263d5143 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -638,11 +638,17 @@ void kvm_xen_inject_vcpu_vector(struct kvm_vcpu *v)
> */
> void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
> {
> - unsigned long evtchn_pending_sel = READ_ONCE(v->arch.xen.evtchn_pending_sel);
> struct gfn_to_pfn_cache *gpc = &v->arch.xen.vcpu_info_cache;
> + bool has_64bit_shinfo = kvm_xen_has_64bit_shinfo(v->kvm);
> + union evtchn_pending_sel {
> + u64 sel64;
> + u32 sel32[2];
> + } pending, *sel_addr;
> + struct vcpu_info *vi;
> unsigned long flags;
>
> - if (!evtchn_pending_sel)
> + pending.sel64 = READ_ONCE(v->arch.xen.evtchn_pending_sel);
> + if (!pending.sel64)
> return;
>
> /*
> @@ -661,31 +667,50 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
> }
>
> /* Now gpc->khva is a valid kernel address for the vcpu_info */
> - if (kvm_xen_has_64bit_shinfo(v->kvm)) {
> - struct vcpu_info *vi = gpc->khva;
> + vi = gpc->khva;
> + sel_addr = gpc->khva + (has_64bit_shinfo ?
> + offsetof(struct vcpu_info, evtchn_pending_sel) :
> + offsetof(struct compat_vcpu_info, evtchn_pending_sel));
>
> + if (has_64bit_shinfo && IS_ALIGNED((unsigned long)sel_addr, sizeof(u64))) {
> + /*
> + * 64-bit shinfo with 8-byte aligned vcpu_info (the common
> + * case): use a single 64-bit atomic.
Nit, the "single 64-bit atomic" is confusing because there are obviously two
atomic operations in the assembly below.
> + */
> asm volatile(LOCK_PREFIX "orq %0, %1\n"
> "notq %0\n"
> LOCK_PREFIX "andq %0, %2\n"
> - : "=r" (evtchn_pending_sel),
> - "+m" (vi->evtchn_pending_sel),
> + : "=r" (pending.sel64),
> + "+m" (sel_addr->sel64),
> "+m" (v->arch.xen.evtchn_pending_sel)
> - : "0" (evtchn_pending_sel));
> - WRITE_ONCE(vi->evtchn_upcall_pending, 1);
> + : "0" (pending.sel64));
> } else {
> - u32 evtchn_pending_sel32 = evtchn_pending_sel;
> - struct compat_vcpu_info *vi = gpc->khva;
> -
> - asm volatile(LOCK_PREFIX "orl %0, %1\n"
> - "notl %0\n"
> - LOCK_PREFIX "andl %0, %2\n"
> - : "=r" (evtchn_pending_sel32),
> - "+m" (vi->evtchn_pending_sel),
> - "+m" (v->arch.xen.evtchn_pending_sel)
> - : "0" (evtchn_pending_sel32));
> - WRITE_ONCE(vi->evtchn_upcall_pending, 1);
> + /*
> + * Use 32-bit operations to avoid splitlock on a vcpu_info
> + * that is only 4-byte aligned (registered in 32-bit mode).
> + * The loop copes with the extremely rare case that the
> + * vcpu_info was registered in 32-bit mode and only enforced
> + * 4-byte alignment, and then the VM was latched to 64-bit
> + * mode afterwards. Which Xen tolerates, so so should KVM.
> + */
> + int i = 0;
> + do {
> + asm volatile(LOCK_PREFIX "orl %0, %1\n"
> + "notl %0\n"
> + LOCK_PREFIX "andl %0, %2\n"
> + : "=r" (pending.sel32[i]),
> + "+m" (sel_addr->sel32[i]),
> + "+m" (((u32 *)&v->arch.xen.evtchn_pending_sel)[i])
> + : "0" (pending.sel32[i]));
> + i++;
> + } while (has_64bit_shinfo && i < 2 && pending.sel32[i]);
This is... impressive? Related to the above comment about there being two separate
atomic operation, only the access to vi->evtchn_pending_sel needs to deal with
potential split-lock issues. And there's zero to handle the NOT in the asm blob.
Rather than munge the 32-bit and 64-bit cases together, just manually handle the
case where the bitwise-OR needs to be chunked in two.
--
From: Sean Christopherson <seanjc@google.com>
Date: Wed, 8 Jul 2026 08:09:19 -0700
Subject: [PATCH] KVM: x86/xen: Use 32-bit atomics if vCPU's evtchn_pending_sel
isn't aligned
When propagating pending Xen events from KVM's "cache" to the guest-visible
structure, use two 32-bit atomic operations to do the bitwise-OR into the
guest-controlled structure if the structure isn't 64-bit aligned, i.e. if
the guest registered its vcpu_info in 32-bit mode and then switched to
64-bit mode, in which case using a 64-bit atomic OR will generate a
split-lock #AC (if enabled).
Opportunistically isolate the clearing of the bits from KVM's cache, as
that structure is KVM-controlled, i.e. is guaranteed to be 64-bit aligned.
This will allow dropping the open-coded inline asm blobs in the future.
Fixes: 14243b387137 ("KVM: x86/xen: Add KVM_IRQ_ROUTING_XEN_EVTCHN and event channel delivery")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/xen.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 24e939ef5d64..7a7f90710847 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -664,13 +664,21 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
if (kvm_xen_has_64bit_shinfo(v->kvm)) {
struct vcpu_info *vi = gpc->khva;
- asm volatile(LOCK_PREFIX "orq %0, %1\n"
- "notq %0\n"
- LOCK_PREFIX "andq %0, %2\n"
- : "=r" (evtchn_pending_sel),
- "+m" (vi->evtchn_pending_sel),
- "+m" (v->arch.xen.evtchn_pending_sel)
- : "0" (evtchn_pending_sel));
+ if (IS_ALIGNED((unsigned long)&vi->evtchn_pending_sel, sizeof(u64)))
+ asm volatile(LOCK_PREFIX "orq %[src], %[dst]\n"
+ : [dst] "+m" (vi->evtchn_pending_sel)
+ : [src] "r" (evtchn_pending_sel));
+ else
+ asm volatile(LOCK_PREFIX "orl %[src_lo], %[dst_lo]\n"
+ LOCK_PREFIX "orl %[src_hi], %[dst_hi]\n"
+ : [dst_lo] "+m" (vi->evtchn_pending_sel),
+ [dst_hi] "+m" (*(((u32 *)&vi->evtchn_pending_sel) + 1))
+ : [src_lo] "r" ((u32)evtchn_pending_sel),
+ [src_hi] "r" ((u32)(evtchn_pending_sel >> 32)));
+
+ asm volatile(LOCK_PREFIX "andq %1, %0\n"
+ : "+m" (v->arch.xen.evtchn_pending_sel)
+ : "r" (~evtchn_pending_sel));
WRITE_ONCE(vi->evtchn_upcall_pending, 1);
} else {
u32 evtchn_pending_sel32 = evtchn_pending_sel;
base-commit: 0c393754b28263323bed3ac091744ff8456c97d0
--
And then as a follow-up, drop the inline asm:
--
From: Sean Christopherson <seanjc@google.com>
Date: Wed, 8 Jul 2026 08:16:22 -0700
Subject: [PATCH] KVM: x86/xen: Use atomic*() APIs instead of open coded
equivalents
Replace the open coded atomic asm blobs in the Xen event injection code
with equivalent atomic{,64}_xxx() operations. Casting the event channel
to atomic types is ugly, but not as ugly as asm blobs.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/xen.c | 32 ++++++++++----------------------
1 file changed, 10 insertions(+), 22 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 7a7f90710847..94d1644ca6d1 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -663,34 +663,22 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
/* Now gpc->khva is a valid kernel address for the vcpu_info */
if (kvm_xen_has_64bit_shinfo(v->kvm)) {
struct vcpu_info *vi = gpc->khva;
+ void *vi_pending_sel = &vi->evtchn_pending_sel;
- if (IS_ALIGNED((unsigned long)&vi->evtchn_pending_sel, sizeof(u64)))
- asm volatile(LOCK_PREFIX "orq %[src], %[dst]\n"
- : [dst] "+m" (vi->evtchn_pending_sel)
- : [src] "r" (evtchn_pending_sel));
- else
- asm volatile(LOCK_PREFIX "orl %[src_lo], %[dst_lo]\n"
- LOCK_PREFIX "orl %[src_hi], %[dst_hi]\n"
- : [dst_lo] "+m" (vi->evtchn_pending_sel),
- [dst_hi] "+m" (*(((u32 *)&vi->evtchn_pending_sel) + 1))
- : [src_lo] "r" ((u32)evtchn_pending_sel),
- [src_hi] "r" ((u32)(evtchn_pending_sel >> 32)));
+ if (IS_ALIGNED((unsigned long)vi_pending_sel, sizeof(u64))) {
+ atomic64_or(evtchn_pending_sel, vi_pending_sel);
+ } else {
+ atomic_or(evtchn_pending_sel, vi_pending_sel);
+ atomic_or(evtchn_pending_sel >> 32, vi_pending_sel + 4);
+ }
- asm volatile(LOCK_PREFIX "andq %1, %0\n"
- : "+m" (v->arch.xen.evtchn_pending_sel)
- : "r" (~evtchn_pending_sel));
+ atomic64_andnot(evtchn_pending_sel, (void *)&v->arch.xen.evtchn_pending_sel);
WRITE_ONCE(vi->evtchn_upcall_pending, 1);
} else {
- u32 evtchn_pending_sel32 = evtchn_pending_sel;
struct compat_vcpu_info *vi = gpc->khva;
- asm volatile(LOCK_PREFIX "orl %0, %1\n"
- "notl %0\n"
- LOCK_PREFIX "andl %0, %2\n"
- : "=r" (evtchn_pending_sel32),
- "+m" (vi->evtchn_pending_sel),
- "+m" (v->arch.xen.evtchn_pending_sel)
- : "0" (evtchn_pending_sel32));
+ atomic_or(evtchn_pending_sel, (void *)&vi->evtchn_pending_sel);
+ atomic_andnot(evtchn_pending_sel, (void *)&v->arch.xen.evtchn_pending_sel);
WRITE_ONCE(vi->evtchn_upcall_pending, 1);
}
base-commit: f7a8319462668aa415333f2853b70eb82ea17f34
--
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 8/8] KVM: x86/xen: Use 32-bit locked ops in kvm_xen_inject_pending_events()
2026-07-08 15:23 ` Sean Christopherson
@ 2026-07-08 15:41 ` David Woodhouse
0 siblings, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2026-07-08 15:41 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paul Durrant, Hyunwoo Kim, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 596 bytes --]
On Wed, 2026-07-08 at 08:23 -0700, Sean Christopherson wrote:
> This is... impressive? Related to the above comment about there being two separate
> atomic operation, only the access to vi->evtchn_pending_sel needs to deal with
> potential split-lock issues. And there's zero to handle the NOT in the asm blob.
>
> Rather than munge the 32-bit and 64-bit cases together, just manually handle the
> case where the bitwise-OR needs to be chunked in two.
Yeah... that seems a whole lot more sensible than the ... "impressive"
version. I think I got a bit lost there... :)
Thanks.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread