coreboot-gerrit October 2022

coreboot-gerrit@coreboot.org

1 participants
5881 discussions

[S] Change in coreboot[master]: src/commonlib: Clean up includes
by Felix Held (Code Review) 26 Oct '22

26 Oct '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68201 ) Change subject: src/commonlib: Clean up includes ...................................................................... src/commonlib: Clean up includes Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> Change-Id: Ieba5a5291209e50dc8b3816efb25bb5b2515fa6a Reviewed-on: https://review.coreboot.org/c/coreboot/+/68201 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Martin Roth <martin.roth(a)amd.corp-partner.google.com> --- M src/commonlib/iobuf.c M src/commonlib/region.c 2 files changed, 15 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved diff --git a/src/commonlib/iobuf.c b/src/commonlib/iobuf.c index 042e8dd..378793e 100644 --- a/src/commonlib/iobuf.c +++ b/src/commonlib/iobuf.c @@ -3,6 +3,7 @@ #include <commonlib/endian.h> #include <commonlib/iobuf.h> #include <string.h> +#include <stdint.h> static int ibuf_check_size(const struct ibuf *ib, size_t sz) { diff --git a/src/commonlib/region.c b/src/commonlib/region.c index 252f6fa..4153f0a 100644 --- a/src/commonlib/region.c +++ b/src/commonlib/region.c @@ -2,6 +2,7 @@ #include <commonlib/helpers.h> #include <commonlib/region.h> +#include <stdint.h> #include <string.h> int region_is_subregion(const struct region *p, const struct region *c) -- To view, visit https://review.coreboot.org/c/coreboot/+/68201 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieba5a5291209e50dc8b3816efb25bb5b2515fa6a Gerrit-Change-Number: 68201 Gerrit-PatchSet: 3 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: arch/x86: Clean up includes
by Felix Held (Code Review) 26 Oct '22

26 Oct '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68200 ) Change subject: arch/x86: Clean up includes ...................................................................... arch/x86: Clean up includes Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> Change-Id: Id2db229dec2ed44333faaa8c53f3a2f9d66d52e0 Reviewed-on: https://review.coreboot.org/c/coreboot/+/68200 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Martin Roth <martin.roth(a)amd.corp-partner.google.com> --- M src/arch/x86/cpu_common.c M src/arch/x86/ebda.c M src/arch/x86/mpspec.c M src/arch/x86/null_breakpoint.c M src/arch/x86/post.c M src/arch/x86/postcar_loader.c M src/arch/x86/rdrand.c 7 files changed, 29 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved diff --git a/src/arch/x86/cpu_common.c b/src/arch/x86/cpu_common.c index 302a14e..e674afa 100644 --- a/src/arch/x86/cpu_common.c +++ b/src/arch/x86/cpu_common.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <cpu/cpu.h> +#include <types.h> #if ENV_X86_32 /* Standard macro to see if a specific flag is changeable */ diff --git a/src/arch/x86/ebda.c b/src/arch/x86/ebda.c index e576a14..e835fce 100644 --- a/src/arch/x86/ebda.c +++ b/src/arch/x86/ebda.c @@ -3,7 +3,7 @@ #include <acpi/acpi.h> #include <bootstate.h> #include <commonlib/endian.h> -#include <stdint.h> +#include <types.h> #define X86_BDA_SIZE 0x200 #define X86_BDA_BASE ((void *)0x400) diff --git a/src/arch/x86/mpspec.c b/src/arch/x86/mpspec.c index 739d185..d94308e 100644 --- a/src/arch/x86/mpspec.c +++ b/src/arch/x86/mpspec.c @@ -1,14 +1,17 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#include <console/console.h> -#include <device/path.h> -#include <device/pci_ids.h> +#include <acpi/acpi.h> #include <arch/ioapic.h> #include <arch/smp/mpspec.h> -#include <string.h> +#include <console/console.h> #include <cpu/cpu.h> #include <cpu/x86/lapic.h> +#include <device/device.h> +#include <device/path.h> +#include <device/pci_ids.h> #include <drivers/generic/ioapic/chip.h> +#include <stdint.h> +#include <string.h> /* Initialize the specified "mc" struct with initial values. */ void mptable_init(struct mp_config_table *mc) diff --git a/src/arch/x86/null_breakpoint.c b/src/arch/x86/null_breakpoint.c index 70b94b6..43e3727 100644 --- a/src/arch/x86/null_breakpoint.c +++ b/src/arch/x86/null_breakpoint.c @@ -1,9 +1,10 @@ /* SPDX-License-Identifier: GPL-2.0-only */ + #include <arch/breakpoint.h> #include <arch/null_breakpoint.h> #include <bootstate.h> #include <console/console.h> -#include <stdint.h> +#include <types.h> static struct breakpoint_handle null_deref_bp; static struct breakpoint_handle null_fetch_bp; diff --git a/src/arch/x86/post.c b/src/arch/x86/post.c index 3c1ca43..908cc3b 100644 --- a/src/arch/x86/post.c +++ b/src/arch/x86/post.c @@ -1,9 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <arch/io.h> +#include <console/console.h> #include <post.h> #include <stdint.h> -#include <console/console.h> -#include <arch/io.h> void arch_post_code(uint8_t value) { diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c index c6a128b..be4a235 100644 --- a/src/arch/x86/postcar_loader.c +++ b/src/arch/x86/postcar_loader.c @@ -9,9 +9,10 @@ #include <program_loading.h> #include <reset.h> #include <rmodule.h> +#include <security/vboot/vboot_common.h> #include <stage_cache.h> #include <timestamp.h> -#include <security/vboot/vboot_common.h> +#include <types.h> static size_t var_mtrr_ctx_size(void) { diff --git a/src/arch/x86/rdrand.c b/src/arch/x86/rdrand.c index 5374933..985aeba 100644 --- a/src/arch/x86/rdrand.c +++ b/src/arch/x86/rdrand.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <random.h> +#include <stdint.h> /* * Intel recommends that applications attempt 10 retries in a tight loop -- To view, visit https://review.coreboot.org/c/coreboot/+/68200 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id2db229dec2ed44333faaa8c53f3a2f9d66d52e0 Gerrit-Change-Number: 68200 Gerrit-PatchSet: 3 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: lint/checkpatch: consider leading + in the line length limit check
by Tim Wawrzynczak (Code Review) 26 Oct '22

26 Oct '22

Attention is currently required from: Felix Singer, Martin L Roth, Angel Pons, Michael Niewöhner. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68869 ) Change subject: lint/checkpatch: consider leading + in the line length limit check ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/68869/comment/5cfe9324_dc432228 PS1, Line 12: for the is one character longer is one character shorter than the content? so it needs to be bumped by 1 -- To view, visit https://review.coreboot.org/c/coreboot/+/68869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8da45bb0d5fbe7d0e12c8b181cf01e5685186bf6 Gerrit-Change-Number: 68869 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Comment-Date: Wed, 26 Oct 2022 16:02:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/amd/mendocino: Add GSVCD range
by Raul Rangel (Code Review) 26 Oct '22

26 Oct '22

Raul Rangel has submitted this change. ( https://review.coreboot.org/c/coreboot/+/66945 ) Change subject: soc/amd/mendocino: Add GSVCD range ...................................................................... soc/amd/mendocino: Add GSVCD range Add region/range of SPI ROM to be verified by Google Security Chip (GSC). BUG=b:227809919 TEST=Build and boot to OS in Skyrim with CBFS verification enabled. Change-Id: If8a766d9a7ef26f94e3ab002a9384ba9d444dd1f Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/66945 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- M src/soc/amd/common/Makefile.inc M src/soc/amd/mendocino/Makefile.inc 2 files changed, 44 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/common/Makefile.inc b/src/soc/amd/common/Makefile.inc index c8e9d0d..63ee773 100644 --- a/src/soc/amd/common/Makefile.inc +++ b/src/soc/amd/common/Makefile.inc @@ -28,6 +28,18 @@ ifeq ($(CONFIG_VBOOT_GSCVD),y) build_complete:: $(AMDFWREAD) + +amdfwread-offset-size-cmd = $(AMDFWREAD) --ro-list $(obj)/coreboot.rom | \ + awk --non-decimal-data '/$(1)/ {printf "%x:%x", $$3, $$4}' + +amdfwread-range-cmd = $(shell ( \ + range=$$($(call amdfwread-offset-size-cmd,$(1))) ;\ + if [ -n "$$range" ]; then \ + printf $$range ;\ + else \ + printf "error" ;\ + fi ;\ +)) endif # ifeq ($(CONFIG_VBOOT_GSCVD),y) endif diff --git a/src/soc/amd/mendocino/Makefile.inc b/src/soc/amd/mendocino/Makefile.inc index 306f228..44a0e29 100644 --- a/src/soc/amd/mendocino/Makefile.inc +++ b/src/soc/amd/mendocino/Makefile.inc @@ -327,4 +327,17 @@ -n apu/amdfw_b_hash -t raw endif +# Add ranges for all components up until the first segment of BIOS to be verified by GSC +ifeq ($(CONFIG_VBOOT_GSCVD),y) +# Adding range for Bootblock +vboot-gscvd-ranges += $(call amdfwread-range-cmd,BIOSL2: 0x62) +# Adding range for PSP Stage1 Bootloader +vboot-gscvd-ranges += $(call amdfwread-range-cmd,PSPL2: 0x01) + +ifeq ($(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),y) +# Adding range for PSP Verstage +vboot-gscvd-ranges += $(call amdfwread-range-cmd,PSPL2: 0x52) +endif # ifeq ($(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),y) +endif # ifeq ($(CONFIG_VBOOT_GSCVD),y) + endif # ($(CONFIG_SOC_AMD_MENDOCINO),y) -- To view, visit https://review.coreboot.org/c/coreboot/+/66945 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If8a766d9a7ef26f94e3ab002a9384ba9d444dd1f Gerrit-Change-Number: 66945 Gerrit-PatchSet: 14 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> 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: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(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-MessageType: merged

1 0

[S] Change in coreboot[master]: soc/amd/mendocino: Update build rules for PSP BIOS image
by Raul Rangel (Code Review) 26 Oct '22

26 Oct '22

Raul Rangel has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68136 ) Change subject: soc/amd/mendocino: Update build rules for PSP BIOS image ...................................................................... soc/amd/mendocino: Update build rules for PSP BIOS image Do not compress PSP BIOS image when CBFS verification is enabled. Otherwise when a file is added to CBFS, cbfstool is not able to find the metadata hash anchor magic in the compressed PSP BIOS image. BUG=b:227809919 TEST=Build and boot to OS in Skyrim with CBFS verification enabled for both x86 and PSP verstage. Change-Id: Iaed888b81d14ede77132ff48abcfbeb138c01ce4 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/68136 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- M src/soc/amd/mendocino/Makefile.inc 1 file changed, 27 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/mendocino/Makefile.inc b/src/soc/amd/mendocino/Makefile.inc index 41aeb20..306f228 100644 --- a/src/soc/amd/mendocino/Makefile.inc +++ b/src/soc/amd/mendocino/Makefile.inc @@ -261,11 +261,17 @@ --location $(shell printf "%#x" $(MENDOCINO_FWM_POSITION)) \ --output $@ +ifeq ($(CONFIG_CBFS_VERIFICATION)$(CONFIG_VBOOT_STARTS_IN_BOOTBLOCK),yy) +$(PSP_BIOSBIN_FILE): $(PSP_ELF_FILE) + rm -f $@ + $(OBJCOPY_bootblock) -O binary $< $@ +else $(PSP_BIOSBIN_FILE): $(PSP_ELF_FILE) $(AMDCOMPRESS) rm -f $@ @printf " AMDCOMPRS $(subst $(obj)/,,$(@))\n" $(AMDCOMPRESS) --infile $(PSP_ELF_FILE) --outfile $@ --compress \ --maxsize $(PSP_BIOSBIN_SIZE) +endif $(obj)/amdfw_a.rom: $(obj)/amdfw.rom rm -f $@ -- To view, visit https://review.coreboot.org/c/coreboot/+/68136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaed888b81d14ede77132ff48abcfbeb138c01ce4 Gerrit-Change-Number: 68136 Gerrit-PatchSet: 11 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> 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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: soc/amd/mendocino: Reserve more space for metadata
by Raul Rangel (Code Review) 26 Oct '22

26 Oct '22

Raul Rangel has submitted this change. ( https://review.coreboot.org/c/coreboot/+/66944 ) ( 12 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: soc/amd/mendocino: Reserve more space for metadata ...................................................................... soc/amd/mendocino: Reserve more space for metadata With CBFS verification enabled, CBFS file header + file name + metadata consumes more than 64 bytes. Hence reserve additional space aligned to the next 64 bytes. BUG=b:227809919 TEST=Build and boot to OS in Skyrim with CBFS verification enabled. Change-Id: I2b7346e2150835443425179048415f3b27d89d89 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/66944 Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/mendocino/Makefile.inc 1 file changed, 23 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, but someone else must approve Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/mendocino/Makefile.inc b/src/soc/amd/mendocino/Makefile.inc index fe283bd..41aeb20 100644 --- a/src/soc/amd/mendocino/Makefile.inc +++ b/src/soc/amd/mendocino/Makefile.inc @@ -73,9 +73,9 @@ $(call int-shift-left, \ 0x80000 $(CONFIG_AMD_FWM_POSITION_INDEX))) 0x20000 1) -# 0x40 accounts for the cbfs_file struct + filename + metadata structs, aligned to 64 bytes +# 0x80 accounts for the cbfs_file struct + filename + metadata structs, aligned to 64 bytes # Building the cbfs image will fail if the offset isn't large enough -AMD_FW_AB_POSITION := 0x40 +AMD_FW_AB_POSITION := 0x80 MENDOCINO_FW_A_POSITION=$(call int-add, \ $(shell awk '$$2 == "FMAP_SECTION_FW_MAIN_A_START" {print $$3}' $(obj)/fmap_config.h) \ -- To view, visit https://review.coreboot.org/c/coreboot/+/66944 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2b7346e2150835443425179048415f3b27d89d89 Gerrit-Change-Number: 66944 Gerrit-PatchSet: 14 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> 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: Marshall Dawson <marshalldawson3rd(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: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: soc/amd: Add amdfw.rom in coreboot.pre
by Raul Rangel (Code Review) 26 Oct '22

26 Oct '22

Raul Rangel has submitted this change. ( https://review.coreboot.org/c/coreboot/+/66943 ) Change subject: soc/amd: Add amdfw.rom in coreboot.pre ...................................................................... soc/amd: Add amdfw.rom in coreboot.pre This change ensures that amdfw.rom binary containing metadata hash anchor is added before any file is added to CBFS. This will allow to verify all the CBFS files that are not excluded from verification. BUG=b:227809919 TEST=Build and boot to OS in Skyrim with CBFS verification enabled using x86 and PSP verstages. Change-Id: Id4d1a2d8b145cbbbf2da27aa73b296c9c8a65209 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/66943 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- M src/soc/amd/cezanne/Makefile.inc M src/soc/amd/common/Makefile.inc M src/soc/amd/mendocino/Makefile.inc M src/soc/amd/morgana/Makefile.inc M src/soc/amd/picasso/Makefile.inc 5 files changed, 33 insertions(+), 20 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/cezanne/Makefile.inc b/src/soc/amd/cezanne/Makefile.inc index 508ea1d..cdbbcfe 100644 --- a/src/soc/amd/cezanne/Makefile.inc +++ b/src/soc/amd/cezanne/Makefile.inc @@ -274,11 +274,6 @@ --output $@ -cbfs-files-y += apu/amdfw -apu/amdfw-file := $(obj)/amdfw.rom -apu/amdfw-position := $(CEZANNE_FWM_POSITION) -apu/amdfw-type := raw - ifeq ($(CONFIG_VBOOT_SLOTS_RW_A)$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),yy) cbfs-files-y += apu/amdfw_a apu/amdfw_a-file := $(obj)/amdfw_a.rom diff --git a/src/soc/amd/common/Makefile.inc b/src/soc/amd/common/Makefile.inc index 75b1142..c8e9d0d 100644 --- a/src/soc/amd/common/Makefile.inc +++ b/src/soc/amd/common/Makefile.inc @@ -14,6 +14,18 @@ amd_microcode_bins += $(wildcard ${FIRMWARE_LOCATION}/*UcodePatch_*.bin) +ifeq ($(CONFIG_RESET_VECTOR_IN_RAM),y) +$(objcbfs)/bootblock.bin: $(obj)/amdfw.rom + cp $< $@ + +add_bootblock = \ + $(CBFSTOOL) $(1) add -f $(2) -n apu/amdfw -t amdfw \ + -b $(call int-add, \ + $(call int-subtract, 0xffffffff \ + $(call int-shift-left, \ + 0x80000 $(CONFIG_AMD_FWM_POSITION_INDEX))) 0x20000 1) +endif # ifeq ($(CONFIG_RESET_VECTOR_IN_RAM),y) + ifeq ($(CONFIG_VBOOT_GSCVD),y) build_complete:: $(AMDFWREAD) endif # ifeq ($(CONFIG_VBOOT_GSCVD),y) diff --git a/src/soc/amd/mendocino/Makefile.inc b/src/soc/amd/mendocino/Makefile.inc index 8f3425d..fe283bd 100644 --- a/src/soc/amd/mendocino/Makefile.inc +++ b/src/soc/amd/mendocino/Makefile.inc @@ -296,11 +296,6 @@ --output $@ -cbfs-files-y += apu/amdfw -apu/amdfw-file := $(obj)/amdfw.rom -apu/amdfw-position := $(MENDOCINO_FWM_POSITION) -apu/amdfw-type := raw - ifeq ($(CONFIG_VBOOT_SLOTS_RW_A)$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),yy) cbfs-files-y += apu/amdfw_a apu/amdfw_a-file := $(obj)/amdfw_a.rom diff --git a/src/soc/amd/morgana/Makefile.inc b/src/soc/amd/morgana/Makefile.inc index f4243bc..a0065b8 100644 --- a/src/soc/amd/morgana/Makefile.inc +++ b/src/soc/amd/morgana/Makefile.inc @@ -298,11 +298,6 @@ --output $@ -cbfs-files-y += apu/amdfw -apu/amdfw-file := $(obj)/amdfw.rom -apu/amdfw-position := $(MORGANA_FWM_POSITION) -apu/amdfw-type := raw - ifeq ($(CONFIG_VBOOT_SLOTS_RW_AB)$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),yy) cbfs-files-y += apu/amdfw_a apu/amdfw_a-file := $(obj)/amdfw_a.rom diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index cff47af..397c54d 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -286,11 +286,6 @@ --anywhere \ --output $@ -cbfs-files-y += apu/amdfw -apu/amdfw-file := $(obj)/amdfw.rom -apu/amdfw-position := $(PICASSO_FWM_POSITION) -apu/amdfw-type := raw - ifeq ($(CONFIG_VBOOT_SLOTS_RW_A)$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),yy) cbfs-files-y += apu/amdfw_a apu/amdfw_a-file := $(obj)/amdfw_a.rom -- To view, visit https://review.coreboot.org/c/coreboot/+/66943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4d1a2d8b145cbbbf2da27aa73b296c9c8a65209 Gerrit-Change-Number: 66943 Gerrit-PatchSet: 15 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> 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: Julius Werner <jwerner(a)chromium.org> 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-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: util/cbfstool: Check for metadata hash in verstage
by Raul Rangel (Code Review) 26 Oct '22

26 Oct '22

Raul Rangel has submitted this change. ( https://review.coreboot.org/c/coreboot/+/66942 ) ( 12 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: util/cbfstool: Check for metadata hash in verstage ...................................................................... util/cbfstool: Check for metadata hash in verstage Metadata Hash is usually present inside the first segment of BIOS. On board where vboot starts in bootblock, it is present in bootblock. On boards where vboot starts before bootblock, it is present in file containing verstage. Update cbfstool to check for metadata hash in file containing verstage besides bootblock. Add a new CBFS file type for the concerned file and exclude it from CBFS verification. BUG=b:227809919 TEST=Build and boot to OS in Skyrim with CBFS verification enabled using x86 and PSP verstages. Change-Id: Ib4dfba6a9cdbda0ef367b812f671c90e5f90caf8 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/66942 Reviewed-by: Julius Werner <jwerner(a)chromium.org> Reviewed-by: Yu-Ping Wu <yupingso(a)google.com> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h M util/cbfstool/cbfs.h M util/cbfstool/cbfstool.c 3 files changed, 46 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved Raul Rangel: Looks good to me, approved Yu-Ping Wu: Looks good to me, approved diff --git a/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h b/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h index 5b70d1a..605412b 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h +++ b/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h @@ -36,6 +36,7 @@ CBFS_TYPE_MMA = 0x62, CBFS_TYPE_EFI = 0x63, CBFS_TYPE_STRUCT = 0x70, + CBFS_TYPE_AMDFW = 0x80, CBFS_TYPE_CMOS_DEFAULT = 0xaa, CBFS_TYPE_SPD = 0xab, CBFS_TYPE_MRC_CACHE = 0xac, diff --git a/util/cbfstool/cbfs.h b/util/cbfstool/cbfs.h index e905162..eca7168 100644 --- a/util/cbfstool/cbfs.h +++ b/util/cbfstool/cbfs.h @@ -52,6 +52,7 @@ {CBFS_TYPE_STRUCT, "struct"}, {CBFS_TYPE_DELETED, "deleted"}, {CBFS_TYPE_NULL, "null"}, + {CBFS_TYPE_AMDFW, "amdfw"}, {0, NULL} }; diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index 5cb787d..04b91e5 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -148,7 +148,7 @@ if (!fmap) goto no_metadata_hash; - /* Find the bootblock. If there is a "BOOTBLOCK" FMAP section, it's + /* Find the metadata_hash container. If there is a "BOOTBLOCK" FMAP section, it's there. If not, it's a normal file in the primary CBFS section. */ size_t offset, size; struct buffer buffer; @@ -161,22 +161,27 @@ size = buffer.size; } else { struct cbfs_image cbfs; - struct cbfs_file *bootblock; + struct cbfs_file *mh_container; if (!partitioned_file_read_region(&buffer, param.image_file, SECTION_NAME_PRIMARY_CBFS)) goto no_metadata_hash; mhc.region = SECTION_NAME_PRIMARY_CBFS; if (cbfs_image_from_buffer(&cbfs, &buffer, param.headeroffset)) goto no_metadata_hash; - bootblock = cbfs_get_entry(&cbfs, "bootblock"); - if (!bootblock || be32toh(bootblock->type) != CBFS_TYPE_BOOTBLOCK) - goto no_metadata_hash; - offset = (void *)bootblock + be32toh(bootblock->offset) - + mh_container = cbfs_get_entry(&cbfs, "bootblock"); + if (!mh_container || be32toh(mh_container->type) != CBFS_TYPE_BOOTBLOCK) { + /* Check for apu/amdfw file */ + mh_container = cbfs_get_entry(&cbfs, "apu/amdfw"); + if (!mh_container || be32toh(mh_container->type) != CBFS_TYPE_AMDFW) + goto no_metadata_hash; + } + + offset = (void *)mh_container + be32toh(mh_container->offset) - buffer_get(&cbfs.buffer); - size = be32toh(bootblock->len); + size = be32toh(mh_container->len); } - /* Find and validate the metadata hash anchor inside the bootblock and + /* Find and validate the metadata hash anchor inside the containing file and record its exact byte offset from the start of the FMAP region. */ struct metadata_hash_anchor *anchor = memmem(buffer_get(&buffer) + offset, size, METADATA_HASH_ANCHOR_MAGIC, sizeof(anchor->magic)); @@ -264,7 +269,8 @@ { if (strcmp(param.region_name, SECTION_NAME_BOOTBLOCK) && strcmp(param.region_name, SECTION_NAME_FMAP) && - param.type != CBFS_TYPE_BOOTBLOCK) + param.type != CBFS_TYPE_BOOTBLOCK && + param.type != CBFS_TYPE_AMDFW) return 0; /* FMAP and bootblock didn't change. */ struct mh_cache *mhc = get_mh_cache(); @@ -285,6 +291,7 @@ case CBFS_TYPE_BOOTBLOCK: case CBFS_TYPE_CBFSHEADER: case CBFS_TYPE_INTEL_FIT: + case CBFS_TYPE_AMDFW: return true; default: return false; -- To view, visit https://review.coreboot.org/c/coreboot/+/66942 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib4dfba6a9cdbda0ef367b812f671c90e5f90caf8 Gerrit-Change-Number: 66942 Gerrit-PatchSet: 14 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: soc/amd: Add an optional unsigned section in PSP verstage
by Raul Rangel (Code Review) 26 Oct '22

26 Oct '22

Raul Rangel has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68135 ) ( 9 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: soc/amd: Add an optional unsigned section in PSP verstage ...................................................................... soc/amd: Add an optional unsigned section in PSP verstage To enable RO CBFS verification in AMD platforms with PSP verstage, metadata hash for RO CBFS is kept as part of verstage. This means any updates to RO CBFS, before WP is enabled, requires updating the metadata hash in the verstage. Hence keep the metadata hash outside the signed range of PSP verstage. This means the metadata hash gets loaded as part of loading PSP verstage while still being excluded from the verification of PSP verstage. This change keeps the metadata hash outside the PSP footer data. This will help to keep it outside the signed range of PSP verstage & aligned to 64 bytes. BUG=b:227809919 TEST=Build and boot to OS in Skyrim with CBFS verification enabled with both x86 and PSP verstage. Change-Id: I308223be8fbca1c0bec8c2e1c86ed65d9f91b966 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/68135 Reviewed-by: Julius Werner <jwerner(a)chromium.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/block/cpu/noncar/memlayout_psp_verstage.ld 1 file changed, 38 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/common/block/cpu/noncar/memlayout_psp_verstage.ld b/src/soc/amd/common/block/cpu/noncar/memlayout_psp_verstage.ld index f386823..e0278ab 100644 --- a/src/soc/amd/common/block/cpu/noncar/memlayout_psp_verstage.ld +++ b/src/soc/amd/common/block/cpu/noncar/memlayout_psp_verstage.ld @@ -17,8 +17,14 @@ .text : { *(.text*) } .rodata : { *(.rodata*) } - .data : { *(.data*) } - .data : { *(PSP_FOOTER_DATA) } + .data : { + *(.data*); + *(PSP_FOOTER_DATA); +#if CONFIG(CBFS_VERIFICATION) + *(.metadata_hash_anchor); + . = ALIGN(64); +#endif + } _bss_start = .; .bss : { *(.bss*) } -- To view, visit https://review.coreboot.org/c/coreboot/+/68135 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I308223be8fbca1c0bec8c2e1c86ed65d9f91b966 Gerrit-Change-Number: 68135 Gerrit-PatchSet: 11 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> 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: Julius Werner <jwerner(a)chromium.org> 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-MessageType: merged

1 0

[S] Change in coreboot[master]: lib/metadata_hash: Include metadata_hash in verstage
by Raul Rangel (Code Review) 26 Oct '22

26 Oct '22

Raul Rangel has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68134 ) ( 8 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: lib/metadata_hash: Include metadata_hash in verstage ...................................................................... lib/metadata_hash: Include metadata_hash in verstage On boards where vboot starts before bootblock, build metadata_hash in verstage. This will allow to enable CBFS verification for such platforms. BUG=b:227809919 TEST=Build and boot to OS in Skyrim with CBFS verification enabled using x86 verstage and PSP verstage. Change-Id: I4269069b66ed66c7b1a47fdef2fd0a8054b2e6a1 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/68134 Reviewed-by: Julius Werner <jwerner(a)chromium.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/lib/Makefile.inc 1 file changed, 26 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved Raul Rangel: Looks good to me, approved diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index 8cd01ad..1c7bc22 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -50,7 +50,11 @@ bootblock-y += cbfs.c bootblock-$(CONFIG_GENERIC_GPIO_LIB) += gpio.c bootblock-y += libgcc.c +ifneq ($(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),y) bootblock-$(CONFIG_CBFS_VERIFICATION) += metadata_hash.c +else # ($(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),y) +verstage-$(CONFIG_CBFS_VERIFICATION) += metadata_hash.c +endif # ($(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),y) bootblock-$(CONFIG_GENERIC_UDELAY) += timer.c bootblock-$(CONFIG_COLLECT_TIMESTAMPS) += timestamp.c -- To view, visit https://review.coreboot.org/c/coreboot/+/68134 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4269069b66ed66c7b1a47fdef2fd0a8054b2e6a1 Gerrit-Change-Number: 68134 Gerrit-PatchSet: 11 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2022