[SeaBIOS] [PATCH] x86emu: Correctly handle 0x66 prefix for some instructions
Julian Pidancet
julian.pidancet at gmail.com
Wed Mar 7 18:54:57 CET 2012
On Wed, Mar 7, 2012 at 1:46 PM, Guillem Jover <guillem at hadrons.org> wrote:
> Hi!
>
> 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. 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() & 0xffff;
>> + } 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() & 0xffff;
>> + } 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() & 0xffff;
>> + 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 discarded *)
tempEFLAGS <- Pop();
EFLAGS <- (tempEFLAGS AND 257FD5H) OR (EFLAGS AND 1A0000H);
ELSE (* OperandSize = 16 *)
IF top 6 bytes of stack are not within stack limits
THEN #SS; FI;
EIP <- Pop(); (* 16-bit pop; clear upper 16 bits *)
CS <- Pop(); (* 16-bit pop *)
EFLAGS[15:0] <- Pop();
FI;
END;
[...]
I will respin a patch shortly.
--
Julian
More information about the SeaBIOS
mailing list