From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gleb Natapov <gleb@redhat.com>
Subject: Re: [PATCH 1/8] KVM: x86 emulator: add support for writing back the
 source operand
Date: Sun, 10 Feb 2013 13:56:07 +0200
Message-ID: <20130210115607.GP7837@redhat.com>
References: <1360402311-19904-1-git-send-email-avi.kivity@gmail.com>
 <1360402311-19904-2-git-send-email-avi.kivity@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Marcelo Tosatti <mtosatti@redhat.com>, kvm@vger.kernel.org
To: Avi Kivity <avi.kivity@gmail.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:6037 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753917Ab3BJL4K (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sun, 10 Feb 2013 06:56:10 -0500
Content-Disposition: inline
In-Reply-To: <1360402311-19904-2-git-send-email-avi.kivity@gmail.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Sat, Feb 09, 2013 at 11:31:44AM +0200, Avi Kivity wrote:
> Some instructions write back the source operand, not just the destination.
> Add support for doing this via the decode flags.
> 
We cannot really write back to memory operands. I feel it's deceiving to make
the code look like we can. 

> Signed-off-by: Avi Kivity <avi.kivity@gmail.com>
> ---
>  arch/x86/kvm/emulate.c | 47 ++++++++++++++++++++++++++---------------------
>  1 file changed, 26 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 2b11318..18c86b5 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -152,6 +152,7 @@
>  #define Avx         ((u64)1 << 43)  /* Advanced Vector Extensions */
>  #define Fastop      ((u64)1 << 44)  /* Use opcode::u.fastop */
>  #define NoWrite     ((u64)1 << 45)  /* No writeback */
> +#define SrcWrite    ((u64)1 << 46)  /* Write back src operand */
>  
>  #define X2(x...) x, x
>  #define X3(x...) X2(x), x
> @@ -1708,45 +1709,42 @@ static void write_register_operand(struct operand *op)
>  	}
>  }
>  
> -static int writeback(struct x86_emulate_ctxt *ctxt)
> +static int writeback(struct x86_emulate_ctxt *ctxt, struct operand *op)
>  {
>  	int rc;
>  
> -	if (ctxt->d & NoWrite)
> -		return X86EMUL_CONTINUE;
> -
> -	switch (ctxt->dst.type) {
> +	switch (op->type) {
>  	case OP_REG:
> -		write_register_operand(&ctxt->dst);
> +		write_register_operand(op);
>  		break;
>  	case OP_MEM:
>  		if (ctxt->lock_prefix)
>  			rc = segmented_cmpxchg(ctxt,
> -					       ctxt->dst.addr.mem,
> -					       &ctxt->dst.orig_val,
> -					       &ctxt->dst.val,
> -					       ctxt->dst.bytes);
> +					       op->addr.mem,
> +					       &op->orig_val,
> +					       &op->val,
> +					       op->bytes);
>  		else
>  			rc = segmented_write(ctxt,
> -					     ctxt->dst.addr.mem,
> -					     &ctxt->dst.val,
> -					     ctxt->dst.bytes);
> +					     op->addr.mem,
> +					     &op->val,
> +					     op->bytes);
>  		if (rc != X86EMUL_CONTINUE)
>  			return rc;
>  		break;
>  	case OP_MEM_STR:
>  		rc = segmented_write(ctxt,
> -				ctxt->dst.addr.mem,
> -				ctxt->dst.data,
> -				ctxt->dst.bytes * ctxt->dst.count);
> +				op->addr.mem,
> +				op->data,
> +				op->bytes * op->count);
>  		if (rc != X86EMUL_CONTINUE)
>  			return rc;
>  		break;
>  	case OP_XMM:
> -		write_sse_reg(ctxt, &ctxt->dst.vec_val, ctxt->dst.addr.xmm);
> +		write_sse_reg(ctxt, &op->vec_val, op->addr.xmm);
>  		break;
>  	case OP_MM:
> -		write_mmx_reg(ctxt, &ctxt->dst.mm_val, ctxt->dst.addr.mm);
> +		write_mmx_reg(ctxt, &op->mm_val, op->addr.mm);
>  		break;
>  	case OP_NONE:
>  		/* no writeback */
> @@ -4717,9 +4715,16 @@ special_insn:
>  		goto done;
>  
>  writeback:
> -	rc = writeback(ctxt);
> -	if (rc != X86EMUL_CONTINUE)
> -		goto done;
> +	if (!(ctxt->d & NoWrite)) {
> +		rc = writeback(ctxt, &ctxt->dst);
> +		if (rc != X86EMUL_CONTINUE)
> +			goto done;
> +	}
> +	if (ctxt->d & SrcWrite) {
> +		rc = writeback(ctxt, &ctxt->src);
> +		if (rc != X86EMUL_CONTINUE)
> +			goto done;
> +	}
>  
>  	/*
>  	 * restore dst type in case the decoding will be reused
> -- 
> 1.8.1.2

--
			Gleb.