Re: [PATCH 05/10] 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 2 potential issue(s) to consider:
- [High] kvm: infinite loop in vcpu_run due to Xen timer delivery failure
- [High] kvm: ABBA deadlock between xen_lock and kvm->srcu in KVM_XEN_HVM_SET_ATTR
--

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

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

Event channel delivery now returns -EAGAIN when it races a memslot
update. It always failed; now the failure is distinguishable, and
instead of dropping the event each caller reacts according to its
context.

Closes: https://lore.kernel.org/all/20260920215341.5ECE91F000FF@smtp.kernel.org

> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index aea6572163d0c..ad7708be6c049 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -113,7 +114,7 @@ void kvm_xen_inject_timer_irqs(struct kvm_vcpu *vcpu)
>  		e.port = vcpu->arch.xen.timer_virq;
>  		e.priority = KVM_IRQ_ROUTING_XEN_EVTCHN_PRIO_2LEVEL;
>  
> -		rc = kvm_xen_set_evtchn(&e, vcpu->kvm);
> +		rc = kvm_xen_set_evtchn(&e, vcpu->kvm, wait);
>  
>  		/*
>  		 * Deliberately leave timer_pending set if the memslot is

[Severity: High]
Does this code cause an infinite loop in vcpu_run() due to the Xen timer
delivery failure?

If rc is -EAGAIN or -EINVAL, timer_pending remains set. A set
timer_pending causes block attempts to instantly abort:

virt/kvm/kvm_main.c:kvm_vcpu_check_block() {
    ...
	if (kvm_cpu_has_pending_timer(vcpu))
		goto out;
	if (signal_pending(current))
		goto out;
    ...
}

Because the block is aborted and kvm_vcpu_has_events() evaluates to false
(as it does not check timer_pending), the vCPU's mp_state remains halted.

This forces vcpu_run() into an infinite retry loop on the host core
without entering the guest:

arch/x86/kvm/x86.c:vcpu_run() {
    ...
		if (kvm_vcpu_running(vcpu)) {
			r = vcpu_enter_guest(vcpu);
		} else {
			r = vcpu_block(vcpu);
		}

		if (r <= 0)
			break;

		kvm_clear_request(KVM_REQ_UNBLOCK, vcpu);
		...
		if (kvm_cpu_has_pending_timer(vcpu))
			kvm_inject_pending_timer_irqs(vcpu);
    ...
}

[ ... ]

> @@ -2020,16 +2036,13 @@ static int kvm_xen_set_evtchn(struct kvm_xen_evtchn *xe, struct kvm *kvm)
>  	 */
>  	do {
>  		struct gfn_to_pfn_cache *gpc = &kvm->arch.xen.shinfo_cache;
> -		int idx;
>  
>  		rc = kvm_xen_set_evtchn_fast(xe, kvm);
>  		if (rc != -EWOULDBLOCK)
>  			break;
>  
> -		idx = srcu_read_lock(&kvm->srcu);
> -		rc = kvm_gpc_refresh(gpc, PAGE_SIZE, false);
> -		srcu_read_unlock(&kvm->srcu, idx);
> -	} while(!rc);
> +		rc = kvm_gpc_refresh(gpc, PAGE_SIZE, wait);
> +	} while (!rc);
>  
>  	if (mm_borrowed)
>  		kthread_unuse_mm(kvm->mm);

[Severity: High]
Can this cause a deadlock between xen_lock and kvm->srcu?

The SET_ATTR ioctl for LONG_MODE acquires xen_lock and calls
kvm_xen_shared_info_init() with wait=true:

arch/x86/kvm/xen.c:kvm_xen_hvm_set_attr() {
    mutex_lock(&kvm->arch.xen.xen_lock);
    ...
    r = kvm->arch.xen.shinfo_cache.active ?
        kvm_xen_shared_info_init(kvm, true) : 0;
    mutex_unlock(&kvm->arch.xen.xen_lock);
}

This causes it to sleep waiting for memslot updates while holding
xen_lock.

Meanwhile, a vCPU running in guest mode (holding kvm->srcu read lock)
can block trying to acquire xen_lock in kvm_xen_write_hypercall_page():

arch/x86/kvm/xen.c:kvm_xen_write_hypercall_page() {
    mutex_lock(&kvm->arch.xen.xen_lock);
}

This prevents synchronize_srcu() in the memslot update from completing,
resulting in an ABBA deadlock.

The SHARED_INFO case similarly acquires xen_lock and explicitly loops in
kvm_gpc_activate() and kvm_gpc_refresh() with wait=true while holding
the lock.

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