Fix Stage1 StackTop computation for Ia32, and align stack to 16 bytes

Adding the following bit of debug code to DetectUsedStackBottom() in
BootloaderCommonPkg/Library/BootloaderCommonLib/BootloaderCommonLib.c:

@@ -290,7 +290,10 @@ DetectUsedStackBottom (
 {
   UINT32  *StackBot;

+  DEBUG ((DEBUG_INFO, "StackTop is %x\n", StackTop));
   StackBot = (UINT32 *) ((StackTop - StackSize) & ~ (sizeof (UINTN) - 1));
+  DEBUG ((DEBUG_INFO, "StackBot is %p %x\n", StackBot, *StackBot));
   ASSERT (*StackBot == STACK_DEBUG_FILL_PATTERN);

   while ((UINT32)(UINTN)StackBot < StackTop) {

shows this for the Stage1 stack on qemu X64, as expected:

	StackTop is 2000
	StackBot is 0 5AA55AA5

but it shows this on qemu Ia32, which appears to be incorrect:

	StackTop is 2004
	StackBot is 4 5AA55AA5

This Stage1 StackTop mismatch on Ia32 seems to be caused by the setup
code in BootloaderCorePkg/Stage1A/Ia32/SecEntry.nasm pushing only a single
32-bit word onto the stack for the 'Status' field of STAGE1A_ASM_PARAM,
while that field is actually a 64-bit field, which causes this line in
BootloaderCorePkg/Stage1A/Stage1A.c to compute a stack top address that
is off by 4 bytes:

  StackTop = (UINT32)(UINTN)Params + sizeof (STAGE1A_ASM_PARAM);

This patch makes the Ia32 Stage1A setup code push an extra 32-bit
word onto the stack before calling SecStartup, which fixes the Stage1
StackTop computation.

While we are at it, let's push another dummy word onto the stack in
the Stage1A setup code to make the Stage1A stack be 16-byte aligned,
like what is already the case for X64, so that we follow Version 1.0
of the System V Intel386 ABI supplement, and satisfy any expectations
our compiler may have regarding stack alignment.

Also add a comment to both the Ia32 and X64 Stage1A setup code to
remind the reader that the structure we build on the stack before
calling SecStartup has to match the layout of STAGE1A_ASM_PARAM.

Signed-off-by: Lennert Buytenhek <buytenh@arista.com>
This commit is contained in:
Lennert Buytenhek 2021-11-19 05:44:44 +02:00 committed by Maurice Ma
parent 5b51c32146
commit 644b8474da
2 changed files with 11 additions and 2 deletions

View File

@ -95,14 +95,20 @@ CheckStackRangeDone:
bts ebx, 0 ; Set BIT0 in Status
CheckStatusDone:
;
; Setup HOB
push ebx ; Status
; This structure has to match the layout of STAGE1A_ASM_PARAM
;
push $0 ; Status[63:32]
push ebx ; Status[31:0]
push edi ; TimeStamp[0] [63:32]
push esi ; TimeStamp[0] [31:0]
push edx ; CarTop
push ecx ; CarBase
push $0 ; Keep the stack 16-byte aligned
push esp
lea ecx, [esp + 4]
push ecx
call ASM_PFX(SecStartup) ; Jump to C code
jmp $

View File

@ -82,7 +82,10 @@ CheckStackRangeDone:
bts rbx, 0 ; Set BIT0 in Status
CheckStatusDone:
;
; Setup HOB
; This structure has to match the layout of STAGE1A_ASM_PARAM
;
push rbx ; Status
push rdi ; TimeStamp[0] [63:0]
shl rdx, 32 ; Move CarTop to high 32bit