coreboot-gerrit December 2021

coreboot-gerrit@coreboot.org

1 participants
5525 discussions

Change in coreboot[master]: drivers/intel/usb4/retimer: Add function to correct EC port mapping
by Tim Wawrzynczak (Code Review) 03 Dec '21

03 Dec '21

Attention is currently required from: Maulik V Vaghela, Sugnan Prabhu S, Subrata Banik, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59665 ) Change subject: drivers/intel/usb4/retimer: Add function to correct EC port mapping ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/59665 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia7a5e63838e6529196bd211516e4d665b084f79e Gerrit-Change-Number: 59665 Gerrit-PatchSet: 8 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> 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: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 03 Dec 2021 21:25:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Add support for ADL-N CPU Type
by Tim Wawrzynczak (Code Review) 03 Dec '21

03 Dec '21

Attention is currently required from: Maulik V Vaghela, Paul Menzel, Reka Norman, Rizwan Qureshi, Subrata Banik, Sridhar Siricilla, Krishna P Bhat D, Usha P, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59836 ) Change subject: soc/intel/alderlake: Add support for ADL-N CPU Type ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File src/soc/intel/alderlake/cpu.c: PS3: For another patch: do we still need this function? Or should we rely on the Kconfigs, e.g. SOC_INTEL_ALDERLAKE_PCH_M / PCH_N / PCH_M, etc. ? -- To view, visit https://review.coreboot.org/c/coreboot/+/59836 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If2917ac356fd80f84bcaf70ed710d329e77f7a6d Gerrit-Change-Number: 59836 Gerrit-PatchSet: 3 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> 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: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Usha P <usha.p(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 03 Dec 2021 21:24:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cbfs | tspi: Join hash calculation for verification and measurement
by Felix Held (Code Review) 03 Dec '21

03 Dec '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/59681 ) 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 Reviewed-on: https://review.coreboot.org/c/coreboot/+/59681 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- 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, 59 insertions(+), 97 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index b4b6eb1..0b58913 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -81,10 +81,6 @@ if (rdev_chain(rdev, &cbd->rdev, data_offset, be32toh(mdata->h.len))) return CB_ERR; - if (tspi_measure_cbfs_hook(rdev, name, be32toh(mdata->h.type))) { - printk(BIOS_ERR, "CBFS ERROR: error when measuring '%s'\n", name); - } - return CB_SUCCESS; } @@ -134,9 +130,8 @@ type = &dummy_type; ret = cbfs_locate(fh, &rdev, name, type); - if (!ret) - if (tspi_measure_cbfs_hook(&rdev, name, *type)) - LOG("error measuring %s in region %s\n", name, region_name); + /* No more measuring here, this function will be removed next patch. */ + return ret; } @@ -193,19 +188,36 @@ static bool cbfs_file_hash_mismatch(const void *buffer, size_t size, const union cbfs_mdata *mdata, bool skip_verification) { - /* Avoid linking hash functions when verification is disabled. */ - if (!CONFIG(CBFS_VERIFICATION) || skip_verification) + /* Avoid linking hash functions when verification and measurement are disabled. */ + if (!CONFIG(CBFS_VERIFICATION) && !CONFIG(TPM_MEASURED_BOOT)) return false; - const struct vb2_hash *hash = cbfs_file_hash(mdata); - if (!hash) { - ERROR("'%s' does not have a file hash!\n", mdata->h.filename); - return true; + const struct vb2_hash *hash = NULL; + + if (CONFIG(CBFS_VERIFICATION) && !skip_verification) { + hash = cbfs_file_hash(mdata); + if (!hash) { + ERROR("'%s' does not have a file hash!\n", mdata->h.filename); + return true; + } + if (vb2_hash_verify(buffer, size, hash) != VB2_SUCCESS) { + ERROR("'%s' file hash mismatch!\n", mdata->h.filename); + return true; + } } - if (vb2_hash_verify(buffer, size, hash) != VB2_SUCCESS) { - ERROR("'%s' file hash mismatch!\n", mdata->h.filename); - return true; + if (CONFIG(TPM_MEASURED_BOOT) && !ENV_SMM) { + struct vb2_hash calculated_hash; + + /* No need to re-hash file if we already have it from verification. */ + if (!hash || hash->algo != TPM_MEASURE_ALGO) { + vb2_hash_calculate(buffer, size, TPM_MEASURE_ALGO, &calculated_hash); + hash = &calculated_hash; + } + + if (tspi_cbfs_measurement(mdata->h.filename, be32toh(mdata->h.type), hash)) + ERROR("failed to measure '%s' into TCPA log\n", mdata->h.filename); + /* We intentionally continue to boot on measurement errors. */ } return false; @@ -528,9 +540,6 @@ if (rdev_chain(&file_rdev, &area_rdev, data_offset, be32toh(mdata.h.len))) return NULL; - if (tspi_measure_cbfs_hook(&file_rdev, name, be32toh(mdata.h.type))) - ERROR("error measuring '%s' in '%s'\n", name, area); - return do_alloc(&mdata, &file_rdev, allocator, arg, size_out, true); } diff --git a/src/security/tpm/tspi.h b/src/security/tpm/tspi.h index ed642c3..7157b4d 100644 --- a/src/security/tpm/tspi.h +++ b/src/security/tpm/tspi.h @@ -51,7 +51,7 @@ * @return TPM_SUCCESS on success. If not a tpm error is returned */ uint32_t tpm_extend_pcr(int pcr, enum vb2_hash_algorithm digest_algo, - uint8_t *digest, size_t digest_len, + const uint8_t *digest, size_t digest_len, const char *name); /** diff --git a/src/security/tpm/tspi/crtm.c b/src/security/tpm/tspi/crtm.c index b64bbbf..8cd0793 100644 --- a/src/security/tpm/tspi/crtm.c +++ b/src/security/tpm/tspi/crtm.c @@ -6,37 +6,6 @@ #include "crtm.h" #include <string.h> -/* - * This function sets the TCPA log namespace - * for the cbfs file (region) lookup. - */ -static int create_tcpa_metadata(const struct region_device *rdev, - const char *cbfs_name, char log_string[TCPA_PCR_HASH_NAME]) -{ - int i; - struct region_device fmap; - static const char *const fmap_cbfs_names[] = { - "COREBOOT", - "FW_MAIN_A", - "FW_MAIN_B", - "RW_LEGACY" - }; - - for (i = 0; i < ARRAY_SIZE(fmap_cbfs_names); i++) { - if (fmap_locate_area_as_rdev(fmap_cbfs_names[i], &fmap) == 0) { - if (region_is_subregion(region_device_region(&fmap), - region_device_region(rdev))) { - snprintf(log_string, TCPA_PCR_HASH_NAME, - "FMAP: %s CBFS: %s", - fmap_cbfs_names[i], cbfs_name); - return 0; - } - } - } - - return -1; -} - static int tcpa_log_initialized; static inline int tcpa_log_available(void) { @@ -64,8 +33,6 @@ */ static uint32_t tspi_init_crtm(void) { - struct prog bootblock = PROG_INIT(PROG_BOOTBLOCK, "bootblock"); - /* Initialize TCPA PRERAM log. */ if (!tcpa_log_available()) { tcpa_preram_log_clear(); @@ -87,7 +54,6 @@ } /* measure bootblock from RO */ - struct cbfsf bootblock_data; struct region_device bootblock_fmap; if (fmap_locate_area_as_rdev("BOOTBLOCK", &bootblock_fmap) == 0) { if (tpm_measure_region(&bootblock_fmap, @@ -95,16 +61,16 @@ "FMAP: BOOTBLOCK")) return VB2_ERROR_UNKNOWN; } else { - if (cbfs_boot_locate(&bootblock_data, - prog_name(&bootblock), NULL)) { - /* - * measurement is done in - * tspi_measure_cbfs_hook() - */ + /* Mapping measures the file. We know we can safely map here because + bootblock-as-a-file is only used on x86, where we don't need cache to map. */ + enum cbfs_type type = CBFS_TYPE_BOOTBLOCK; + void *mapping = cbfs_ro_type_map("bootblock", NULL, &type); + if (!mapping) { printk(BIOS_INFO, "TSPI: Couldn't measure bootblock into CRTM!\n"); return VB2_ERROR_UNKNOWN; } + cbfs_unmap(mapping); } return VB2_SUCCESS; @@ -129,8 +95,7 @@ return !strcmp(allowlist, name); } -uint32_t tspi_measure_cbfs_hook(const struct region_device *rdev, const char *name, - uint32_t cbfs_type) +uint32_t tspi_cbfs_measurement(const char *name, uint32_t type, const struct vb2_hash *hash) { uint32_t pcr_index; char tcpa_metadata[TCPA_PCR_HASH_NAME]; @@ -144,7 +109,7 @@ printk(BIOS_DEBUG, "CRTM initialized.\n"); } - switch (cbfs_type) { + switch (type) { case CBFS_TYPE_MRC_CACHE: pcr_index = TPM_RUNTIME_DATA_PCR; break; @@ -166,10 +131,10 @@ break; } - if (create_tcpa_metadata(rdev, name, tcpa_metadata) < 0) - return VB2_ERROR_UNKNOWN; + snprintf(tcpa_metadata, TCPA_PCR_HASH_NAME, "CBFS: %s", name); - return tpm_measure_region(rdev, pcr_index, tcpa_metadata); + return tpm_extend_pcr(pcr_index, hash->algo, hash->raw, vb2_digest_size(hash->algo), + tcpa_metadata); } int tspi_measure_cache_to_pcr(void) diff --git a/src/security/tpm/tspi/crtm.h b/src/security/tpm/tspi/crtm.h index 011fa26..8ebb661 100644 --- a/src/security/tpm/tspi/crtm.h +++ b/src/security/tpm/tspi/crtm.h @@ -7,6 +7,7 @@ #include <program_loading.h> #include <security/tpm/tspi.h> #include <types.h> +#include <vb2_sha.h> /* CRTM */ #define TPM_CRTM_PCR 2 @@ -16,21 +17,16 @@ */ #define TPM_RUNTIME_DATA_PCR 3 +#define TPM_MEASURE_ALGO (CONFIG(TPM1) ? VB2_HASH_SHA1 : VB2_HASH_SHA256) + /** * Measure digests cached in TCPA log entries into PCRs */ int tspi_measure_cache_to_pcr(void); -#if !ENV_SMM && CONFIG(TPM_MEASURED_BOOT) -/* - * Measures cbfs data via hook (cbfs) - * rdev covers the file data (not metadata) - * return 0 if successful, else an error +/** + * Extend a measurement hash taken for a CBFS file into the appropriate PCR. */ -uint32_t tspi_measure_cbfs_hook(const struct region_device *rdev, - const char *name, uint32_t cbfs_type); -#else -#define tspi_measure_cbfs_hook(rdev, name, cbfs_type) 0 -#endif +uint32_t tspi_cbfs_measurement(const char *name, uint32_t type, const struct vb2_hash *hash); #endif /* __SECURITY_TSPI_CRTM_H__ */ diff --git a/src/security/tpm/tspi/tspi.c b/src/security/tpm/tspi/tspi.c index b1bea41..e2d6d1e 100644 --- a/src/security/tpm/tspi/tspi.c +++ b/src/security/tpm/tspi/tspi.c @@ -220,7 +220,7 @@ } uint32_t tpm_extend_pcr(int pcr, enum vb2_hash_algorithm digest_algo, - uint8_t *digest, size_t digest_len, const char *name) + const uint8_t *digest, size_t digest_len, const char *name) { uint32_t result; @@ -234,16 +234,22 @@ return result; } - printk(BIOS_DEBUG, "TPM: Extending digest for %s into PCR %d\n", name, pcr); + printk(BIOS_DEBUG, "TPM: Extending digest for `%s` into PCR %d\n", name, pcr); result = tlcl_extend(pcr, digest, NULL); - if (result != TPM_SUCCESS) + if (result != TPM_SUCCESS) { + printk(BIOS_ERR, "TPM: Extending hash for `%s` into PCR %d failed.\n", + name, pcr); return result; + } } if (CONFIG(TPM_MEASURED_BOOT)) tcpa_log_add_table_entry(name, pcr, digest_algo, digest, digest_len); + printk(BIOS_DEBUG, "TPM: Digest of `%s` to PCR %d %s\n", + name, pcr, tspi_tpm_is_setup() ? "measured" : "logged"); + return TPM_SUCCESS; } @@ -253,23 +259,16 @@ { uint8_t digest[TPM_PCR_MAX_LEN], digest_len; uint8_t buf[HASH_DATA_CHUNK_SIZE]; - uint32_t result, offset; + uint32_t offset; size_t len; struct vb2_digest_context ctx; - enum vb2_hash_algorithm hash_alg; if (!rdev || !rname) return TPM_E_INVALID_ARG; - if (CONFIG(TPM1)) { - hash_alg = VB2_HASH_SHA1; - } else { /* CONFIG_TPM2 */ - hash_alg = VB2_HASH_SHA256; - } - - digest_len = vb2_digest_size(hash_alg); + digest_len = vb2_digest_size(TPM_MEASURE_ALGO); assert(digest_len <= sizeof(digest)); - if (vb2_digest_init(&ctx, hash_alg)) { + if (vb2_digest_init(&ctx, TPM_MEASURE_ALGO)) { printk(BIOS_ERR, "TPM: Error initializing hash.\n"); return TPM_E_HASH_ERROR; } @@ -294,13 +293,6 @@ printk(BIOS_ERR, "TPM: Error finalizing hash.\n"); return TPM_E_HASH_ERROR; } - result = tpm_extend_pcr(pcr, hash_alg, digest, digest_len, rname); - if (result != TPM_SUCCESS) { - printk(BIOS_ERR, "TPM: Extending hash into PCR failed.\n"); - return result; - } - printk(BIOS_DEBUG, "TPM: Digest of %s to PCR %d %s\n", - rname, pcr, tspi_tpm_is_setup() ? "measured" : "logged"); - return TPM_SUCCESS; + return tpm_extend_pcr(pcr, TPM_MEASURE_ALGO, digest, digest_len, rname); } #endif /* VBOOT_LIB */ -- 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: 4 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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: mb/google/brya/var/felwinter: Add WiFi SAR table for felwinter
by Felix Held (Code Review) 03 Dec '21

03 Dec '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/59718 ) Change subject: mb/google/brya/var/felwinter: Add WiFi SAR table for felwinter ...................................................................... mb/google/brya/var/felwinter: Add WiFi SAR table for felwinter Add WiFi SAR table for felwinter. BUG=b:206901900 TEST=emerge-brya chromeos-config chromeos-config-bsp-private coreboot-private-files-baseboard-brya coreboot chromeos-bootimage and checked SAR table can load by WiFi driver. Signed-off-by: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Change-Id: I0de710f4447302ee545a67cbd79373bdd2077637 Reviewed-on: https://review.coreboot.org/c/coreboot/+/59718 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Reviewed-by: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> --- M src/mainboard/google/brya/Kconfig.name M src/mainboard/google/brya/variants/felwinter/variant.c 2 files changed, 7 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified EricR Lai: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved diff --git a/src/mainboard/google/brya/Kconfig.name b/src/mainboard/google/brya/Kconfig.name index cd00815..ead3348 100644 --- a/src/mainboard/google/brya/Kconfig.name +++ b/src/mainboard/google/brya/Kconfig.name @@ -108,6 +108,7 @@ bool "-> Felwinter" select BOARD_GOOGLE_BASEBOARD_BRYA select DRIVERS_GENERIC_GPIO_KEYS + select CHROMEOS_WIFI_SAR if CHROMEOS config BOARD_GOOGLE_ANAHERA bool "-> Anahera" diff --git a/src/mainboard/google/brya/variants/felwinter/variant.c b/src/mainboard/google/brya/variants/felwinter/variant.c index ee39ef0..8b270de 100644 --- a/src/mainboard/google/brya/variants/felwinter/variant.c +++ b/src/mainboard/google/brya/variants/felwinter/variant.c @@ -3,6 +3,12 @@ #include <chip.h> #include <fw_config.h> #include <baseboard/variants.h> +#include <sar.h> + +const char *get_wifi_sar_cbfs_filename(void) +{ + return "wifi_sar_0.hex"; +} void variant_update_soc_chip_config(struct soc_intel_alderlake_config *config) { -- To view, visit https://review.coreboot.org/c/coreboot/+/59718 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0de710f4447302ee545a67cbd79373bdd2077637 Gerrit-Change-Number: 59718 Gerrit-PatchSet: 6 Gerrit-Owner: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com> Gerrit-CC: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Van Chen <van_chen(a)compal.corp-partner.google.com> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/intel/alderlake: Define the helper functions
by Tim Wawrzynczak (Code Review) 03 Dec '21

03 Dec '21

Attention is currently required from: Arthur Heymans, Sridhar Siricilla, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59362 ) Change subject: soc/intel/alderlake: Define the helper functions ...................................................................... Patch Set 7: (5 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/59362/comment/faf1b5e7_195d5e16 PS7, Line 7: the helper functions `Define CPPCv3 hybrid helper functions` https://review.coreboot.org/c/coreboot/+/59362/comment/dff765c5_b70dfc8f PS7, Line 9: defines following `defines the following` https://review.coreboot.org/c/coreboot/+/59362/comment/5dee461c_3ba9ca19 PS7, Line 12: TRUE `true` https://review.coreboot.org/c/coreboot/+/59362/comment/0bd8cbd1_d83956d7 PS7, Line 13: FASLE `false` Patchset: PS7: could possibly be squashed with next patch? -- To view, visit https://review.coreboot.org/c/coreboot/+/59362 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I963690a4fadad322095d202bcc08c92dcd845360 Gerrit-Change-Number: 59362 Gerrit-PatchSet: 7 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(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: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 03 Dec 2021 21:13:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Define soc_get_pcie_rp_type
by Subrata Banik (Code Review) 03 Dec '21

03 Dec '21

Attention is currently required from: Tim Wawrzynczak, Patrick Rudolph. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59853 ) Change subject: soc/intel/tigerlake: Define soc_get_pcie_rp_type ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File src/soc/intel/common/block/include/intelblocks/pcie_rp.h: https://review.coreboot.org/c/coreboot/+/59853/comment/b77f7c18_f5ee2104 PS3, Line 120: /*For PCIe RTD3 support, each SoC that uses it must implement this function. */ @Tim, one space after `/*` -- To view, visit https://review.coreboot.org/c/coreboot/+/59853 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic3f7d3f2fc12ae2b53604cd8f8b694a7674c3620 Gerrit-Change-Number: 59853 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 03 Dec 2021 21:13:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Enable CPPCv3
by Tim Wawrzynczak (Code Review) 03 Dec '21

03 Dec '21

Attention is currently required from: Paul Menzel, Sridhar Siricilla, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59363 ) Change subject: soc/intel/alderlake: Enable CPPCv3 ...................................................................... Patch Set 7: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/59363/comment/8d46a37d_2aedec5a PS5, Line 7: for Intel Alderlake > Remove as it’s in the prefix. Done https://review.coreboot.org/c/coreboot/+/59363/comment/ceb324f4_31aa023d PS5, Line 9: Alderlake > Alder Lake Done Commit Message: https://review.coreboot.org/c/coreboot/+/59363/comment/e31681b2_f2de0b3d PS7, Line 12: OS patches link to the patches? -- To view, visit https://review.coreboot.org/c/coreboot/+/59363 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icaacd4a4ba953d8337f557903ed2ea9da4a60fb9 Gerrit-Change-Number: 59363 Gerrit-PatchSet: 7 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(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-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 03 Dec 2021 21:13:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: acpi: Add #define for Mutex "no timeout" value
by Subrata Banik (Code Review) 03 Dec '21

03 Dec '21

Attention is currently required from: Tim Wawrzynczak. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59851 ) Change subject: acpi: Add #define for Mutex "no timeout" value ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/59851/comment/9ae64210_17bb37cb PS1, Line 8: > Done Thanks -- To view, visit https://review.coreboot.org/c/coreboot/+/59851 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I16bc9f3f04dd1e3dc0f3eca3e56377e6f48132b2 Gerrit-Change-Number: 59851 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> 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-Comment-Date: Fri, 03 Dec 2021 21:12:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Implement ACPI CPPCv3 package to support hybrid core
by Tim Wawrzynczak (Code Review) 03 Dec '21

03 Dec '21

Attention is currently required from: Arthur Heymans, Nico Huber, Paul Menzel, Ravindra, Sridhar Siricilla, Subrata Banik, Michael Niewöhner, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59359 ) Change subject: soc/intel/common: Implement ACPI CPPCv3 package to support hybrid core ...................................................................... Patch Set 7: (7 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/59359/comment/48291d36_0b55bd74 PS7, Line 15: XPPPC `XPPC` File src/soc/intel/common/block/acpi/cpu_hybrid.c: https://review.coreboot.org/c/coreboot/+/59359/comment/71360317_a7402d40 PS7, Line 26: u8 global_cpu_type[MAX_CPU_COUNT]; please don't reach into other modules' data; use an accessor function https://review.coreboot.org/c/coreboot/+/59359/comment/1faaf074_0343345b PS7, Line 62: void __weak get_cpu_scaling_factor(u16 *big_core_scal_factor, u16 *small_core_scal_factor) : { : } : : bool __weak cpu_is_nominal_freq_supported(void) : { : return 0; : } I don't think the weak definitions are necessary; this file is already guarded by the Kconfig SOC_INTEL_COMMON_BLOCK_ACPI_CPU_HYBRID, so if an SoC selects that, then it also has to implement these methods as part of that contract. https://review.coreboot.org/c/coreboot/+/59359/comment/c2f0aa52_fabb3d1a PS7, Line 115: 12 `sizeof(scope)` https://review.coreboot.org/c/coreboot/+/59359/comment/56ba0c09_43f7ecc1 PS7, Line 118: 16 `sizeof(scope)` File src/soc/intel/common/block/include/intelblocks/acpi.h: https://review.coreboot.org/c/coreboot/+/59359/comment/7de508d4_b6e10bda PS7, Line 21: /* It gets scaling factor for small and big core */ Since this has to be implemented per-SoC, I'd add a comment that mentions that and also prefix it with `soc_` https://review.coreboot.org/c/coreboot/+/59359/comment/c2bf418f_11ef764c PS7, Line 139: bool cpu_is_nominal_freq_supported(void); Since this has to be implemented per-SoC, I'd add a comment that mentions that and also prefix it with `soc_` -- To view, visit https://review.coreboot.org/c/coreboot/+/59359 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icd5ea9e70bebd1e66d3cea2bcf8a6678e5cc95ca Gerrit-Change-Number: 59359 Gerrit-PatchSet: 7 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Ravindra <ravindra(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> 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: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Ravindra <ravindra(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 03 Dec 2021 21:11:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: acpi/acpigen, drivers/tpm: Move definition of set_package_element_op
by Tim Wawrzynczak (Code Review) 03 Dec '21

03 Dec '21

Attention is currently required from: Lance Zhao, Sridhar Siricilla. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59729 ) Change subject: acpi/acpigen, drivers/tpm: Move definition of set_package_element_op ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/59729 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4fe70a1f369c97c67bbc1717e6adc7c5f4ba5839 Gerrit-Change-Number: 59729 Gerrit-PatchSet: 4 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Lance Zhao 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: Lance Zhao Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Comment-Date: Fri, 03 Dec 2021 21:00:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2021