> From: Nadav Har'El [mailto:nyh@xxxxxxxxxxxxxxxxxxx] > 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
