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));
}
--
Nadav Har'El | Wednesday, May 25 2011, 21 Iyyar 5771
nyh@xxxxxxxxxxxxxxxxxxx |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |If at first you don't succeed, redefine
http://nadav.harel.org.il |success.
--
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
