Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH] KVM: x86/xen: Use read_trylock() for GPC locks in hardirq paths
@ 2026-08-07 12:19 sai madhu
  2026-08-07 12:19 ` sai madhu
  0 siblings, 1 reply; 3+ messages in thread
From: sai madhu @ 2026-08-07 12:19 UTC (permalink / raw)
  To: kvm; +Cc: dwmw2, seanjc, pbonzini, syzbot+919877893c9d28162dc2, sai madhu

Hi KVM maintainers,

This patch fixes a lockdep WARNING in kvm_xen_set_evtchn_fast() when
called from xen_timer_callback() in hard IRQ context.

Bug report:
  https://syzkaller.appspot.com/bug?extid=919877893c9d28162dc2

Observed crash (unpatched):
  [ BUG: Invalid wait context ]
  (&gpc->lock) at kvm_xen_set_evtchn_fast+0x1fb
  context-{2:2}  (hard IRQ, from xen_timer_callback)

Fix:
  Use read_trylock() on gpc->lock and return -EWOULDBLOCK on failure.
  The existing slow path (timer_pending + kvm_xen_inject_timer_irqs)
  handles deferred delivery safely in vcpu_run context.

This patch is based on David Woodhouse's May 2026 series (patch 3/7).
I reproduced the syzbot C repro on x86_64 with nested KVM:

  Unpatched bzImage-b69053dd: 1x lockdep BUG at ~69s
  Patched bzImage-xen-fix:    0x lockdep BUG (same repro)

Tested-by: sai madhu <suryasaimadhu369@gmail.com>

Thanks,
sai madhu

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] KVM: x86/xen: Use read_trylock() for GPC locks in hardirq paths
  2026-08-07 12:19 [PATCH] KVM: x86/xen: Use read_trylock() for GPC locks in hardirq paths sai madhu
@ 2026-08-07 12:19 ` sai madhu
  2026-08-07 12:28   ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: sai madhu @ 2026-08-07 12:19 UTC (permalink / raw)
  To: kvm; +Cc: dwmw2, seanjc, pbonzini, syzbot+919877893c9d28162dc2, sai madhu

kvm_xen_set_evtchn_fast() is called from hardirq context (xen timer
callback, kvm_arch_set_irq_inatomic()). On PREEMPT_RT (and under
lockdep), read_lock_irqsave(&gpc->lock) is invalid there because
rwlock_t may sleep.

Switch to read_trylock() and return -EWOULDBLOCK when the lock is
contended or the cache is invalid. xen_timer_callback() already
defers to the slow path (timer_pending + KVM_REQ_UNBLOCK +
kvm_xen_inject_timer_irqs()).

Also fix __kvm_xen_has_interrupt() the same way for atomic callers.

Based on David Woodhouse's upstream series (May 2026, patch 3/7).
Fixes syzbot: https://syzkaller.appspot.com/bug?extid=919877893c9d28162dc2

Tested-by: sai madhu <suryasaimadhu369@gmail.com>
Signed-off-by: sai madhu <suryasaimadhu369@gmail.com>
---
 arch/x86/kvm/xen.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 694b31c1f..02f30d795 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -697,6 +697,7 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
 int __kvm_xen_has_interrupt(struct kvm_vcpu *v)
 {
 	struct gfn_to_pfn_cache *gpc = &v->arch.xen.vcpu_info_cache;
+	bool atomic = in_atomic() || !task_is_running(current);
 	unsigned long flags;
 	u8 rc = 0;
 
@@ -713,7 +714,15 @@ int __kvm_xen_has_interrupt(struct kvm_vcpu *v)
 	BUILD_BUG_ON(sizeof(rc) !=
 		     sizeof_field(struct compat_vcpu_info, evtchn_upcall_pending));
 
-	read_lock_irqsave(&gpc->lock, flags);
+	if (atomic) {
+		local_irq_save(flags);
+		if (!read_trylock(&gpc->lock)) {
+			local_irq_restore(flags);
+			return 1;
+		}
+	} else {
+		read_lock_irqsave(&gpc->lock, flags);
+	}
 	while (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
 		read_unlock_irqrestore(&gpc->lock, flags);
 
@@ -725,7 +734,7 @@ int __kvm_xen_has_interrupt(struct kvm_vcpu *v)
 		 * and we'll end up getting called again from a context where we *can*
 		 * fault in the page and wait for it.
 		 */
-		if (in_atomic() || !task_is_running(current))
+		if (atomic)
 			return 1;
 
 		if (kvm_gpc_refresh(gpc, sizeof(struct vcpu_info))) {
@@ -1797,7 +1806,6 @@ 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;
 	unsigned long *pending_bits, *mask_bits;
-	unsigned long flags;
 	int port_word_bit;
 	bool kick_vcpu = false;
 	int vcpu_idx, idx, rc;
@@ -1819,9 +1827,10 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
 
 	idx = srcu_read_lock(&kvm->srcu);
 
-	read_lock_irqsave(&gpc->lock, flags);
-	if (!kvm_gpc_check(gpc, PAGE_SIZE))
+	if (!read_trylock(&gpc->lock))
 		goto out_rcu;
+	if (!kvm_gpc_check(gpc, PAGE_SIZE))
+		goto out_unlock;
 
 	if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode) {
 		struct shared_info *shinfo = gpc->khva;
@@ -1850,11 +1859,10 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
 	} else {
 		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);
+		read_unlock(&gpc->lock);
 		gpc = &vcpu->arch.xen.vcpu_info_cache;
 
-		read_lock_irqsave(&gpc->lock, flags);
-		if (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
+		if (!read_trylock(&gpc->lock)) {
 			/*
 			 * Could not access the vcpu_info. Set the bit in-kernel
 			 * and prod the vCPU to deliver it for itself.
@@ -1863,6 +1871,11 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
 				kick_vcpu = true;
 			goto out_rcu;
 		}
+		if (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
+			if (!test_and_set_bit(port_word_bit, &vcpu->arch.xen.evtchn_pending_sel))
+				kick_vcpu = true;
+			goto out_unlock;
+		}
 
 		if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode) {
 			struct vcpu_info *vcpu_info = gpc->khva;
@@ -1886,8 +1899,9 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
 		}
 	}
 
+ out_unlock:
+	read_unlock(&gpc->lock);
  out_rcu:
-	read_unlock_irqrestore(&gpc->lock, flags);
 	srcu_read_unlock(&kvm->srcu, idx);
 
 	if (kick_vcpu) {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] KVM: x86/xen: Use read_trylock() for GPC locks in hardirq paths
  2026-08-07 12:19 ` sai madhu
@ 2026-08-07 12:28   ` sashiko-bot
  0 siblings, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-07 12:28 UTC (permalink / raw)
  To: sai madhu; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hardware interrupt leak on PREEMPT_RT in __kvm_xen_has_interrupt()
--

--- Patch [1]: [PATCH] KVM: x86/xen: Use read_trylock() for GPC locks in hardirq paths ---
commit 094309ad41a72add2b4de793f00e2fe6f42178a3
Author: sai madhu <suryasaimadhu369@gmail.com>

KVM: x86/xen: Use read_trylock() for GPC locks in hardirq paths

This commit switches GPC locks in hardirq paths to use read_trylock()
instead of read_lock_irqsave(). This prevents sleeping on PREEMPT_RT
and under lockdep, returning -EWOULDBLOCK when the lock is contended.

Fixes syzbot: https://syzkaller.appspot.com/bug?extid=919877893c9d28162dc2

> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 694b31c1fcc9a..02f30d79513db 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -713,7 +714,15 @@ int __kvm_xen_has_interrupt(struct kvm_vcpu *v)
>  	BUILD_BUG_ON(sizeof(rc) !=
>  		     sizeof_field(struct compat_vcpu_info, evtchn_upcall_pending));
>  
> -	read_lock_irqsave(&gpc->lock, flags);
> +	if (atomic) {
> +		local_irq_save(flags);
> +		if (!read_trylock(&gpc->lock)) {
> +			local_irq_restore(flags);
> +			return 1;
> +		}
> +	} else {
> +		read_lock_irqsave(&gpc->lock, flags);
> +	}
>  	while (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
>  		read_unlock_irqrestore(&gpc->lock, flags);

[Severity: High]
Does this leave hardware interrupts disabled on PREEMPT_RT?

Since local_irq_save(flags) is called manually here when atomic is true, but
the lock is later released using read_unlock_irqrestore(&gpc->lock, flags),
will the hardware interrupt state be correctly restored?

Looking at include/linux/rwlock_rt.h, read_unlock_irqrestore() for rwlock_t
on PREEMPT_RT is mapped to rt_read_unlock(), which drops the lock but
completely ignores the flags parameter:

include/linux/rwlock_rt.h:
static __always_inline void read_unlock_irqrestore(rwlock_t *rwlock,
						   unsigned long flags)
{
	rt_read_unlock(rwlock);
}

If atomic is true, doesn't this mean the previously saved hardware interrupt
state is never restored here or at the end of the function, causing IRQs to
remain disabled indefinitely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807121906.925704-2-suryasaimadhu369@gmail.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-07 12:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-07 12:19 [PATCH] KVM: x86/xen: Use read_trylock() for GPC locks in hardirq paths sai madhu
2026-08-07 12:19 ` sai madhu
2026-08-07 12:28   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox