Message ID | 20240920080012.74405-2-mankku@gmail.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | KVM: nVMX: update VPPR on vmlaunch/vmresume | expand |
On Fri, Sep 20, 2024, Markku Ahvenjärvi wrote: > Running certain hypervisors under KVM on VMX suffered L1 hangs after > launching a nested guest. The external interrupts were not processed on > vmlaunch/vmresume due to stale VPPR, and L2 guest would resume without > allowing L1 hypervisor to process the events. > > The patch ensures VPPR to be updated when checking for pending > interrupts. This is architecturally incorrect, PPR isn't refreshed at VM-Enter. Aha! I wonder if the missing PPR update is due to the nested VM-Enter path directly clearing IRR when processing a posted interrupt. On top of https://github.com/kvm-x86/linux/tree/next, does this fix things? diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index a8e7bc04d9bf..a8255c6f0d51 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -3731,7 +3731,7 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) kvm_apic_has_interrupt(vcpu) == vmx->nested.posted_intr_nv) { vmx->nested.pi_pending = true; kvm_make_request(KVM_REQ_EVENT, vcpu); - kvm_apic_clear_irr(vcpu, vmx->nested.posted_intr_nv); + kvm_apic_ack_interrupt(vcpu, vmx->nested.posted_intr_nv); } /* Hide L1D cache contents from the nested guest. */
> This is architecturally incorrect, PPR isn't refreshed at VM-Enter. Ah, I see. Thanks for pointing out. > Aha! I wonder if the missing PPR update is due to the nested VM-Enter path > directly clearing IRR when processing a posted interrupt. > > On top of https://github.com/kvm-x86/linux/tree/next, does this fix things? > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index a8e7bc04d9bf..a8255c6f0d51 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -3731,7 +3731,7 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) > kvm_apic_has_interrupt(vcpu) == vmx->nested.posted_intr_nv) { > vmx->nested.pi_pending = true; > kvm_make_request(KVM_REQ_EVENT, vcpu); > - kvm_apic_clear_irr(vcpu, vmx->nested.posted_intr_nv); > + kvm_apic_ack_interrupt(vcpu, vmx->nested.posted_intr_nv); > } > > /* Hide L1D cache contents from the nested guest. */ No, unfortunately. The vmexits L1 is not receiving originate from deprivileged host (like timer ticks), running in vmx non-root mode. Thanks, Markku
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 5bb481aefcbc..7747c7d672ea 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -952,7 +952,7 @@ static int apic_has_interrupt_for_ppr(struct kvm_lapic *apic, u32 ppr) return highest_irr; } -static bool __apic_update_ppr(struct kvm_lapic *apic, u32 *new_ppr) +bool __kvm_apic_update_ppr(struct kvm_lapic *apic, u32 *new_ppr) { u32 tpr, isrv, ppr, old_ppr; int isr; @@ -973,12 +973,13 @@ static bool __apic_update_ppr(struct kvm_lapic *apic, u32 *new_ppr) return ppr < old_ppr; } +EXPORT_SYMBOL_GPL(__kvm_apic_update_ppr); static void apic_update_ppr(struct kvm_lapic *apic) { u32 ppr; - if (__apic_update_ppr(apic, &ppr) && + if (__kvm_apic_update_ppr(apic, &ppr) && apic_has_interrupt_for_ppr(apic, ppr) != -1) kvm_make_request(KVM_REQ_EVENT, apic->vcpu); } @@ -2895,7 +2896,7 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu) if (!kvm_apic_present(vcpu)) return -1; - __apic_update_ppr(apic, &ppr); + __kvm_apic_update_ppr(apic, &ppr); return apic_has_interrupt_for_ppr(apic, ppr); } EXPORT_SYMBOL_GPL(kvm_apic_has_interrupt); @@ -2954,7 +2955,7 @@ int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu) * triggered KVM_REQ_EVENT already. */ apic_set_isr(vector, apic); - __apic_update_ppr(apic, &ppr); + __kvm_apic_update_ppr(apic, &ppr); } return vector; diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h index 7ef8ae73e82d..1d0bc13a6794 100644 --- a/arch/x86/kvm/lapic.h +++ b/arch/x86/kvm/lapic.h @@ -106,6 +106,7 @@ int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2); void kvm_apic_clear_irr(struct kvm_vcpu *vcpu, int vec); bool __kvm_apic_update_irr(u32 *pir, void *regs, int *max_irr); bool kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir, int *max_irr); +bool __kvm_apic_update_ppr(struct kvm_lapic *apic, u32 *new_ppr); void kvm_apic_update_ppr(struct kvm_vcpu *vcpu); int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq, struct dest_map *dest_map); diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 2392a7ef254d..dacc92b150dd 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -3431,10 +3431,11 @@ static int nested_vmx_check_permission(struct kvm_vcpu *vcpu) static u8 vmx_has_apicv_interrupt(struct kvm_vcpu *vcpu) { + u32 vppr; u8 rvi = vmx_get_rvi(); - u8 vppr = kvm_lapic_get_reg(vcpu->arch.apic, APIC_PROCPRI); + __kvm_apic_update_ppr(vcpu->arch.apic, &vppr); - return ((rvi & 0xf0) > (vppr & 0xf0)); + return ((rvi & 0xf0) > (u8) (vppr & 0xf0)); } static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,