coreboot-gerrit May 2023

coreboot-gerrit@coreboot.org

1 participants
5262 discussions

[S] Change in coreboot[master]: payloads/edk2: Remove ABOVE_4G_MEMORY option
by Matt DeVillier (Code Review) 02 May '23

02 May '23

Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74336 ) Change subject: payloads/edk2: Remove ABOVE_4G_MEMORY option ...................................................................... payloads/edk2: Remove ABOVE_4G_MEMORY option Remove the ABOVE_4G_MEMORY option as the option was removed in edk2 in commit dc5f2905ebfdf68ae28ce1081d435af0f8641dd9 (UefiPayloadPkg: Always build MemoryTypeInformation HOB for DXE GCD - https://github.com/tianocore/edk2/pull/4231). Signed-off-by: Sean Rhodes <sean(a)starlabs.systems> Change-Id: I8d5ee79ef3f7ecfcd1463c612aad2e3d629df22a Reviewed-on: https://review.coreboot.org/c/coreboot/+/74336 Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M payloads/external/Makefile.inc M payloads/external/edk2/Kconfig M payloads/external/edk2/Makefile 3 files changed, 20 insertions(+), 16 deletions(-) Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Matt DeVillier: Looks good to me, approved Lean Sheng Tan: Looks good to me, approved diff --git a/payloads/external/Makefile.inc b/payloads/external/Makefile.inc index 5f29063..d10cb15 100644 --- a/payloads/external/Makefile.inc +++ b/payloads/external/Makefile.inc @@ -172,7 +172,6 @@ CONFIG_EDK2_DEBUG=$(CONFIG_EDK2_DEBUG) \ CONFIG_EDK2_RELEASE=$(CONFIG_EDK2_RELEASE) \ CONFIG_EDK2_VERBOSE_BUILD=$(CONFIG_EDK2_VERBOSE_BUILD) \ - CONFIG_EDK2_ABOVE_4G_MEMORY=$(CONFIG_EDK2_ABOVE_4G_MEMORY) \ CONFIG_EDK2_BOOTSPLASH_FILE=$(CONFIG_EDK2_BOOTSPLASH_FILE) \ CONFIG_EDK2_BOOT_MANAGER_ESCAPE=$(CONFIG_EDK2_BOOT_MANAGER_ESCAPE) \ CONFIG_EDK2_BOOT_TIMEOUT=$(CONFIG_EDK2_BOOT_TIMEOUT) \ @@ -213,7 +212,6 @@ CONFIG_EDK2_CUSTOM_BUILD_PARAMS=$(CONFIG_EDK2_CUSTOM_BUILD_PARAMS) \ CONFIG_EDK2_DEBUG=$(CONFIG_EDK2_DEBUG) \ CONFIG_EDK2_RELEASE=$(CONFIG_EDK2_RELEASE) \ - CONFIG_EDK2_ABOVE_4G_MEMORY=$(CONFIG_EDK2_ABOVE_4G_MEMORY) \ CONFIG_EDK2_BOOTSPLASH_FILE=$(CONFIG_EDK2_BOOTSPLASH_FILE) \ CONFIG_EDK2_BOOT_MANAGER_ESCAPE=$(CONFIG_EDK2_BOOT_MANAGER_ESCAPE) \ CONFIG_EDK2_BOOT_TIMEOUT=$(CONFIG_EDK2_BOOT_TIMEOUT) \ diff --git a/payloads/external/edk2/Kconfig b/payloads/external/edk2/Kconfig index c166975..fd6bc9b 100644 --- a/payloads/external/edk2/Kconfig +++ b/payloads/external/edk2/Kconfig @@ -111,16 +111,6 @@ build log extremely verbose. This can be used to troubleshoot failed builds which are usually down to missing tools or toolchain. -config EDK2_ABOVE_4G_MEMORY - bool "Enable above 4G memory" - default n - help - Select this option to enable Above 4G Decode. This will allow the - payload to use all of the memory, rather than an maximum of 4G. - - Disabling memory above 4G is useful for bootloaders that are not - fully 64-bit aware such as Qubes R4.0.4 bootloader. - config EDK2_BOOTSPLASH_FILE string "edk2 Bootsplash path and filename" default "Documentation/coreboot_logo.bmp" diff --git a/payloads/external/edk2/Makefile b/payloads/external/edk2/Makefile index 2181242..a2a133d 100644 --- a/payloads/external/edk2/Makefile +++ b/payloads/external/edk2/Makefile @@ -43,10 +43,6 @@ # # OPTION = DEFAULT_VALUE # -# ABOVE_4G_MEMORY = TRUE -ifneq ($(CONFIG_EDK2_ABOVE_4G_MEMORY),y) -BUILD_STR += -D ABOVE_4G_MEMORY=FALSE -endif # BOOTSPLASH_IMAGE = FALSE ifneq ($(CONFIG_EDK2_BOOTSPLASH_FILE),) BUILD_STR += -D BOOTSPLASH_IMAGE=TRUE -- To view, visit https://review.coreboot.org/c/coreboot/+/74336 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8d5ee79ef3f7ecfcd1463c612aad2e3d629df22a Gerrit-Change-Number: 74336 Gerrit-PatchSet: 5 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(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: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: cpu/intel/speedstep: Use acpigen_write_processor_device()
by Nico Huber (Code Review) 02 May '23

02 May '23

Attention is currently required from: Arthur Heymans, Kyösti Mälkki. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74441 ) Change subject: cpu/intel/speedstep: Use acpigen_write_processor_device() ...................................................................... Patch Set 4: (3 comments) File src/southbridge/intel/i82801gx/fadt.c: https://review.coreboot.org/c/coreboot/+/74441/comment/617fb54c_662de538 PS4, Line 29: fadt->p_lvl3_lat = chip->c3_latency; This leaves the `c3_latency` field blind. Would it be much hassle to port all ports setting it to _CST and drop it completely? File src/southbridge/intel/i82801jx/fadt.c: https://review.coreboot.org/c/coreboot/+/74441/comment/856c237e_0082e37e PS4, Line 25: fadt->p_lvl3_lat = 0; /* FIXME: Is this correct? */ ACPI spec: "A value > 1000 indicates the system does not support a C3 state.". I think it's ok to change spurious values like this 0 to ffff or similar. But why should we drop valid values like the 1 above? File src/southbridge/intel/i82801jx/lpc.c: https://review.coreboot.org/c/coreboot/+/74441/comment/ab2eddd6_fbb8d8a0 PS4, Line 159: return 0; AIUI, this is to keep the current behavior and not to reflect the hardware support? Then a comment would be good. (there's also a chance that the CPU emulates it. on newer CPUs it's part of the MSR_PMG_IO_CAPTURE_ADDR configuration (even though there's the +4 offset given). couldn't find any documentation for older ones, it seems coreboot code preceded public documentation.) -- To view, visit https://review.coreboot.org/c/coreboot/+/74441 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If1950ceac7daf8d8e91c74f1090c7451cb92e100 Gerrit-Change-Number: 74441 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Tue, 02 May 2023 13:47:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: payloads/edk2: Add Kconfig to enable UEFI Secure Boot support
by Matt DeVillier (Code Review) 02 May '23

02 May '23

Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74869 ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: payloads/edk2: Add Kconfig to enable UEFI Secure Boot support ...................................................................... payloads/edk2: Add Kconfig to enable UEFI Secure Boot support Now that MrChromebox's default edk2 branch supports Secure Boot, add a Kconfig to enable it, and do so by default when MrChromebox's branch is used and SMMSTORE_V2 is enabled (which is a prerequisite). TEST=build/boot google boards link, panther, lulu,reef, ampton, akemi, and banshee, verify Secure Boot options available in payload, Secure Boot status reported properly by Linux/Windows. Change-Id: I4be58c3315cabe08729d717c59203fdc6a3e2958 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/74869 Reviewed-by: Sean Rhodes <sean(a)starlabs.systems> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M payloads/external/Makefile.inc M payloads/external/edk2/Kconfig M payloads/external/edk2/Makefile 3 files changed, 35 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Sean Rhodes: Looks good to me, approved diff --git a/payloads/external/Makefile.inc b/payloads/external/Makefile.inc index effab43..5f29063 100644 --- a/payloads/external/Makefile.inc +++ b/payloads/external/Makefile.inc @@ -188,6 +188,7 @@ CONFIG_ECAM_MMCONF_LENGTH=$(CONFIG_ECAM_MMCONF_LENGTH) \ CONFIG_CPU_XTAL_HZ=$(CONFIG_CPU_XTAL_HZ) \ CONFIG_SMMSTORE_V2=$(CONFIG_SMMSTORE_v2) \ + CONFIG_EDK2_SECURE_BOOT_SUPPORT=$(CONFIG_EDK2_SECURE_BOOT_SUPPORT) \ GCC_CC_x86_32=$(GCC_CC_x86_32) \ GCC_CC_x86_64=$(GCC_CC_x86_64) \ GCC_CC_arm=$(GCC_CC_arm) \ diff --git a/payloads/external/edk2/Kconfig b/payloads/external/edk2/Kconfig index 2c8152f..c166975 100644 --- a/payloads/external/edk2/Kconfig +++ b/payloads/external/edk2/Kconfig @@ -242,6 +242,14 @@ Enable serial port output in edk2. Serial output limits the performance of edk2's FrontPage. +config EDK2_SECURE_BOOT_SUPPORT + bool "Enable UEFI Secure Boot support" + depends on EDK2_REPO_MRCHROMEBOX && SMMSTORE_V2 + default y if EDK2_REPO_MRCHROMEBOX && SMMSTORE_V2 + help + Select this option to enable UEFI SecureBoot support in edk2. + UEFI SecureBoot will be disabled by default and can be enabled from the menu option. + config EDK2_CUSTOM_BUILD_PARAMS string "edk2 additional custom build parameters" default "-D VARIABLE_SUPPORT=SMMSTORE" if EDK2_REPO_MRCHROMEBOX && SMMSTORE_V2 diff --git a/payloads/external/edk2/Makefile b/payloads/external/edk2/Makefile index b241cf9..2181242 100644 --- a/payloads/external/edk2/Makefile +++ b/payloads/external/edk2/Makefile @@ -111,6 +111,10 @@ ifneq ($(CONFIG_EDK2_SD_MMC_TIMEOUT),) BUILD_STR += -D SD_MMC_TIMEOUT=$(shell echo $$(( $(CONFIG_EDK2_SD_MMC_TIMEOUT) * 1000)) ) endif +# EDK2_SECURE_BOOT_SUPPORT = FALSE +ifeq ($(CONFIG_EDK2_SECURE_BOOT_SUPPORT), y) +BUILD_STR += -D SECURE_BOOT_ENABLE=TRUE +endif # # EDKII has the below PCDs that are relevant to coreboot: -- To view, visit https://review.coreboot.org/c/coreboot/+/74869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4be58c3315cabe08729d717c59203fdc6a3e2958 Gerrit-Change-Number: 74869 Gerrit-PatchSet: 5 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: payloads/edk2: Update default branch for MrChromebox repo to 2023-04
by Matt DeVillier (Code Review) 02 May '23

02 May '23

Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74868 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: payloads/edk2: Update default branch for MrChromebox repo to 2023-04 ...................................................................... payloads/edk2: Update default branch for MrChromebox repo to 2023-04 Update the default branch used for MrChromebox's edk2 fork from 2022-07 to 2023-04. This updated branch has been rebased on the latest upstream stable tag (edk2-stable202302), and adds support for UEFI Secure Boot and TPM 1.2/2.0 management (though it does not currently support Google CR50/Ti50 TPMs). TEST=build/boot google boards link, panther, lulu,reef, ampton, akemi, and banshee with edk2 payload selected. Change-Id: I096eaa4e065db731a70ba238ba5a3bb49e5db867 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/74868 Reviewed-by: Sean Rhodes <sean(a)starlabs.systems> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M payloads/external/edk2/Kconfig 1 file changed, 24 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Sean Rhodes: Looks good to me, approved diff --git a/payloads/external/edk2/Kconfig b/payloads/external/edk2/Kconfig index e3bb157..2c8152f 100644 --- a/payloads/external/edk2/Kconfig +++ b/payloads/external/edk2/Kconfig @@ -53,7 +53,7 @@ config EDK2_TAG_OR_REV string "Insert a commit's SHA-1 or a branch name" - default "origin/uefipayload_202207" if EDK2_REPO_MRCHROMEBOX + default "origin/uefipayload_202304" if EDK2_REPO_MRCHROMEBOX default "origin/master" if EDK2_REPO_OFFICIAL default "" if EDK2_REPO_CUSTOM help -- To view, visit https://review.coreboot.org/c/coreboot/+/74868 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I096eaa4e065db731a70ba238ba5a3bb49e5db867 Gerrit-Change-Number: 74868 Gerrit-PatchSet: 4 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: mb/google/link: Apply symmetry for EC events defines
by Matt DeVillier (Code Review) 02 May '23

02 May '23

Attention is currently required from: Kyösti Mälkki. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74602 ) Change subject: mb/google/link: Apply symmetry for EC events defines ...................................................................... Patch Set 4: (1 comment) File src/mainboard/google/link/mainboard.c: https://review.coreboot.org/c/coreboot/+/74602/comment/506b3993_e5d134ea PS4, Line 111: mainboard_ec_init(); looks like this is missing a declaration -- To view, visit https://review.coreboot.org/c/coreboot/+/74602 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I97d9d28963c97e780156d75b39deac069028866a Gerrit-Change-Number: 74602 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Tue, 02 May 2023 13:40:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: Drop many cases of CONFIG_MAINBOARD_PART_NUMBER
by Felix Singer (Code Review) 02 May '23

02 May '23

Attention is currently required from: Michał Żygowski, Jason Nien, Michał Kopeć, Martin Roth, Kyösti Mälkki, Yidi Lin, Yu-Ping Wu, Piotr Król. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74450 ) Change subject: Drop many cases of CONFIG_MAINBOARD_PART_NUMBER ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74450 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I917222922560c6273b4be91cd7d99ce2ff8e4231 Gerrit-Change-Number: 74450 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> 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: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Comment-Date: Tue, 02 May 2023 13:24:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: Drop many cases of CONFIG_MAINBOARD_PART_NUMBER
by Arthur Heymans (Code Review) 02 May '23

02 May '23

Attention is currently required from: Michał Żygowski, Jason Nien, Michał Kopeć, Martin Roth, Kyösti Mälkki, Yidi Lin, Yu-Ping Wu, Piotr Król. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74450 ) Change subject: Drop many cases of CONFIG_MAINBOARD_PART_NUMBER ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74450 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I917222922560c6273b4be91cd7d99ce2ff8e4231 Gerrit-Change-Number: 74450 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> 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: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Comment-Date: Tue, 02 May 2023 13:21:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: cpu/intel/microcode: Implement microcode info caching inside cbmem
by Subrata Banik (Code Review) 02 May '23

02 May '23

Attention is currently required from: Tarun Tuli, Reka Norman, Kapil Porwal, Julius Werner, Arthur Heymans, Lean Sheng Tan. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74873 ) Change subject: cpu/intel/microcode: Implement microcode info caching inside cbmem ...................................................................... Patch Set 4: (1 comment) Patchset: PS3: > > > > > > I assume this patch is in response to b/242473942? And your time savings aren't really from "searching", they're from bypassing CBFS verification? > > > > > > > > > > > > If we determined that we actually *want* to bypass CBFS verification, and that it is safe to do so, there are more straight-forward ways to do that (e.g. cbfs_unverified_area_map()). But my understanding from the previous discussion on the bug was that we don't actually want to do that because there could actually be some attack vector here (even if it's e.g. just a rollback attack). In that case, we obviously also can't do something like this. > > > > > > > > > > yes, your understanding is correct that we don't want to bypass the CBFS verification check > > > > > > > > also i have noticed ~10ms boot time impact with every new ucode (per cpu stepping) being added into the cpu_microcode_blob.bin > > > > > > > > for example: current REX (MTL) added 2 ucode for ES1 and ES2 hence, the timestamp ID 971 was around 20ms. if we drop ES2 support and keep only ES1 then the boot time (timestamp ID 971) is 10ms > > > > > > Instead of concatenating microcode you could add a cbfs file per cpuid? That would not bypass CBFS verification and result in the same speedup. > > > > isn't the expectation inside find_cbfs_microcode() function is that, there would be one single microcode entry inside CBFS? > > Yes. My suggestion implies changing that :-) Ah, that should be long pole as we would like to get some solution for existing devices where we are not meeting the KPI numbers by 10-15ms 😭 -- To view, visit https://review.coreboot.org/c/coreboot/+/74873 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I208a2e0e1dce96b2d4d63f882e8aaf4c25f77e3f Gerrit-Change-Number: 74873 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Tue, 02 May 2023 13:07:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: cpu/intel/microcode: Implement microcode info caching inside cbmem
by Arthur Heymans (Code Review) 02 May '23

02 May '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Reka Norman, Kapil Porwal, Julius Werner, Lean Sheng Tan. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74873 ) Change subject: cpu/intel/microcode: Implement microcode info caching inside cbmem ...................................................................... Patch Set 4: (1 comment) Patchset: PS3: > > > > > I assume this patch is in response to b/242473942? And your time savings aren't really from "searching", they're from bypassing CBFS verification? > > > > > > > > > > If we determined that we actually *want* to bypass CBFS verification, and that it is safe to do so, there are more straight-forward ways to do that (e.g. cbfs_unverified_area_map()). But my understanding from the previous discussion on the bug was that we don't actually want to do that because there could actually be some attack vector here (even if it's e.g. just a rollback attack). In that case, we obviously also can't do something like this. > > > > > > > > yes, your understanding is correct that we don't want to bypass the CBFS verification check > > > > > > also i have noticed ~10ms boot time impact with every new ucode (per cpu stepping) being added into the cpu_microcode_blob.bin > > > > > > for example: current REX (MTL) added 2 ucode for ES1 and ES2 hence, the timestamp ID 971 was around 20ms. if we drop ES2 support and keep only ES1 then the boot time (timestamp ID 971) is 10ms > > > > Instead of concatenating microcode you could add a cbfs file per cpuid? That would not bypass CBFS verification and result in the same speedup. > > isn't the expectation inside find_cbfs_microcode() function is that, there would be one single microcode entry inside CBFS? Yes. My suggestion implies changing that :-) -- To view, visit https://review.coreboot.org/c/coreboot/+/74873 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I208a2e0e1dce96b2d4d63f882e8aaf4c25f77e3f Gerrit-Change-Number: 74873 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Tue, 02 May 2023 13:01:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: mb/google/hatch: Add SOF chip driver
by Martin Roth (Code Review) 02 May '23

02 May '23

Attention is currently required from: Tarun Tuli, Matt DeVillier. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74814 ) Change subject: mb/google/hatch: Add SOF chip driver ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74814 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie791fa873fc7bbab84644f5ea5743bdcdc124908 Gerrit-Change-Number: 74814 Gerrit-PatchSet: 5 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: CoolStar <coolstarorganization(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> 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: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Comment-Date: Tue, 02 May 2023 12:58:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2023