Attention is currently required from: Arthur Heymans, Julius Werner, Jérémy Compostella, Kapil Porwal, Nico Huber.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81960?usp=email )
Change subject: arch/x86: Enable long mode entry into payload for x86_64 support ......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81960/comment/cec64804_35640213 : PS1, Line 15: transition.
Why would there be such issues? wouldn't libpayload be responsible to switch to long mode properly? The payload (depthcharge) should be only C code, isn't it?
Please take a look into https://github.com/coreboot/coreboot/blob/main/payloads/libpayload/arch/x86/... In this case, the expectation is static, like libpayload enters into protected mode. Therefore, all the information expected from coreboot are based on 32-bit calling convention.
When we implement the x64 architecture for libpayload, we design the ABI expectation according to long mode and also build the libpayload using a 64-bit compiler (I will submit those CLs next). However, if coreboot always switches back to protected mode, then the expectation at the x64/head.S file will not match compared to the 32-bit libpayload entry. for example: to fetch the coreboot table in 32-bit mode
``` /* save pointer to coreboot tables */ movl 4(%esp), %eax movl %eax, cb_header_ptr ```
ideally a 64-bit lb code would expect the same data from RSI
``` movq %rsi, cb_header_ptr ```
Now that we are in 64-bit LB and expect to see the proper data in RSI, we noticed that the calling convention is not aligned between coreboot and libpayload.
Therefore, we opted for this option, where we decide at compile time how we would like to pass this information to libpayload. Since building libpayload in 64-bit is a static information, a platform that decides to build LB in 64-bit can also select this Kconfig to ensure there is no intermediate switch between 64-bit and 32-bit.
File src/arch/x86/boot.c:
https://review.coreboot.org/c/coreboot/+/81960/comment/0b82af5d_90de517c : PS1, Line 25: if (CONFIG(PAYLOAD_X86_64_SUPPORT)) {
AIUI, the payload handover and the coreboot tables are the most important ABI pieces of coreboot. Making this a compile-time option would mean that the resulting coreboot is suddenly incompatible to all prior (x86) payload builds. So, shouldn't this be decided at runtime, maybe based on information from CBFS?
Is it not a fact that using a 64-bit toolchain to compile the libpayload is also a static piece of information? If so, then why are we not allowing a configuration to also enter LB in the desired 64-bit mode instead of limiting it to 32-bit?
Currently, the default/non-configurable decision about LB entry is always in protected mode.
This CL allows for one more option where 64-bit direct entry is also possible, hence all of the following options are now valid:
1. 64-bit CB / 32-bit LB (using protected mode) 2. 64-bit CB / 64-bit LB (using long mode) 3. 32-bit CB / 32-bit LB
If we are relying on the runtime information about coreboot operational mode and using this information to decide whether to jump into LB, then wouldn't that limit #1 (where a protected mode entry into the libpayload is not possible while using 64-bit coreboot)? Perhaps not everyone wishes to use a 64-bit payload, so it would be better to keep those selections static based on the selection of payload mode.