From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH v6 10/15] nEPT: Nested INVEPT Date: Fri, 2 Aug 2013 13:00:14 +0300 Message-ID: <20130802100014.GC30072@redhat.com> References: <1375366117-9014-1-git-send-email-gleb@redhat.com> <1375366117-9014-11-git-send-email-gleb@redhat.com> <51FB6868.4060307@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, Jun Nakajima , Yang Zhang , pbonzini@redhat.com To: Xiao Guangrong Return-path: Received: from mx1.redhat.com ([209.132.183.28]:36733 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758179Ab3HBKAf (ORCPT ); Fri, 2 Aug 2013 06:00:35 -0400 Content-Disposition: inline In-Reply-To: <51FB6868.4060307@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, Aug 02, 2013 at 04:06:00PM +0800, Xiao Guangrong wrote: > On 08/01/2013 10:08 PM, Gleb Natapov wrote: > > > +/* Emulate the INVEPT instruction */ > > +static int handle_invept(struct kvm_vcpu *vcpu) > > +{ > > + u32 vmx_instruction_info; > > + bool ok; > > + unsigned long type; > > + gva_t gva; > > + struct x86_exception e; > > + struct { > > + u64 eptp, gpa; > > + } operand; > > + > > + if (!(nested_vmx_secondary_ctls_high & SECONDARY_EXEC_ENABLE_EPT) || > > + !(nested_vmx_ept_caps & VMX_EPT_INVEPT_BIT)) { > > + kvm_queue_exception(vcpu, UD_VECTOR); > > + return 1; > > + } > > + > > + if (!nested_vmx_check_permission(vcpu)) > > + return 1; > > + > > + if (!kvm_read_cr0_bits(vcpu, X86_CR0_PE)) { > > + kvm_queue_exception(vcpu, UD_VECTOR); > > + return 1; > > + } > > + > > + /* According to the Intel VMX instruction reference, the memory > > + * operand is read even if it isn't needed (e.g., for type==global) > > + */ > > + vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO); > > + if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION), > > + vmx_instruction_info, &gva)) > > + return 1; > > + if (kvm_read_guest_virt(&vcpu->arch.emulate_ctxt, gva, &operand, > > + sizeof(operand), &e)) { > > + kvm_inject_page_fault(vcpu, &e); > > + return 1; > > + } > > + > > + type = kvm_register_read(vcpu, (vmx_instruction_info >> 28) & 0xf); > > + > > + switch (type) { > > + case VMX_EPT_EXTENT_GLOBAL: > > + case VMX_EPT_EXTENT_CONTEXT: > > + ok = !!(nested_vmx_ept_caps & > > + (1UL << (type + VMX_EPT_EXTENT_SHIFT))); > > + break; > > + default: > > + ok = false; > > + } > > + > > + if (ok) { > > + kvm_mmu_sync_roots(vcpu); > > + kvm_mmu_flush_tlb(vcpu); > > + nested_vmx_succeed(vcpu); > > + } > > + else > > + nested_vmx_failValid(vcpu, > > + VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID); > > + > > + skip_emulated_instruction(vcpu); > > + return 1; > > Can this code be changed to: > > switch (type) { > case VMX_EPT_EXTENT_GLOBAL: > case VMX_EPT_EXTENT_CONTEXT: > if (nested_vmx_ept_caps & > (1UL << (type + VMX_EPT_EXTENT_SHIFT) { > kvm_mmu_sync_roots(vcpu); > kvm_mmu_flush_tlb(vcpu); > nested_vmx_succeed(vcpu); > break; > } > default: > nested_vmx_failValid(vcpu, > VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID); > } > ? > That's clearer i think. > > Also, we can sync the shadow page table only if the current eptp is the required > eptp, that means: > > if (type == GLOBAL || operand.eptp == nested_ept_get_cr3(vcpu)) { > kvm_mmu_sync_roots(vcpu); > ...... > } Good point. Will do. -- Gleb.