Re: [PATCH 10/11] KVM: nVMX: Synchronize VMCS12 content with the shadow vmcs

[Gleb Natapov <gleb@redhat.com>]

On Mon, Mar 11, 2013 at 09:54:47AM +0200, Abel Gordon wrote:
> "Nadav Har'El" <nyh@math.technion.ac.il> wrote on 11/03/2013 12:43:35 AM:
> 
> 
> > On Sun, Mar 10, 2013, Abel Gordon wrote about "[PATCH 10/11] KVM:
> > nVMX: Synchronize VMCS12 content with the shadow vmcs":
> > >     nested_vmx_vmexit(vcpu);
> > > +   if (enable_shadow_vmcs)
> > > +      copy_vmcs12_to_shadow(to_vmx(vcpu));
> >
> > I was curious why your patch adds this call to copy_vmcs12_to_shadow
> after
> > every nested_vmx_vmexit (3 times), instead of making this call inside
> > nested_vmx_vmexit(), say right after prepare_vmcs12(). Until I saw:
> 
> Because nested code sometimes modifies vmcs fileds "after"
> nested_vmx_vmexit (see below). I was afraid nested logic
> may be changed in the future and some field may become out-of-sync.
> 
> If we do have to call copy_vmcs12_to_shadow explicitly, then, it will be
> more difficult to miss some field.
> 
I think the patch already miss some fields. What if nested_vmx_run()
fails and calls nested_vmx_entry_failure(). nested_vmx_entry_failure()
sets vmcs12->vm_exit_reason and vmcs12->exit_qualification, but where do
we copy them back to shadow before going back to L1?

May be we need to introduce vmcs12 accessors to track what is changes
and if something need to be copied to shadow before going back to L1.

--
			Gleb.