From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH 1/3] KVM: x86 emulator: fix negative bit offset BitOp
 instruction emulation
Date: Fri, 06 Aug 2010 10:10:19 +0200
Message-ID: <4C5BC36B.3060601@redhat.com>
References: <4C5BB725.3060609@cn.fujitsu.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=GB2312
Content-Transfer-Encoding: 7bit
Cc: Avi Kivity <avi@redhat.com>, kvm@vger.kernel.org
To: Wei Yongjun <yjwei@cn.fujitsu.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-wy0-f174.google.com ([74.125.82.174]:39515 "EHLO
	mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1754998Ab0HFIKY (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 6 Aug 2010 04:10:24 -0400
Received: by wyb39 with SMTP id 39so7492195wyb.19
        for <kvm@vger.kernel.org>; Fri, 06 Aug 2010 01:10:23 -0700 (PDT)
In-Reply-To: <4C5BB725.3060609@cn.fujitsu.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 08/06/2010 09:17 AM, Wei Yongjun wrote:
> If bit offset operands is a negative number, BitOp instruction
> will return wrong value. This patch fix it.
> 
> Signed-off-by: Wei Yongjun<yjwei@cn.fujitsu.com>
> ---
>   arch/x86/kvm/emulate.c |   32 ++++++++++++++++++++++++++------
>   1 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 0e360c6..470c7eb 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -724,6 +724,30 @@ done:
>   	return rc;
>   }
> 
> +static void fetch_bit_operand(struct decode_cache *c)
> +{
> +	unsigned long mask, byte_offset;
> +
> +	if (c->dst.type == OP_MEM) {
> +		if (c->src.bytes == 2)
> +			c->src.val = (s16)c->src.val;
> +		else if (c->src.bytes == 4)
> +			c->src.val = (s32)c->src.val;
> +
> +		mask = ~(c->dst.bytes * 8 - 1);
> +
> +		if ((long)c->src.val<  0) {
> +			/* negative bit offset */
> +			byte_offset = c->dst.bytes +
> +				      ((-c->src.val - 1)&  mask) / 8;
> +			c->dst.addr.mem -= byte_offset;
> +		} else {
> +			/* positive bit offset */
> +			c->dst.addr.mem += (c->src.val&  mask) / 8;
> +		}
> +	}
> +}
> +
>   static int read_emulated(struct x86_emulate_ctxt *ctxt,
>   			 struct x86_emulate_ops *ops,
>   			 unsigned long addr, void *dest, unsigned size)
> @@ -2646,12 +2670,8 @@ done_prefixes:
>   			c->dst.bytes = 8;
>   		else
>   			c->dst.bytes = (c->d&  ByteOp) ? 1 : c->op_bytes;
> -		if (c->dst.type == OP_MEM&&  (c->d&  BitOp)) {
> -			unsigned long mask = ~(c->dst.bytes * 8 - 1);
> -
> -			c->dst.addr.mem = c->dst.addr.mem +
> -						   (c->src.val&  mask) / 8;
> -		}
> +		if (c->d&  BitOp)
> +			fetch_bit_operand(c);
>   		c->dst.orig_val = c->dst.val;
>   		break;
>   	case DstAcc:

The whole series looks good now.  Thanks!

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo