From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH 09/10] KVM: nVMX: Synchronize VMCS12 content with the shadow vmcs Date: Thu, 18 Apr 2013 10:10:20 +0300 Message-ID: <20130418071020.GJ8997@redhat.com> References: <1366218306-abelg@il.ibm.com> <20130417170940.7F6DF3806E7@moren.haifa.ibm.com> <20130418064116.GH8997@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dongxiao.xu@intel.com, jun.nakajima@intel.com, kvm@vger.kernel.org, nadav@harel.org.il, owasserm@redhat.com To: Abel Gordon Return-path: Received: from mx1.redhat.com ([209.132.183.28]:33222 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752567Ab3DRHKa (ORCPT ); Thu, 18 Apr 2013 03:10:30 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Apr 18, 2013 at 10:07:36AM +0300, Abel Gordon wrote: > > > Gleb Natapov wrote on 18/04/2013 09:41:16 AM: > > > > On Wed, Apr 17, 2013 at 08:09:40PM +0300, Abel Gordon wrote: > > > Synchronize between the VMCS12 software controlled structure and the > > > processor-specific shadow vmcs > > > > > > Signed-off-by: Abel Gordon > > > --- > > > arch/x86/kvm/vmx.c | 32 ++++++++++++++++++++++++++++++++ > > > 1 file changed, 32 insertions(+) > > > > > > --- .before/arch/x86/kvm/vmx.c 2013-04-17 19:58:33.000000000 +0300 > > > +++ .after/arch/x86/kvm/vmx.c 2013-04-17 19:58:33.000000000 +0300 > > > @@ -356,6 +356,11 @@ struct nested_vmx { > > > struct page *current_vmcs12_page; > > > struct vmcs12 *current_vmcs12; > > > struct vmcs *current_shadow_vmcs; > > > + /* > > > + * Indicates if the shadow vmcs must be updated with the > > > + * data hold by vmcs12 > > > + */ > > > + bool sync_shadow_vmcs; > > > > > > /* vmcs02_list cache of VMCSs recently used to run L2 guests */ > > > struct list_head vmcs02_pool; > > > @@ -5596,6 +5601,14 @@ static int nested_vmx_check_permission(s > > > > > > static inline void nested_release_vmcs12(struct vcpu_vmx *vmx) > > > { > > > + 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; > > > + } > > > + } > > > kunmap(vmx->nested.current_vmcs12_page); > > > nested_release_page(vmx->nested.current_vmcs12_page); > > > } > > > @@ -5724,6 +5737,10 @@ static void nested_vmx_failValid(struct > > > X86_EFLAGS_SF | X86_EFLAGS_OF)) > > > | X86_EFLAGS_ZF); > > > get_vmcs12(vcpu)->vm_instruction_error = vm_instruction_error; > > > + /* > > > + * We don't need to force a shadow sync because > > > + * VM_INSTRUCTION_ERROR is not shadowed > > > + */ > > > } > > > > > > /* Emulate the VMCLEAR instruction */ > > > @@ -6122,6 +6139,9 @@ static int handle_vmptrld(struct kvm_vcp > > > vmx->nested.current_vmptr = vmptr; > > > vmx->nested.current_vmcs12 = new_vmcs12; > > > vmx->nested.current_vmcs12_page = page; > > > + if (enable_shadow_vmcs) { > > > + vmx->nested.sync_shadow_vmcs = true; > > > + } > > No need braces around one statement according to kernel coding style. > > The last patch introduces code within the braces... I usually prefer > to add them in advance so you don't see the "if" statement as modified > in future patches. If this is not a common practice, I'll remove the > braces here and add them again later. Please confirm. Ah, I haven't noticed that the later patch adds code here. In this case the braces a welcome. -- Gleb.