coreboot-gerrit June 2023

coreboot-gerrit@coreboot.org

1 participants
5387 discussions

[XS] Change in coreboot[master]: allocator_v4: Fix top-level allocations w/o IORESOURCE_ABOVE_4G
by Paul Menzel (Code Review) 23 Jun '23

23 Jun '23

Attention is currently required from: Angel Pons, Arthur Heymans, Eric Lai, Felix Held, Nico Huber, Raul Rangel, Tim Wawrzynczak. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76090?usp=email ) Change subject: allocator_v4: Fix top-level allocations w/o IORESOURCE_ABOVE_4G ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/76090?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: Ied3a0695ef5e91f092bf2d442c1c482057643483 Gerrit-Change-Number: 76090 Gerrit-PatchSet: 4 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 23 Jun 2023 04:58:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[master]: mb/google/skyrim/variants/baseboard:Update system_configuration
by Paul Menzel (Code Review) 23 Jun '23

23 Jun '23

Attention is currently required from: Jason Glenesk, Jason Nien, Martin Roth, Tim Van Patten. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76091?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: mb/google/skyrim/variants/baseboard:Update system_configuration ...................................................................... Patch Set 2: (6 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/76091/comment/bcbeac04_dd8ccb6d : PS2, Line 7: :Update Please add a space. https://review.coreboot.org/c/coreboot/+/76091/comment/ea1f429f_70e3e8e8 : PS2, Line 7: system_configuration That parameter name seems too generic. https://review.coreboot.org/c/coreboot/+/76091/comment/49e96d9b_7296f1d9 : PS2, Line 7: mb/google/skyrim/variants/baseboard:Update system_configuration Maybe more specific: > mb/google/skyrim: Set system_configuration = 4 to avoid SMU call https://review.coreboot.org/c/coreboot/+/76091/comment/4293ad27_97610df8 : PS2, Line 10: " Is that quote in there? https://review.coreboot.org/c/coreboot/+/76091/comment/62993b9b_1974f823 : PS2, Line 12: that is not needed Is that causing some delay, or it’s just for correctness? https://review.coreboot.org/c/coreboot/+/76091/comment/d4d8a5b1_0fc6b1cf : PS2, Line 15: TEST=Confirm extra message is not sent in serial log. Could you please paste the message that is not sent? -- To view, visit https://review.coreboot.org/c/coreboot/+/76091?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: I1f3e305c48801b4e499de56d06c0dcd3eeacc626 Gerrit-Change-Number: 76091 Gerrit-PatchSet: 2 Gerrit-Owner: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Tim Van Patten <timvp(a)google.com> Gerrit-Comment-Date: Fri, 23 Jun 2023 04:57:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[master]: mb/google/rex: Enable GPIO for BT I2S offload for soundwire config
by Uday Bhat (Code Review) 23 Jun '23

23 Jun '23

Attention is currently required from: Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun Tuli. Uday Bhat has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76003?usp=email ) Change subject: mb/google/rex: Enable GPIO for BT I2S offload for soundwire config ...................................................................... Patch Set 3: (2 comments) File src/mainboard/google/rex/variants/rex0/fw_config.c: https://review.coreboot.org/c/coreboot/+/76003/comment/525da63f_4c44a475 : PS3, Line 94: BT I2S offload > shouldn't this is SNDW ? […] Will merge this CL and will change the print to "Configure GPIOs for BT offload mode" As there is already a print related to "Configure GPIOs for SoundWire audio" above BT offload configuration - explicit mentioning of SNDW may not be required https://review.coreboot.org/c/coreboot/+/76003/comment/5248ed8a_49252928 : PS3, Line 99: printk(BIOS_INFO, "Configure GPIOs for BT offload mode.\n"); > Configure GPIOs for BT offload mode with I2S audio ? […] As there is already a print related to "Configure GPIOs for I2S audio" above BT offload configuration - explicit mentioning of I2S audio in the print may not be required for BT offload. Hence, dropping this CL - hope its ok -- To view, visit https://review.coreboot.org/c/coreboot/+/76003?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: I65eae8839d72b266f5239a694c7e57cec856eedf Gerrit-Change-Number: 76003 Gerrit-PatchSet: 3 Gerrit-Owner: Uday Bhat <uday.m.bhat(a)intel.com> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Fri, 23 Jun 2023 04:51:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/rex: Enable Fast V-Mode for MTL-U 28W
by Subrata Banik (Code Review) 23 Jun '23

23 Jun '23

Attention is currently required from: Eric Herrmann, Jakub Czapiga, Jay Patel, Jérémy Compostella, Kapil Porwal, Tarun Tuli. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75764?usp=email ) Change subject: mb/google/rex: Enable Fast V-Mode for MTL-U 28W ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: > I have reached out internally to get all the necessary clarification. I will keep you posted. Thanks a lot. Looking forward to get further feedback from you. -- To view, visit https://review.coreboot.org/c/coreboot/+/75764?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: If834c1af8141a19fd2daae653579225b77cfa6c8 Gerrit-Change-Number: 75764 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Herrmann <eherrmann(a)chromium.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Jay Patel <jay2.patel(a)intel.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Eric Herrmann <eherrmann(a)chromium.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Jay Patel <jay2.patel(a)intel.com> Gerrit-Comment-Date: Fri, 23 Jun 2023 04:51:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: {commonlib/drivers}: Have option to store MRC version inside CBMEM
by Subrata Banik (Code Review) 23 Jun '23

23 Jun '23

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75921?usp=email ) ( 4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: {commonlib/drivers}: Have option to store MRC version inside CBMEM ...................................................................... {commonlib/drivers}: Have option to store MRC version inside CBMEM This patch introduces CBMEM ID to store the MRC version (similar to existing implementation that stores the FSP-M version inside CBMEM ID) inside cbmem so the version information is available across the different coreboot stages. For example: * romstage: Use the CBMEM ID version information to check if the MRC cache is valid and need to erase the MRC cache * ramstage: Use the CBMEM ID to store the MRC cache into the non-volatile space. BUG=b:261689642 TEST=Able to build and boot google/rex and dump the MRC version as below. cbmem --list CBMEM table of contents: NAME ID START LENGTH ... 21. MRC VERSION 5f43524d 75ffeb60 00000004 ... localhost ~ # cbmem -r 5f43524d | hexdump 00000000 01 12 07 00 Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I91f735239b33c6f8ba41c076048903e4b213c6a2 Reviewed-on: https://review.coreboot.org/c/coreboot/+/75921 Reviewed-by: Ronak Kanabar <ronak.kanabar(a)intel.com> Reviewed-by: Tarun Tuli <taruntuli(a)google.com> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/save_mrc_data.c 3 files changed, 28 insertions(+), 11 deletions(-) Approvals: Tarun Tuli: Looks good to me, approved Ronak Kanabar: Looks good to me, approved Kapil Porwal: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h b/src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h index 5080812..8e6e6cd 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h +++ b/src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h @@ -27,6 +27,7 @@ #define CBMEM_ID_FSP_RESERVED_MEMORY 0x46535052 #define CBMEM_ID_FSP_RUNTIME 0x52505346 #define CBMEM_ID_FSPM_VERSION 0x56505346 +#define CBMEM_ID_MRC_VERSION 0x5f43524d #define CBMEM_ID_GDT 0x4c474454 #define CBMEM_ID_HOB_POINTER 0x484f4221 #define CBMEM_ID_IGD_OPREGION 0x4f444749 @@ -112,6 +113,7 @@ { CBMEM_ID_FSP_RESERVED_MEMORY, "FSP MEMORY " }, \ { CBMEM_ID_FSP_RUNTIME, "FSP RUNTIME" }, \ { CBMEM_ID_FSPM_VERSION, "FSPM VERSION" }, \ + { CBMEM_ID_MRC_VERSION, "MRC VERSION" }, \ { CBMEM_ID_GDT, "GDT " }, \ { CBMEM_ID_HOB_POINTER, "HOB " }, \ { CBMEM_ID_IGD_OPREGION, "IGD OPREGION" }, \ diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index fa24a7e..28e4d72 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -30,10 +30,28 @@ static uint8_t temp_ram[CONFIG_FSP_TEMP_RAM_SIZE] __aligned(sizeof(uint64_t)); +/* + * Helper function to store the MRC cache version into CBMEM + * + * ramstage uses either the MRC version or FSP-M version (depending on the config) + * when updating the MRC cache + */ +static void do_cbmem_version_entry(uint32_t cbmem_id, uint32_t version) +{ + uint32_t *cbmem_version_entry = cbmem_add(cbmem_id, sizeof(version)); + if (!cbmem_version_entry) { + printk(BIOS_ERR, "Failed to add %s version to cbmem.\n", + CONFIG(MRC_CACHE_USING_MRC_VERSION) ? "MRC" : "FSP-M"); + return; + } + *cbmem_version_entry = version; +} + static void do_fsp_post_memory_init(bool s3wake, uint32_t version) { struct range_entry fsp_mem; - uint32_t *fsp_version_cbmem; + uint32_t cbmem_id = CONFIG(MRC_CACHE_USING_MRC_VERSION) ? CBMEM_ID_MRC_VERSION : + CBMEM_ID_FSPM_VERSION; fsp_find_reserved_memory(&fsp_mem); @@ -56,14 +74,8 @@ (uintptr_t)cbmem_find(CBMEM_ID_FSP_RESERVED_MEMORY)) die("Failed to accommodate FSP reserved memory request!\n"); - /* ramstage uses the FSP-M version when updating the MRC cache */ - if (CONFIG(CACHE_MRC_SETTINGS) && !s3wake) { - fsp_version_cbmem = cbmem_add(CBMEM_ID_FSPM_VERSION, - sizeof(version)); - if (!fsp_version_cbmem) - printk(BIOS_ERR, "Failed to add FSP-M version to cbmem.\n"); - *fsp_version_cbmem = version; - } + if (CONFIG(CACHE_MRC_SETTINGS) && !s3wake) + do_cbmem_version_entry(cbmem_id, version); /* Create romstage handof information */ romstage_handoff_init(s3wake); diff --git a/src/drivers/intel/fsp2_0/save_mrc_data.c b/src/drivers/intel/fsp2_0/save_mrc_data.c index d124942..19e8a52 100644 --- a/src/drivers/intel/fsp2_0/save_mrc_data.c +++ b/src/drivers/intel/fsp2_0/save_mrc_data.c @@ -11,14 +11,17 @@ { size_t mrc_data_size; const void *mrc_data; + uint32_t cbmem_id = CONFIG(MRC_CACHE_USING_MRC_VERSION) ? CBMEM_ID_MRC_VERSION : + CBMEM_ID_FSPM_VERSION; uint32_t *version; if (acpi_is_wakeup_s3()) return; - version = cbmem_find(CBMEM_ID_FSPM_VERSION); + version = cbmem_find(cbmem_id); if (!version) { - printk(BIOS_ERR, "Failed to read FSP-M version from cbmem.\n"); + printk(BIOS_ERR, "Failed to read %s version from cbmem.\n", + CONFIG(MRC_CACHE_USING_MRC_VERSION) ? "MRC" : "FSP-M"); return; } -- To view, visit https://review.coreboot.org/c/coreboot/+/75921?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: I91f735239b33c6f8ba41c076048903e4b213c6a2 Gerrit-Change-Number: 75921 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: driver/intel/fsp2_0: Add support to store MRC cache using MRC version
by Subrata Banik (Code Review) 23 Jun '23

23 Jun '23

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75920?usp=email ) Change subject: driver/intel/fsp2_0: Add support to store MRC cache using MRC version ...................................................................... driver/intel/fsp2_0: Add support to store MRC cache using MRC version This patch uses the "generic" variable name as "version" while storing the MRC cache data instead referring to the FSP-M version or MRC version. Hence, updated all the instances of `fsp_version/fspm_version` with `version`. Also introduces the new option to the MRC cache version that allows SoC users to store the MRC cache version based on the supported EDK2 version. Intel FSP built with EDK2 version 202302 onwards has support to retrieve the MRC version by directly parsing the binary. Additionally, added the helper function `fsp_mrc_version()` and corresponding header file to read the MRC version from the FSP binary. BUG=b:261689642 TEST=Able to build and boot google/rex and google/omnigul. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: Ia8af53aed674ad4a3b426264706264df91d9c6b0 Reviewed-on: https://review.coreboot.org/c/coreboot/+/75920 Reviewed-by: Tarun Tuli <taruntuli(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Ronak Kanabar <ronak.kanabar(a)intel.com> Reviewed-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> --- M src/drivers/intel/fsp2_0/include/fsp/soc_binding.h M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/save_mrc_data.c M src/drivers/mrc_cache/Kconfig 4 files changed, 63 insertions(+), 16 deletions(-) Approvals: Nico Huber: Looks good to me, but someone else must approve build bot (Jenkins): Verified Kapil Porwal: Looks good to me, approved Tarun Tuli: Looks good to me, approved Ronak Kanabar: Looks good to me, approved diff --git a/src/drivers/intel/fsp2_0/include/fsp/soc_binding.h b/src/drivers/intel/fsp2_0/include/fsp/soc_binding.h index 02cd4e0..952bc4c 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/soc_binding.h +++ b/src/drivers/intel/fsp2_0/include/fsp/soc_binding.h @@ -27,6 +27,10 @@ #include <Base.h> #include <FspmUpd.h> #include <FspsUpd.h> +#if CONFIG(MRC_CACHE_USING_MRC_VERSION) +#define BUILD_TIME_STAMP_SIZE 12 +#include <FspProducerDataHeader.h> +#endif #if CONFIG(DISPLAY_FSP_VERSION_INFO) #include <FirmwareVersionInfoHob.h> #elif CONFIG(DISPLAY_FSP_VERSION_INFO_2) diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index 6d25844..fa24a7e 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -30,7 +30,7 @@ static uint8_t temp_ram[CONFIG_FSP_TEMP_RAM_SIZE] __aligned(sizeof(uint64_t)); -static void do_fsp_post_memory_init(bool s3wake, uint32_t fsp_version) +static void do_fsp_post_memory_init(bool s3wake, uint32_t version) { struct range_entry fsp_mem; uint32_t *fsp_version_cbmem; @@ -59,17 +59,17 @@ /* ramstage uses the FSP-M version when updating the MRC cache */ if (CONFIG(CACHE_MRC_SETTINGS) && !s3wake) { fsp_version_cbmem = cbmem_add(CBMEM_ID_FSPM_VERSION, - sizeof(fsp_version)); + sizeof(version)); if (!fsp_version_cbmem) printk(BIOS_ERR, "Failed to add FSP-M version to cbmem.\n"); - *fsp_version_cbmem = fsp_version; + *fsp_version_cbmem = version; } /* Create romstage handof information */ romstage_handoff_init(s3wake); } -static void fsp_fill_mrc_cache(FSPM_ARCH_UPD *arch_upd, uint32_t fsp_version) +static void fsp_fill_mrc_cache(FSPM_ARCH_UPD *arch_upd, uint32_t version) { void *data; size_t mrc_size; @@ -82,7 +82,7 @@ /* Assume boot device is memory mapped. */ assert(CONFIG(BOOT_DEVICE_MEMORY_MAPPED)); - data = mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, fsp_version, + data = mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, version, &mrc_size); if (data == NULL) return; @@ -134,7 +134,7 @@ } static enum cb_err fsp_fill_common_arch_params(FSPM_ARCH_UPD *arch_upd, - bool s3wake, uint32_t fsp_version, + bool s3wake, uint32_t version, const struct memranges *memmap) { /* @@ -152,7 +152,7 @@ return CB_ERR; } - fsp_fill_mrc_cache(arch_upd, fsp_version); + fsp_fill_mrc_cache(arch_upd, version); /* Configure bootmode */ if (s3wake) { @@ -210,19 +210,50 @@ struct memranges memmap; }; +/* + * Helper function to read MRC version + * + * There are multiple ways to read the MRC version using + * Intel FSP. Currently the only supported method to get the + * MRC version is by reading the FSP_PRODUCDER_DATA_TABLES + * from the FSP-M binary (by parsing the FSP header). + */ +static uint32_t fsp_mrc_version(void) +{ + uint32_t ver = 0; +#if CONFIG(MRC_CACHE_USING_MRC_VERSION) + size_t fspm_blob_size; + void *fspm_blob_file = cbfs_map(CONFIG_FSP_M_CBFS, &fspm_blob_size); + if (!fspm_blob_file) + return 0; + + FSP_PRODUCER_DATA_TABLES *ft = fspm_blob_file + FSP_HDR_OFFSET; + FSP_PRODUCER_DATA_TYPE2 *table2 = &ft->FspProduceDataType2; + size_t mrc_version_size = sizeof(table2->MrcVersion); + for (size_t i = 0; i < mrc_version_size; i++) { + ver |= (table2->MrcVersion[i] << ((mrc_version_size - 1) - i) * 8); + } + cbfs_unmap(fspm_blob_file); +#endif + return ver; +} + static void do_fsp_memory_init(const struct fspm_context *context, bool s3wake) { uint32_t status; fsp_memory_init_fn fsp_raminit; FSPM_UPD fspm_upd, *upd; FSPM_ARCH_UPD *arch_upd; - uint32_t fsp_version; + uint32_t version; const struct fsp_header *hdr = &context->header; const struct memranges *memmap = &context->memmap; post_code(POST_MEM_PREINIT_PREP_START); - fsp_version = fsp_memory_settings_version(hdr); + if (CONFIG(MRC_CACHE_USING_MRC_VERSION)) + version = fsp_mrc_version(); + else + version = fsp_memory_settings_version(hdr); upd = (FSPM_UPD *)(uintptr_t)(hdr->cfg_region_offset + hdr->image_base); @@ -254,7 +285,7 @@ arch_upd->BootLoaderTolumSize = cbmem_overhead_size(); /* Fill common settings on behalf of chipset. */ - if (fsp_fill_common_arch_params(arch_upd, s3wake, fsp_version, + if (fsp_fill_common_arch_params(arch_upd, s3wake, version, memmap) != CB_SUCCESS) die_with_post_code(POST_INVALID_VENDOR_BINARY, "FSPM_ARCH_UPD not found!\n"); @@ -266,7 +297,7 @@ #endif /* Give SoC and mainboard a chance to update the UPD */ - platform_fsp_memory_init_params_cb(&fspm_upd, fsp_version); + platform_fsp_memory_init_params_cb(&fspm_upd, version); /* * For S3 resume case, if valid mrc cache data is not found or @@ -309,7 +340,7 @@ "FspMemoryInit returned with error 0x%08x!\n", status); } - do_fsp_post_memory_init(s3wake, fsp_version); + do_fsp_post_memory_init(s3wake, version); /* * fsp_debug_after_memory_init() checks whether the end of the tolum diff --git a/src/drivers/intel/fsp2_0/save_mrc_data.c b/src/drivers/intel/fsp2_0/save_mrc_data.c index 8550b68..d124942 100644 --- a/src/drivers/intel/fsp2_0/save_mrc_data.c +++ b/src/drivers/intel/fsp2_0/save_mrc_data.c @@ -11,13 +11,13 @@ { size_t mrc_data_size; const void *mrc_data; - uint32_t *fspm_version; + uint32_t *version; if (acpi_is_wakeup_s3()) return; - fspm_version = cbmem_find(CBMEM_ID_FSPM_VERSION); - if (!fspm_version) { + version = cbmem_find(CBMEM_ID_FSPM_VERSION); + if (!version) { printk(BIOS_ERR, "Failed to read FSP-M version from cbmem.\n"); return; } @@ -34,7 +34,7 @@ * code which saves the data to flash doesn't write if the latest * training data matches this one. */ - if (mrc_cache_stash_data(MRC_TRAINING_DATA, *fspm_version, mrc_data, + if (mrc_cache_stash_data(MRC_TRAINING_DATA, *version, mrc_data, mrc_data_size) < 0) printk(BIOS_ERR, "Failed to stash MRC data\n"); } diff --git a/src/drivers/mrc_cache/Kconfig b/src/drivers/mrc_cache/Kconfig index 616e5f5..403c501 100644 --- a/src/drivers/mrc_cache/Kconfig +++ b/src/drivers/mrc_cache/Kconfig @@ -52,4 +52,16 @@ Store a hash of the MRC_CACHE training data in a TPM NVRAM space to ensure that it cannot be tampered with. +config MRC_CACHE_USING_MRC_VERSION + bool + default y if UDK_VERSION >= UDK_202302_VERSION + default n + help + Use the MRC version info from FSP extended header to store the MRC cache data. + This method relies on the FSP_PRODUCER_DATA_TABLES belongs to the + `FspProducerDataHeader.h`file to get the MRC version. + + Intel FSP built with EDK2 version 202302 onwards has support to retrieve the + MRC version by directly parsing the binary. + endif # CACHE_MRC_SETTINGS -- To view, visit https://review.coreboot.org/c/coreboot/+/75920?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: Ia8af53aed674ad4a3b426264706264df91d9c6b0 Gerrit-Change-Number: 75920 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.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-MessageType: merged

1 0

[XS] Change in coreboot[master]: soc/amd/block/ivrs: Add NULL check for IVRS
by Naresh Solanki (Code Review) 23 Jun '23

23 Jun '23

Attention is currently required from: Arthur Heymans, Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, Raul Rangel. Naresh Solanki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75506?usp=email ) Change subject: soc/amd/block/ivrs: Add NULL check for IVRS ...................................................................... Patch Set 3: (1 comment) File src/soc/amd/common/block/acpi/ivrs.c: https://review.coreboot.org/c/coreboot/+/75506/comment/54babe0c_b2a396ad : PS3, Line 339: return 0; > That will really mess things up. Returning current is better. […] Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/75506?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: Ibeb0ea3bcaa3512a93500588ad4f11046edee61f Gerrit-Change-Number: 75506 Gerrit-PatchSet: 3 Gerrit-Owner: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 23 Jun 2023 03:52:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[XL] Change in coreboot[master]: arch/x86,lib: Migrate SMBIOS implementation to common code
by 9elements QA (Code Review) 23 Jun '23

23 Jun '23

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75886?usp=email ) Change subject: arch/x86,lib: Migrate SMBIOS implementation to common code ...................................................................... Patch Set 4: Automatic boot test returned (PASS/FAIL/TOTAL): 14 / 2 / 16 PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES_ and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/170141 PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/170139 PASS: x86_32 "ThinkPad T500" , build config LENOVO_T500 and payload SeaBIOS : https://lava.9esec.io/r/170138 PASS: x86_32 "HP Z220 SFF Workstation" , build config HP_Z220_SFF_WORKSTATION and payload LinuxBoot_BB_kexec : https://lava.9esec.io/r/170137 FAIL: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload TianoCore : https://lava.9esec.io/r/170136 FAIL: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload SeaBIOS : https://lava.9esec.io/r/170135 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload TianoCore : https://lava.9esec.io/r/170134 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload SeaBIOS : https://lava.9esec.io/r/170133 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload TianoCore : https://lava.9esec.io/r/170132 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload SeaBIOS : https://lava.9esec.io/r/170131 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload TianoCore : https://lava.9esec.io/r/170130 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload SeaBIOS : https://lava.9esec.io/r/170129 PASS: x86_64 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_X86_64 and payload SeaBIOS : https://lava.9esec.io/r/170128 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ASAN and payload SeaBIOS : https://lava.9esec.io/r/170127 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ and payload SeaBIOS : https://lava.9esec.io/r/170126 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX and payload SeaBIOS : https://lava.9esec.io/r/170125 Please note: This test is under development and might not be accurate at all! -- To view, visit https://review.coreboot.org/c/coreboot/+/75886?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: I030c853f83f8427da4a4c661b82a6487938b24e6 Gerrit-Change-Number: 75886 Gerrit-PatchSet: 4 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-Comment-Date: Fri, 23 Jun 2023 03:27:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/{cfl,cml,whl}: Use true/false macros for s0ix_enable dt option
by Felix Singer (Code Review) 23 Jun '23

23 Jun '23

Attention is currently required from: Christian Walter, Michael Niewöhner, Patrick Rudolph, Tarun Tuli, Yuchen He. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75960?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: mb/{cfl,cml,whl}: Use true/false macros for s0ix_enable dt option ...................................................................... Patch Set 4: -Code-Review (1 comment) Patchset: PS4: Have a look at the CB:75888. I made the script more generic and I let it iterate over all related options allowing us to submit only one patch, which makes the git history much less noisy. -- To view, visit https://review.coreboot.org/c/coreboot/+/75960?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: Ibfd1b1dc7d3a210e30d7007876f5aa8a8a0d4d34 Gerrit-Change-Number: 75960 Gerrit-PatchSet: 4 Gerrit-Owner: Yuchen He <yuchenhe126(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Yuchen He <yuchenhe126(a)gmail.com> Gerrit-Comment-Date: Fri, 23 Jun 2023 03:15:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/{skl,kbl}: Use true/false macros for eist_enable dt option
by Felix Singer (Code Review) 23 Jun '23

23 Jun '23

Felix Singer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/75965?usp=email ) Change subject: mb/{skl,kbl}: Use true/false macros for eist_enable dt option ...................................................................... Abandoned Covered by CB:75888 now. -- To view, visit https://review.coreboot.org/c/coreboot/+/75965?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: I9ff24082370d70b9f645ca936cf2c66d58ae4a48 Gerrit-Change-Number: 75965 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2023