Re: [PATCH 05/11] KVM: nVMX: Refactor handle_vmwrite

From: Gleb Natapov <gleb@redhat.com>
To: Abel Gordon <abelg@il.ibm.com>
Cc: kvm@vger.kernel.org, owasserm@redhat.com, nadav@harel.org.il,
	jun.nakajima@intel.com, dongxiao.xu@intel.com
Subject: Re: [PATCH 05/11] KVM: nVMX: Refactor handle_vmwrite
Date: Tue, 9 Apr 2013 14:05:07 +0300	[thread overview]
Message-ID: <20130409110507.GV17919@redhat.com> (raw)
In-Reply-To: <20130310160555.C2117380134@moren.haifa.ibm.com>

On Sun, Mar 10, 2013 at 06:05:55PM +0200, Abel Gordon wrote:
> Refactor existent code so we re-use vmcs12_write_any to copy fields from the
> shadow vmcs specified by the link pointer (used by the processor,
> implementation-specific) to the VMCS12 software format used by L0 to hold
> the fields in L1 memory address space.
> 
> Signed-off-by: Abel Gordon <abelg@il.ibm.com>
> ---
>  arch/x86/kvm/vmx.c |   52 +++++++++++++++++++++++--------------------
>  1 file changed, 28 insertions(+), 24 deletions(-)
> 
> --- .before/arch/x86/kvm/vmx.c	2013-03-10 18:00:54.000000000 +0200
> +++ .after/arch/x86/kvm/vmx.c	2013-03-10 18:00:54.000000000 +0200
> @@ -5741,6 +5741,33 @@ static inline bool vmcs12_read_any(struc
>  	}
>  }
>  
> +
> +static inline bool vmcs12_write_any(struct kvm_vcpu *vcpu,
> +				    unsigned long field, u64 field_value){
> +	short offset = vmcs_field_to_offset(field);
> +	char *p = ((char *) get_vmcs12(vcpu)) + offset;
> +	if (offset < 0)
> +		return 0;
The function returns bool, so use true/false please.

> +
> +	switch (vmcs_field_type(field)) {
> +	case VMCS_FIELD_TYPE_U16:
> +		*(u16 *)p = field_value;
> +		return 1;
> +	case VMCS_FIELD_TYPE_U32:
> +		*(u32 *)p = field_value;
> +		return 1;
> +	case VMCS_FIELD_TYPE_U64:
> +		*(u64 *)p = field_value;
> +		return 1;
> +	case VMCS_FIELD_TYPE_NATURAL_WIDTH:
> +		*(natural_width *)p = field_value;
> +		return 1;
> +	default:
> +		return 0; /* can never happen. */
> +	}
> +
> +}
> +
>  /*
>   * VMX instructions which assume a current vmcs12 (i.e., that VMPTRLD was
>   * used before) all generate the same failure when it is missing.
> @@ -5806,8 +5833,6 @@ static int handle_vmwrite(struct kvm_vcp
>  	gva_t gva;
>  	unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
>  	u32 vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
> -	char *p;
> -	short offset;
>  	/* The value to write might be 32 or 64 bits, depending on L1's long
>  	 * mode, and eventually we need to write that into a field of several
>  	 * possible lengths. The code below first zero-extends the value to 64
> @@ -5846,28 +5871,7 @@ static int handle_vmwrite(struct kvm_vcp
>  		return 1;
>  	}
>  
> -	offset = vmcs_field_to_offset(field);
> -	if (offset < 0) {
> -		nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT);
> -		skip_emulated_instruction(vcpu);
> -		return 1;
> -	}
> -	p = ((char *) get_vmcs12(vcpu)) + offset;
> -
> -	switch (vmcs_field_type(field)) {
> -	case VMCS_FIELD_TYPE_U16:
> -		*(u16 *)p = field_value;
> -		break;
> -	case VMCS_FIELD_TYPE_U32:
> -		*(u32 *)p = field_value;
> -		break;
> -	case VMCS_FIELD_TYPE_U64:
> -		*(u64 *)p = field_value;
> -		break;
> -	case VMCS_FIELD_TYPE_NATURAL_WIDTH:
> -		*(natural_width *)p = field_value;
> -		break;
> -	default:
> +	if (!vmcs12_write_any(vcpu, field, field_value)) {
>  		nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT);
>  		skip_emulated_instruction(vcpu);
>  		return 1;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.