On Thu, Dec 04, 2003 at 08:35:02PM -0500, Sergey Babkin wrote:
>Hi,
>
>I've been running XFree86 4.3.0.1 on a machine with a particularly
>weird videocard and I've got the VESA driver craching. If anyone
>wonders, the BIOS data in the log is:
>
>(II) VESA(0): VESA BIOS detected
>(II) VESA(0): VESA VBE Version 3.0
>(II) VESA(0): VESA VBE Total Mem: 1024 kB
>(II) VESA(0): VESA VBE OEM: Intel(R) 810, Intel(R) 815 Chipset Video BIOS
>(II) VESA(0): VESA VBE OEM Software Rev: 37.16
>(II) VESA(0): VESA VBE OEM Vendor: Intel Corporation
>(II) VESA(0): VESA VBE OEM Product: Intel(R) 810, Intel(R) 815 Chipset
>(II) VESA(0): VESA VBE OEM Product Rev: Hardware Version 0.0
>
>The machine is a retail box with embedded LCD touch-screen.
>
>A little investigation has shown that it crashes in
>x86emuOp_mov_word_SR_RM() (in programs/Xserver/hw/xfree86/int10/ops.c
>which is symlinked from extras/x86emu/src/x86emu) on the underlined line:
>
> case 3: /* register to register */
> destreg = decode_rm_seg_register(rh);
> DECODE_PRINTF(",");
> srcreg = DECODE_RM_WORD_REGISTER(rl);
> DECODE_PRINTF("\n");
> TRACE_AND_STEP();
> *destreg = *srcreg;
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> break;
>
>This happens because destreg is 0, because it's returned that way
>from decode_rm_seg_register(). rh is 4, and that's the value that
>decode_rm_seg_register() in decode.c (also linked from extras) does
>not understand. I've looked it up in the manual and actually
>the value 4 is for FS and value 5 is for GS. So here is the
>patch (also attached, to avoid corruption by the mailer):
>
>------------------ cut here ---------------
>*** decode.c.0 Thu Dec 4 15:42:51 2003
>--- decode.c Thu Dec 4 17:15:29 2003
>***************
>*** 699,705 ****
>--- 699,709 ----
> DECODE_PRINTF("DS");
> return &M.x86.R_DS;
> case 4:
>+ DECODE_PRINTF("FS");
>+ return &M.x86.R_FS;
> case 5:
>+ DECODE_PRINTF("GS");
>+ return &M.x86.R_GS;
> case 6:
> case 7:
> DECODE_PRINTF("ILLEGAL SEGREG");
>------------------ cut here ---------------
>
>This patch made the X server to work with this card.
>Could someone please check it in? I've looked in CVS and the most
>recent version still has this bug in it.
I'll commit this now -- thanks.
>There is also a more general question: in case when an instruction opcode
>can't be decoded, many routines in decode.c rely on just calling
>HALT_SYS() for error handling. However HALT_SYS() expands to
>X86EMU_halt_sys() which does nothing but sets a flag. The decode
>routines return 0 instead of all kinds of pointers to their caller
>which would immediately try to reference that pointer and crash.
>That means that any misformatted routine met in BIOS will make the
>X server to crash. I think that it's not good. I think that in case
>of a bad opcode an error message has to be printed into the log,
>so that the user would have some clue, and then the decoders must
>return not NULL pointer but a valid pointer to some trash value,
>so that their caller would be able to reference it without crashing
>before the interpreter has a chance to halt.
Yes, that is a problem. The INTR_HALTED flag isn't tested until too
late. The potential problem of returning a trash value is that could
cause bad things too. Maybe the callers of functions that can return
NULL need to check for that, and abort the instruction? Then INTR_HALTED
will be noticed before moving on to the next instruction.
>If I make a patch to fix it as described, would anyone be willing to
>review and commit it? (BTW, any pointers to how to print an error
>message properly from the bowels of the i86 interpreters would be
>appreciated too).
Yes.
Use "printk". It should get supplied by whatever uses the emulator.
For the XFree86 server it provided as a function that calls VErrorF(),
the generic logging function.
David
--
David Dawes
developer/release engineer The XFree86 Project
www.XFree86.org/~dawes
_______________________________________________
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel
