Attention is currently required from: Arthur Heymans, Christian Walter, Jeremy Soller, Paul Menzel, Philipp Deppenwiese.
Hello Christian Walter, Jeremy Soller, Matt DeVillier, Philipp Deppenwiese, Sean Rhodes, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/73297?usp=email
to look at the new patch set (#7).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: security/tpm/tspi/log: Respect CBMEM TPM log size when copying preram entries
......................................................................
security/tpm/tspi/log: Respect CBMEM TPM log size when copying preram entries
The preram TPM log was being copied to the end of the CBMEM TPM log no
matter what the size of the CBMEM TPM log was. Eventually, it would
overwrite anything else in CBMEM beyond the TPM log.
This can currently be reproduced by enabling TPM_MEASURED_BOOT and
performing multiple S3 suspends, as coreboot is incorrectly performing
TPM measurements on S3 resume.
Change-Id: If76299e68eb5ed2ed20c947be35cea46c51fcdec
Signed-off-by: Jeremy Soller <jeremy(a)system76.com>
Signed-off-by: Tim Crawford <tcrawford(a)system76.com>
---
M src/security/tpm/tspi/log-tpm1.c
M src/security/tpm/tspi/log-tpm2.c
M src/security/tpm/tspi/log.c
3 files changed, 15 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/73297/7
--
To view, visit https://review.coreboot.org/c/coreboot/+/73297?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If76299e68eb5ed2ed20c947be35cea46c51fcdec
Gerrit-Change-Number: 73297
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newpatchset
Attention is currently required from: Christian Walter, Tim Crawford.
Hello Christian Walter, Tim Crawford,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/75997?usp=email
to look at the new patch set (#2).
Change subject: UNTESTED: security/crtm: Don't measure anything on S3 resume
......................................................................
UNTESTED: security/crtm: Don't measure anything on S3 resume
To quote the TCG PC Client Platform Firmware Profile
Specification:
"7.3.9 S3 (Sleep) to S0 (Working)
This transition is a resume from an S3 suspend state. Host Platform
Reset and TPM_INIT are asserted. The SRTM issues the TPM2_Startup(STATE)
command, loading the previously saved state, without re-measuring Pre-OS
components. The SRTM passes2395 control to the OS. If there are any
changes to the Host Platform’s components or configuration, measuring
these changes is the responsibility of the OS"
Therefore coreboot should not measure anything in either the logs or PCR
on S3 resume.
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Change-Id: Ic4ed5a3ca8bb2f82931e08348754c173d7a78c53
---
M src/security/tpm/tspi/crtm.c
1 file changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/75997/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/75997?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic4ed5a3ca8bb2f82931e08348754c173d7a78c53
Gerrit-Change-Number: 75997
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Christian Walter, Paul Menzel, Philipp Deppenwiese.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73297?usp=email )
Change subject: security/tpm/tspi/log: Respect CBMEM TPM log size when copying preram entries
......................................................................
Patch Set 6:
(1 comment)
Patchset:
PS6:
Can you test https://review.coreboot.org/c/coreboot/+/75997 ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/73297?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If76299e68eb5ed2ed20c947be35cea46c51fcdec
Gerrit-Change-Number: 73297
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Jeremy Soller <jeremy(a)system76.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Wed, 21 Jun 2023 16:08:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Christian Walter, Paul Menzel, Philipp Deppenwiese.
Tim Crawford has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73297?usp=email )
Change subject: security/tpm/tspi/log: Respect CBMEM TPM log size when copying preram entries
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS1:
> > Okay, but this change is about preventing logs from clobbering CBMEM when being copied. […]
Changed "Fixes" to a repro case.
--
To view, visit https://review.coreboot.org/c/coreboot/+/73297?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If76299e68eb5ed2ed20c947be35cea46c51fcdec
Gerrit-Change-Number: 73297
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Jeremy Soller <jeremy(a)system76.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Wed, 21 Jun 2023 16:07:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Crawford <tcrawford(a)system76.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/75997?usp=email )
Change subject: UNTESTED: security/crtm: Don't measure anything on S3 resume
......................................................................
UNTESTED: security/crtm: Don't measure anything on S3 resume
To quote the TCG PC Client Platform Firmware Profile
Specification:
"7.3.9 S3 (Sleep) to S0 (Working)
This transition is a resume from an S3 suspend state. Host Platform
Reset and TPM_INIT are asserted. The SRTM issues the TPM2_Startup(STATE)
command, loading the previously saved state, without re-measuring Pre-OS
components. The SRTM passes2395 control to the OS. If there are any
changes to the Host Platform’s components or configuration, measuring
these changes is the responsibility of the OS"
Therefore coreboot should not measure anything in either the logs or PCR
on S3 resume.
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Change-Id: Ic4ed5a3ca8bb2f82931e08348754c173d7a78c53
---
M src/security/tpm/tspi/crtm.c
1 file changed, 11 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/75997/1
diff --git a/src/security/tpm/tspi/crtm.c b/src/security/tpm/tspi/crtm.c
index 36dffb8..02304fe 100644
--- a/src/security/tpm/tspi/crtm.c
+++ b/src/security/tpm/tspi/crtm.c
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
+#include <acpi/acpi.h>
#include <console/console.h>
#include <fmap.h>
#include <bootstate.h>
@@ -35,6 +36,9 @@
*/
static uint32_t tspi_init_crtm(void)
{
+ if (acpi_is_wakeup_s3())
+ return VB2_SUCCESS;
+
/* Initialize TPM PRERAM log. */
if (!tpm_log_available()) {
tpm_preram_log_clear();
@@ -113,6 +117,9 @@
uint32_t pcr_index;
char tpm_log_metadata[TPM_CB_LOG_PCR_HASH_NAME];
+ if (acpi_is_wakeup_s3())
+ return TPM_SUCCESS;
+
if (!tpm_log_available()) {
if (tspi_init_crtm() != VB2_SUCCESS) {
printk(BIOS_WARNING,
@@ -156,8 +163,7 @@
/* We are dealing here with pre CBMEM environment.
* If cbmem isn't available use CAR or SRAM */
- if (!cbmem_possibly_online() &&
- !CONFIG(VBOOT_RETURN_FROM_VERSTAGE))
+ if (!ENV_HAS_CBMEM && !CONFIG(VBOOT_RETURN_FROM_VERSTAGE))
return _tpm_log;
else if (ENV_CREATES_CBMEM
&& !CONFIG(VBOOT_RETURN_FROM_VERSTAGE)) {
@@ -207,6 +213,9 @@
#if !CONFIG(VBOOT_RETURN_FROM_VERSTAGE)
static void recover_tpm_log(int is_recovery)
{
+ if (is_recovery)
+ return;
+
const void *preram_log = _tpm_log;
void *ram_log = tpm_log_cbmem_init();
--
To view, visit https://review.coreboot.org/c/coreboot/+/75997?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic4ed5a3ca8bb2f82931e08348754c173d7a78c53
Gerrit-Change-Number: 75997
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newchange
Attention is currently required from: Christian Walter, Paul Menzel, Philipp Deppenwiese, Tim Crawford.
Hello Christian Walter, Jeremy Soller, Matt DeVillier, Philipp Deppenwiese, Sean Rhodes, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/73297?usp=email
to look at the new patch set (#6).
Change subject: security/tpm/tspi/log: Respect CBMEM TPM log size when copying preram entries
......................................................................
security/tpm/tspi/log: Respect CBMEM TPM log size when copying preram entries
The preram TPM log was being copied to the end of the CBMEM TPM log no
matter what the size of the CBMEM TPM log was. Eventually, it would
overwrite anything else in CBMEM beyond the TPM log.
This can currently be reproduced by enabling TPM_MEASURED_BOOT and
performing multiple S3 suspends, as coreboot is incorrectly performing
TPM measurements on S3 resume.
Change-Id: If76299e68eb5ed2ed20c947be35cea46c51fcdec
Signed-off-by: Jeremy Soller <jeremy(a)system76.com>
Signed-off-by: Tim Crawford <tcrawford(a)system76.com>
---
M src/security/tpm/tspi/log.c
1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/73297/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/73297?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If76299e68eb5ed2ed20c947be35cea46c51fcdec
Gerrit-Change-Number: 73297
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Jeremy Soller <jeremy(a)system76.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Paul Menzel.
Tim Crawford has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73975?usp=email )
Change subject: mb/system76/rpl: Add Adder WS 3 as a variant
......................................................................
Set Ready For Review
--
To view, visit https://review.coreboot.org/c/coreboot/+/73975?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I165a434fe18f8c0aac49cb872bb87f98551d8f2c
Gerrit-Change-Number: 73975
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Wed, 21 Jun 2023 16:03:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Tarun Tuli.
Hello Subrata Banik, Tarun Tuli, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/72926?usp=email
to look at the new patch set (#7).
Change subject: soc/intel/adl: Add Raptorlake-HX definitions
......................................................................
soc/intel/adl: Add Raptorlake-HX definitions
Tested by booting System76 Adder WS 3 (addw3) and Serval WS 13 (serw13)
to edk2 payload and then OS.
Ref: Intel Raptor Lake EDS, Volume 1 (#640555, rev. 2.5)
Change-Id: I6098e9121a3afc4160c8a0c96d597e88095fd65d
Signed-off-by: Tim Crawford <tcrawford(a)system76.com>
---
M src/include/cpu/intel/cpu_ids.h
M src/include/device/pci_ids.h
M src/soc/intel/alderlake/bootblock/report_platform.c
M src/soc/intel/alderlake/chip.h
M src/soc/intel/alderlake/chipset.cb
M src/soc/intel/alderlake/cpu.c
M src/soc/intel/alderlake/fsp_params.c
M src/soc/intel/alderlake/include/soc/cpu.h
M src/soc/intel/alderlake/vr_config.c
M src/soc/intel/common/block/cpu/mp_init.c
M src/soc/intel/common/block/graphics/graphics.c
M src/soc/intel/common/block/systemagent/systemagent.c
12 files changed, 129 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/72926/7
--
To view, visit https://review.coreboot.org/c/coreboot/+/72926?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6098e9121a3afc4160c8a0c96d597e88095fd65d
Gerrit-Change-Number: 72926
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jeremy Soller <jeremy(a)system76.com>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Christian Walter, Jeremy Soller, Paul Menzel, Philipp Deppenwiese, Tim Crawford.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73297?usp=email )
Change subject: security/tpm/tspi/log: Respect CBMEM TPM log size when copying preram entries
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS1:
> Okay, but this change is about preventing logs from clobbering CBMEM when being copied. It's obvious to hit right now *because* S3 resume does remeasure, but if that's fixed are you saying this check is not needed at all?
The check is not a bad idea but is for sure the wrong fix for your problem. Maybe change the commit message to reflect that this is just a sanity check?
--
To view, visit https://review.coreboot.org/c/coreboot/+/73297?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If76299e68eb5ed2ed20c947be35cea46c51fcdec
Gerrit-Change-Number: 73297
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Comment-Date: Wed, 21 Jun 2023 15:54:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Crawford <tcrawford(a)system76.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment