All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Sean Christopherson" <seanjc@google.com>
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH v2 02/20] KVM: x86/xen: Use read_trylock() for GPC locks in hardirq/atomic paths
Date: Fri, 29 May 2026 17:20:16 +0000	[thread overview]
Message-ID: <20260529172016.ACE851F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260529165114.748639-3-seanjc@google.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbalanced interrupt disable causes permanent hard IRQ disable on PREEMPT_RT
--

commit 13e357726c8cbadaf777f9052a529db65b824338
Author: David Woodhouse <dwmw@amazon.co.uk>
KVM: x86/xen: Use read_trylock() for GPC locks in hardirq/atomic paths

This patch modifies the GPC lock acquisition paths to use read_trylock()
instead of read_lock_irqsave() in atomic contexts. It introduces an explicit
local_irq_save() to prevent issues on PREEMPT_RT kernels where rwlock_t
cannot be used with read_lock_irqsave() from hardirq context.

Reported-by: syzbot+208f7f3e5f59c11aeb90@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=208f7f3e5f59c11aeb90
Fixes: 14243b387137 ("KVM: x86/xen: Add KVM_IRQ_ROUTING_XEN_EVTCHN and event channel delivery")

> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 91fd3673c09a2..9bdb8e3cad58b 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 result in unbalanced interrupt disables on PREEMPT_RT?

When atomic is true, local_irq_save() explicitly disables hardware interrupts.
On standard kernels, read_unlock_irqrestore() restores the interrupt state
correctly.

However, on PREEMPT_RT, rwlock_t is a sleepable lock and the implementation
of read_unlock_irqrestore() ignores the flags parameter, merely dropping the
lock without re-enabling interrupts. The interrupt state saved by
local_irq_save() is never restored, which would lead to hardware interrupts
being permanently disabled on the CPU.

I noticed this issue is resolved later in the series by commit 625a2fc07daa6
("KVM: x86/xen: Remove unnecessary irqsave from GPC lock usage in xen.c").

Should this fix be folded into this patch to ensure the kernel remains fully
functional and bisectable at this step?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529165114.748639-1-seanjc@google.com?part=2

  reply	other threads:[~2026-05-29 17:20 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-29 16:50 [PATCH v2 00/20] KVM: x86/xen: Fix Xen/GP/PREEMPT_RT issues with rwlock_t Sean Christopherson
2026-05-29 16:50 ` [PATCH v2 01/20] locking/rt: Use raw_spin_lock_irqsave() in __rwbase_read_unlock() Sean Christopherson
2026-05-29 19:32   ` Peter Zijlstra
2026-05-29 19:34     ` Peter Zijlstra
2026-05-29 20:05       ` Sean Christopherson
2026-05-29 20:13         ` Peter Zijlstra
2026-05-29 20:38           ` Peter Zijlstra
2026-05-30  0:54             ` Sean Christopherson
2026-05-30 10:26               ` Paolo Bonzini
2026-05-30 12:47                 ` David Woodhouse
2026-05-30 14:40                   ` Paolo Bonzini
2026-06-01 10:52                     ` David Woodhouse
2026-06-01 13:01                       ` David Woodhouse
2026-06-01 13:40                         ` Sebastian Andrzej Siewior
2026-06-01 13:53                           ` David Woodhouse
2026-06-01 14:47                             ` Sebastian Andrzej Siewior
2026-06-01 15:11                               ` David Woodhouse
2026-06-01  9:40                   ` Peter Zijlstra
2026-06-01 10:04                     ` David Woodhouse
2026-05-30 13:02                 ` Paolo Bonzini
2026-06-01  8:40                 ` Peter Zijlstra
2026-06-01 11:11                   ` Sebastian Andrzej Siewior
2026-06-01 11:40                     ` Peter Zijlstra
2026-06-01 19:13                     ` Paolo Bonzini
2026-06-02  7:34                       ` Sebastian Andrzej Siewior
2026-06-04 23:58             ` David Woodhouse
2026-05-29 16:50 ` [PATCH v2 02/20] KVM: x86/xen: Use read_trylock() for GPC locks in hardirq/atomic paths Sean Christopherson
2026-05-29 17:20   ` sashiko-bot [this message]
2026-05-29 23:28   ` Hillf Danton
2026-05-29 16:50 ` [PATCH v2 03/20] KVM: x86/xen: Remove unnecessary irqsave from GPC lock usage in xen.c Sean Christopherson
2026-05-29 17:36   ` sashiko-bot
2026-05-29 16:50 ` [PATCH v2 04/20] KVM: x86: Remove unnecessary irqsave from kvm_setup_guest_pvclock() Sean Christopherson
2026-05-29 16:50 ` [PATCH v2 05/20] KVM: Remove unnecessary IRQ disabling from GPC lock in pfncache.c Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 06/20] KVM: x86/xen: Use guard() to grab kvm->srcu around gpc critical sections Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 07/20] KVM: x86/xen: Extract delivery of event to vCPU into a separate helper Sean Christopherson
2026-05-29 17:47   ` sashiko-bot
2026-05-29 16:51 ` [PATCH v2 08/20] KVM: x86/xen: Explicitly tag "shared info" page as never being dirty tracked Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 09/20] KVM: x86/xen: Don't dirty track "vCPU info" page Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 10/20] KVM: Move {g,p}fn <=> {g,h}pa conversion helpers to kvm_types.h Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 11/20] KVM: Add CLASS() constructs to automagically handle lock+check of gpc Sean Christopherson
2026-05-29 17:59   ` sashiko-bot
2026-05-29 16:51 ` [PATCH v2 12/20] KVM: x86/xen: Convert kvm_xen_shared_info_init() to gpc's CLASS() APIs Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 13/20] KVM: x86/xen: Don't bother waiting on gpc->lock in SCHEDOP_poll Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 14/20] KVM: x86/xen: Convert wait_pending_event() to gpc's CLASS() APIs Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 15/20] KVM: x86/xen: Convert kvm_xen_set_evtchn_fast() " Sean Christopherson
2026-05-29 19:01   ` sashiko-bot
2026-05-29 19:11     ` Sean Christopherson
2026-06-02 12:37       ` David Woodhouse
2026-05-29 16:51 ` [PATCH v2 16/20] KVM: x86/xen: Convert xen_get_guest_pvclock() " Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 17/20] KVM: x86/xen: Drop local "kick_vcpu" from __kvm_xen_set_evtchn_fast() Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 18/20] KVM: x86/xen: Convert event injection to gpc's CLASS() APIs Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 19/20] KVM: Add "extended" gpc CLASS() APIs for sometimes-atomic cases Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 20/20] KVM: x86: Use gfn_to_pfn_cache for record_steal_time Sean Christopherson
2026-05-30  6:19   ` sashiko-bot
2026-06-02 12:29     ` David Woodhouse

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=20260529172016.ACE851F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=seanjc@google.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.