From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Tian, Kevin" Subject: RE: [PATCH 20/31] nVMX: Exiting from L2 to L1 Date: Wed, 25 May 2011 16:23:38 +0800 Message-ID: <625BA99ED14B2D499DC4E29D8138F1505C9BFA3A3B@shsmsx502.ccr.corp.intel.com> References: <1305575004-nyh@il.ibm.com> <201105161954.p4GJs9XB001955@rice.haifa.ibm.com> <625BA99ED14B2D499DC4E29D8138F1505C9BFA3524@shsmsx502.ccr.corp.intel.com> <20110524134302.GA10363@fermat.math.technion.ac.il> <625BA99ED14B2D499DC4E29D8138F1505C9BFA35FD@shsmsx502.ccr.corp.intel.com> <20110525080617.GA10509@fermat.math.technion.ac.il> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT Cc: "kvm@vger.kernel.org" , "gleb@redhat.com" , "avi@redhat.com" To: Nadav Har'El Return-path: Received: from mga02.intel.com ([134.134.136.20]:36845 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757399Ab1EYIY1 convert rfc822-to-8bit (ORCPT ); Wed, 25 May 2011 04:24:27 -0400 In-Reply-To: <20110525080617.GA10509@fermat.math.technion.ac.il> Content-Language: en-US Sender: kvm-owner@vger.kernel.org List-ID: > From: Nadav Har'El [mailto:nyh@math.technion.ac.il] > Sent: Wednesday, May 25, 2011 4:06 PM > > On Wed, May 25, 2011, Tian, Kevin wrote about "RE: [PATCH 20/31] nVMX: > Exiting from L2 to L1": > > IOW, I disagree that if L0/L1 set same bit in cr0_guest_host_mask, then > > the bit is identical in vmcs02.guest_cr0 and vmcs12.guest_cr0 because L1 > > has no permission to set its bit effectively in this case. > >... > > this is the problem: > > > > (vmcs12->guest_cr0 & vmcs12->cr0_guest_host_mask) != > > (vmcs02->guest_cr0 & vmcs12->cr0_guest_host_mask) > > Sorry for arguing previously, this is a very good, and correct, point, which > I missed. When both L0 and L1 are KVM, this didn't cause problems because > the > only problematic bit has been the TS bit, and when KVM wants to override this > bit it always does it to 1. > > So I've rewritten this function, based on my new understanding following your > insights. I believe it now implements your formula *exactly*. Please take a > look at the comments and the code, and see if you now agree with them: > > /* > * On a nested exit from L2 to L1, vmcs12.guest_cr0 might not be up-to-date > * because L2 may have changed some cr0 bits directly > (CRO_GUEST_HOST_MASK). > * This function returns the new value we should put in vmcs12.guest_cr0. > * It's not enough to just return the vmcs02 GUEST_CR0. Rather, > * 1. Bits that neither L0 nor L1 trapped, were set directly by L2 and are now > * available in vmcs02 GUEST_CR0. (Note: It's enough to check that L0 > * didn't trap the bit, because if L1 did, so would L0). > * 2. Bits that L1 asked to trap (and therefore L0 also did) could not have > * been modified by L2, and L1 knows it. So just leave the old value of > * the bit from vmcs12.guest_cr0. Note that the bit from vmcs02 > GUEST_CR0 > * isn't relevant, because if L0 traps this bit it can set it to anything. > * 3. Bits that L1 didn't trap, but L0 did. L1 believes the guest could have > * changed these bits, and therefore they need to be updated, but L0 > * didn't necessarily allow them to be changed in GUEST_CR0 - and > rather > * put them in vmcs02 CR0_READ_SHADOW. So take these bits from > there. > */ > static inline unsigned long > vmcs12_guest_cr0(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12) > { > return > /*1*/ (vmcs_readl(GUEST_CR0) & vcpu->arch.cr0_guest_owned_bits) > | > /*2*/ (vmcs12->guest_cr0 & vmcs12->cr0_guest_host_mask) | > /*3*/ (vmcs_readl(CR0_READ_SHADOW) & > ~(vmcs12->cr0_guest_host_mask | > vcpu->arch.cr0_guest_owned_bits)); > } > > This looks good to me. :-) Thanks Kevin