Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29840 )
Change subject: security/vboot: Fix measured boot issues ......................................................................
Patch Set 13:
(3 comments)
https://review.coreboot.org/#/c/29840/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/29840/13//COMMIT_MSG@9 PS13, Line 9: * Remove legacy and never published google purin. Hmm... the reason we haven't done this before is that Purin is the only board that builds for MIPS architecture. It may have bitrotted a bit by now anyway, but at least it's still getting compiled. If we remove this, we may as well remove all of MIPS support because nothing will prevent it from rotting away.
Is fixing Purin that hard? I don't even care whether the memory map makes sense for the board, actually, I'd just like to keep something for compile testing if possible. Just make it as big as it needs to be. (Long-term it would be great if we could get a MIPS QEMU target instead, but of course nobody has time for that.)
If you do think we need to remove this, please also remove the Cygnus SoC with it.
https://review.coreboot.org/#/c/29840/13//COMMIT_MSG@13 PS13, Line 13: AMD stoneyridge. Can you please make separate commits for each of these? Changes should be grouped in commits based on what they do, not why you need them.
https://review.coreboot.org/#/c/29840/13/src/soc/rockchip/rk3288/include/soc... File src/soc/rockchip/rk3288/include/soc/memlayout.ld:
https://review.coreboot.org/#/c/29840/13/src/soc/rockchip/rk3288/include/soc... PS13, Line 36: OVERLAP_VERSTAGE_ROMSTAGE(0xFF70C800, 42K) Are these changes needed even if you don't compile CONFIG_VBOOT_MEASURED_BOOT into those systems? Because I don't think your feature should increase code size when it's not enabled. If it does, maybe we can track down how and fix that.
-
Julius Werner (Code Review)