Re: [PATCH v6 10/15] nEPT: Nested INVEPT

[Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>]

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