Attention is currently required from: Julius Werner, Jérémy Compostella, Kapil Porwal, Nico Huber, Subrata Banik.
Arthur Heymans 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 2:
(1 comment)
File src/arch/x86/boot.c:
https://review.coreboot.org/c/coreboot/+/81960/comment/7ddcf017_beae94a3 : PS1, Line 25: if (CONFIG(PAYLOAD_X86_64_SUPPORT)) {
[Subrata] I don't under why all of sudden the "multiple entry point approach" considered critical. The problem of supporting 64-bit direct entry can be handled w/o that as well. Having multiple entry point approach like kernel may not be that critical at this stage. we can jump into 64-bit entry point using boot.c code (CB:81960). For that we don't need to support multiple entry point approach. If you wish to implement multiple entry point approach, please do that w/o blocking the current task is my request.
So multiple entry points is for sure not a must to proceed I'd say. The point I tried to make was that you don't want different ways to inform coreboot ramstage to straight jump to 64bit payload entry (even if there is only one 64bit entry). So we should carefully discuss the route we want to take.
So what do you think of the following suggestion:
- Add a 64bit entry (maybe call it x86_long_mode, to avoid confusion?) in cbfs_payload_segment_type: maybe PAYLOAD_SEGMENT_X86_LONG_MODE_ENTRY ?
- Add a way to cbfstool to mark the entry of the payload as such (similar to your --64 patch, but then not with file attr, but as payload segment)
- 64bit ramstage coreboot at runtime decides to jump to PAYLOAD_SEGMENT_X86_LONG_MODE_ENTRY directly if found
- If not found keep the old 32bit entering code
[Subrata] I really like this approach and isn't this is what I've tried since morning till now? I have used file attribute (over segment_type) as I felt this is more meaningful as we are injecting it at header.
A few reason why segment_type is better in my opinion: - I think they 'fail' better in some cases. For instance if an older coreboot ramstage or chainloading payload does not find the ENTRY segment of a 64bit only payload it will complain more loudly about what's wrong. With and attribute old ramstage/chainload would jump to new 64bit payload entry and likely fail on some 64bit instruction in 32bit mode. - It makes multiple entry points easier to implement