The size of 'struct bregs' is not evenly divisible by four and where the assembler placed a 'struct bregs' on the extra stack as part of entering into the C functions it caused the C functions to run with a non-aligned stack. It's technically not correct to use an unaligned stack and it is certainly less efficient.
This patch avoids using BREGS_size (the sizeof struct bregs) and instead introduces PUSHBREGS_size (the size of the general purpose registers in struct bregs) in the assembler. Where the code actually did use the %cs:%ip and flags, an extra 8 (instead of 6) bytes are added to maintain a sane alignment.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/asm-offsets.c | 1 - src/entryfuncs.S | 2 ++ src/romlayout.S | 24 ++++++++++++------------ vgasrc/vgaentry.S | 20 ++++++++++---------- 4 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/src/asm-offsets.c b/src/asm-offsets.c index 576bf34..b98f3b5 100644 --- a/src/asm-offsets.c +++ b/src/asm-offsets.c @@ -20,5 +20,4 @@ void foo(void) OFFSET(BREGS_edi, bregs, edi); OFFSET(BREGS_flags, bregs, flags); OFFSET(BREGS_code, bregs, code); - DEFINE(BREGS_size, sizeof(struct bregs)); } diff --git a/src/entryfuncs.S b/src/entryfuncs.S index cc1f433..7368bb6 100644 --- a/src/entryfuncs.S +++ b/src/entryfuncs.S @@ -9,6 +9,8 @@ * Macros for save and restore of 'struct bregs' registers ****************************************************************/
+#define PUSHBREGS_size 32 + // Save registers (matches struct bregs) to stack .macro PUSHBREGS pushl %eax diff --git a/src/romlayout.S b/src/romlayout.S index 7a5df0e..c1751e1 100644 --- a/src/romlayout.S +++ b/src/romlayout.S @@ -257,7 +257,7 @@ entry_pmm: movw %cx, %ds shll $4, %ecx movl $_cfunc32flat_handle_pmm, %eax // Setup: call32(handle_pmm, args, -1) - leal BREGS_size-6+12(%esp, %ecx), %edx // %edx points to start of args + leal PUSHBREGS_size+12(%esp, %ecx), %edx // %edx points to start of args movl $-1, %ecx calll call32 movw %ax, BREGS_eax(%esp) // Modify %ax:%dx to return %eax @@ -284,7 +284,7 @@ entry_pnp_real: PUSHBREGS movw %ss, %cx // Move %ss to %ds movw %cx, %ds - leal BREGS_size-6+12(%esp), %eax // %eax points to start of u16 args + leal PUSHBREGS_size+12(%esp), %eax // %eax points to start of u16 args calll handle_pnp movw %ax, BREGS_eax(%esp) // Modify %eax to return %ax POPBREGS @@ -433,11 +433,11 @@ irqentry_extrastack: movl $_zonelow_seg, %eax movl %eax, %ds movl StackPos, %eax - subl $BREGS_size+8, %eax + subl $PUSHBREGS_size+8, %eax SAVEBREGS_POP_DSEAX popl %ecx - movl %esp, BREGS_size(%eax) - movw %ss, BREGS_size+4(%eax) + movl %esp, PUSHBREGS_size(%eax) + movw %ss, PUSHBREGS_size+4(%eax)
movw %ds, %dx // Setup %ss/%esp and call function movw %dx, %ss @@ -445,8 +445,8 @@ irqentry_extrastack: calll *%ecx
movl %esp, %eax // Restore registers and return - movw BREGS_size+4(%eax), %ss - movl BREGS_size(%eax), %esp + movw PUSHBREGS_size+4(%eax), %ss + movl PUSHBREGS_size(%eax), %esp RESTOREBREGS_DSEAX iretw
@@ -460,11 +460,11 @@ irqentry_arg_extrastack: movl $_zonelow_seg, %eax movl %eax, %ds movl StackPos, %eax - subl $BREGS_size+8, %eax + subl $PUSHBREGS_size+16, %eax SAVEBREGS_POP_DSEAX // Save registers on extra stack popl %ecx - movl %esp, BREGS_size+0(%eax) - movw %ss, BREGS_size+4(%eax) + movl %esp, PUSHBREGS_size+8(%eax) + movw %ss, PUSHBREGS_size+12(%eax) popl BREGS_code(%eax) popw BREGS_flags(%eax)
@@ -474,8 +474,8 @@ irqentry_arg_extrastack: calll *%ecx
movl %esp, %eax // Restore registers and return - movw BREGS_size+4(%eax), %ss - movl BREGS_size+0(%eax), %esp + movw PUSHBREGS_size+12(%eax), %ss + movl PUSHBREGS_size+8(%eax), %esp popl %edx popw %dx pushw BREGS_flags(%eax) diff --git a/vgasrc/vgaentry.S b/vgasrc/vgaentry.S index c05502d..f9cf656 100644 --- a/vgasrc/vgaentry.S +++ b/vgasrc/vgaentry.S @@ -112,10 +112,10 @@ entry_10_extrastack: pushw %ds // Set %ds:%eax to space on ExtraStack pushl %eax movw %cs:ExtraStackSeg, %ds - movl $(CONFIG_VGA_EXTRA_STACK_SIZE-BREGS_size-8), %eax + movl $(CONFIG_VGA_EXTRA_STACK_SIZE-PUSHBREGS_size-16), %eax SAVEBREGS_POP_DSEAX // Save registers on extra stack - movl %esp, BREGS_size+0(%eax) - movw %ss, BREGS_size+4(%eax) + movl %esp, PUSHBREGS_size+8(%eax) + movw %ss, PUSHBREGS_size+12(%eax) popl BREGS_code(%eax) popw BREGS_flags(%eax)
@@ -125,8 +125,8 @@ entry_10_extrastack: VGA_CALLL handle_10
movl %esp, %eax // Restore registers and return - movw BREGS_size+4(%eax), %ss - movl BREGS_size+0(%eax), %esp + movw PUSHBREGS_size+12(%eax), %ss + movl PUSHBREGS_size+8(%eax), %esp popl %edx popw %dx pushw BREGS_flags(%eax) @@ -148,10 +148,10 @@ entry_timer_hook_extrastack: pushw %ds // Set %ds:%eax to space on ExtraStack pushl %eax movw %cs:ExtraStackSeg, %ds - movl $(CONFIG_VGA_EXTRA_STACK_SIZE-BREGS_size-8), %eax + movl $(CONFIG_VGA_EXTRA_STACK_SIZE-PUSHBREGS_size-8), %eax SAVEBREGS_POP_DSEAX - movl %esp, BREGS_size(%eax) - movw %ss, BREGS_size+4(%eax) + movl %esp, PUSHBREGS_size(%eax) + movw %ss, PUSHBREGS_size+4(%eax)
movw %ds, %dx // Setup %ss/%esp and call function movw %dx, %ss @@ -159,7 +159,7 @@ entry_timer_hook_extrastack: calll handle_timer_hook
movl %esp, %eax // Restore registers and return - movw BREGS_size+4(%eax), %ss - movl BREGS_size(%eax), %esp + movw PUSHBREGS_size+4(%eax), %ss + movl PUSHBREGS_size(%eax), %esp RESTOREBREGS_DSEAX ljmpw *%cs:Timer_Hook_Resume
On Wed, Nov 05, 2014 at 09:23:12AM -0500, Kevin O'Connor wrote:
The size of 'struct bregs' is not evenly divisible by four and where the assembler placed a 'struct bregs' on the extra stack as part of entering into the C functions it caused the C functions to run with a non-aligned stack. It's technically not correct to use an unaligned stack and it is certainly less efficient.
This patch avoids using BREGS_size (the sizeof struct bregs) and instead introduces PUSHBREGS_size (the size of the general purpose registers in struct bregs) in the assembler. Where the code actually did use the %cs:%ip and flags, an extra 8 (instead of 6) bytes are added to maintain a sane alignment.
FYI, I have pushed this patch to the main seabios repo.
-Kevin
-
Kevin O'Connor