From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH 4/5] Nested VMX patch 4 implements vmread and vmwrite
Date: Tue, 20 Oct 2009 13:44:41 +0900
Message-ID: <4ADD4039.2050708@redhat.com>
References: <1255617706-13564-1-git-send-email-oritw@il.ibm.com> <1255617706-13564-2-git-send-email-oritw@il.ibm.com> <1255617706-13564-3-git-send-email-oritw@il.ibm.com> <1255617706-13564-4-git-send-email-oritw@il.ibm.com> <1255617706-13564-5-git-send-email-oritw@il.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, benami@il.ibm.com, abelg@il.ibm.com,
	muli@il.ibm.com, aliguori@us.ibm.com, mdday@us.ibm.com
To: oritw@il.ibm.com
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:9417 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751186AbZJTEo4 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 20 Oct 2009 00:44:56 -0400
In-Reply-To: <1255617706-13564-5-git-send-email-oritw@il.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 10/15/2009 11:41 PM, oritw@il.ibm.com wrote:
>
> +static int nested_map_shadow_vmcs(struct kvm_vcpu *vcpu)
> +{
> +	struct vcpu_vmx *vmx = to_vmx(vcpu);
> +	struct page *vmcs_page = nested_get_page(vcpu, vmx->nested.vmptr);
> +
> +	if (vmcs_page == NULL) {
> +		printk(KERN_INFO "%s: failure in nested_get_page\n",__func__);
> +		return 0;
> +	}
> +
> +	if (vmx->nested.l2_state->shadow_vmcs) {
> +		printk(KERN_INFO "%s: shadow vmcs already mapped\n",__func__);
> +		return 0;
> +	}
> +
>    

Consider dropping shadow_vmcs from l2_state and just passing it 
everywhere.  Less convenient but safer.

> +	vmx->nested.l2_state->shadow_vmcs = kmap_atomic(vmcs_page, KM_USER0);
> +
> +	if (!vmx->nested.l2_state->shadow_vmcs) {
> +		printk(KERN_INFO "%s: error in kmap_atomic\n",__func__);
> +		return 0;
> +	}
>    

kmap_atomic() can't fail.
>
> +static int handle_vmread(struct kvm_vcpu *vcpu)
> +{
> +#ifndef CONFIG_X86_64
> +	u64 value;
> +#endif
> +
> +	if (!nested_vmx_check_permission(vcpu))
> +		return 1;
> +
> +	if (!nested_map_shadow_vmcs(vcpu)) {
> +		printk(KERN_INFO "%s invalid shadow vmcs\n", __func__);
> +		set_rflags_to_vmx_fail_invalid(vcpu);
> +		return 1;
> +	}
>    

return an error.

> +
> +	switch (vmcs_field_length(vcpu->arch.regs[VCPU_REGS_RDX])) {
> +	case VMCS_FIELD_TYPE_U16:
> +		vcpu->arch.regs[VCPU_REGS_RAX] =
> +			nested_vmcs_read16(vcpu,
> +					   vcpu->arch.regs[VCPU_REGS_RDX]);
> +		break;
>    

Use the emulator to decode operands.


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.