Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/22949
Change subject: x86/smm: Preserve local variables whan aligning stack ......................................................................
x86/smm: Preserve local variables whan aligning stack
Align the stack earlier and account for all local variables.
Ensure %edi, containing the fxsave/fxrstor location, can be properly retrieved from the stack prior to the rsm instruction.
This corrects an observed bug where the stack had a 16-byte alignment, %edi was pushed, then the stack was realigned for the call to c_handler. After returning, garbage was popped into %edi and the system rebooted as a result of the fxrstor instruction.
BUG=b:70027919 TEST=Boot Kahlee to the OS
Change-Id: Ic06cdf8bd95d9aaa9634ecf3546a5033150fe82b Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/cpu/x86/smm/smm_stub.S 1 file changed, 8 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/22949/1
diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S index 32435a0..f5a30d4 100644 --- a/src/cpu/x86/smm/smm_stub.S +++ b/src/cpu/x86/smm/smm_stub.S @@ -145,6 +145,14 @@ add %eax, %edi
2: + /* Align the stack before saving any local information. This takes + * into account that the stack must be in proper alignment at the time + * the call to c_handler is made. That is, c_handler's stack must be + * aligned so that %esp+4 (or %rsp+8) is a multiple of 16. + */ + andl $0xfffffff0, %esp + sub $12, %esp + /* Save location of fxsave area. */ push %edi mov %esp, %ebp @@ -160,9 +168,6 @@ fxsave (%edi)
1: - /* Align stack to 16 bytes. Another 16 bytes are pushed below. */ - andl $0xfffffff0, %esp - /* Call into the c-based SMM relocation function with the platform * parameters. Equivalent to: * struct arg = { c_handler_params, cpu_num, smm_runtime {;