Re: [PATCH] KVM: nVMX: Fix setting of CR0 and CR4 in guest mode

[Gleb Natapov <gleb@redhat.com>]

On Mon, Mar 04, 2013 at 09:37:17PM +0100, Jan Kiszka wrote:
> On 2013-03-04 21:24, Gleb Natapov wrote:
> >>> That doesn't make sense to me. I do not even sure what you are saying
> >>> since you do not specify what shadow is matched. From the code I see
> >>> that on CR0 exit to L0 from L2 we check if L2 tries to change CR0 bits
> >>> that L1 claims to belong to it and do #vmexit to L1 if it is:
> >>>
> >>>    if (vmcs12->cr0_guest_host_mask & (val ^ vmcs12->cr0_read_shadow))
> >>>             return 1;
> >>>
> >>> We never reach handle_set_cr0() in that case.
> >>>
> >>> Can you provide an example with actual values for L2/L1/L0 of what you
> >>> are trying to say?
> >>
> >> I already provided a concrete one: L1 clears PE/PG from its
> >> guest_host_mask (assuming we support unrestricted guest mode for L1), L2
> >> switches from real to protected mode, thus sets PE=1 while the shadow
> >> (set by L0) holds 0 => we end up in handle_set_cr0.
> >>
> > So how is this "inverse of what the comment suggest"? I do not
> > understand your grudge against the comment. Just clarify that TS is the
> > one example of how we can get here if you think that it is not clear enough.
> > The TS part was useful to me.
> 
> Hmm, if the comment was helpful, why did you have to ask me what was
> wrong? :)
> 
Comment helped me understand what's the case that should be handled
here. I didn't asked you what was wrong, Nadav did, but that's because
I haven't wrapped my mind around this code yet. I will ask it later :)

> If you insist on clarification, let's try it again:
> 
> "We get here when L2 changed cr0 in a way that did not change
> any of L1's shadowed bits (see nested_vmx_exit_handled_cr),
> but did change L0 shadowed bits. KVM only hands over TS to L1,
> and that only if the FPU is enabled. So we will be called for
> every change that L1 allows L2 to perform natively."

I find this much more confusing that original comment. Actually
re-reading the original one I do not think it's about KVM as L2 either.
What bout:

     We get here when L2 changed cr0 in a way that did not change
     any of L1's shadowed bits (see nested_vmx_exit_handled_cr),
     but did change L0 shadowed bits. This may happen with
     the TS bit: L0 may want to leave TS on (for lazy fpu loading)
     while pretending to allow the guest to change it.

So in your case it may happen to PE, but TS is only an example anyway.

> 
> For me, ignoring TS made things clearer and simpler.
> 
Because you had other example to work with.

--
			Gleb.