Attention is currently required from: Tim Wawrzynczak, Curtis Chen, Patrick Rudolph.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59644 )
Change subject: soc/intel/alderlake: Add TDP to give correct VR configuration.
......................................................................
Patch Set 1:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/59644/comment/999533eb_934bd76c
PS1, Line 7: soc/intel/alderlake: Add TDP to give correct VR configuration.
Please remove the dot/period at the end [1].
[1]: https://chris.beams.io/posts/git-commit/https://review.coreboot.org/c/coreboot/+/59644/comment/3a9a5274_20823103
PS1, Line 11: Add TDP to recognize the correct SKU and give the correct power setting.
Please add a blank line between paragraphs.
File src/soc/intel/alderlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/59644/comment/78235ea9_ffd1a953
PS1, Line 36: u8 tdp;
Please be consistent, so `uint8_t`.
https://review.coreboot.org/c/coreboot/+/59644/comment/9525417f_0c86a47d
PS1, Line 83: { PCI_DEVICE_ID_INTEL_ADL_P_ID_3, 28, VR_CFG_ALL_DOMAINS_TDC_CURRENT(40, 40) },
That line is new. Maybe make it a separate commit?
--
To view, visit https://review.coreboot.org/c/coreboot/+/59644
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4d31e7afc76d9a8c772781671f92ec08f9d8713f
Gerrit-Change-Number: 59644
Gerrit-PatchSet: 1
Gerrit-Owner: Curtis Chen <curtis.chen(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Curtis Chen <curtis.chen(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 25 Nov 2021 22:31:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jakub Czapiga, Christian Walter, Yu-Ping Wu.
Hello Jakub Czapiga, Christian Walter, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/59682
to look at the new patch set (#2).
Change subject: cbfs: Remove deprecated APIs
......................................................................
cbfs: Remove deprecated APIs
This patch removes all remaining pieces of the old CBFS API, now that
the last straggling use cases of it have been ported to the new one.
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: I1cec0ca2d9d311626a087318d1d78163243bfc3c
---
M src/commonlib/Makefile.inc
D src/commonlib/cbfs.c
D src/commonlib/include/commonlib/cbfs.h
M src/include/cbfs.h
M src/lib/cbfs.c
M src/security/tpm/tspi/crtm.h
M src/security/vboot/Kconfig
7 files changed, 2 insertions(+), 483 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/59682/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/59682
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1cec0ca2d9d311626a087318d1d78163243bfc3c
Gerrit-Change-Number: 59682
Gerrit-PatchSet: 2
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jakub Czapiga, Christian Walter, Arthur Heymans, Werner Zeh.
Hello Jakub Czapiga, Christian Walter, Arthur Heymans, Werner Zeh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/59681
to look at the new patch set (#2).
Change subject: cbfs | tspi: Join hash calculation for verification and measurement
......................................................................
cbfs | tspi: Join hash calculation for verification and measurement
This patch moves the CBFS file measurement when CONFIG_TPM_MEASURED_BOOT
is enabled from the lookup step into the code where a file is actually
loaded or mapped from flash. This has the advantage that CBFS routines
which just look up a file to inspect its metadata (e.g. cbfs_get_size())
do not cause the file to be measured twice. It also removes the existing
inefficiency that files are loaded twice when measurement is enabled
(once to measure and then again when they are used). When CBFS
verification is enabled and uses the same hash algorithm as the TPM, we
are even able to only hash the file a single time and use the result for
both purposes.
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: I70d7066c6768195077f083c7ffdfa30d9182b2b7
---
M src/lib/cbfs.c
M src/security/tpm/tspi.h
M src/security/tpm/tspi/crtm.c
M src/security/tpm/tspi/crtm.h
M src/security/tpm/tspi/tspi.c
5 files changed, 57 insertions(+), 94 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/59681/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/59681
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I70d7066c6768195077f083c7ffdfa30d9182b2b7
Gerrit-Change-Number: 59681
Gerrit-PatchSet: 2
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Raul Rangel, Jakub Czapiga.
Hello Raul Rangel, Jakub Czapiga,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/59678
to look at the new patch set (#2).
Change subject: cbfs: Add unverified_area APIs
......................................................................
cbfs: Add unverified_area APIs
This patch adds a new ..._unverified_area_... group of functions to the
cbfs_map/_load/_alloc() APIs. These functions can be used to access
custom FMAP sections and are meant to replace the existing
cbfs_locate_file_in_region(). The name is intended to highlight that
accesses through this API will not be verified when CBFS_VERIFICATION is
enabled and should always be treated as if they may return malicious
data. (Due to laziness I'm not adding the combination of this API with
the ..._type_... variant at this point, since it seems very unlikely
that we'll ever have a use case for that. If we ever do, it should be
easy to add later.)
(Also remove the 'inline' from cbfs_file_hash_mismatch(). I'm not sure
why I put it there in the first place, probably a bad copy&paste.)
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: I402265900f7075aa0c2f58d812c67ea63ddf2900
---
M src/include/cbfs.h
M src/lib/cbfs.c
2 files changed, 151 insertions(+), 78 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/59678/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/59678
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I402265900f7075aa0c2f58d812c67ea63ddf2900
Gerrit-Change-Number: 59678
Gerrit-PatchSet: 2
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Raul Rangel, Jakub Czapiga.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59678 )
Change subject: cbfs: Add unverified_area APIs
......................................................................
Patch Set 1:
(1 comment)
File src/lib/cbfs.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-134409):
https://review.coreboot.org/c/coreboot/+/59678/comment/5e051cf4_adc8fa49
PS1, Line 531: if (tspi_measure_cbfs_hook(&file_rdev, name, be32toh(mdata.h.type))) {
braces {} are not necessary for single statement blocks
--
To view, visit https://review.coreboot.org/c/coreboot/+/59678
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I402265900f7075aa0c2f58d812c67ea63ddf2900
Gerrit-Change-Number: 59678
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Comment-Date: Thu, 25 Nov 2021 22:19:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Julius Werner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59680 )
Change subject: drivers/smmstore: Remove SMMSTORE_IN_CBFS
......................................................................
drivers/smmstore: Remove SMMSTORE_IN_CBFS
The SMMSTORE_IN_CBFS option was just meant as a workaround for an
attempt to backport SMMSTORE into older Chromebooks that never actually
happened. All current and future users of coreboot should be using
SMMSTORE in an FMAP region. The APIs needed for SMMSTORE_IN_CBFS clash
with the CBFS rdev isolation needed for CBFS_VERIFICATION, so let's just
get rid of it.
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: Ia0604a4ffd20b46774631d585925311b65d5a0e9
---
M src/drivers/smmstore/Kconfig
M src/drivers/smmstore/store.c
2 files changed, 7 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/59680/1
diff --git a/src/drivers/smmstore/Kconfig b/src/drivers/smmstore/Kconfig
index 13b7312..3e20e3e 100644
--- a/src/drivers/smmstore/Kconfig
+++ b/src/drivers/smmstore/Kconfig
@@ -19,27 +19,10 @@
By using version 2 you cannot make use of software that expects
a version 1 SMMSTORE.
-config SMMSTORE_IN_CBFS
- bool
- default n
- help
- Select this if you want to add an SMMSTORE region to a
- cbfsfile in a cbfs FMAP region
-
if SMMSTORE
-config SMMSTORE_REGION
- string "fmap region in which SMM store file is kept" if SMMSTORE_IN_CBFS
- default "RW_LEGACY" if CHROMEOS && SMMSTORE_IN_CBFS
- default "COREBOOT" if SMMSTORE_IN_CBFS
- default "SMMSTORE"
-
-config SMMSTORE_FILENAME
- string "SMM store file name" if SMMSTORE_IN_CBFS
- default "smm_store"
config SMMSTORE_SIZE
hex "size of the SMMSTORE FMAP region"
- depends on !SMMSTORE_IN_CBFS
default 0x40000
help
Sets the size of the default SMMSTORE FMAP region.
diff --git a/src/drivers/smmstore/store.c b/src/drivers/smmstore/store.c
index a12cd58..24e8a88 100644
--- a/src/drivers/smmstore/store.c
+++ b/src/drivers/smmstore/store.c
@@ -1,7 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
#include <boot_device.h>
-#include <cbfs.h>
#include <fmap.h>
#include <commonlib/helpers.h>
#include <commonlib/region.h>
@@ -9,6 +8,8 @@
#include <smmstore.h>
#include <types.h>
+#define SMMSTORE_REGION "SMMSTORE"
+
/*
* The region format is still not finalized, but so far it looks like this:
* (
@@ -33,26 +34,11 @@
static enum cb_err lookup_store_region(struct region *region)
{
- if (CONFIG(SMMSTORE_IN_CBFS)) {
- struct cbfsf file;
- if (cbfs_locate_file_in_region(&file,
- CONFIG_SMMSTORE_REGION,
- CONFIG_SMMSTORE_FILENAME, NULL) < 0) {
- printk(BIOS_WARNING,
- "smm store: Unable to find SMM store file in region '%s'\n",
- CONFIG_SMMSTORE_REGION);
- return CB_ERR;
- }
- struct region_device rdev;
- cbfs_file_data(&rdev, &file);
- *region = *region_device_region(&rdev);
- } else {
- if (fmap_locate_area(CONFIG_SMMSTORE_REGION, region)) {
- printk(BIOS_WARNING,
- "smm store: Unable to find SMM store FMAP region '%s'\n",
- CONFIG_SMMSTORE_REGION);
- return CB_ERR;
- }
+ if (fmap_locate_area(SMMSTORE_REGION, region)) {
+ printk(BIOS_WARNING,
+ "smm store: Unable to find SMM store FMAP region '%s'\n",
+ SMMSTORE_REGION);
+ return CB_ERR;
}
return CB_SUCCESS;
--
To view, visit https://review.coreboot.org/c/coreboot/+/59680
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia0604a4ffd20b46774631d585925311b65d5a0e9
Gerrit-Change-Number: 59680
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: newchange