Re: [SeaBIOS] [PATCH] x86emu: Correctly handle 0x66 prefix for some instructions

[Guillem Jover]

On Wed, 2012-03-07 at 17:54:57 +0000, Julian Pidancet wrote:
> On Wed, Mar 7, 2012 at 1:46 PM, Guillem Jover  wrote:
> > On Mon, 2012-03-05 at 17:49:08 +0000, Julian Pidancet wrote:
> > > diff --git a/hw/xfree86/x86emu/ops.c b/hw/xfree86/x86emu/ops.c
> > > index 5d3cac1..440b8dc 100644
> > > --- a/hw/xfree86/x86emu/ops.c
> > > +++ b/hw/xfree86/x86emu/ops.c
> > > @@ -8787,11 +8795,16 @@ static void x86emuOp_enter(u8 X86EMU_UNUSED(op1))
> > >      frame_pointer = M.x86.R_SP;
> > >      if (nesting > 0) {
> > >          for (i = 1; i < nesting; i++) {
> > > M.x86.R_BP -= 2;
> > > push_word(fetch_data_word_abs(M.x86.R_SS, M.x86.R_BP));
> > > if (M.x86.mode & SYSMODE_PREFIX_DATA) {
> > > M.x86.R_EBP -= 4;
> > > push_long(fetch_data_long_abs(M.x86.R_SS, M.x86.R_BP));
> >
> > Shouldn't this be:
> >
> > push_long(fetch_data_long_abs(M.x86.R_SS, M.x86.R_EBP))
> >
> > ?
>
> From the Intel® 64 and IA-32 Architectures Software Developer Manuals,
> here is the pseudo code associated with the enter instruction:
>
> [...]
> IF (NestingLevel > 1)
>     THEN FOR i <- 1 to (NestingLevel - 1)
>         DO
>             IF 64-Bit Mode (StackSize = 64)
>                 THEN
>                     RBP <- RBP - 8;
>                     Push([RBP]); (* Quadword push *)
>                 ELSE IF OperandSize = 32
>                     THEN
>                         IF StackSize = 32
>                             EBP <- EBP - 4;
>                             Push([EBP]); (* Doubleword push *)
>                         ELSE (* StackSize = 16 *)
>                             BP <- BP - 4;
>                             Push([BP]); (* Doubleword push *)
>                         FI;
>                     FI;
>                 ELSE (* OperandSize = 16 *)
>                     IF StackSize = 32
>                         THEN
>                             EBP <- EBP - 2;
>                             Push([EBP]); (* Word push *)
>                         ELSE (* StackSize = 16 *)
>                             BP <- BP - 2;
>                             Push([BP]); (* Word push *)
>                     FI;
>             FI;
>         OD;
> FI;
> [...]
>
> I believe the 0x66 prefix only affects OperandSize, not StackSize.

Right. What affects the stack size is the B flag on the stack descriptor.
> So according to the manual, it should be BP, not EBP.

The register being decreased should match the one being used to address
the stack, and the one to use depends on the descriptor as per above.
> In any case, It won't be a problem, because the 16 high bits of EBP
> will most likely be zero in real-mode code.

Well, not if the the code is using some 32-bit instructions. :)
> I will respin a patch shortly.

thanks,
guillem