Attention is currently required from: Arthur Heymans, David Hendricks, Jérémy Compostella, Kapil Porwal, Nico Huber, Patrick Rudolph, Subrata Banik, Werner Zeh.
Julius Werner 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 14:
(2 comments)
Patchset:
PS14:
I can understand Subrata's rational to transfer control to the payload in 64 bit seamlessly withou […]
I think the answer is "no"? I don't think there's really a way to do that without getting very crazy (e.g. running the payload under VMX). I strongly doubt that there's a need for this that would justify going to such lengths... when people care about running older payloads under new coreboot I think they usually care about doing it on the same old platform that payload was originally developed for. If you're developing for an entirely new platform, I think you can recompile your payload once.
PS14:
And you are in favor of a new CBFS type (not a new segment type), is that correct?
Yes, I think doing that (and not having multiple entry points) would be the cleanest and easiest solution.
Dual entry aside, do we already have code in coreboot that considers archi-
tecture-specific types or would we have to add code?
It's not directly architecture-specific, but we do have the precedent of CBFS_TYPE_FIT, which is a different payload format (different from SELF) that is only used on Arm (and I think maybe also RISC-V now?). So I think treating "SELF but in 64-bit" as another case like that and adding CBFS_TYPE_SELF64 would be consistent.
The nice part about this is that `payload_load()` already extracts the CBFS type and stores it in `struct prog` anyway, so all the code to get this type and pipe it through to where it needs to go is already in place. All we'd need to do to support it is to add another case label to the same code path for running `selfload()` that already exists in `payload_load()`, and then check `prog->cbfs_type` in `arch_prog_run()` to do a different hand-off. Any of the other solutions about using CBFS attributes or SELF segments would instead require adding extra piping to `struct prog` (in addition to the other concerns mentioned previously).
I don't want to repeat my whole email here, my basic thought was how we could do it with minimal (or no) additional complexity in coreboot. And preferably the least ifs in coreboot and payload build systems.
I think the solution I'm suggesting would be pretty much that, I don't think you can make it any simpler.
If we do the long-mode handover now before X86S, we have to answer more questions, e.g. What should a 64-bit coreboot on AMD64 do when it encounters a 32-bit payload?
I think in order to keep the default configuration compatible with the way people who've been building coreboot for X86_64 used it before, we need to support that (unless everyone here says that nobody has been using it yet and nobody cares?). So we keep the current code that switches back down to protected mode, and decide whether to use it based on the CBFS type.
I would suggest to also add a (default-off) Kconfig to disable building in that code, so that platforms which know they'll never need it can save on code size (and then they'll just `die()` if they encounter the wrong payload type). But that's also something that could be discussed and added later and doesn't need to be part of the immediate decision here.