Explicitly remove dying vCPUs that were blocking when the VM was released from the Posted Interrupts wakeup list instead of very, very subtly relying on the vcpu_load() in nested_vmx_free_vcpu() to take the vCPU off said list. In addition to helping avoid future regressions, e.g. if an enterprising developer were to optimize nested_vmx_free_vcpu() by bailing early if "nested" is completely disabled, this will allow dropping the load+put pair from nested_vmx_free_vcpu().
Provide a dedicated API to remove a vCPU from the wakeup list, i.e. don't _completely_ abuse vt_vcpu_pi_load(), but do reuse the core logic for updating the PID control word. Using a CMPXCHG8B _should_ be unnecessary as IRQ routing has already been torn down, i.e. no IRTEs should be pointing at updating the PID, and there are obviously no other references to the VM or vCPU, and so concurrently modifications of the PID should be impossible. For the same reasons, updating the destination should also be unnecessary, as nothing should ever send an IRQ. However, relying on those assumptions begs for WARNs to ensure they hold, along with fallback code to avoid the worst case scenario, at which point trying to take advantage of the fact that the PID is unreachable would be *more* complex than what is now __vt_vcpu_pi_load(), not to mention KVM would have two paths to maintain. Signed-off-by: Sean Christopherson <[email protected]> --- arch/x86/kvm/vmx/posted_intr.c | 68 +++++++++++++++++++++------------- arch/x86/kvm/vmx/posted_intr.h | 1 + arch/x86/kvm/vmx/vmx.c | 2 + 3 files changed, 45 insertions(+), 26 deletions(-) diff --git a/arch/x86/kvm/vmx/posted_intr.c b/arch/x86/kvm/vmx/posted_intr.c index 2644add54b76..7414fb551d2e 100644 --- a/arch/x86/kvm/vmx/posted_intr.c +++ b/arch/x86/kvm/vmx/posted_intr.c @@ -56,35 +56,13 @@ static int pi_try_set_control(struct pi_desc *pi_desc, u64 *pold, u64 new) return 0; } -void vt_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu) +static void __vt_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu) { struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); - struct vcpu_vt *vt = to_vt(vcpu); struct pi_desc old, new; - unsigned long flags; unsigned int dest; - /* - * To simplify hot-plug and dynamic toggling of APICv, keep PI.NDST and - * PI.SN up-to-date even if there is no assigned device or if APICv is - * deactivated due to a dynamic inhibit bit, e.g. for Hyper-V's SyncIC. - */ - if (!enable_apicv || !lapic_in_kernel(vcpu)) - return; - - /* - * If the vCPU wasn't on the wakeup list and wasn't migrated, then the - * full update can be skipped as neither the vector nor the destination - * needs to be changed. Clear SN even if there is no assigned device, - * again for simplicity. - */ - if (pi_desc->nv != POSTED_INTR_WAKEUP_VECTOR && vcpu->cpu == cpu) { - if (pi_test_and_clear_sn(pi_desc)) - goto after_clear_sn; - return; - } - - local_irq_save(flags); + guard(irqsave)(); /* * If the vCPU was waiting for wakeup, remove the vCPU from the wakeup @@ -103,7 +81,7 @@ void vt_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu) */ raw_spin_lock(spinlock); spin_acquire(&spinlock->dep_map, PI_LOCK_SCHED_OUT, 0, _RET_IP_); - list_del(&vt->pi_wakeup_list); + list_del(&to_vt(vcpu)->pi_wakeup_list); spin_release(&spinlock->dep_map, _RET_IP_); raw_spin_unlock(spinlock); } @@ -129,8 +107,33 @@ void vt_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu) */ new.nv = POSTED_INTR_VECTOR; } while (pi_try_set_control(pi_desc, &old.control, new.control)); +} - local_irq_restore(flags); +void vt_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu) +{ + struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); + + /* + * To simplify hot-plug and dynamic toggling of APICv, keep PI.NDST and + * PI.SN up-to-date even if there is no assigned device or if APICv is + * deactivated due to a dynamic inhibit bit, e.g. for Hyper-V's SyncIC. + */ + if (!enable_apicv || !lapic_in_kernel(vcpu)) + return; + + /* + * If the vCPU wasn't on the wakeup list and wasn't migrated, then the + * full update can be skipped as neither the vector nor the destination + * needs to be changed. Clear SN even if there is no assigned device, + * again for simplicity. + */ + if (pi_desc->nv != POSTED_INTR_WAKEUP_VECTOR && vcpu->cpu == cpu) { + if (pi_test_and_clear_sn(pi_desc)) + goto after_clear_sn; + return; + } + + __vt_vcpu_pi_load(vcpu, cpu); after_clear_sn: @@ -146,6 +149,19 @@ void vt_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu) pi_set_on(pi_desc); } +void vt_vcpu_pi_remove_from_wakeup_list(struct kvm_vcpu *vcpu) +{ + WARN_ON_ONCE(refcount_read(&vcpu->kvm->users_count)); + + if (!enable_apicv || !lapic_in_kernel(vcpu) || + vcpu_to_pi_desc(vcpu)->nv != POSTED_INTR_WAKEUP_VECTOR) + return; + + __vt_vcpu_pi_load(vcpu, get_cpu()); + + put_cpu(); +} + static bool vt_can_use_vtd_pi(struct kvm *kvm) { /* diff --git a/arch/x86/kvm/vmx/posted_intr.h b/arch/x86/kvm/vmx/posted_intr.h index c5860ea828ae..1d9ccbc2bb6f 100644 --- a/arch/x86/kvm/vmx/posted_intr.h +++ b/arch/x86/kvm/vmx/posted_intr.h @@ -10,6 +10,7 @@ void vt_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu); void vt_vcpu_pi_put(struct kvm_vcpu *vcpu); +void vt_vcpu_pi_remove_from_wakeup_list(struct kvm_vcpu *vcpu); void pi_wakeup_handler(void); void __init pi_init_cpu(int cpu); void pi_apicv_pre_state_restore(struct kvm_vcpu *vcpu); diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 292001bbcbff..1f9e43c5b4c4 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -7514,6 +7514,8 @@ void vmx_vcpu_free(struct kvm_vcpu *vcpu) { struct vcpu_vmx *vmx = to_vmx(vcpu); + vt_vcpu_pi_remove_from_wakeup_list(vcpu); + if (enable_pml) vmx_destroy_pml_buffer(vcpu); free_vpid(vmx->vpid); -- 2.56.0.307.g5fe7bd680c-goog
