Re: [PATCH 10/10] KVM: nVMX: Enable and disable shadow vmcs functionality

[Gleb Natapov <gleb@redhat.com>]

On Wed, Apr 17, 2013 at 06:18:27PM +0300, Abel Gordon wrote:
> 
> 
> Gleb Natapov <gleb@redhat.com> wrote on 17/04/2013 05:41:07 PM:
> 
> > On Wed, Apr 17, 2013 at 02:55:40PM +0300, Abel Gordon wrote:
> > > Once L1 loads VMCS12 we enable shadow-vmcs capability and copy allthe
> VMCS12
> > > shadowed fields to the shadow vmcs.  When we release the VMCS12, we
> also
> > > disable shadow-vmcs capability.
> > >
> > > Signed-off-by: Abel Gordon <abelg@il.ibm.com>
> > > ---
> > >  arch/x86/kvm/vmx.c |   11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > --- .before/arch/x86/kvm/vmx.c   2013-04-17 14:20:51.000000000 +0300
> > > +++ .after/arch/x86/kvm/vmx.c   2013-04-17 14:20:51.000000000 +0300
> > > @@ -5590,12 +5590,17 @@ static int nested_vmx_check_permission(s
> > >
> > >  static inline void nested_release_vmcs12(struct vcpu_vmx *vmx)
> > >  {
> > > +   u32 exec_control;
> > >     if (enable_shadow_vmcs) {
> > >        if (vmx->nested.current_vmcs12 != NULL) {
> > >           /* copy to memory all shadowed fields in case
> > >              they were modified */
> > >           copy_shadow_to_vmcs12(vmx);
> > >           vmx->nested.sync_shadow_vmcs = false;
> > > +         exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
> > > +         exec_control &= ~SECONDARY_EXEC_SHADOW_VMCS;
> > > +         vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_control);
> > > +         vmcs_write64(VMCS_LINK_POINTER, -1ull);
> > >           free_vmcs(vmx->nested.current_shadow_vmcs);
> > >        }
> > >     }
> > > @@ -6084,6 +6089,7 @@ static int handle_vmptrld(struct kvm_vcp
> > >     gpa_t vmptr;
> > >     struct x86_exception e;
> > >     struct vmcs *shadow_vmcs;
> > > +   u32 exec_control;
> > >
> > >     if (!nested_vmx_check_permission(vcpu))
> > >        return 1;
> > > @@ -6140,6 +6146,11 @@ static int handle_vmptrld(struct kvm_vcp
> > >           /* init shadow vmcs */
> > >           vmcs_clear(shadow_vmcs);
> > >           vmx->nested.current_shadow_vmcs = shadow_vmcs;
> > > +         exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
> > > +         exec_control |= SECONDARY_EXEC_SHADOW_VMCS;
> > > +         vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_control);
> > > +         vmcs_write64(VMCS_LINK_POINTER,
> > > +                 __pa(shadow_vmcs));
> > How hard would it be to disable shadowing for individual vmcs if shadow
> > vmcs allocation fails? It bothers me a little that we can fail perfectly
> > valid vmptrld() because of failed allocation.
> 
> That's really a corner case... IMHO, if we fail to allocate a shadow vmcs
> we may experience bigger issues, like failing  to allocate VMCS02.
> Anyway, if we reuse the shadow vmcs as you requested, then we can allocate
> the shadow vmcs once in handle_vmon. In this case, handle_vmon will fail
> and
> not handle_vmptrld.
Yes, I agree that with shadow vmcs reuse the issue is almost non
existent.

--
			Gleb.