[PATCH] x86emu: Fix more mis-decoding of the data prefix

[Julien Cristau]

On Mon, Dec 20, 2010 at 16:55:18 -0500, Adam Jackson wrote:

> On Mon, 2010-12-20 at 19:41 +0100, Julien Cristau wrote:
> > On Fri, Dec 10, 2010 at 14:47:20 -0500, Adam Jackson wrote:
> > > @@ -7084,8 +7089,12 @@ static void x86emuOp_call_far_IMM(u8 X86EMU_UNUSED(op1))
> > >      TRACE_AND_STEP();
> > >      push_word(M.x86.R_CS);
> > >      M.x86.R_CS = farseg;
> > > -    push_word(M.x86.R_IP);
> > > -    M.x86.R_IP = faroff;
> > > +    if (M.x86.mode & SYSMODE_PREFIX_DATA) {
> > > +	push_long(M.x86.R_EIP);
> > > +    } else {
> > > +	push_word(M.x86.R_IP);
> > > +    }
> > > +    M.x86.R_EIP = faroff & 0xffff;
> > >      DECODE_CLEAR_SEGOVR();
> > >      END_OF_INSTR();
> > >  }
> > 
> > The intel doc says (for far calls in virtual-8086 mode):
> > 
> > IF OperandSize = 32
> >   THEN
> >     ...
> >     EIP ← DEST[31:0]; (* DEST is ptr16:32 or [m16:32] *)
> >   ELSE
> >     ...
> >     EIP ← DEST[15:0]; (* DEST is ptr16:16 or [m16:16]; clear upper 16 bits *)
> > FI;
> > 
> > which suggests that you don't need the & 0xffff in the last assignment?
> 
> Bit of artistic license on my part.  The relevant excerpt is:
> 
> IF OperandSize = 32
> THEN
> 	IF stack not large enough for a 6-byte return address
> 	THEN #SS(0); FI;
> @	IF DEST[31:16] is not zero THEN #GP(0); FI;
> 	Push(CS); (* Padded with 16 high-order bits *)
> 	Push(EIP);
> 	CS <- DEST[47:32]; (* DEST is ptr16:32 or [m16:32] *)
> 	EIP <- DEST[31:0]; (* DEST is ptr16:32 or [m16:32] *)
> ELSE (* OperandSize = 16 *)
> 	IF stack not large enough for a 4-byte return address
> 	THEN #SS(0); FI;
> 	Push(CS);
> 	Push(IP);
> 	CS <- DEST[31:16]; (* DEST is ptr16:16 or [m16:16] *)
> 	EIP <- DEST[15:0]; (* DEST is ptr16:16 or [m16:16]; clear upper 16 bits *)
> FI;
> 
> The @'d line is where I'm taking a small liberty, as x86emu doesn't
> emulate faults.  We'll assume (since x86emu tends to work) that the
> number of BIOSes that need fault handling is very low.  So by the time
> you hit the EIP load, either:
> 
> - operand size is 16, high word of EIP explicitly 0
> - operand size is 32 but you'd have faulted if the high word wasn't 0
> 
> Practically I think this matters only if the later code looks at the
> high word of EIP, instruction fetch is only going to look at cs:ip not
> cs:eip.  But, might as well be screechingly correct.
> 
Thanks for the explanation.  Feel free to add my
Reviewed-by: Julien Cristau <jcristau at debian.org>

Cheers,
Julien