From: Sean Christopherson <[email protected]> Move the marking of an event as pending in the target vCPU's vcpu_info into a separate helper, __kvm_xen_set_evtchn_fast(), invoked after the shared_info processing is complete and its RCU read-side critical section has been exited. This makes both halves easier to read.
No functional change intended. Signed-off-by: Sean Christopherson <[email protected]> [dwmw2: rebased onto the -EAGAIN series. Sean's series first converted the GPC locks to read_trylock(), which this series does not carry, so the helper takes the plain read lock and the original's trylock failure path does not apply. The caller's kvm->srcu section extends across the helper call, since kvm_gpc_check() on the vcpu_info cache consults the memslot generation and the irqfd path enters holding only irq_srcu; in Sean's series that was covered by a guard(srcu) spanning the whole function.] Signed-off-by: David Woodhouse <[email protected]> Assisted-by: LLM --- arch/x86/kvm/xen.c | 158 ++++++++++++++++++++++++++++----------------- 1 file changed, 97 insertions(+), 61 deletions(-) diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c index ad7708be6c04..a931458f0c71 100644 --- a/arch/x86/kvm/xen.c +++ b/arch/x86/kvm/xen.c @@ -1862,6 +1862,89 @@ static void kvm_xen_check_poller(struct kvm_vcpu *vcpu, int port) } } +static void __kvm_xen_set_evtchn_fast(struct kvm_vcpu *vcpu, int port_word_bit, + bool has_64bit_shinfo) +{ + struct gfn_to_pfn_cache *gpc = &vcpu->arch.xen.vcpu_info_cache; + unsigned long vi_pending_sel_ofs; + bool inject_upcall = false; + bool kick_vcpu = false; + unsigned long flags; + bool old; + + vi_pending_sel_ofs = has_64bit_shinfo ? + offsetof(struct vcpu_info, evtchn_pending_sel) : + offsetof(struct compat_vcpu_info, evtchn_pending_sel); + + /* Now switch to the vCPU's vcpu_info to set the index and pending_sel */ + read_lock_irqsave(&gpc->lock, flags); + if (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) { + /* + * Could not access the vcpu_info. Set the bit in-kernel and + * prod the vCPU to deliver it for itself. Note that an MSI is + * *not* an acceptable substitute here even if the vCPU has an + * upcall vector: the pending bit only exists in + * evtchn_pending_sel so far, and the vCPU has to be forced + * out of the guest so that kvm_xen_inject_pending_events() + * can refresh the cache and propagate it to the guest. + */ + if (!test_and_set_bit(port_word_bit, &vcpu->arch.xen.evtchn_pending_sel)) + kick_vcpu = true; + goto out_unlock; + } + inject_upcall = true; + + /* + * Explicitly use a 32-bit btsl instead of test_and_set_bit(), which + * would use btsq on x86-64. The vcpu_info is guest-controlled and only + * required to be 32-bit aligned, so a 64-bit access could generate a + * split-lock #AC. + * + * Note, this does not apply to the test_and_set_bit() on pending_bits + * in the caller: that is in the per-VM shared_info, which is page + * aligned, so the access is guaranteed to be 64-bit aligned. + */ + old = GEN_BINARY_RMWcc(LOCK_PREFIX "btsl", + *(u32 *)(gpc->khva + vi_pending_sel_ofs), + c, "Ir", port_word_bit); + if (!old) { + struct vcpu_info *vi = gpc->khva; + + /* No need for compat handling */ + BUILD_BUG_ON(offsetof(struct vcpu_info, evtchn_upcall_pending) != + offsetof(struct compat_vcpu_info, evtchn_upcall_pending)); + + WRITE_ONCE(vi->evtchn_upcall_pending, 1); + kick_vcpu = true; + } + +out_unlock: + read_unlock_irqrestore(&gpc->lock, flags); + + /* + * Deliver the upcall or kick the vCPU only after dropping the GPC + * read lock. Both paths end up in kvm_vcpu_kick(), and the MSI + * delivery also walks the APIC map and takes APIC locks; none of + * that wants to be nested inside the GPC read lock, which a + * concurrent invalidation may be waiting to take for write. + */ + if (!kick_vcpu) + return; + + /* + * For the per-vCPU lapic vector, deliver it as MSI — but only if the + * vcpu_info was actually updated above. If it wasn't, the vCPU must + * be kicked instead (see above). + */ + if (inject_upcall && vcpu->arch.xen.upcall_vector) { + kvm_xen_inject_vcpu_vector(vcpu); + return; + } + + kvm_make_request(KVM_REQ_UNBLOCK, vcpu); + kvm_vcpu_kick(vcpu); +} + /* * The return value from this function is propagated to kvm_set_irq() API, * so it returns: @@ -1876,11 +1959,10 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm) { struct gfn_to_pfn_cache *gpc = &kvm->arch.xen.shinfo_cache; bool has_64bit_shinfo = kvm_xen_has_64bit_shinfo(kvm); - unsigned long *pending_bits, *mask_bits, vi_pending_sel_ofs; + unsigned long *pending_bits, *mask_bits; struct kvm_vcpu *vcpu; unsigned long flags; int port_word_bit; - bool kick_vcpu = false; int vcpu_idx, idx, rc; vcpu_idx = READ_ONCE(xe->vcpu_idx); @@ -1902,23 +1984,19 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm) read_lock_irqsave(&gpc->lock, flags); if (!kvm_gpc_check(gpc, PAGE_SIZE)) - goto out_rcu; + goto out_unlock; if (has_64bit_shinfo) { struct shared_info *shinfo = gpc->khva; pending_bits = (unsigned long *)&shinfo->evtchn_pending; mask_bits = (unsigned long *)&shinfo->evtchn_mask; port_word_bit = xe->port / 64; - - vi_pending_sel_ofs = offsetof(struct vcpu_info, evtchn_pending_sel); } else { struct compat_shared_info *shinfo = gpc->khva; pending_bits = (unsigned long *)&shinfo->evtchn_pending; mask_bits = (unsigned long *)&shinfo->evtchn_mask; port_word_bit = xe->port / 32; - vi_pending_sel_ofs = offsetof(struct compat_vcpu_info, evtchn_pending_sel); - /* test_and_set_bit() needs 64-bit alignment, but that's OK */ BUILD_BUG_ON(offsetof(struct compat_shared_info, evtchn_pending) & 7); } @@ -1934,66 +2012,24 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm) rc = 0; /* It was already raised */ } else if (test_bit(xe->port, mask_bits)) { rc = -ENOTCONN; /* Masked */ - kvm_xen_check_poller(vcpu, xe->port); } else { - bool old; - rc = 1; /* Delivered to the bitmap in shared_info. */ - /* Now switch to the vCPU's vcpu_info to set the index and pending_sel */ - read_unlock_irqrestore(&gpc->lock, flags); - gpc = &vcpu->arch.xen.vcpu_info_cache; - - read_lock_irqsave(&gpc->lock, flags); - if (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) { - /* - * Could not access the vcpu_info. Set the bit in-kernel - * and prod the vCPU to deliver it for itself. - */ - if (!test_and_set_bit(port_word_bit, &vcpu->arch.xen.evtchn_pending_sel)) - kick_vcpu = true; - goto out_rcu; - } - - /* - * Explicitly use a 32-bit btsl instead of test_and_set_bit(), - * which would use btsq on x86-64. The vcpu_info is guest- - * controlled and only required to be 32-bit aligned, so a - * 64-bit access could generate a split-lock #AC. - * - * Note, this does not apply to the test_and_set_bit() on - * pending_bits above: that is in the per-VM shared_info, which - * is page aligned, so the access is guaranteed to be 64-bit - * aligned. - */ - old = GEN_BINARY_RMWcc(LOCK_PREFIX "btsl", - *(u32 *)(gpc->khva + vi_pending_sel_ofs), - c, "Ir", port_word_bit); - if (!old) { - struct vcpu_info *vi = gpc->khva; - - /* No need for compat handling */ - BUILD_BUG_ON(offsetof(struct vcpu_info, evtchn_upcall_pending) != - offsetof(struct compat_vcpu_info, evtchn_upcall_pending)); - - WRITE_ONCE(vi->evtchn_upcall_pending, 1); - kick_vcpu = true; - } - - /* For the per-vCPU lapic vector, deliver it as MSI. */ - if (kick_vcpu && vcpu->arch.xen.upcall_vector) { - kvm_xen_inject_vcpu_vector(vcpu); - kick_vcpu = false; - } } - out_rcu: + out_unlock: read_unlock_irqrestore(&gpc->lock, flags); - srcu_read_unlock(&kvm->srcu, idx); - if (kick_vcpu) { - kvm_make_request(KVM_REQ_UNBLOCK, vcpu); - kvm_vcpu_kick(vcpu); - } + /* + * Both of these can kick the vCPU, so keep them outside the GPC + * read lock; an invalidation may be waiting to take it for write. + */ + if (rc == -ENOTCONN) + kvm_xen_check_poller(vcpu, xe->port); + else if (rc == 1) + __kvm_xen_set_evtchn_fast(vcpu, port_word_bit, + has_64bit_shinfo); + + srcu_read_unlock(&kvm->srcu, idx); return rc; } -- 2.43.0
