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

[Julian Pidancet]

On Wed, Mar 7, 2012 at 7:04 PM, Guillem Jover <guillem at hadrons.org> wrote:
> On Wed, 2012-03-07 at 17:54:57 +0000, Julian Pidancet wrote:
>> On Wed, Mar 7, 2012 at 1:46 PM, Guillem Jover <guillem at hadrons.org> 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.
>

Can StackSize equal something else than 16 in Real-Mode ? If yes,
should I handle the case StackSize=32 in the next version of this
patch ?

-- 
Julian