Re: [PATCH v4 06/12] KVM: x86/xen: Retry event channel delivery which races a memslot update

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM: x86/xen: 3-way deadlock via wait=true in evtchn_set_fn() from PIT worker
--

commit deb1cf394d02a7487c9bd0713d5fed7a4be72540
Author: David Woodhouse <dwmw@amazon.co.uk>

KVM: x86/xen: Retry event channel delivery which races a memslot update

This commit updates Xen event channel delivery callers to handle -EAGAIN
when delivery races with a memslot update, either waiting for the update
to complete or retrying depending on the execution context.

> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 8a9d6963fc05a..c0b9ac674255d 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c

[ ... ]

> @@ -2153,7 +2165,7 @@ static int evtchn_set_fn(struct kvm_kernel_irq_routing_entry *e, struct kvm *kvm
>  	if (!level)
>  		return -EINVAL;
>  
> -	return kvm_xen_set_evtchn(&e->xen_evtchn, kvm);
> +	return kvm_xen_set_evtchn(&e->xen_evtchn, kvm, true);
>  }

[Severity: High]
Can passing wait=true unconditionally in evtchn_set_fn() cause a 3-way
deadlock when GSI 0 is routed to KVM_IRQ_ROUTING_XEN_EVTCHN and the
in-kernel PIT is active?

evtchn_set_fn() is the .set callback invoked by kvm_set_irq(). In addition
to the irqfd workqueue, pit_do_work() in arch/x86/kvm/i8254.c runs on the
pit->worker kthread and fires GSI 0:

arch/x86/kvm/i8254.c:pit_do_work() {
    ...
    kvm_set_irq(kvm, KVM_PIT_IRQ_SOURCE_ID, 0, 1, false);
    kvm_set_irq(kvm, KVM_PIT_IRQ_SOURCE_ID, 0, 0, false);
    ...
}

Because pit->worker is a kthread with current->mm == NULL,
kvm_xen_set_evtchn() borrows kvm->mm via kthread_use_mm() and calls
kvm_gpc_refresh(gpc, PAGE_SIZE, true).

If a memslot move or deletion is concurrently updating the memslot backing
shinfo_cache, kvm_invalidate_memslot() -> kvm_swap_active_memslots() marks
the slot KVM_MEMSLOT_INVALID and blocks in
synchronize_srcu_expedited(&kvm->srcu).

If a vCPU thread inside vcpu_run()'s kvm->srcu read section concurrently
emulates a guest PIO write to PIT ports 0x40-0x43, pit_ioport_write() ->
pit_load_count() calls create_pit_timer() or destroy_pit_timer(), both of
which flush pit->expired while holding kvm->srcu:

arch/x86/kvm/i8254.c:create_pit_timer() {
    ...
    hrtimer_cancel(&ps->timer);
    kthread_flush_work(&pit->expired);
    ...
}

At that point:
1. The memslot update thread waits for the vCPU thread to exit its
   kvm->srcu read section.
2. The vCPU thread holds kvm->srcu and waits in
   kthread_flush_work(&pit->expired) for pit_do_work() to finish.
3. pit_do_work() loops in kvm_gpc_refresh(..., wait=true) on -EAGAIN
   waiting for the memslot update to finish, and because pit->worker is a
   kthread, fatal_signal_pending(current) never triggers.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019162445.37219-1-dwmw2@infradead.org?part=6