From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liran Alon Subject: Re: [PATCH v3 10/11] KVM: nVMX: Wake halted L2 on nested posted-interrupt Date: Wed, 27 Dec 2017 17:33:16 +0200 Message-ID: <5A43BD3C.3010804@ORACLE.COM> References: <1514131983-24305-1-git-send-email-liran.alon@oracle.com> <1514131983-24305-11-git-send-email-liran.alon@oracle.com> <5A438B8C.3080109@ORACLE.COM> <5A43979A.5050400@ORACLE.COM> <9d603bfb-b16b-9195-58de-fd6c4946a14e@redhat.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 , Konrad Rzeszutek Wilk To: Paolo Bonzini , rkrcmar@redhat.com, kvm@vger.kernel.org Return-path: Received: from userp2130.oracle.com ([156.151.31.86]:54026 "EHLO userp2130.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751043AbdL0Pd1 (ORCPT ); Wed, 27 Dec 2017 10:33:27 -0500 In-Reply-To: <9d603bfb-b16b-9195-58de-fd6c4946a14e@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 27/12/17 15:05, Paolo Bonzini wrote: > On 27/12/2017 13:52, Liran Alon wrote: >> >> The race-condition described in commit message of patch 9 is fixed >> in patch 9. It is fixed because we get rid of the dependency on >> KVM_REQ_EVENT. > > Or is it fixed because, while getting rid of the dependency on > KVM_REQ_EVENT, you had to move kvm_vcpu_trigger_posted_interrupt to the > correct place: > > > - /* the PIR and ON have been set by L1. */ > - kvm_vcpu_trigger_posted_interrupt(vcpu, true); > /* > * If a posted intr is not recognized by hardware, > * we will accomplish it in the next vmentry. > */ > vmx->nested.pi_pending = true; > - kvm_make_request(KVM_REQ_EVENT, vcpu); > + /* the PIR and ON have been set by L1. */ > + kvm_vcpu_trigger_posted_interrupt(vcpu, true); > return 0; > > ? > > Getting rid of KVM_REQ_EVENT is not a requirement to fix the > vcpu->requests race. There is canonical way to avoid that, which is to > use kvm_vcpu_kick. > > To me the main value of patch 9 is introducing the nested PI handler. > The fact that patch 9 fixes that race condition is almost a side effect > (and in fact it's not entirely fixed until patch 10, in my opinion). > That's a good point. Haven't thought about it like that. I now tend to agree with you. I will re-think about how to change patches 5-11 such that we will: 1. Get rid of pi_pending and instead use virtual LAPIC IRR and process the vmcs12->posted_intr_nv specially in case vCPU is in non-root-mode (and posted-interrupts is active). Similar to what a real CPU does. 2. Re-order patches to be similar to the following: (a) Simple bug-fix for the race-condition issue: Just changing order like I did in v1 and adding a kvm_vcpu_kick() like in patch 10 of this series. (b) Get rid of pi_pending and instead use virtual LAPIC IRR bit and process it specially in case vCPU in non-root-mode & posted-interrupts is active. (c) Get rid of software simulation of nested posted-interrupts processing and instead use self-IPI trick to make CPU process it for us. What do you think? Regards, -Liran > Paolo > >> If the target vCPU thread passes the check for pending >> kvm requests, it means it is already running with interrupts disabled >> and therefore the physical IPI of POSTED_INTR_NESTED_VECTOR will be >> received in guest which will process nested posted-interrupts >> correctly. If guest will exit because of another external-interrupt >> before the physical IPI will be received, on next VMEntry, code will >> note there are pending nested posted-interrupts and re-trigger >> physical IPI of POSTED_INTR_NESTED_VECTOR. Therefore, nested >> posted-interrupts will eventually be processed in guest. This is in >> contrast to what happens before patch 9 where L2 guest will continue >> running until next time KVM_REQ_EVENT will be consumed. Therefore, >> bug is indeed fixed in patch 9. >