coreboot-gerrit October 2022

coreboot-gerrit@coreboot.org

1 participants
5881 discussions

[M] Change in coreboot[master]: util/amdfwtool/amdfwread: Update relative_offset function
by Karthik Ramasubramanian (Code Review) 21 Oct '22

21 Oct '22

Attention is currently required from: Robert Zieba, Jon Murphy, Rob Barnes, Karthik Ramasubramanian, Felix Held. Hello build bot (Jenkins), Robert Zieba, Raul Rangel, Jon Murphy, Rob Barnes, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/66572 to look at the new patch set (#14). Change subject: util/amdfwtool/amdfwread: Update relative_offset function ...................................................................... util/amdfwtool/amdfwread: Update relative_offset function * AMD_ADDR_PHYSICAL refers to physical address in the memory map * AMD_ADDR_REL_BIOS is relative to the start of the BIOS image * AMD_ADDR_REL_TAB is relative to the start of concerned PSP or BIOS tables Update the relative_offset implementation accordingly. Though AMD_ADDR_REL_SLOT is defined it is not used. Removing that to simplify the relative_offset implementation so that it can be used for both PSP and BIOS firmware tables. Hence update the relative_offset function signature as well. BUG=None TEST=Build and use amdfwread to read the Soft-fuse bits from Guybrush BIOS image. Observed no changes before and after the changes. Change-Id: I74603dd08eda87393c14b746c4435eaf2bb34126 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M util/amdfwtool/amdfwread.c 1 file changed, 45 insertions(+), 20 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/66572/14 -- To view, visit https://review.coreboot.org/c/coreboot/+/66572 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I74603dd08eda87393c14b746c4435eaf2bb34126 Gerrit-Change-Number: 66572 Gerrit-PatchSet: 14 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Robert Zieba <robertzieba(a)google.com> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[XL] Change in coreboot[master]: vendorcode/intel/fsp: Add Raptor Lake FSP headers for FSP RPL.3361.07
by Selma Bensaid (Code Review) 21 Oct '22

21 Oct '22

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/68495 to look at the new patch set (#2). Change subject: vendorcode/intel/fsp: Add Raptor Lake FSP headers for FSP RPL.3361.07 ...................................................................... vendorcode/intel/fsp: Add Raptor Lake FSP headers for FSP RPL.3361.07 The headers added are generated as per FSP v3361.07 In the future, when Alder Lake and Raptor Lake fsp align, Raptor Lake fsp headers can be deleted and Raptor Lake soc will also use headers from alderlake/ folder. BUG=b:254054169 BRANCH=firmware-brya-14505.B TEST=Boot to OS Signed-off-by: Selma Bensaid <selma.bensaid(a)intel.com> Change-Id: If486867477c88ad3e2ec5041ef94a0c364f5dfd6 --- M src/vendorcode/intel/fsp/fsp2_0/raptorlake/FspmUpd.h M src/vendorcode/intel/fsp/fsp2_0/raptorlake/FspsUpd.h 2 files changed, 905 insertions(+), 861 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/68495/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/68495 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If486867477c88ad3e2ec5041ef94a0c364f5dfd6 Gerrit-Change-Number: 68495 Gerrit-PatchSet: 2 Gerrit-Owner: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: arch/x86/postcar_loader: Don't add postcar to stage cache
by Reka Norman (Code Review) 21 Oct '22

21 Oct '22

Attention is currently required from: Subrata Banik, Kangheui Won, Nick Vaccaro, Julius Werner, Angel Pons, Arthur Heymans, Karthikeyan Ramasubramanian. Reka Norman has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68641 ) Change subject: arch/x86/postcar_loader: Don't add postcar to stage cache ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Let me know what you think. I'm happy to change it for nissa only, but it seems likely to benefit most devices. -- To view, visit https://review.coreboot.org/c/coreboot/+/68641 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3614c0874a6c71d13606b0b782ea445692d88bb1 Gerrit-Change-Number: 68641 Gerrit-PatchSet: 2 Gerrit-Owner: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Karthikeyan Ramasubramanian <kramasub(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kangheui Won <khwon(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Karthikeyan Ramasubramanian <kramasub(a)chromium.org> Gerrit-Comment-Date: Fri, 21 Oct 2022 00:12:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: mb/google/kahlee: use override devicetrees for variants
by Matt DeVillier (Code Review) 21 Oct '22

21 Oct '22

1 0

[S] Change in coreboot[master]: soc/amd: move all AOAC function prototypes to amdblocks/aoac.h
by 9elements QA (Code Review) 21 Oct '22

21 Oct '22

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68559 ) Change subject: soc/amd: move all AOAC function prototypes to amdblocks/aoac.h ...................................................................... Patch Set 5: Automatic boot test returned (PASS/FAIL/TOTAL): 15 / 1 / 16 PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES_ and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/129302 PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/129301 PASS: x86_32 "ThinkPad T500" , build config LENOVO_T500 and payload SeaBIOS : https://lava.9esec.io/r/129300 PASS: x86_32 "HP Z220 SFF Workstation" , build config HP_Z220_SFF_WORKSTATION and payload LinuxBoot_BB_kexec : https://lava.9esec.io/r/129299 PASS: 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/129298 PASS: 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/129297 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/129296 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/129295 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload TianoCore : https://lava.9esec.io/r/129294 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload SeaBIOS : https://lava.9esec.io/r/129293 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload TianoCore : https://lava.9esec.io/r/129292 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload SeaBIOS : https://lava.9esec.io/r/129291 FAIL: x86_64 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_X86_64 and payload SeaBIOS : https://lava.9esec.io/r/129290 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ASAN and payload SeaBIOS : https://lava.9esec.io/r/129289 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ and payload SeaBIOS : https://lava.9esec.io/r/129288 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX and payload SeaBIOS : https://lava.9esec.io/r/129287 Please note: This test is under development and might not be accurate at all! -- To view, visit https://review.coreboot.org/c/coreboot/+/68559 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3deae150cd1e20fff6507a0f0ba6a375fca430e5 Gerrit-Change-Number: 68559 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(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-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-Comment-Date: Fri, 21 Oct 2022 00:09:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: mb/google/kahlee: use override devicetrees for variants
by Matt DeVillier (Code Review) 21 Oct '22

21 Oct '22

Attention is currently required from: Jason Nien, Martin Roth, Felix Held. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68634 ) Change subject: mb/google/kahlee: use override devicetrees for variants ...................................................................... Patch Set 3: (4 comments) File src/mainboard/google/kahlee/variants/aleena/overridetree.cb: https://review.coreboot.org/c/coreboot/+/68634/comment/53fb52e3_4a75f8b2 PS3, Line 38: chip drivers/generic/adau7002 : device generic 0.0 on end : end can move to baseboard https://review.coreboot.org/c/coreboot/+/68634/comment/65095064_bcc8ac96 PS3, Line 59: chip drivers/generic/max98357a : register "hid" = ""MX98357A"" : register "sdmode_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_119)" : register "sdmode_delay" = "5" : device generic 0.1 on end : end can go to baseboard too https://review.coreboot.org/c/coreboot/+/68634/comment/f34c57d0_224631bb PS3, Line 67: chip drivers/i2c/generic : register "hid" = ""ELAN0000"" : register "desc" = ""ELAN Touchpad"" : register "irq_gpio" = "ACPI_GPIO_IRQ_LEVEL_LOW(GPIO_5)" : register "wake" = "7" : register "detect" = "1" : device i2c 15 on end : end FYI, I tested on LIARA moving the ELAN touchpad to the baseboard and leaving the other (synaptics) in the overridetree, and coreboot will pick up / attempt detection on both. So good to move ELAN pad to baseboard https://review.coreboot.org/c/coreboot/+/68634/comment/ef48c835_6bbf6567 PS3, Line 85: device ref i2c_3 on : chip drivers/i2c/generic : register "hid" = ""RAYD0001"" : register "desc" = ""Raydium Touchscreen"" : register "probed" = "1" : register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_LOW(GPIO_11)" : register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_85)" : register "reset_delay_ms" = "20" : register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_76)" : register "enable_delay_ms" = "1" : register "has_power_resource" = "1" : device i2c 39 on end : end : chip drivers/i2c/generic : register "hid" = ""ELAN0001"" : register "desc" = ""ELAN Touchscreen"" : register "probed" = "1" : register "irq_gpio" = "ACPI_GPIO_IRQ_LEVEL_LOW(GPIO_11)" : register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_85)" : register "reset_delay_ms" = "20" : register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_76)" : register "enable_delay_ms" = "1" : register "has_power_resource" = "1" : device i2c 10 on end : end : end same here - the ELAN and Raydium touchscreens are common and can be moved to the baseboard -- To view, visit https://review.coreboot.org/c/coreboot/+/68634 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie050c4624327b904e8cb0959b40421339e43f825 Gerrit-Change-Number: 68634 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 21 Oct 2022 00:06:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: mb/google/brya: Create gladios ADL variant
by Nick Vaccaro (Code Review) 21 Oct '22

21 Oct '22

Nick Vaccaro has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68569 ) Change subject: mb/google/brya: Create gladios ADL variant ...................................................................... mb/google/brya: Create gladios ADL variant Create the gladios variant of the brask reference board by copying the template files to a new directory named for the variant. (Auto-Generated by create_coreboot_variant.sh version 4.5.0). BUG=b:239513596 BRANCH=None TEST=util/abuild/abuild -p none -t google/brya -x -a make sure the build includes GOOGLE_gladios Signed-off-by: Kevin Chiu <kevin.chiu.17802(a)gmail.com> Change-Id: I3dc99d97d8e30d9641f56616222dd68e3a0d548d Reviewed-on: https://review.coreboot.org/c/coreboot/+/68569 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nick Vaccaro <nvaccaro(a)google.com> Reviewed-by: Zhuohao Lee <zhuohao(a)google.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/mainboard/google/brya/Kconfig M src/mainboard/google/brya/Kconfig.name A src/mainboard/google/brya/variants/gladios/include/variant/ec.h A src/mainboard/google/brya/variants/gladios/include/variant/gpio.h A src/mainboard/google/brya/variants/gladios/overridetree.cb 5 files changed, 53 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Nick Vaccaro: Looks good to me, approved Zhuohao Lee: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve diff --git a/src/mainboard/google/brya/Kconfig b/src/mainboard/google/brya/Kconfig index b86005a..577c2ca 100644 --- a/src/mainboard/google/brya/Kconfig +++ b/src/mainboard/google/brya/Kconfig @@ -213,6 +213,7 @@ default "Yaviks" if BOARD_GOOGLE_YAVIKS default "Lisbon" if BOARD_GOOGLE_LISBON default "Zydron" if BOARD_GOOGLE_ZYDRON + default "Gladios" if BOARD_GOOGLE_GLADIOS config VARIANT_DIR default "brya0" if BOARD_GOOGLE_BRYA0 @@ -252,6 +253,7 @@ default "yaviks" if BOARD_GOOGLE_YAVIKS default "lisbon" if BOARD_GOOGLE_LISBON default "zydron" if BOARD_GOOGLE_ZYDRON + default "gladios" if BOARD_GOOGLE_GLADIOS config VBOOT select VBOOT_EARLY_EC_SYNC if !BOARD_GOOGLE_BASEBOARD_NISSA diff --git a/src/mainboard/google/brya/Kconfig.name b/src/mainboard/google/brya/Kconfig.name index 3c98dca..9992eea 100644 --- a/src/mainboard/google/brya/Kconfig.name +++ b/src/mainboard/google/brya/Kconfig.name @@ -281,3 +281,7 @@ select DRIVERS_I2C_NAU8825 select DRIVERS_INTEL_MIPI_CAMERA select SOC_INTEL_COMMON_BLOCK_IPU + +config BOARD_GOOGLE_GLADIOS + bool "-> Gladios" + select BOARD_GOOGLE_BASEBOARD_BRASK diff --git a/src/mainboard/google/brya/variants/gladios/include/variant/ec.h b/src/mainboard/google/brya/variants/gladios/include/variant/ec.h new file mode 100644 index 0000000..7a2a6ff --- /dev/null +++ b/src/mainboard/google/brya/variants/gladios/include/variant/ec.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef __VARIANT_EC_H__ +#define __VARIANT_EC_H__ + +#include <baseboard/ec.h> + +#endif diff --git a/src/mainboard/google/brya/variants/gladios/include/variant/gpio.h b/src/mainboard/google/brya/variants/gladios/include/variant/gpio.h new file mode 100644 index 0000000..c4fe342 --- /dev/null +++ b/src/mainboard/google/brya/variants/gladios/include/variant/gpio.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef VARIANT_GPIO_H +#define VARIANT_GPIO_H + +#include <baseboard/gpio.h> + +#endif diff --git a/src/mainboard/google/brya/variants/gladios/overridetree.cb b/src/mainboard/google/brya/variants/gladios/overridetree.cb new file mode 100644 index 0000000..4f2c04a --- /dev/null +++ b/src/mainboard/google/brya/variants/gladios/overridetree.cb @@ -0,0 +1,6 @@ +chip soc/intel/alderlake + + device domain 0 on + end + +end -- To view, visit https://review.coreboot.org/c/coreboot/+/68569 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3dc99d97d8e30d9641f56616222dd68e3a0d548d Gerrit-Change-Number: 68569 Gerrit-PatchSet: 2 Gerrit-Owner: Kevin Chiu <kevin.chiu.17802(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)chromium.org> Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: arch/x86/postcar_loader: Don't add postcar to stage cache
by Reka Norman (Code Review) 21 Oct '22

21 Oct '22

Reka Norman has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/68641 ) Change subject: arch/x86/postcar_loader: Don't add postcar to stage cache ...................................................................... arch/x86/postcar_loader: Don't add postcar to stage cache In romstage, CBMEM and TSEG are not yet cached, so reading/writing to the stage cache is slow. This means there isn't really any benefit to adding postcar to the stage cache: - During boot, adding it to the stage cache adds a few ms of boot time, since writing to uncached memory is slow. - During S3 resume, reading from stage cache takes roughly the same time as reading from SPI, so we get little or no savings anyway. E.g. on nivviks, this change saves 6 ms of boot time and 2 ms of S3 resume time. Adding postcar to stage cache: Boot: rmodule_stage_load() - 4 ms stage_cache_add() - 6 ms S3 resume: stage_cache_load_stage() - 6 ms Not adding postcar to stage cache: Boot: rmodule_stage_load() - 4 ms S3 resume: rmodule_stage_load() - 4 ms Savings: Boot time - 6 ms S3 resume time - 2 ms ------------------------------------ Savings on other devices: nivviks anahera kindred treeya rmodule_stage_load() 4.0 1.4 0.8 2.8 stage_cache_add() 6.0 2.8 1.5 2.0 stage_cache_load_stage() 6.0 2.8 1.5 2.0 Boot time savings 6.0 2.8 1.5 2.0 S3 resume time savings 2.0 1.4 0.7 -0.8 BUG=b:247940538 TEST=On nivviks, boot and check S3 resume still works. Add logging to check postcar is added to stage cache when POSTCAR_IN_STAGE_CACHE is selected, and not added otherwise. Change-Id: I3614c0874a6c71d13606b0b782ea445692d88bb1 Signed-off-by: Reka Norman <rekanorman(a)chromium.org> --- M src/arch/x86/Kconfig M src/arch/x86/postcar_loader.c 2 files changed, 67 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/68641/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/68641 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3614c0874a6c71d13606b0b782ea445692d88bb1 Gerrit-Change-Number: 68641 Gerrit-PatchSet: 2 Gerrit-Owner: Reka Norman <rekanorman(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: arch/x86/postcar_loader: Don't add postcar to stage cache
by build bot (Jenkins) (Code Review) 21 Oct '22

21 Oct '22

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68641 ) Change subject: arch/x86/postcar_loader: Don't add postcar to stage cache ...................................................................... Patch Set 1: (1 comment) Commit Message: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-160926): https://review.coreboot.org/c/coreboot/+/68641/comment/8a40b9dd_ec21bd0c PS1, Line 13: since writing to uncached memory is slow. 'rougly' may be misspelled - perhaps 'roughly'? -- To view, visit https://review.coreboot.org/c/coreboot/+/68641 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3614c0874a6c71d13606b0b782ea445692d88bb1 Gerrit-Change-Number: 68641 Gerrit-PatchSet: 1 Gerrit-Owner: Reka Norman <rekanorman(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 20 Oct 2022 23:55:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: arch/x86/postcar_loader: Don't add postcar to stage cache
by Reka Norman (Code Review) 21 Oct '22

21 Oct '22

Reka Norman has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/68641 ) Change subject: arch/x86/postcar_loader: Don't add postcar to stage cache ...................................................................... arch/x86/postcar_loader: Don't add postcar to stage cache In romstage, CBMEM and TSEG are not yet cached, so reading/writing to the stage cache is slow. This means there isn't really any benefit to adding postcar to the stage cache: - During boot, adding it to the stage cache adds a few ms of boot time, since writing to uncached memory is slow. - During S3 resume, reading from stage cache takes rougly the same time as reading from SPI, so we get little or no savings anyway. E.g. on nivviks, this change saves 6 ms of boot time and 2 ms of S3 resume time. Adding postcar to stage cache: Boot: rmodule_stage_load() - 4 ms stage_cache_add() - 6 ms S3 resume: stage_cache_load_stage() - 6 ms Not adding postcar to stage cache: Boot: rmodule_stage_load() - 4 ms S3 resume: rmodule_stage_load() - 4 ms Savings: Boot time - 6 ms S3 resume time - 2 ms ------------------------------------ Savings on other devices: nivviks anahera kindred treeya rmodule_stage_load() 4.0 1.4 0.8 2.8 stage_cache_add() 6.0 2.8 1.5 2.0 stage_cache_load_stage() 6.0 2.8 1.5 2.0 Boot time savings 6.0 2.8 1.5 2.0 S3 resume time savings 2.0 1.4 0.7 -0.8 BUG=b:247940538 TEST=On nivviks, boot and check S3 resume still works. Add logging to check postcar is added to stage cache when POSTCAR_IN_STAGE_CACHE is selected, and not added otherwise. Change-Id: I3614c0874a6c71d13606b0b782ea445692d88bb1 Signed-off-by: Reka Norman <rekanorman(a)chromium.org> --- M src/arch/x86/Kconfig M src/arch/x86/postcar_loader.c 2 files changed, 67 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/68641/1 diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig index c00eb0c..bba3eba 100644 --- a/src/arch/x86/Kconfig +++ b/src/arch/x86/Kconfig @@ -207,6 +207,16 @@ depends on ARCH_X86 depends on !RESET_VECTOR_IN_RAM +config POSTCAR_IN_STAGE_CACHE + bool + depends on !NO_STAGE_CACHE && POSTCAR_STAGE + help + In romstage, CBMEM and TSEG are not yet cached, so reading and + writing to stage cache is slow. This means on most platforms + adding postcar to the stage cache gives little if any S3 resume + time savings, while also adding a few ms of boot time. Therefore + this config is disabled by default. + config VERSTAGE_DEBUG_SPINLOOP bool default n diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c index c6a128b..8987e25 100644 --- a/src/arch/x86/postcar_loader.c +++ b/src/arch/x86/postcar_loader.c @@ -110,7 +110,8 @@ finalize_load(rsl.params, (uintptr_t)pcf->mtrr); - stage_cache_add(STAGE_POSTCAR, prog); + if (CONFIG(POSTCAR_IN_STAGE_CACHE)) + stage_cache_add(STAGE_POSTCAR, prog); } /* @@ -141,7 +142,7 @@ struct prog prog = PROG_INIT(PROG_POSTCAR, CONFIG_CBFS_PREFIX "/postcar"); - if (resume_from_stage_cache()) { + if (CONFIG(POSTCAR_IN_STAGE_CACHE) && resume_from_stage_cache()) { stage_cache_load_stage(STAGE_POSTCAR, &prog); /* This is here to allow platforms to pass different stack parameters between S3 resume and normal boot. On the -- To view, visit https://review.coreboot.org/c/coreboot/+/68641 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3614c0874a6c71d13606b0b782ea445692d88bb1 Gerrit-Change-Number: 68641 Gerrit-PatchSet: 1 Gerrit-Owner: Reka Norman <rekanorman(a)chromium.org> Gerrit-MessageType: newchange

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2022