Attention is currently required from: Elyes HAOUAS, Maximilian Brune, Ronak Kanabar, Sridhar Siricilla, Subrata Banik.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76878?usp=email )
Change subject: arch/x86: Adjust stack pointer to point to dummy argument ......................................................................
Patch Set 1: Code-Review-2
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/76878/comment/f7196e60_d97dc949 : PS1, Line 9: The patch pushes dummy argument so that stack pointer points to this : dummy element instead of coreboot table address as depthcharge picks the : second element from top of the stack as coreboot table address. The coreboot Documentation specifies that payloads need to be entered in 32bit mode and while not documented the argument containing the coreboot tables just follows the ABI which here means pushing a 32bit variable on the stack. I think you should fix this in depthcharge rather than break all other payloads that consume the calling argument to pass the coreboot tables.
Patchset:
PS1: This looks like a depthcharge problem and not a coreboot problem.
File src/arch/x86/c_exit.S:
https://review.coreboot.org/c/coreboot/+/76878/comment/6aa843ed_5f514c7d : PS1, Line 32: subl $12, %esp Stack is not aligned to 16 any longer when calling a function.
https://review.coreboot.org/c/coreboot/+/76878/comment/ae7c9802_a04d364a : PS1, Line 34: pushl $0 /* Dummy argument */ The 32bit ABI has argument 0 pushed last to the stack. This will break all payloads that expect it.