From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH 10/24] Implement VMPTRLD
Date: Mon, 14 Jun 2010 12:07:52 +0300
Message-ID: <4C15F168.3030305@redhat.com>
References: <1276431753-nyh@il.ibm.com> <201006131227.o5DCRfIJ012978@rice.haifa.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org
To: "Nadav Har'El" <nyh@il.ibm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:34194 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1755631Ab0FNJHz (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 14 Jun 2010 05:07:55 -0400
In-Reply-To: <201006131227.o5DCRfIJ012978@rice.haifa.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 06/13/2010 03:27 PM, Nadav Har'El wrote:
> This patch implements the VMPTRLD instruction.
>
>
>   static void clear_rflags_cf_zf(struct kvm_vcpu *vcpu)
>   {
>   	unsigned long rflags;
> @@ -3869,6 +3889,57 @@ static int handle_vmclear(struct kvm_vcp
>   	return 1;
>   }
>
> +static bool verify_vmcs12_revision(struct kvm_vcpu *vcpu, gpa_t guest_vmcs_addr)
> +{
> +	bool ret;
> +	struct vmcs12 *vmcs12;
> +	struct page *vmcs_page = nested_get_page(vcpu, guest_vmcs_addr);
>    

Blank line so I can catch my breath.

> +	if (vmcs_page == NULL)
> +		return 0;
>    

Doesn't seem right.

> +	vmcs12 = (struct vmcs12 *)kmap_atomic(vmcs_page, KM_USER0);
> +	if (vmcs12->revision_id == VMCS12_REVISION)
> +		ret = 1;
> +	else {
> +		set_rflags_to_vmx_fail_valid(vcpu);
> +		ret = 0;
> +	}
> +	kunmap_atomic(vmcs12, KM_USER0);
> +	kvm_release_page_dirty(vmcs_page);
>    

Can release a clean page here.  But what happened to those mapping helpers?

> +	return ret;
> +}
> +
> +/* Emulate the VMPTRLD instruction */
> +static int handle_vmptrld(struct kvm_vcpu *vcpu)
> +{
> +	struct vcpu_vmx *vmx = to_vmx(vcpu);
> +	gpa_t guest_vmcs_addr;
> +
> +	if (!nested_vmx_check_permission(vcpu))
> +		return 1;
> +
> +	if (read_guest_vmcs_gpa(vcpu,&guest_vmcs_addr)) {
> +		set_rflags_to_vmx_fail_invalid(vcpu);
>    

Need to skip_emulated_instruction() in this case.

> +		return 1;
> +	}
> +
> +	if (!verify_vmcs12_revision(vcpu, guest_vmcs_addr))
> +		return 1;
>    

Here too.

> +
> +	if (vmx->nested.current_vmptr != guest_vmcs_addr) {
> +		vmx->nested.current_vmptr = guest_vmcs_addr;
> +
> +		if (nested_create_current_vmcs(vcpu)) {
> +			printk(KERN_ERR "%s error could not allocate memory",
> +				__func__);
>    

In general ftrace and the ENOMEM itself are sufficient documentation 
that something went wrong.

> +			return -ENOMEM;
> +		}
> +	}
> +
> +	clear_rflags_cf_zf(vcpu);
> +	skip_emulated_instruction(vcpu);
> +	return 1;
> +}
> +
>    

-- 
error compiling committee.c: too many arguments to function