From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liran Alon Subject: Re: [PATCH v3 11/11] KVM: nVMX: Wake L2 from HLT when nested posted-interrupt pending Date: Wed, 27 Dec 2017 12:51:25 +0200 Message-ID: <5A437B2D.30805@ORACLE.COM> References: <1514131983-24305-1-git-send-email-liran.alon@oracle.com> <1514131983-24305-12-git-send-email-liran.alon@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: jmattson@google.com, wanpeng.li@hotmail.com, idan.brown@ORACLE.COM, Liam Merwick To: Paolo Bonzini , rkrcmar@redhat.com, kvm@vger.kernel.org Return-path: Received: from userp2130.oracle.com ([156.151.31.86]:59868 "EHLO userp2130.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750790AbdL0Kve (ORCPT ); Wed, 27 Dec 2017 05:51:34 -0500 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 27/12/17 12:15, Paolo Bonzini wrote: > On 24/12/2017 17:13, Liran Alon wrote: >> +static bool vmx_cpu_has_nested_posted_interrupt(struct kvm_vcpu *vcpu) >> +{ >> + struct vcpu_vmx *vmx = to_vmx(vcpu); >> + >> + return (vcpu->arch.apicv_active && >> + is_guest_mode(vcpu) && >> + vmx->nested.pi_pending && >> + vmx->nested.pi_desc && >> + pi_test_on(vmx->nested.pi_desc)); >> +} >> + >> /* >> * Set up the vmcs's constant host-state fields, i.e., host-state fields that >> * will not change in the lifetime of the guest. >> @@ -12142,6 +12153,8 @@ static int enable_smi_window(struct kvm_vcpu *vcpu) >> .deliver_posted_interrupt = vmx_deliver_posted_interrupt, >> .complete_nested_posted_interrupt = >> vmx_complete_nested_posted_interrupt, >> + .cpu_has_nested_posted_interrupt = >> + vmx_cpu_has_nested_posted_interrupt, >> >> .set_tss_addr = vmx_set_tss_addr, >> .get_tdp_level = get_ept_level, >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index fa088951afc9..a840f2c9bd66 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -8542,7 +8542,8 @@ static inline bool kvm_vcpu_has_events(struct kvm_vcpu *vcpu) >> return true; >> >> if (kvm_arch_interrupt_allowed(vcpu) && >> - kvm_cpu_has_interrupt(vcpu)) >> + (kvm_cpu_has_interrupt(vcpu) || >> + kvm_x86_ops->cpu_has_nested_posted_interrupt(vcpu))) >> return true; > > > kvm_cpu_has_interrupt ultimately calls apic_has_interrupt_for_ppr, which > calls kvm_x86_ops->sync_pir_to_irr. > > You already have > > + if (is_guest_mode(vcpu)) > + kvm_x86_ops->complete_nested_posted_interrupt(vcpu); > > earlier in the series right after a call to kvm_x86_ops->sync_pir_to_irr. > > So I wonder if: > > 1) kvm_x86_ops->complete_nested_posted_interrupt would do the job here as > well, removing the need for the new kvm_x86_ops member; > > 2) The call to kvm_x86_ops->complete_nested_posted_interrupt actually > applies to all callers of sync_pir_to_irr, which would remove the need for > that other kvm_x86_ops member. Maybe I misunderstand you, but I don't think this is true. complete_nested_posted_interrupt() relies on being called at a very specific call-site: Right before VMEntry and after interrupts are disabled. It works by issue a self-IPI to cause CPU to actually process the nested posted-interrupts. In addition, I don't see how we can utilize the fact that kvm_cpu_has_interrupt() calls apic_has_interrupt_for_ppr() as vmx.nested.pi_desc->pir should never be synced into L0 vLAPIC IRR. In contrast to vmx.pi_desc->pir which does after sync_pir_to_irr(). > > I'm leaning towards applying patches 1-4, what do you think? > I don't see any reason not to do so if it passes your review :) Logically these patches are separated from the patches we still debate on. Regards, -Liran > Paolo >