Attention is currently required from: Christian Walter, Filip Lewiński, Julius Werner, Yu-Ping Wu.
Michał Żygowski has posted comments on this change by Filip Lewiński. ( https://review.coreboot.org/c/coreboot/+/82695?usp=email )
Change subject: security: Allow vboot when INTEL_TXT enabled
......................................................................
Patch Set 5:
(1 comment)
File src/lib/bootblock.c:
https://review.coreboot.org/c/coreboot/+/82695/comment/1aa033a7_af83ad28?us… :
PS5, Line 62: if (CONFIG(TPM_MEASURED_BOOT_INIT_BOOTBLOCK) && !CONFIG(VBOOT_STARTS_IN_BOOTBLOCK)) {
> No, sorry, this doesn't make sense. You're mixing different things together here. […]
Right, I kind of mixed the redundancy of TPM startup in bootblock when vboot runs in bootblock with the VBOOT + TXT. When I saw what vboot_setup_tpm does, I tried to minimize the loss of vboot information output from that function, and that's what I have come up with.
Regarding the TPM_MEASURED_BOOT_INIT_BOOTBLOCK dependency in INTEL_TXT, I think it has to do with how server TXT works. On servers the CPU comes out of reset with IBB modules (defined in FIT type 7 entries) already measured to PCR0 (and TPM is already started). To avoid measuring to cached log, TPM is initialized right in bootblock, to keep sending the measurements directly to the TPM. So, it would make even more sense to select INVALID_POSTINIT on servers with TXT. Summing it up, doing
```
select TPM_MEASURED_BOOT_INIT_BOOTBLOCK if TPM_MEASURED_BOOT && !VBOOT_STARTS_IN_BOOTBLOCK
```
still doesn't help much, because vboot will fail the TPM setup (INVALID_POSTINIT) and proceed to recovery with code VB2_RECOVERY_RO_TPM_S_ERROR. We don't want that either, do we?
So what could possibly work:
```
select TPM_MEASURED_BOOT_INIT_BOOTBLOCK if TPM_MEASURED_BOOT && !VBOOT_STARTS_IN_BOOTBLOCK
select TPM_STARTUP_IGNORE_POSTINIT
```
We don't have any way to distinguish server TXT from client TXT, so TPM_STARTUP_IGNORE_POSTINIT will nearly always be needed, except on client platforms before TigerLake (CBnT can work like server TXT even on client) where VBOOT_STARTS_IN_BOOTBLOCK and we don't select TPM_MEASURED_BOOT_INIT_BOOTBLOCK.
I hope this sheds some light on the matter. Let's discuss further how we can solve it, as it is not yet clear to me how I may satisfy your requirements,
--
To view, visit https://review.coreboot.org/c/coreboot/+/82695?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I19dc3d910c23fcfd8732465c488f47dd86a96781
Gerrit-Change-Number: 82695
Gerrit-PatchSet: 5
Gerrit-Owner: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Tue, 13 Aug 2024 08:43:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Attention is currently required from: Felix Singer, Jérémy Compostella, yuchi.chen(a)intel.com.
Shuo Liu has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83318?usp=email )
Change subject: soc/intel/common/systemagent: Improve systemagent
......................................................................
Patch Set 12:
(1 comment)
File src/soc/intel/common/block/systemagent/systemagent.c:
https://review.coreboot.org/c/coreboot/+/83318/comment/497e4916_1dfed710?us… :
PS10, Line 146: value = ALIGN_DOWN(value, 1 * MiB);
> The original logic is kept in the `soc_systemagent_fixup_address()`, please see it at the bottom of […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/83318?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If32c2a6524c9d55ce7f9c3dd203bcf85cab76c2c
Gerrit-Change-Number: 83318
Gerrit-PatchSet: 12
Gerrit-Owner: yuchi.chen(a)intel.com
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Vasiliy Khoruzhick <vasilykh(a)arista.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: yuchi.chen(a)intel.com
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Comment-Date: Tue, 13 Aug 2024 08:39:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: yuchi.chen(a)intel.com
Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>
Attention is currently required from: Matt DeVillier.
Hello Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83893?usp=email
to look at the new patch set (#2).
Change subject: Revert "mb/starlabs/starbook/adl: Update the VBT"
......................................................................
Revert "mb/starlabs/starbook/adl: Update the VBT"
This reverts commit 2eb5c1e83ef5206f384846d7514c3aebcaec5bb8.
Reason for revert: The latest release of FSP will not boot
without a display being connected using this VBT. The original
VBT does not have this issue, nor is the original issue that
commit 2eb5c1e83ef5206f384846d7514c3aebcaec5bb8 fixed.
Revert it to restore booting when there is no display.
Change-Id: I05f9037cd68b8b29e69156e2372a544985f4442e
Signed-off-by: Sean Rhodes <sean(a)starlabs.systems>
---
M src/mainboard/starlabs/starbook/variants/adl/data.vbt
1 file changed, 0 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/83893/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83893?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I05f9037cd68b8b29e69156e2372a544985f4442e
Gerrit-Change-Number: 83893
Gerrit-PatchSet: 2
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Sean Rhodes has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/83893?usp=email )
Change subject: Revert "mb/starlabs/starbook/adl: Update the VBT"
......................................................................
Set Ready For Review
--
To view, visit https://review.coreboot.org/c/coreboot/+/83893?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I05f9037cd68b8b29e69156e2372a544985f4442e
Gerrit-Change-Number: 83893
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Tue, 13 Aug 2024 08:26:02 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Sean Rhodes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/83892?usp=email )
Change subject: Revert "mb/starlabs/starbook/adl: Update the VBT"
......................................................................
Abandoned
--
To view, visit https://review.coreboot.org/c/coreboot/+/83892?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: abandon
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I05f9037cd68b8b29e69156e2372a544985f4442e
Gerrit-Change-Number: 83892
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Sean Rhodes has created a revert of this change. ( https://review.coreboot.org/c/coreboot/+/77135?usp=email )
Change subject: mb/starlabs/starbook/adl: Update the VBT
......................................................................
--
To view, visit https://review.coreboot.org/c/coreboot/+/77135?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: revert
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifcb4a086106f90c70926f44a7566330efd185544
Gerrit-Change-Number: 77135
Gerrit-PatchSet: 3
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Attention is currently required from: Felix Held.
Hello build bot (Jenkins), Felix Held,
I'd like you to do a code review.
Please visit
https://review.coreboot.org/c/coreboot/+/83892?usp=email
to review the following change.
Change subject: Revert "mb/starlabs/starbook/adl: Update the VBT"
......................................................................
Revert "mb/starlabs/starbook/adl: Update the VBT"
This reverts commit 2eb5c1e83ef5206f384846d7514c3aebcaec5bb8.
Reason for revert: 4251 FSP doesn't like this change
Change-Id: I05f9037cd68b8b29e69156e2372a544985f4442e
---
M src/mainboard/starlabs/starbook/variants/adl/data.vbt
1 file changed, 0 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/83892/1
diff --git a/src/mainboard/starlabs/starbook/variants/adl/data.vbt b/src/mainboard/starlabs/starbook/variants/adl/data.vbt
index 8ecfffb..2c7db18 100644
--- a/src/mainboard/starlabs/starbook/variants/adl/data.vbt
+++ b/src/mainboard/starlabs/starbook/variants/adl/data.vbt
Binary files differ
--
To view, visit https://review.coreboot.org/c/coreboot/+/83892?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I05f9037cd68b8b29e69156e2372a544985f4442e
Gerrit-Change-Number: 83892
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Filip Lewiński, Jérémy Compostella, Krystian Hebel, Michał Kopeć, Michał Żygowski, Paul Menzel.
Michał Żygowski has posted comments on this change by Filip Lewiński. ( https://review.coreboot.org/c/coreboot/+/82696?usp=email )
Change subject: cpu/x86/mp_init: Add code to restart APs
......................................................................
Patch Set 7:
(1 comment)
File src/cpu/x86/mp_init.c:
https://review.coreboot.org/c/coreboot/+/82696/comment/53c40d1f_d754df48?us… :
PS1, Line 1214: mdelay(1000);
> It is just a copied fragment from mp_init function in this file. […]
@paulepanter@mailbox.org any action required for me?
--
To view, visit https://review.coreboot.org/c/coreboot/+/82696?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ief2a7629d3075de29b363d05330e3a76cef48971
Gerrit-Change-Number: 82696
Gerrit-PatchSet: 7
Gerrit-Owner: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Michał Żygowski <miczyg94(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Michał Żygowski <miczyg94(a)gmail.com>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 13 Aug 2024 08:22:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>