Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30118 )
Change subject: arch/x86/boot: Jump to payload in protected mode ......................................................................
Patch Set 30:
(6 comments)
https://review.coreboot.org/c/coreboot/+/30118/29//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/30118/29//COMMIT_MSG@9 PS29, Line 9: On ARCH_RAMSTAGE_X86_64 jump to the payload in protected mode.
I think this should be documented somewhere?
Done
https://review.coreboot.org/c/coreboot/+/30118/29/src/arch/x86/c_exit.S File src/arch/x86/c_exit.S:
https://review.coreboot.org/c/coreboot/+/30118/29/src/arch/x86/c_exit.S@25 PS29, Line 25: .section .bss, "aw", @nobits
What is this doing here?
Done
https://review.coreboot.org/c/coreboot/+/30118/29/src/arch/x86/c_exit.S@27 PS29, Line 27: .section ".text.SetCodeSelector32", "ax", @progbits
Is just specifying the text section not enough?
Done
https://review.coreboot.org/c/coreboot/+/30118/8/src/arch/x86/c_start.S File src/arch/x86/c_start.S:
https://review.coreboot.org/c/coreboot/+/30118/8/src/arch/x86/c_start.S@214 PS8, Line 214: SetCodeSelector64:
I guess the changes from SetCodeSelector32 could be applied here too, but other than that looks good […]
moved to separate file, comment no longer applies
https://review.coreboot.org/c/coreboot/+/30118/14/src/arch/x86/c_start.S File src/arch/x86/c_start.S:
https://review.coreboot.org/c/coreboot/+/30118/14/src/arch/x86/c_start.S@a22... PS14, Line 220: # save rsp because iret will align it to a 16 byte boundary
I don't see why not, they should be identical now.
comment is no longer valid in latest patchset
https://review.coreboot.org/c/coreboot/+/30118/29/src/include/program_loadin... File src/include/program_loading.h:
https://review.coreboot.org/c/coreboot/+/30118/29/src/include/program_loadin... PS29, Line 201: uint32_t
This cuts off a part of prog_entry_arg which returns a pointer to void.
Done
-
Patrick Rudolph (Code Review)