[PATCH] KVM: x86 emulator: fix %rip-relative addressing with immediate source operand

[PATCH] KVM: x86 emulator: fix %rip-relative addressing with immediate source operand

[Avi Kivity]

%rip-relative addressing is relative to the first byte of the next instruction,
so we need to add %rip only after we've fetched any immediate bytes.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 14891ad..6f08bc9 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3342,7 +3342,7 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
 	int def_op_bytes, def_ad_bytes, goffset, simd_prefix;
 	bool op_prefix = false;
 	struct opcode opcode;
-	struct operand memop = { .type = OP_NONE };
+	struct operand memop = { .type = OP_NONE }, *memopp = NULL;
 
 	ctxt->_eip = ctxt->eip;
 	ctxt->fetch.start = ctxt->_eip;
@@ -3517,9 +3517,6 @@ done_prefixes:
 	if (memop.type == OP_MEM && ctxt->ad_bytes != 8)
 		memop.addr.mem.ea = (u32)memop.addr.mem.ea;
 
-	if (memop.type == OP_MEM && ctxt->rip_relative)
-		memop.addr.mem.ea += ctxt->_eip;
-
 	/*
 	 * Decode and fetch the source operand: register, memory
 	 * or immediate.
@@ -3541,6 +3538,7 @@ done_prefixes:
 							   ctxt->op_bytes;
 	srcmem_common:
 		ctxt->src = memop;
+		memopp = &ctxt->src;
 		break;
 	case SrcImmU16:
 		rc = decode_imm(ctxt, &ctxt->src, 2, false);
@@ -3637,6 +3635,7 @@ done_prefixes:
 	case DstMem:
 	case DstMem64:
 		ctxt->dst = memop;
+		memopp = &ctxt->dst;
 		if ((ctxt->d & DstMask) == DstMem64)
 			ctxt->dst.bytes = 8;
 		else
@@ -3670,10 +3669,13 @@ done_prefixes:
 		/* Special instructions do their own operand decoding. */
 	default:
 		ctxt->dst.type = OP_NONE; /* Disable writeback. */
-		return 0;
+		break;
 	}
 
 done:
+	if (memopp && memopp->type == OP_MEM && ctxt->rip_relative)
+		memopp->addr.mem.ea += ctxt->_eip;
+
 	return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED : EMULATION_OK;
 }
 
-- 
1.7.5.3

RE: [PATCH] KVM: x86 emulator: fix %rip-relative addressing with immediate source operand

[Li, Xin]

> %rip-relative addressing is relative to the first byte of the next instruction,
> so we need to add %rip only after we've fetched any immediate bytes.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>

Acked-by: Li Xin <xin.li@intel.com>

> ---
>  arch/x86/kvm/emulate.c |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 14891ad..6f08bc9 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -3342,7 +3342,7 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt,
> void *insn, int insn_len)
>  	int def_op_bytes, def_ad_bytes, goffset, simd_prefix;
>  	bool op_prefix = false;
>  	struct opcode opcode;
> -	struct operand memop = { .type = OP_NONE };
> +	struct operand memop = { .type = OP_NONE }, *memopp = NULL;
> 
>  	ctxt->_eip = ctxt->eip;
>  	ctxt->fetch.start = ctxt->_eip;
> @@ -3517,9 +3517,6 @@ done_prefixes:
>  	if (memop.type == OP_MEM && ctxt->ad_bytes != 8)
>  		memop.addr.mem.ea = (u32)memop.addr.mem.ea;
> 
> -	if (memop.type == OP_MEM && ctxt->rip_relative)
> -		memop.addr.mem.ea += ctxt->_eip;
> -
>  	/*
>  	 * Decode and fetch the source operand: register, memory
>  	 * or immediate.
> @@ -3541,6 +3538,7 @@ done_prefixes:
>  							   ctxt->op_bytes;
>  	srcmem_common:
>  		ctxt->src = memop;
> +		memopp = &ctxt->src;
>  		break;
>  	case SrcImmU16:
>  		rc = decode_imm(ctxt, &ctxt->src, 2, false);
> @@ -3637,6 +3635,7 @@ done_prefixes:
>  	case DstMem:
>  	case DstMem64:
>  		ctxt->dst = memop;
> +		memopp = &ctxt->dst;
>  		if ((ctxt->d & DstMask) == DstMem64)
>  			ctxt->dst.bytes = 8;
>  		else
> @@ -3670,10 +3669,13 @@ done_prefixes:
>  		/* Special instructions do their own operand decoding. */
>  	default:
>  		ctxt->dst.type = OP_NONE; /* Disable writeback. */
> -		return 0;
> +		break;
>  	}
> 
>  done:
> +	if (memopp && memopp->type == OP_MEM && ctxt->rip_relative)
> +		memopp->addr.mem.ea += ctxt->_eip;
> +
>  	return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED :
> EMULATION_OK;
>  }
> 
> --
> 1.7.5.3

Re: [PATCH] KVM: x86 emulator: fix %rip-relative addressing with immediate source operand

[Avi Kivity]

On 06/21/2011 12:09 PM, Li, Xin wrote:
> >  %rip-relative addressing is relative to the first byte of the next instruction,
> >  so we need to add %rip only after we've fetched any immediate bytes.
> >
> >  Signed-off-by: Avi Kivity<avi@redhat.com>
>
> Acked-by: Li Xin<xin.li@intel.com>

And should also be, Based on original patch by Li Xin 
<xin.li@intel.com>.  Sorry.

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

Re: [PATCH] KVM: x86 emulator: fix %rip-relative addressing with immediate source operand

[Marcelo Tosatti]

On Sun, Jun 19, 2011 at 07:21:11PM +0300, Avi Kivity wrote:
> %rip-relative addressing is relative to the first byte of the next instruction,
> so we need to add %rip only after we've fetched any immediate bytes.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/kvm/emulate.c |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)

Applied, thanks.