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

2012-03-08 Thread Guillem Jover 
On Thu, 2012-03-08 at 22:57:30 +, Julian Pidancet wrote:
> On Wed, Mar 7, 2012 at 7:04 PM, Guillem Jover  wrote:
> > On Wed, 2012-03-07 at 17:54:57 +, Julian Pidancet wrote:
> > > 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.
> 
> There is no "descriptor" in real mode. Default stack size is always 16-bit.

Certainly! And sorry, somehow missed the fact this is exclusively
real-mode being emulated.

> Also, I think the right thing to do is to decrement BP instead of EBP
> when SYSMODE_PREFIX_DATA is set:
> 
> M.x86.R_BP -= 4;
> push_long(fetch_data_long_abs(M.x86.R_SS, M.x86.R_BP));
> 
> instead of:
> 
> M.x86.R_EBP -= 4;
> push_long(fetch_data_long_abs(M.x86.R_SS, M.x86.R_BP));
> 
> ...to remain exactly consistent with the manual.

Indeed.

> > > 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. :)

> They are not "32-bit instructions". The processor is still functioning
> in real-mode, therefore, addressing still follows the rules of
> real-mode addressing, as mentioned in Volume 1:Basic Architecture :

> 3.3.5 32-Bit and 16-Bit Address and Operand Sizes

Regardless of the mode, if the instructions are being modified by the
prefixes to use 32-bit operands or addresses, they are in my book
32-bit instructions. In any case what I meant was that ebp can have
any value because the register can be assigned directly, for example,
and wrap-around might vary depending on what part of it it's being
operated on.

thanks,
guillem

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

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

2012-03-08 Thread Julian Pidancet 
Some instructions are not emulated correctly by x86emu when they
are prefixed by the 0x66 opcode.
I've identified problems in the emulation of these intructions: ret,
enter, leave, iret and some forms of call.

Most of the time, the problem is that these instructions should push or
pop 32-bit values to/from the stack, instead of 16bit, when they are
prefixed by the 0x66 special opcode.

The SeaBIOS project aims to produce a complete legacy BIOS
implementation as well as a VGA option ROM, entirely written in C and
using the GCC compiler.

In 16bit code produced by the GCC compiler, the 0x66 prefix is used
almost everywhere. This patch is necessary to allow the SeaBIOS VGA
option ROM to function with Xorg when using the vesa driver.

v2: - Decrement BP instead of EBP in accordance with the Intel Manual
- Assign EIP instead of IP when poping the return address from the
stack in 32-bit operand size mode in ret_far_IMM, ret_far, and iret
- When poping EFLAGS from the stack in iret in 32-bit operand size
mode, apply some mask to preserve Read-only flags.

Signed-off-by: Julian Pidancet 
---
 hw/xfree86/x86emu/ops.c |  194 ++
 1 files changed, 143 insertions(+), 51 deletions(-)

diff --git a/hw/xfree86/x86emu/ops.c b/hw/xfree86/x86emu/ops.c
index 5d3cac1..c5d5139 100644
--- a/hw/xfree86/x86emu/ops.c
+++ b/hw/xfree86/x86emu/ops.c
@@ -8487,7 +8487,11 @@ static void x86emuOp_ret_near_IMM(u8 X86EMU_UNUSED(op1))
 DECODE_PRINTF2("%x\n", imm);
RETURN_TRACE("RET",M.x86.saved_cs,M.x86.saved_ip);
TRACE_AND_STEP();
-M.x86.R_IP = pop_word();
+if (M.x86.mode & SYSMODE_PREFIX_DATA) {
+M.x86.R_EIP = pop_long();
+} else {
+M.x86.R_IP = pop_word();
+}
 M.x86.R_SP += imm;
 DECODE_CLEAR_SEGOVR();
 END_OF_INSTR();
@@ -8503,7 +8507,11 @@ static void x86emuOp_ret_near(u8 X86EMU_UNUSED(op1))
 DECODE_PRINTF("RET\n");
RETURN_TRACE("RET",M.x86.saved_cs,M.x86.saved_ip);
TRACE_AND_STEP();
-M.x86.R_IP = pop_word();
+if (M.x86.mode & SYSMODE_PREFIX_DATA) {
+M.x86.R_EIP = pop_long();
+} else {
+M.x86.R_IP = pop_word();
+}
 DECODE_CLEAR_SEGOVR();
 END_OF_INSTR();
 }
@@ -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_BP -= 4;
+push_long(fetch_data_long_abs(M.x86.R_SS, M.x86.R_BP));
+} else {
+M.x86.R_BP -= 2;
+push_word(fetch_data_word_abs(M.x86.R_SS, M.x86.R_BP));
 }
-push_word(frame_pointer);
 }
+push_word(frame_pointer);
+}
 M.x86.R_BP = frame_pointer;
 M.x86.R_SP = (u16)(M.x86.R_SP - local);
 DECODE_CLEAR_SEGOVR();
@@ -8808,7 +8821,11 @@ static void x86emuOp_leave(u8 X86EMU_UNUSED(op1))
 DECODE_PRINTF("LEAVE\n");
 TRACE_AND_STEP();
 M.x86.R_SP = M.x86.R_BP;
-M.x86.R_BP = pop_word();
+if (M.x86.mode & SYSMODE_PREFIX_DATA) {
+M.x86.R_EBP = pop_long();
+} else {
+M.x86.R_BP = pop_word();
+}
 DECODE_CLEAR_SEGOVR();
 END_OF_INSTR();
 }
@@ -8827,8 +8844,13 @@ static void x86emuOp_ret_far_IMM(u8 X86EMU_UNUSED(op1))
 DECODE_PRINTF2("%x\n", imm);
RETURN_TRACE("RETF",M.x86.saved_cs,M.x86.saved_ip);
TRACE_AND_STEP();
-M.x86.R_IP = pop_word();
-M.x86.R_CS = pop_word();
+if (M.x86.mode & SYSMODE_PREFIX_DATA) {
+M.x86.R_EIP = pop_long();
+M.x86.R_CS = pop_long() & 0x;
+} else {
+M.x86.R_IP = pop_word();
+M.x86.R_CS = pop_word();
+}
 M.x86.R_SP += imm;
 DECODE_CLEAR_SEGOVR();
 END_OF_INSTR();
@@ -8844,8 +8866,13 @@ static void x86emuOp_ret_far(u8 X86EMU_UNUSED(op1))
 DECODE_PRINTF("RETF\n");
RETURN_TRACE("RETF",M.x86.saved_cs,M.x86.saved_ip);
TRACE_AND_STEP();
-M.x86.R_IP = pop_word();
-M.x86.R_CS = pop_word();
+if (M.x86.mode & SYSMODE_PREFIX_DATA) {
+M.x86.R_EIP = pop_long();
+M.x86.R_CS = pop_long() & 0x;
+} else {
+M.x86.R_IP = pop_word();
+M.x86.R_CS = pop_word();
+}
 DECODE_CLEAR_SEGOVR();
 END_OF_INSTR();
 }
@@ -8939,9 +8966,15 @@ static void x86emuOp_iret(u8 X86EMU_UNUSED(op1))
 
 TRACE_AND_STEP();
 
-M.x86.R_IP = pop_word();
-M.x86.R_CS = pop_word();
-M.x86.R_FLG = pop_word();
+if (M.x86.mode & SYSMODE_PREFIX_DATA) {
+M.x86.R_EIP = pop_long();
+M.x86.R_CS = pop_long() & 0x;
+M.x86.R_EFLG = (pop_long() & 0x257FD5) | (M.x86.R_EFLG & 0x1A);
+} else {
+M.x86.R_IP = pop_word();
+M.x86.R_CS = pop_word();
+M.x86.R_FLG = pop_word();
+}
 DECODE_CLEAR_SEGOVR();
 EN

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

2012-03-08 Thread Julian Pidancet 
On Wed, Mar 7, 2012 at 7:04 PM, Guillem Jover  wrote:
> On Wed, 2012-03-07 at 17:54:57 +, Julian Pidancet wrote:
>> 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.
>

There is no "descriptor" in real mode. Default stack size is always 16-bit.
Also, I think the right thing to do is to decrement BP instead of EBP
when SYSMODE_PREFIX_DATA is set:

M.x86.R_BP -= 4;
push_long(fetch_data_long_abs(M.x86.R_SS, M.x86.R_BP));

instead of:

M.x86.R_EBP -= 4;
push_long(fetch_data_long_abs(M.x86.R_SS, M.x86.R_BP));

...to remain exactly consistent with the manual.

>> 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. :)
>

They are not "32-bit instructions". The processor is still functioning
in real-mode, therefore, addressing still follows the rules of
real-mode addressing, as mentioned in Volume 1:Basic Architecture :

3.3.5 32-Bit and 16-Bit Address and Operand Sizes

[...]

When operating in real-address mode, the default addressing and
operand size is 16 bits. An address-size override can be used in
real-address mode to enable 32-bit addressing. However, the maximum
allowable 32-bit linear address is still 000FH (2^20-1).

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

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

2012-03-07 Thread Guillem Jover 
On Wed, 2012-03-07 at 17:54:57 +, Julian Pidancet wrote:
> On Wed, Mar 7, 2012 at 1:46 PM, Guillem Jover  wrote:
> > On Mon, 2012-03-05 at 17:49:08 +, 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

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

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

2012-03-07 Thread Guillem Jover 
Hi!

On Mon, 2012-03-05 at 17:49:08 +, 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))

?

> +} else {
> +M.x86.R_BP -= 2;
> +push_word(fetch_data_word_abs(M.x86.R_SS, M.x86.R_BP));
>  }
> -push_word(frame_pointer);
>  }
> +push_word(frame_pointer);
> +}
>  M.x86.R_BP = frame_pointer;
>  M.x86.R_SP = (u16)(M.x86.R_SP - local);
>  DECODE_CLEAR_SEGOVR();


> @@ -8827,8 +8844,13 @@ static void x86emuOp_ret_far_IMM(u8 X86EMU_UNUSED(op1))
>  DECODE_PRINTF2("%x\n", imm);
>   RETURN_TRACE("RETF",M.x86.saved_cs,M.x86.saved_ip);
>   TRACE_AND_STEP();
> -M.x86.R_IP = pop_word();
> -M.x86.R_CS = pop_word();
> +if (M.x86.mode & SYSMODE_PREFIX_DATA) {
> +M.x86.R_IP = pop_long();
> +M.x86.R_CS = pop_long() & 0x;
> +} else {
> +M.x86.R_IP = pop_word();
> +M.x86.R_CS = pop_word();
> +}
>  M.x86.R_SP += imm;
>  DECODE_CLEAR_SEGOVR();
>  END_OF_INSTR();
> @@ -8844,8 +8866,13 @@ static void x86emuOp_ret_far(u8 X86EMU_UNUSED(op1))
>  DECODE_PRINTF("RETF\n");
>   RETURN_TRACE("RETF",M.x86.saved_cs,M.x86.saved_ip);
>   TRACE_AND_STEP();
> -M.x86.R_IP = pop_word();
> -M.x86.R_CS = pop_word();
> +if (M.x86.mode & SYSMODE_PREFIX_DATA) {
> +M.x86.R_IP = pop_long();
> +M.x86.R_CS = pop_long() & 0x;
> +} else {
> +M.x86.R_IP = pop_word();
> +M.x86.R_CS = pop_word();
> +}
>  DECODE_CLEAR_SEGOVR();
>  END_OF_INSTR();
>  }
> @@ -8939,9 +8966,15 @@ static void x86emuOp_iret(u8 X86EMU_UNUSED(op1))
>  
>  TRACE_AND_STEP();
>  
> -M.x86.R_IP = pop_word();
> -M.x86.R_CS = pop_word();
> -M.x86.R_FLG = pop_word();
> +if (M.x86.mode & SYSMODE_PREFIX_DATA) {
> +M.x86.R_IP = pop_long();
> +M.x86.R_CS = pop_long() & 0x;
> +M.x86.R_FLG = pop_long();
> +} else {
> +M.x86.R_IP = pop_word();
> +M.x86.R_CS = pop_word();
> +M.x86.R_FLG = pop_word();
> +}
>  DECODE_CLEAR_SEGOVR();
>  END_OF_INSTR();
>  }

On these three hunks, when on 32-bit mode shouldn't the registers be
M.x86.R_EIP and M.x86.R_EFLG?

regards,
guillem

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

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

2012-03-07 Thread Julian Pidancet 
On Wed, Mar 7, 2012 at 7:04 PM, Guillem Jover  wrote:
> On Wed, 2012-03-07 at 17:54:57 +, Julian Pidancet wrote:
>> On Wed, Mar 7, 2012 at 1:46 PM, Guillem Jover  wrote:
>> > On Mon, 2012-03-05 at 17:49:08 +, 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
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

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

2012-03-07 Thread Julian Pidancet 
On Wed, Mar 7, 2012 at 1:46 PM, Guillem Jover  wrote:
> Hi!
>
> On Mon, 2012-03-05 at 17:49:08 +, 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. So
according to the manual, it should be BP, not EBP.
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.

>> +            } else {
>> +                M.x86.R_BP -= 2;
>> +                push_word(fetch_data_word_abs(M.x86.R_SS, M.x86.R_BP));
>>              }
>> -        push_word(frame_pointer);
>>          }
>> +        push_word(frame_pointer);
>> +    }
>>      M.x86.R_BP = frame_pointer;
>>      M.x86.R_SP = (u16)(M.x86.R_SP - local);
>>      DECODE_CLEAR_SEGOVR();
>
>
>> @@ -8827,8 +8844,13 @@ static void x86emuOp_ret_far_IMM(u8 
>> X86EMU_UNUSED(op1))
>>      DECODE_PRINTF2("%x\n", imm);
>>       RETURN_TRACE("RETF",M.x86.saved_cs,M.x86.saved_ip);
>>       TRACE_AND_STEP();
>> -    M.x86.R_IP = pop_word();
>> -    M.x86.R_CS = pop_word();
>> +    if (M.x86.mode & SYSMODE_PREFIX_DATA) {
>> +        M.x86.R_IP = pop_long();
>> +        M.x86.R_CS = pop_long() & 0x;
>> +    } else {
>> +        M.x86.R_IP = pop_word();
>> +        M.x86.R_CS = pop_word();
>> +    }
>>      M.x86.R_SP += imm;
>>      DECODE_CLEAR_SEGOVR();
>>      END_OF_INSTR();
>> @@ -8844,8 +8866,13 @@ static void x86emuOp_ret_far(u8 X86EMU_UNUSED(op1))
>>      DECODE_PRINTF("RETF\n");
>>       RETURN_TRACE("RETF",M.x86.saved_cs,M.x86.saved_ip);
>>       TRACE_AND_STEP();
>> -    M.x86.R_IP = pop_word();
>> -    M.x86.R_CS = pop_word();
>> +    if (M.x86.mode & SYSMODE_PREFIX_DATA) {
>> +        M.x86.R_IP = pop_long();
>> +        M.x86.R_CS = pop_long() & 0x;
>> +    } else {
>> +        M.x86.R_IP = pop_word();
>> +        M.x86.R_CS = pop_word();
>> +    }
>>      DECODE_CLEAR_SEGOVR();
>>      END_OF_INSTR();
>>  }
>> @@ -8939,9 +8966,15 @@ static void x86emuOp_iret(u8 X86EMU_UNUSED(op1))
>>
>>      TRACE_AND_STEP();
>>
>> -    M.x86.R_IP = pop_word();
>> -    M.x86.R_CS = pop_word();
>> -    M.x86.R_FLG = pop_word();
>> +    if (M.x86.mode & SYSMODE_PREFIX_DATA) {
>> +        M.x86.R_IP = pop_long();
>> +        M.x86.R_CS = pop_long() & 0x;
>> +        M.x86.R_FLG = pop_long();
>> +    } else {
>> +        M.x86.R_IP = pop_word();
>> +        M.x86.R_CS = pop_word();
>> +        M.x86.R_FLG = pop_word();
>> +    }
>>      DECODE_CLEAR_SEGOVR();
>>      END_OF_INSTR();
>>  }
>
> On these three hunks, when on 32-bit mode shouldn't the registers be
> M.x86.R_EIP and M.x86.R_EFLG?
>

You're absolutely right, and in addition, it seems that some of the
bits in EFLAGS must be preserved by the iret instruction, as described
in the manual:

[...]
REAL-ADDRESS-MODE;
IF OperandSize = 32
THEN
IF top 12 bytes of stack not within stack limits
THEN #SS; FI;
tempEIP <- 4 bytes at end of stack
IF tempEIP[31:16] is not zero THEN #GP(0); FI;
EIP <- Pop();
CS <- Pop(); (* 32-bit pop, high-order 16 bits di

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

2012-03-06 Thread Julian Pidancet 
On 03/05/12 15:51, Peter Stuge wrote:
> Julian Pidancet wrote:
>> Sorry for the noise. None of these emails got through to the
>> xorg-devel mailing list. Will retry later.
> 
> I would appreciate if you looked at the coreboot x86emu as well.
> 
> http://review.coreboot.org/gitweb?p=coreboot.git;a=tree;f=src/devices/oprom/x86emu
> 

After having a quick look at coreboot sources, it seems this version of
x86emu needs patching as well. Can you try this patch and see it applies
correctly on top of coreboot sources ?

I never used coreboot before, what does it uses x86emu for ?

-- 
Julian

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

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

2012-03-05 Thread Peter Stuge 
Julian Pidancet wrote:
> Sorry for the noise. None of these emails got through to the
> xorg-devel mailing list. Will retry later.

I would appreciate if you looked at the coreboot x86emu as well.

http://review.coreboot.org/gitweb?p=coreboot.git;a=tree;f=src/devices/oprom/x86emu


//Peter

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

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

2012-03-05 Thread Julian Pidancet 
Sorry for the noise. None of these emails got through to the
xorg-devel mailing list. Will retry later.

-- 
Julian

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

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

2012-03-05 Thread Julian Pidancet 
Some instructions are not emulated correctly by x86emu when they
are prefixed by the 0x66 opcode.
I've identified problems in the emulation of these intructions: ret,
enter, leave, iret and some forms of call.

Most of the time, the problem is that these instructions should push or
pop 32-bit values to/from the stack, instead of 16bit, when they are
prefixed by the 0x66 special opcode.

The SeaBIOS project aims to produce a complete legacy BIOS
implementation as well as a VGA option ROM, entirely written in C and
using the GCC compiler.

In 16bit code produced by the GCC compiler, the 0x66 prefix is used
almost everywhere. This patch is necessary to allow the SeaBIOS VGA
option ROM to function with Xorg when using the vesa driver.

Signed-off-by: Julian Pidancet 
---
 hw/xfree86/x86emu/ops.c |  194 ++
 1 files changed, 143 insertions(+), 51 deletions(-)

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
@@ -8487,7 +8487,11 @@ static void x86emuOp_ret_near_IMM(u8 X86EMU_UNUSED(op1))
 DECODE_PRINTF2("%x\n", imm);
RETURN_TRACE("RET",M.x86.saved_cs,M.x86.saved_ip);
TRACE_AND_STEP();
-M.x86.R_IP = pop_word();
+if (M.x86.mode & SYSMODE_PREFIX_DATA) {
+M.x86.R_EIP = pop_long();
+} else {
+M.x86.R_IP = pop_word();
+}
 M.x86.R_SP += imm;
 DECODE_CLEAR_SEGOVR();
 END_OF_INSTR();
@@ -8503,7 +8507,11 @@ static void x86emuOp_ret_near(u8 X86EMU_UNUSED(op1))
 DECODE_PRINTF("RET\n");
RETURN_TRACE("RET",M.x86.saved_cs,M.x86.saved_ip);
TRACE_AND_STEP();
-M.x86.R_IP = pop_word();
+if (M.x86.mode & SYSMODE_PREFIX_DATA) {
+M.x86.R_EIP = pop_long();
+} else {
+M.x86.R_IP = pop_word();
+}
 DECODE_CLEAR_SEGOVR();
 END_OF_INSTR();
 }
@@ -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));
+} else {
+M.x86.R_BP -= 2;
+push_word(fetch_data_word_abs(M.x86.R_SS, M.x86.R_BP));
 }
-push_word(frame_pointer);
 }
+push_word(frame_pointer);
+}
 M.x86.R_BP = frame_pointer;
 M.x86.R_SP = (u16)(M.x86.R_SP - local);
 DECODE_CLEAR_SEGOVR();
@@ -8808,7 +8821,11 @@ static void x86emuOp_leave(u8 X86EMU_UNUSED(op1))
 DECODE_PRINTF("LEAVE\n");
 TRACE_AND_STEP();
 M.x86.R_SP = M.x86.R_BP;
-M.x86.R_BP = pop_word();
+if (M.x86.mode & SYSMODE_PREFIX_DATA) {
+M.x86.R_EBP = pop_long();
+} else {
+M.x86.R_BP = pop_word();
+}
 DECODE_CLEAR_SEGOVR();
 END_OF_INSTR();
 }
@@ -8827,8 +8844,13 @@ static void x86emuOp_ret_far_IMM(u8 X86EMU_UNUSED(op1))
 DECODE_PRINTF2("%x\n", imm);
RETURN_TRACE("RETF",M.x86.saved_cs,M.x86.saved_ip);
TRACE_AND_STEP();
-M.x86.R_IP = pop_word();
-M.x86.R_CS = pop_word();
+if (M.x86.mode & SYSMODE_PREFIX_DATA) {
+M.x86.R_IP = pop_long();
+M.x86.R_CS = pop_long() & 0x;
+} else {
+M.x86.R_IP = pop_word();
+M.x86.R_CS = pop_word();
+}
 M.x86.R_SP += imm;
 DECODE_CLEAR_SEGOVR();
 END_OF_INSTR();
@@ -8844,8 +8866,13 @@ static void x86emuOp_ret_far(u8 X86EMU_UNUSED(op1))
 DECODE_PRINTF("RETF\n");
RETURN_TRACE("RETF",M.x86.saved_cs,M.x86.saved_ip);
TRACE_AND_STEP();
-M.x86.R_IP = pop_word();
-M.x86.R_CS = pop_word();
+if (M.x86.mode & SYSMODE_PREFIX_DATA) {
+M.x86.R_IP = pop_long();
+M.x86.R_CS = pop_long() & 0x;
+} else {
+M.x86.R_IP = pop_word();
+M.x86.R_CS = pop_word();
+}
 DECODE_CLEAR_SEGOVR();
 END_OF_INSTR();
 }
@@ -8939,9 +8966,15 @@ static void x86emuOp_iret(u8 X86EMU_UNUSED(op1))
 
 TRACE_AND_STEP();
 
-M.x86.R_IP = pop_word();
-M.x86.R_CS = pop_word();
-M.x86.R_FLG = pop_word();
+if (M.x86.mode & SYSMODE_PREFIX_DATA) {
+M.x86.R_IP = pop_long();
+M.x86.R_CS = pop_long() & 0x;
+M.x86.R_FLG = pop_long();
+} else {
+M.x86.R_IP = pop_word();
+M.x86.R_CS = pop_word();
+M.x86.R_FLG = pop_word();
+}
 DECODE_CLEAR_SEGOVR();
 END_OF_INSTR();
 }
@@ -11168,19 +11201,36 @@ static void x86emuOp_opcFF_word_RM(u8 
X86EMU_UNUSED(op1))
 }
 break;
 case 2: /* call word ptr ... */
-destval = fetch_data_word(destoffset);
-TRACE_AND_STEP();
-push_word(M.x86.R_IP);
-M.x86.R_IP = destval;
+if (M.x86.mode & SYSMODE_PREFIX