From mboxrd@z Thu Jan  1 00:00:00 1970
From: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
Subject: Re: [PATCH v6 10/15] nEPT: Nested INVEPT
Date: Fri, 02 Aug 2013 16:06:00 +0800
Message-ID: <51FB6868.4060307@linux.vnet.ibm.com>
References: <1375366117-9014-1-git-send-email-gleb@redhat.com> <1375366117-9014-11-git-send-email-gleb@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org, Jun Nakajima <jun.nakajima@intel.com>,
	Yang Zhang <yang.z.zhang@intel.com>, pbonzini@redhat.com
To: Gleb Natapov <gleb@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from e23smtp09.au.ibm.com ([202.81.31.142]:45850 "EHLO
	e23smtp09.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752452Ab3HBIGW (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 2 Aug 2013 04:06:22 -0400
Received: from /spool/local
	by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted
	for <kvm@vger.kernel.org> from <xiaoguangrong@linux.vnet.ibm.com>;
	Sat, 3 Aug 2013 05:01:06 +1000
Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [9.190.235.21])
	by d23dlp01.au.ibm.com (Postfix) with ESMTP id 5B55C2CE8052
	for <kvm@vger.kernel.org>; Fri,  2 Aug 2013 18:06:04 +1000 (EST)
Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97])
	by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r7285r9B63569930
	for <kvm@vger.kernel.org>; Fri, 2 Aug 2013 18:05:54 +1000
Received: from d23av03.au.ibm.com (localhost [127.0.0.1])
	by d23av03.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r72863af029774
	for <kvm@vger.kernel.org>; Fri, 2 Aug 2013 18:06:03 +1000
In-Reply-To: <1375366117-9014-11-git-send-email-gleb@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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);
	......
}