[SeaBIOS] [PATCH] Fix winxp boot regression introduced in ecdc655a.

Fred . eldmannen at gmail.com
Mon Jun 4 08:43:31 CEST 2012


Maybe it should be documented to prevent this bug from being
re-introduced in the future.

Maybe a comment in the code so developers don't get confused.

On Sun, Jun 3, 2012 at 2:40 AM, Kevin O'Connor <kevin at koconnor.net> wrote:
> The winxp boot loader does something curious - it sets an int 0x1c
> handler, records the stack location, and then spins in place with irqs
> enabled.  The 0x1c handler alters the memory just past the stack
> pointer so that when the timer irq returns the code jumps to a new
> location and stop spinning.  The winxp code relies on the fact that a
> hw irq will always place 6 bytes at a specific location and that it
> can alter those bytes.
>
> The ecdc655a patch does a full backup/restore of the register state.
> Unfortunately, the restore overwrites the changes made by the winxp
> 0x1c handler.
>
> This patch reverts much of ecdc655a.  Hardware irqs are still handled
> on the extra stack, but only the essential register state is backed up
> and restored.
>
> Also, stack_hop_back is changed to only use %sp when changing states -
> this enables the entry code to store just %esp instead of both %esp
> and %sp.
>
> Signed-off-by: Kevin O'Connor <kevin at koconnor.net>
> ---
>  src/clock.c     |    8 ++++----
>  src/disk.c      |    4 ++--
>  src/floppy.c    |    4 ++--
>  src/misc.c      |    8 ++++----
>  src/output.c    |    9 +++++++++
>  src/ps2port.c   |    8 ++++----
>  src/romlayout.S |   53 ++++++++++++++++++++---------------------------------
>  src/stacks.c    |    2 +-
>  src/util.h      |    5 +++++
>  9 files changed, 51 insertions(+), 50 deletions(-)
>
> diff --git a/src/clock.c b/src/clock.c
> index 55dde2e..69e9f17 100644
> --- a/src/clock.c
> +++ b/src/clock.c
> @@ -516,9 +516,9 @@ handle_1a(struct bregs *regs)
>
>  // INT 08h System Timer ISR Entry Point
>  void VISIBLE16
> -handle_08(struct bregs *regs)
> +handle_08(void)
>  {
> -    debug_enter(regs, DEBUG_ISR_08);
> +    debug_isr(DEBUG_ISR_08);
>
>     floppy_tick();
>
> @@ -659,9 +659,9 @@ handle_1583(struct bregs *regs)
>
>  // int70h: IRQ8 - CMOS RTC
>  void VISIBLE16
> -handle_70(struct bregs *regs)
> +handle_70(void)
>  {
> -    debug_enter(regs, DEBUG_ISR_70);
> +    debug_isr(DEBUG_ISR_70);
>
>     // Check which modes are enabled and have occurred.
>     u8 registerB = inb_cmos(CMOS_STATUS_B);
> diff --git a/src/disk.c b/src/disk.c
> index 3ca5697..8eff464 100644
> --- a/src/disk.c
> +++ b/src/disk.c
> @@ -885,9 +885,9 @@ handle_13(struct bregs *regs)
>
>  // record completion in BIOS task complete flag
>  void VISIBLE16
> -handle_76(struct bregs *regs)
> +handle_76(void)
>  {
> -    debug_enter(regs, DEBUG_ISR_76);
> +    debug_isr(DEBUG_ISR_76);
>     SET_BDA(disk_interrupt_flag, 0xff);
>     eoi_pic2();
>  }
> diff --git a/src/floppy.c b/src/floppy.c
> index 5400bb0..72bc79b 100644
> --- a/src/floppy.c
> +++ b/src/floppy.c
> @@ -582,9 +582,9 @@ process_floppy_op(struct disk_op_s *op)
>
>  // INT 0Eh Diskette Hardware ISR Entry Point
>  void VISIBLE16
> -handle_0e(struct bregs *regs)
> +handle_0e(void)
>  {
> -    debug_enter(regs, DEBUG_ISR_0e);
> +    debug_isr(DEBUG_ISR_0e);
>     if (! CONFIG_FLOPPY)
>         goto done;
>
> diff --git a/src/misc.c b/src/misc.c
> index d0d6665..b948778 100644
> --- a/src/misc.c
> +++ b/src/misc.c
> @@ -55,9 +55,9 @@ handle_10(struct bregs *regs)
>
>  // NMI handler
>  void VISIBLE16
> -handle_02(struct bregs *regs)
> +handle_02(void)
>  {
> -    debug_enter(regs, DEBUG_ISR_02);
> +    debug_isr(DEBUG_ISR_02);
>  }
>
>  void
> @@ -71,9 +71,9 @@ mathcp_setup(void)
>
>  // INT 75 - IRQ13 - MATH COPROCESSOR EXCEPTION
>  void VISIBLE16
> -handle_75(struct bregs *regs)
> +handle_75(void)
>  {
> -    debug_enter(regs, DEBUG_ISR_75);
> +    debug_isr(DEBUG_ISR_75);
>
>     // clear irq13
>     outb(0, PORT_MATH_CLEAR);
> diff --git a/src/output.c b/src/output.c
> index 1fe5d91..37c4942 100644
> --- a/src/output.c
> +++ b/src/output.c
> @@ -487,6 +487,15 @@ dump_regs(struct bregs *regs)
>             , regs->code.seg, regs->code.offset, regs->flags);
>  }
>
> +// Report entry to an Interrupt Service Routine (ISR).
> +void
> +__debug_isr(const char *fname)
> +{
> +    puts_cs(&debuginfo, fname);
> +    putc(&debuginfo, '\n');
> +    debug_serial_flush();
> +}
> +
>  // Function called on handler startup.
>  void
>  __debug_enter(struct bregs *regs, const char *fname)
> diff --git a/src/ps2port.c b/src/ps2port.c
> index 601e40d..d4626d6 100644
> --- a/src/ps2port.c
> +++ b/src/ps2port.c
> @@ -356,12 +356,12 @@ ps2_mouse_command(int command, u8 *param)
>
>  // INT74h : PS/2 mouse hardware interrupt
>  void VISIBLE16
> -handle_74(struct bregs *regs)
> +handle_74(void)
>  {
>     if (! CONFIG_PS2PORT)
>         return;
>
> -    debug_enter(regs, DEBUG_ISR_74);
> +    debug_isr(DEBUG_ISR_74);
>
>     u8 v = inb(PORT_PS2_STATUS);
>     if ((v & (I8042_STR_OBF|I8042_STR_AUXDATA))
> @@ -383,12 +383,12 @@ done:
>
>  // INT09h : Keyboard Hardware Service Entry Point
>  void VISIBLE16
> -handle_09(struct bregs *regs)
> +handle_09(void)
>  {
>     if (! CONFIG_PS2PORT)
>         return;
>
> -    debug_enter(regs, DEBUG_ISR_09);
> +    debug_isr(DEBUG_ISR_09);
>
>     // read key from keyboard controller
>     u8 v = inb(PORT_PS2_STATUS);
> diff --git a/src/romlayout.S b/src/romlayout.S
> index aadc9cf..8125277 100644
> --- a/src/romlayout.S
> +++ b/src/romlayout.S
> @@ -394,48 +394,35 @@ entry_elf:
>  irqentry_extrastack:
>         cli
>         cld
> -        pushw %ds
> +        pushw %ds               // Set %ds:%eax to space on ExtraStack
>         pushl %eax
>         movl $_datalow_seg, %eax
>         movl %eax, %ds
>         movl StackPos, %eax
> -        subl $BREGS_size+12, %eax
> -        popl BREGS_eax(%eax)
> -        popw BREGS_ds(%eax)
> -        movl %edi, BREGS_edi(%eax)
> -        movl %esi, BREGS_esi(%eax)
> -        movl %ebp, BREGS_ebp(%eax)
> -        movl %ebx, BREGS_ebx(%eax)
> -        movl %edx, BREGS_edx(%eax)
> -        movl %ecx, BREGS_ecx(%eax)
> -        movw %es, BREGS_es(%eax)
> +        subl $24, %eax
> +        popl 0(%eax)            // Backup %eax, %ds, %es, %ecx, %edx
> +        popw 4(%eax)
> +        movw %es, 6(%eax)
> +        movl %ecx, 8(%eax)
>         popl %ecx
> -        popl BREGS_code(%eax)
> -        popw BREGS_flags(%eax)
> -
> -        movw %ss, BREGS_size+8(%eax)
> -        movzwl %sp, %edx
> -        movl %edx, BREGS_size+4(%eax)
> -        movl %esp, BREGS_size+0(%eax)
> -        movw %ds, %dx
> +        movl %edx, 12(%eax)
> +        movl %esp, 16(%eax)
> +        movzwl %sp, %esp
> +        movw %ss, 20(%eax)
> +
> +        movw %ds, %dx           // Setup %ss/%esp and call function
>         movw %dx, %ss
>         movl %eax, %esp
>         calll *%ecx
>
> -        movl %esp, %eax
> -        movw BREGS_size+8(%eax), %ss
> -        movl BREGS_size+0(%eax), %esp
> -        movl BREGS_edi(%eax), %edi
> -        movl BREGS_esi(%eax), %esi
> -        movl BREGS_ebp(%eax), %ebp
> -        movl BREGS_ebx(%eax), %ebx
> -        movl BREGS_edx(%eax), %edx
> -        movl BREGS_ecx(%eax), %ecx
> -        movw BREGS_es(%eax), %es
> -        pushw BREGS_flags(%eax)
> -        pushl BREGS_code(%eax)
> -        pushw BREGS_ds(%eax)
> -        pushl BREGS_eax(%eax)
> +        movl %esp, %eax         // Restore registers and return
> +        movw 20(%eax), %ss
> +        movl 16(%eax), %esp
> +        movl 12(%eax), %edx
> +        movl 8(%eax), %ecx
> +        movw 6(%eax), %es
> +        pushw 4(%eax)
> +        pushl 0(%eax)
>         popl %eax
>         popw %ds
>         iretw
> diff --git a/src/stacks.c b/src/stacks.c
> index 2804e47..9381729 100644
> --- a/src/stacks.c
> +++ b/src/stacks.c
> @@ -74,7 +74,7 @@ stack_hop_back(u32 eax, u32 edx, void *func)
>         // Restore original callers' %ss/%esp
>         "movl -4(%4), %5\n"
>         "movl %5, %%ss\n"
> -        "movl %%ds:-8(%4), %%esp\n"
> +        "movw %%ds:-8(%4), %%sp\n"
>         "movl %5, %%ds\n"
>         // Call func
>         "calll *%2\n"
> diff --git a/src/util.h b/src/util.h
> index 39350cc..ba39678 100644
> --- a/src/util.h
> +++ b/src/util.h
> @@ -243,6 +243,7 @@ char * znprintf(size_t size, const char *fmt, ...)
>  void __dprintf(const char *fmt, ...)
>     __attribute__ ((format (printf, 1, 2)));
>  void __debug_enter(struct bregs *regs, const char *fname);
> +void __debug_isr(const char *fname);
>  void __debug_stub(struct bregs *regs, int lineno, const char *fname);
>  void __warn_invalid(struct bregs *regs, int lineno, const char *fname);
>  void __warn_unimplemented(struct bregs *regs, int lineno, const char *fname);
> @@ -264,6 +265,10 @@ void hexdump(const void *d, int len);
>         if ((lvl) && (lvl) <= CONFIG_DEBUG_LEVEL)       \
>             __debug_enter((regs), __func__);            \
>     } while (0)
> +#define debug_isr(lvl) do {                             \
> +        if ((lvl) && (lvl) <= CONFIG_DEBUG_LEVEL)       \
> +            __debug_isr(__func__);                      \
> +    } while (0)
>  #define debug_stub(regs)                        \
>     __debug_stub((regs), __LINE__, __func__)
>  #define warn_invalid(regs)                      \
> --
> 1.7.6.5
>
>
> _______________________________________________
> SeaBIOS mailing list
> SeaBIOS at seabios.org
> http://www.seabios.org/mailman/listinfo/seabios



More information about the SeaBIOS mailing list