Attention is currently required from: Arthur Heymans, Felix Held, Martin L Roth, Patrick Rudolph.
Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79160?usp=email )
Change subject: Documentation: Improve x86_64 ......................................................................
Patch Set 1:
(9 comments)
Patchset:
PS1:
Thanks for doing this. […]
So far, I've rebased some commits for 64-bit support onto the main branch, and fixed the merge conflicts. I'll work on updating it to reflect current status as I can.
File Documentation/arch/x86/x86_64.md:
https://review.coreboot.org/c/coreboot/+/79160/comment/81efc031_5a383606 : PS1, Line 3: experimental
Do you have any thoughts about how long this is going to remain experimental? What's preventing it f […]
Likely, it'll take time to test platforms/configurations and ensure there aren't any problems. That might mean fixing compiler issues, but I'm also wondering if turning on page tables will break some platforms if they don't report resources correctly.
On some platforms, callbacks from the FSP to coreboot are an issue now (PPI, console). There is only mode switch code for entry at the moment. This is solvable with a couple of mode switch functions though.
https://review.coreboot.org/c/coreboot/+/79160/comment/dcd7717b_7ac491f7 : PS1, Line 10: a *"hack"* which places page tables in ROM
Add: "This will be explained further below in the section on page tables"?
Sure, I suppose this needs rewriting. It's also somewhat out-of-date. With the follow-up change, romstage generates page tables if memory above 4 GiB is used, and ramstage will do use reported resources to generate tables unconditionally.
https://review.coreboot.org/c/coreboot/+/79160/comment/b7bc187f_124c3b2f : PS1, Line 24: : The large memory model causes the compiler to emit 64bit addressing : instructions, which are not only slower, but also increase code size.
Should this be moved to a section comparing advantages and disadvantages between 32 & 64-bit modes? […]
Page tables can be used to provide security benefits, such as by marking memory as non-executable or removing it entirely. This could be useful for SMM: mark regular DRAM as NX.
(Intel platforms already have SMM code access check, which triggers a protection fault if SMM executes code outside DRAM, but we can't enable it because the MSR requires SMM to set it on all cores.)
https://review.coreboot.org/c/coreboot/+/79160/comment/2896b972_27a1a981 : PS1, Line 28: must generate page tables at build time
Is this really a toolchain requirement, or just an implementation detail?
I'm not sure I understand? If you're asking if long mode architecturally needs page tables in ROM, no. That's just the implementation in-tree, and with the follow-up, the implementation that bootblock sets-up (also used by verstage, and possibly until ramstage).
https://review.coreboot.org/c/coreboot/+/79160/comment/28d74d5a_9688fabb : PS1, Line 107: * Fix running VGA Option ROMs
option roms can't currently be run directly when in long mode, since only going to real mode is only […]
If necessary, I suppose `protected_mode_call_argx()` could be used to run the code that drops from protected to real mode. But of course, that isn't done yet.
https://review.coreboot.org/c/coreboot/+/79160/comment/ed266f0c_cb0f821b : PS1, Line 117: * Place and run code above 4GiB
do we really need this? i'd say that we should just keep all code and stack/heap below 4GB
Probably not, I think. I don't know what the thought process was when this was written though.
https://review.coreboot.org/c/coreboot/+/79160/comment/1b519ae1_3c388fe7 : PS1, Line 140: Here's a list of known issues:
Should this be another subsection, or is this still referring to QEMU?
I'm not sure. I'll check Intel's SDM and some AMD documentation.
https://review.coreboot.org/c/coreboot/+/79160/comment/ee1ed59d_3b9e16a5 : PS1, Line 155: * Entering long mode crashes on AMD host platforms.
i haven't added the code for switching back to 32 bit mode before calling FSP; that code was already […]
Yeah, this might be out of date.