Attention is currently required from: Arthur Heymans, Philipp Hug, ron minnich.
Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68841?usp=email )
Change subject: riscv: add smp support for exception handler ......................................................................
Patch Set 2:
(4 comments)
File src/arch/riscv/payload.c:
https://review.coreboot.org/c/coreboot/+/68841/comment/fdaebb95_3400bca4 : PS2, Line 70: write_csr(mscratch, MACHINE_STACK_TOP()); maybe mention why we save it in a comment. Without looking at this patch it is not really obvious why we save the stack pointer in the mscratch register.
File src/arch/riscv/trap_util.S:
https://review.coreboot.org/c/coreboot/+/68841/comment/b4ef1f1e_61d3c696 : PS2, Line 12: LOAD x3, 3 * REGBYTES(sp) might be worth to note (in a comment) that x2 is the stack pointer otherwise it is not clear why we don't save/restore it here.
https://review.coreboot.org/c/coreboot/+/68841/comment/129ae739_49a19713 : PS2, Line 103: # when exiting coreboot, write sp to mscratch how do we make sure that nothing else writes to MACHINE_TOP_STACK after coreboot is jumping to payload? If we use it here than we effectively overwrite what was written to MACHINE_TOP_STACK after corebot jumped to payload.
https://review.coreboot.org/c/coreboot/+/68841/comment/e0385be3_acc8ed2b : PS2, Line 105: bnez sp, 1f # sp == 0, trap come from current program nit: `bnez sp, 1f # sp == 0, trap comes from coreboot` only because I had to think twice before I realized that you meant coreboot with current program (instead of whatever program comes after coreboot)