From: David Woodhouse <[email protected]> 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:
- kvm_xen_hvm_evtchn_send(): the KVM_XEN_HVM_EVTCHN_SEND ioctl. May sleep: passes wait == true, and kvm_gpc_refresh() rides the update out. - evtchn_set_fn(): irqfd routing, from the irqfd workqueue after the atomic fast path failed. May also wait. - kvm_xen_hcall_evtchn_send(): the guest's own EVTCHNOP_send, in vcpu_run's exit handling under the vCPU's kvm->srcu read section — the very reader the memslot update is waiting out, so waiting there deadlocks, and any guest can trigger it. Instead, leave the hypercall uncompleted, with RIP not advanced, and the guest re-executes it; each attempt cycles the srcu read section on the way back in, letting the update complete. This is how Xen itself restarts preempted hypercalls, and the pending delivery is pure guest state, so there is nothing extra to migrate. - kvm_xen_inject_timer_irqs(): the timer retry added by the previous commit — timer_pending *is* its pending state, so it keeps that and retries on the next entry pass. (Its other caller, the KVM_XEN_VCPU_GET_ATTR timer flush, must not wait under xen_lock — a vCPU writing the hypercall page MSR blocks on that lock inside its kvm->srcu section — and doesn't need to: an undelivered event is reported as still owed in the snapshot.) Reported-by: [email protected] Closes: https://lore.kernel.org/all/[email protected] Signed-off-by: David Woodhouse <[email protected]> Assisted-by: LLM --- arch/x86/kvm/irq.c | 2 +- arch/x86/kvm/xen.c | 65 ++++++++++++++++++++++++++++++---------------- arch/x86/kvm/xen.h | 4 +-- 3 files changed, 46 insertions(+), 25 deletions(-) diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c index 871977288272..4e986eb4c3c3 100644 --- a/arch/x86/kvm/irq.c +++ b/arch/x86/kvm/irq.c @@ -171,7 +171,7 @@ void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu) if (lapic_in_kernel(vcpu)) kvm_inject_apic_timer_irqs(vcpu); if (kvm_xen_timer_enabled(vcpu)) - kvm_xen_inject_timer_irqs(vcpu); + kvm_xen_inject_timer_irqs(vcpu, false); } void __kvm_migrate_timers(struct kvm_vcpu *vcpu) diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c index aea6572163d0..ad7708be6c04 100644 --- a/arch/x86/kvm/xen.c +++ b/arch/x86/kvm/xen.c @@ -30,9 +30,10 @@ #include "cpuid.h" #include "trace.h" -static int kvm_xen_set_evtchn(struct kvm_xen_evtchn *xe, struct kvm *kvm); +static int kvm_xen_set_evtchn(struct kvm_xen_evtchn *xe, struct kvm *kvm, + bool wait); static int kvm_xen_setattr_evtchn(struct kvm *kvm, struct kvm_xen_hvm_attr *data); -static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r); +static int kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r); DEFINE_STATIC_KEY_DEFERRED_FALSE(kvm_xen_enabled, HZ); @@ -102,7 +103,7 @@ static int kvm_xen_shared_info_init(struct kvm *kvm, bool wait) return ret; } -void kvm_xen_inject_timer_irqs(struct kvm_vcpu *vcpu) +void kvm_xen_inject_timer_irqs(struct kvm_vcpu *vcpu, bool wait) { if (atomic_read(&vcpu->arch.xen.timer_pending) > 0) { struct kvm_xen_evtchn e; @@ -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 @@ -661,10 +662,11 @@ void kvm_xen_inject_vcpu_vector(struct kvm_vcpu *v) */ void kvm_xen_inject_pending_events(struct kvm_vcpu *v) { - unsigned long evtchn_pending_sel = READ_ONCE(v->arch.xen.evtchn_pending_sel); + unsigned long evtchn_pending_sel; struct gfn_to_pfn_cache *gpc = &v->arch.xen.vcpu_info_cache; unsigned long flags; + evtchn_pending_sel = READ_ONCE(v->arch.xen.evtchn_pending_sel); if (!evtchn_pending_sel) return; @@ -1284,7 +1286,7 @@ int kvm_xen_vcpu_get_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data) */ if (vcpu->arch.xen.timer_expires) { hrtimer_cancel(&vcpu->arch.xen.timer); - kvm_xen_inject_timer_irqs(vcpu); + kvm_xen_inject_timer_irqs(vcpu, false); } data->u.timer.port = vcpu->arch.xen.timer_virq; @@ -1791,8 +1793,21 @@ int kvm_xen_hypercall(struct kvm_vcpu *vcpu) } break; case __HYPERVISOR_event_channel_op: - if (params[0] == EVTCHNOP_send) - handled = kvm_xen_hcall_evtchn_send(vcpu, params[1], &r); + if (params[0] == EVTCHNOP_send) { + int ret = kvm_xen_hcall_evtchn_send(vcpu, params[1], &r); + + /* + * Delivery raced a memslot update; waiting for it + * here would deadlock under vcpu_run's kvm->srcu + * read section. Leave the hypercall uncompleted — + * RIP not advanced — so the guest re-executes it. + * Each attempt cycles the srcu read section on the + * way back in, letting the update complete. + */ + if (ret == -EAGAIN) + return 1; + handled = !ret; + } break; case __HYPERVISOR_sched_op: handled = kvm_xen_hcall_sched_op(vcpu, is_64bit, params[0], @@ -1983,7 +1998,8 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm) return rc; } -static int kvm_xen_set_evtchn(struct kvm_xen_evtchn *xe, struct kvm *kvm) +static int kvm_xen_set_evtchn(struct kvm_xen_evtchn *xe, struct kvm *kvm, + bool wait) { bool mm_borrowed = false; int rc; @@ -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); @@ -2044,7 +2057,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); } /* @@ -2118,7 +2131,7 @@ int kvm_xen_hvm_evtchn_send(struct kvm *kvm, struct kvm_irq_routing_xen_evtchn * e.vcpu_idx = -1; e.priority = uxe->priority; - ret = kvm_xen_set_evtchn(&e, kvm); + ret = kvm_xen_set_evtchn(&e, kvm, true); /* * None of that 'return 1 if it actually got delivered' nonsense. @@ -2347,7 +2360,11 @@ static int kvm_xen_setattr_evtchn(struct kvm *kvm, struct kvm_xen_hvm_attr *data return kvm_xen_eventfd_assign(kvm, data); } -static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r) +/* + * Returns 0 with *r set if handled, -EAGAIN if the hypercall should be + * re-executed by the guest, or -ENOENT to hand it to userspace. + */ +static int kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r) { struct evtchnfd *evtchnfd; struct evtchn_send send; @@ -2357,7 +2374,7 @@ static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r) BUILD_BUG_ON(sizeof(send) != 4); if (kvm_read_guest_virt(vcpu, param, &send, sizeof(send), &e)) { *r = -EFAULT; - return true; + return 0; } /* @@ -2366,18 +2383,22 @@ static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r) */ evtchnfd = xa_load(&vcpu->kvm->arch.xen.evtchn_ports, send.port); if (!evtchnfd) - return false; + return -ENOENT; if (evtchnfd->deliver.port.port) { - int ret = kvm_xen_set_evtchn(&evtchnfd->deliver.port, vcpu->kvm); + int ret = kvm_xen_set_evtchn(&evtchnfd->deliver.port, + vcpu->kvm, false); + + if (ret == -EAGAIN) + return ret; if (ret < 0 && ret != -ENOTCONN) - return false; + return -ENOENT; } else { eventfd_signal(evtchnfd->deliver.eventfd.ctx); } *r = 0; - return true; + return 0; } void kvm_xen_init_vcpu(struct kvm_vcpu *vcpu) diff --git a/arch/x86/kvm/xen.h b/arch/x86/kvm/xen.h index 9d04e350bdb1..f6202510bbd6 100644 --- a/arch/x86/kvm/xen.h +++ b/arch/x86/kvm/xen.h @@ -100,7 +100,7 @@ static inline int kvm_xen_has_pending_timer(struct kvm_vcpu *vcpu) return 0; } -void kvm_xen_inject_timer_irqs(struct kvm_vcpu *vcpu); +void kvm_xen_inject_timer_irqs(struct kvm_vcpu *vcpu, bool wait); #else static inline int kvm_xen_write_hypercall_page(struct kvm_vcpu *vcpu, u64 data) { @@ -161,7 +161,7 @@ static inline int kvm_xen_has_pending_timer(struct kvm_vcpu *vcpu) return 0; } -static inline void kvm_xen_inject_timer_irqs(struct kvm_vcpu *vcpu) +static inline void kvm_xen_inject_timer_irqs(struct kvm_vcpu *vcpu, bool wait) { } -- 2.43.0
