Re: [PATCH] KVM: x86/xen: bail in IRQ context on PREEMPT_RT in kvm_xen_set_evtchn_fast()

[Mauricio Faria de Oliveira <mfo@igalia.com>]

On 2026-05-07 13:15, David Woodhouse wrote:
> On Thu, 2026-05-07 at 13:02 -0300, Mauricio Faria de Oliveira wrote:
>> On 2026-05-07 12:22, David Woodhouse wrote:
>> > On Thu, 2026-05-07 at 11:56 -0300, Mauricio Faria de Oliveira wrote:
>> > > On 2026-05-07 03:58, David Woodhouse wrote:
>> > > > On Wed, 2026-05-06 at 23:36 -0300, Mauricio Faria de Oliveira wrote:
>> > > > > kvm_xen_set_evtchn_fast() calls read_lock_irqsave(), which might block
>> > > > > on PREEMPT_RT, but that is invalid in IRQ context, as when it's called
>> > > > > by xen_timer_callback() (even on PREEMPT_RT per HRTIMER_MODE_ABS_HARD).
>> > > > > 
>> > > > > Check for that case, and bail out early.
>> > > > > 
>> > > > > Note: there is previous work and discussion on this [1] (~2 years ago),
>> > > > > which involved continuing to execute the function with changes, but it
>> > > > > was not merged. That was a different, more complex approach.
>> > > > > 
>> > > > > [1] https://lore.kernel.org/lkml/ZdPQVP7eejq3eFjc@google.com/
>> > > > 
>> > > > ...
>> > > > 
>> > > > > +	/* Bail in IRQ context on PREEMPT_RT; read_lock_irqsave() might block */
>> > > > > +	if (IS_ENABLED(CONFIG_PREEMPT_RT) && in_hardirq())
>> > > > > +		goto out;
>> > > > 
>> > > > 
>> > > > The approach in Paul's earlier patch was better; we absolutely *want*
>> > > > to deliver the interrupt to the guest immediately whenever we can, and
>> > > > only fall back to the workqueue in the rare case that the shared info
>> > > > page has been invalidated.
>> > > 
>> > > Certainly, that was better. This was a simple workaround, but with this
>> > > clarification, it indeed doesn't fit.
>> > > 
>> > > > We should switch to plain read_trylock(), *without* the
>> > > > local_irq_save(). And since this was the *only* case where the GPC lock
>> > > > was ever taken under IRQ¹, all the GPC locking can drop the _irq part.
>> > > 
>> > > Ok, I can take a look. Or do you plan to work on it yourself (as you
>> > > hit the issue with read_unlock later in this thread)?
>> > 
>> > I'm working on it now; thanks.
>> 
>> Ok, thanks; I'll drop this. Could you please Cc me when you send it out?
>> 
>> > Currently confused by the fact that the read_trylock() seems to fail
>> > more often than it should under RT, causing the fallback path to be
>> > taken... and the fallback path doesn't seem to work properly...
>> > 
>> > Your version should have seen this too, surely? Did the selftest in
>> > linux/tools/testing/selftests/kvm/x86/xen_shinfo_test work with your
>> > patch?
>> 
>> Yes, it works, apparently. Timing is similar with/without PREEMPT_RT.
> 
> Huh. Something weird going on for me. Would you mind confirming it
> still works with my version:
> 
> https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/gpc-stealtime

Yes, it also works with that tree [1].

Is PREEMPT_RT enabled in the Xen guest as well? In earlier tests, that
caused interrupt issues on boot (a few 'nobody cared, try irqpoll' and
'Disabled IRQ #'), but that also happened without changes in the Xen
host kernel, thus not a regression, and I disabled it as couldn't look
further then.

[1] HEAD at commit 916875860566 ("locking/rt: Use
raw_spin_lock_irqsave() in __rwbase_read_unlock()")

Hope this helps,

-- 
Mauricio