On 03/06/2012 10:33 AM, Nadav Har'El wrote:
> Hi,
>
> I just noticed that Nested VMX got broken (at least in my tests) by commit
> 46199f33c29533e7ad2a7d2128dc30175d1d4157.
>
> The specific change causing the problem was:
>
> @@ -2220,7 +2216,6 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32
> msr_index, u64 data)
> break;
> msr = find_msr_entry(vmx, msr_index);
> if (msr) {
> - vmx_load_host_state(vmx);
> msr->data = data;
> break;
> }
>
> And if anyone wants a quick workaround to making nested VMX work again,
> returning this line fixes the problem.
>
> I'm still trying to figure out why this line, which indeed seems unrelated
> and unnecessary, is necessary for the correct functioning of nested VMX.
> My (unsubstantiated) guess is that it isn't that it is actually necessary
> in this point - it's just that it does something that should have been more
> properly done in another place, but I've yet to figure out exactly what.
> I'll send a patch when I have this figured out. If anybody else has any
> guess, I'd love to hear.
A side effect of vmx_load_host_state() is that it schedules a
vmx_save_host_state() for the next entry. Maybe you're missing
something there.
Ah, it's probably
for (i = 0; i < vmx->save_nmsrs; ++i)
kvm_set_shared_msr(vmx->guest_msrs[i].index,
vmx->guest_msrs[i].data,
vmx->guest_msrs[i].mask);
What happened is that nvmx workloads write a lot to MSR_*STAR and
friends, and these are stored both in vmx->guest_msrs[] and on the cpu.
Without the call to vmx_save_host_state(), the values were written only
to memory, not to the cpu register, and the guest saw a corrupted value.
I'll post a patch soon.
--
error compiling committee.c: too many arguments to function
--
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
