Instead of allocating the SeaVGABIOS extra stack via a PCIv3 style PMM call, use the EBDA to store the stack. The stack is placed at the end of the first 1KiB of the EBDA. Normally this EBDA space is reserved for the main BIOS, but SeaBIOS doesn't use the space.
This works around an issue with 16bit code running on Windows Vista (and likely later versions of Windows). The Windows 16bit emulator can not handle a stack in the e-segment. Using the EBDA, and thus relocating the stack to the 9-segment, works around the issue.
Reported-by: Richard Laager rlaager@wiktel.com Signed-off-by: Kevin O'Connor kevin@koconnor.net ---
There were several possible ways to resolve the main issue. I'm leaning torwards the "use the EBDA for the stack" approach, and the patch below implements that.
-Kevin
--- vgasrc/Kconfig | 18 ++++++++---------- vgasrc/vgaentry.S | 20 +++++++++++++------- vgasrc/vgainit.c | 56 +++++++------------------------------------------------ 3 files changed, 28 insertions(+), 66 deletions(-)
diff --git a/vgasrc/Kconfig b/vgasrc/Kconfig index 400e8da..bc1ea48 100644 --- a/vgasrc/Kconfig +++ b/vgasrc/Kconfig @@ -90,19 +90,17 @@ menu "VGA ROM" Support emulating text mode features when only a framebuffer is available.
- config VGA_ALLOCATE_EXTRA_STACK + config VGA_EXTRA_STACK depends on BUILD_VGABIOS - bool "Allocate an internal stack for 16bit interrupt entry point" + bool "Use an extra stack located in the EBDA" default y help - Attempt to allocate (via BIOS PMM call) an internal stack - for the legacy 16bit 0x10 interrupt entry point. This - reduces the amount of space on the caller's stack that - SeaVGABIOS uses. - - config VGA_EXTRA_STACK_SIZE - int - default 512 + Use space at the end of the first 1KiB of the EBDA for an + internal stack when the legacy 16bit 0x10 interrupt entry + point is called. This reduces the amount of space on the + caller's stack that SeaVGABIOS uses. Normally the first + 1KiB of the EBDA is reserved for the main BIOS, but + SeaBIOS does not use this space.
config VGA_VBE depends on BUILD_VGABIOS diff --git a/vgasrc/vgaentry.S b/vgasrc/vgaentry.S index f9cf656..2e4b200 100644 --- a/vgasrc/vgaentry.S +++ b/vgasrc/vgaentry.S @@ -104,15 +104,19 @@ entry_10: ENTRY_ARG_VGA handle_10 iretw
+#define BDA_ebda_seg 0x0e + // Entry point using extra stack DECLFUNC entry_10_extrastack entry_10_extrastack: cli cld - pushw %ds // Set %ds:%eax to space on ExtraStack + pushw %ds // Set %ds:%eax to end of first 1K of EBDA pushl %eax - movw %cs:ExtraStackSeg, %ds - movl $(CONFIG_VGA_EXTRA_STACK_SIZE-PUSHBREGS_size-16), %eax + movw $SEG_BDA, %ax + movw %ax, %ds + movw BDA_ebda_seg, %ds + movl $(1024-PUSHBREGS_size-16), %eax SAVEBREGS_POP_DSEAX // Save registers on extra stack movl %esp, PUSHBREGS_size+8(%eax) movw %ss, PUSHBREGS_size+12(%eax) @@ -145,11 +149,13 @@ entry_timer_hook: entry_timer_hook_extrastack: cli cld - pushw %ds // Set %ds:%eax to space on ExtraStack + pushw %ds // Set %ds:%eax to end of first 1K of EBDA pushl %eax - movw %cs:ExtraStackSeg, %ds - movl $(CONFIG_VGA_EXTRA_STACK_SIZE-PUSHBREGS_size-8), %eax - SAVEBREGS_POP_DSEAX + movw $SEG_BDA, %ax + movw %ax, %ds + movw BDA_ebda_seg, %ds + movl $(1024-PUSHBREGS_size-8), %eax + SAVEBREGS_POP_DSEAX // Save registers on extra stack movl %esp, PUSHBREGS_size(%eax) movw %ss, PUSHBREGS_size+4(%eax)
diff --git a/vgasrc/vgainit.c b/vgasrc/vgainit.c index 8d12261..78d5556 100644 --- a/vgasrc/vgainit.c +++ b/vgasrc/vgainit.c @@ -43,51 +43,6 @@ struct pci_data rom_pci_data VAR16 VISIBLE16 = {
/**************************************************************** - * PMM call and extra stack setup - ****************************************************************/ - -u16 ExtraStackSeg VAR16 VISIBLE16; - -static void -allocate_extra_stack(void) -{ - if (!CONFIG_VGA_ALLOCATE_EXTRA_STACK) - return; - u32 pmmscan; - for (pmmscan=0; pmmscan < BUILD_BIOS_SIZE; pmmscan+=16) { - struct pmmheader *pmm = (void*)pmmscan; - if (GET_FARVAR(SEG_BIOS, pmm->signature) != PMM_SIGNATURE) - continue; - if (checksum_far(SEG_BIOS, pmm, GET_FARVAR(SEG_BIOS, pmm->length))) - continue; - struct segoff_s entry = GET_FARVAR(SEG_BIOS, pmm->entry); - dprintf(1, "Attempting to allocate VGA stack via pmm call to %04x:%04x\n" - , entry.seg, entry.offset); - u16 res1, res2; - asm volatile( - "pushl %0\n" - "pushw $(8|1)\n" // Permanent low memory request - "pushl $0xffffffff\n" // Anonymous handle - "pushl $" __stringify(CONFIG_VGA_EXTRA_STACK_SIZE/16) "\n" - "pushw $0x00\n" // PMM allocation request - "lcallw *12(%%esp)\n" - "addl $16, %%esp\n" - "cli\n" - "cld\n" - : "+r" (entry.segoff), "=a" (res1), "=d" (res2) : : "cc", "memory"); - u32 res = res1 | (res2 << 16); - if (!res || res == PMM_FUNCTION_NOT_SUPPORTED) - return; - dprintf(1, "VGA stack allocated at %x\n", res); - SET_VGA(ExtraStackSeg, res >> 4); - extern void entry_10_extrastack(void); - SET_IVT(0x10, SEGOFF(get_global_seg(), (u32)entry_10_extrastack)); - return; - } -} - - -/**************************************************************** * Timer hook ****************************************************************/
@@ -110,7 +65,7 @@ hook_timer_irq(void) extern void entry_timer_hook_extrastack(void); struct segoff_s oldirq = GET_IVT(0x08); struct segoff_s newirq = SEGOFF(get_global_seg(), (u32)entry_timer_hook); - if (CONFIG_VGA_ALLOCATE_EXTRA_STACK && GET_GLOBAL(ExtraStackSeg)) + if (CONFIG_VGA_EXTRA_STACK) newirq = SEGOFF(get_global_seg(), (u32)entry_timer_hook_extrastack); dprintf(1, "Hooking hardware timer irq (old=%x new=%x)\n" , oldirq.segoff, newirq.segoff); @@ -177,10 +132,13 @@ vga_post(struct bregs *regs) if (CONFIG_VGA_STDVGA_PORTS) stdvga_build_video_param();
+ // Setup int10 entry point extern void entry_10(void); - SET_IVT(0x10, SEGOFF(get_global_seg(), (u32)entry_10)); - - allocate_extra_stack(); + extern void entry_10_extrastack(void); + struct segoff_s newirq = SEGOFF(get_global_seg(), (u32)entry_10); + if (CONFIG_VGA_EXTRA_STACK) + newirq = SEGOFF(get_global_seg(), (u32)entry_10_extrastack); + SET_IVT(0x10, newirq);
hook_timer_irq();
On Mon, Jan 19, 2015 at 01:12:24PM -0500, Kevin O'Connor wrote:
Instead of allocating the SeaVGABIOS extra stack via a PCIv3 style PMM call, use the EBDA to store the stack. The stack is placed at the end of the first 1KiB of the EBDA. Normally this EBDA space is reserved for the main BIOS, but SeaBIOS doesn't use the space.
This works around an issue with 16bit code running on Windows Vista (and likely later versions of Windows). The Windows 16bit emulator can not handle a stack in the e-segment. Using the EBDA, and thus relocating the stack to the 9-segment, works around the issue.
I ran some tests as part of committing this change and found that this patch regresses an old Digital Research "Concurrent DOS" image I was sent some time back. :-(
I do think that 16bit support on recent Windows is a higher priority than "Concurrent DOS", but I am leery of making a change like this just before we tag the next release.
Richard - it sounds like you have a short term solution in place, so I'm inclined to make the next SeaBIOS release without this change. The 16bit support in Windows will, of course, need to be fixed. Maybe a better solution can be found for the following release though.
Thoughts? -Kevin
On Sat, 2015-01-31 at 18:59 -0500, Kevin O'Connor wrote:
Richard - it sounds like you have a short term solution in place, so I'm inclined to make the next SeaBIOS release without this change.
Yeah, I'm good for now. I'd love to see a proper fix in place eventually, of course.
The 16bit support in Windows will, of course, need to be fixed. Maybe a better solution can be found for the following release though.
What about your option 6 patch? That fixed the issue for me and fixed Skifree. You had some concerns about KVM, but I believe Paolo said those weren't an issue. (To be fair, I'm didn't follow this closely, as this BIOS-level stuff is all new to me.)
On Mon, Feb 02, 2015 at 05:01:14PM -0600, Richard Laager wrote:
On Sat, 2015-01-31 at 18:59 -0500, Kevin O'Connor wrote:
Richard - it sounds like you have a short term solution in place, so I'm inclined to make the next SeaBIOS release without this change.
Yeah, I'm good for now. I'd love to see a proper fix in place eventually, of course.
The 16bit support in Windows will, of course, need to be fixed. Maybe a better solution can be found for the following release though.
What about your option 6 patch? That fixed the issue for me and fixed Skifree. You had some concerns about KVM, but I believe Paolo said those weren't an issue. (To be fair, I'm didn't follow this closely, as this BIOS-level stuff is all new to me.)
That's also a possibility. Unfortunately, it's unclear if that approach breaks anything else. Being in protected mode is a symptom of a more modern OS, but it's still possible an older application (that uses a very small stack) is run while in protected mode (eg, with emm386).
-Kevin
-
Kevin O'Connor -
Richard Laager