[coreboot-gerrit] Change in ...coreboot[master]: security/vboot: Fix measured boot issues

[Julius Werner (Code Review)]

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/memlayout.ld 
File src/soc/rockchip/rk3288/include/soc/memlayout.ld:

https://review.coreboot.org/#/c/29840/13/src/soc/rockchip/rk3288/include/soc/memlayout.ld@36 
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.



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/29840
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I35a85b8f137f28cd9960f2c5ce95f8fa31185b82
Gerrit-Change-Number: 29840
Gerrit-PatchSet: 13
Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki at gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi at google.com>
Gerrit-Reviewer: Patrick Rudolph <siro at das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki at gmail.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh at siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Mon, 26 Nov 2018 21:31:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20181126/61da0f20/attachment.html>