Attention is currently required from: Sergii Dmytruk, Timothy Pearson, Ron Minnich. Krystian Hebel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57084 )
Change subject: src/arch/ppc64/*: pass FDT address to payload ......................................................................
Patch Set 20: Code-Review-1
(3 comments)
Patchset:
PS20: All of the code in this commit is just for QEMU, yet it lands in architecture-specific files. This would make incoming commits for Talos2 unnecessarily complicated.
File src/arch/ppc64/boot.c:
https://review.coreboot.org/c/coreboot/+/57084/comment/8be41c6c_56dba2b3 PS20, Line 42: asm("ld 27,%0" :: "m"(fdt) : "27"); Only QEMU prepares FDT in advance, real hardware doesn't, so all of the new code should be moved to platform_prog_run() instead.
Because this line loads to volatile register, it should be put in the same asm block as the final jump to the payload, otherwise compiler is free to move code around, potentially overwriting FDT pointer in register.
File src/arch/ppc64/bootblock_crt0.S:
https://review.coreboot.org/c/coreboot/+/57084/comment/6ea12989_3716f19c PS20, Line 40: /* Store FDT address that's available in %r3 to pass to payload */ Maybe use one of SPRs for holding this value so it won't have to be moved around in RAM? HSPRG0 or HSPRG1 seems good. No need for additional section in memlayout or CBMEM entry either.
-
Krystian Hebel (Code Review)