coreboot-gerrit April 2023

coreboot-gerrit@coreboot.org

1 participants
5761 discussions

[S] Change in coreboot[master]: mb/google/rex: Enable asynchronous End-Of-Post
by Subrata Banik (Code Review) 22 Apr '23

22 Apr '23

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74562 ) Change subject: mb/google/rex: Enable asynchronous End-Of-Post ...................................................................... mb/google/rex: Enable asynchronous End-Of-Post Set the `SOC_INTEL_CSE_SEND_EOP_ASYNC' flag to request End-Of-Post right after PCI enumeration and handle the command response at `BS_PAYLOAD_BOOT'. With these settings we have observed a boot time reduction of about 100ms on google/rex. TEST=Tests on google/rex with `SOC_INTEL_CSE_SEND_EOP_ASYNC' show End-Of-Post after PCI initialization and EOP message received at `BS_PAYLOAD_BOOT'. Change-Id: I27b540eeddcada521eba91fcc51504831d6dc855 Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/74562 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> --- M src/mainboard/google/rex/Kconfig M src/soc/intel/meteorlake/Kconfig 2 files changed, 26 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Kapil Porwal: Looks good to me, approved diff --git a/src/mainboard/google/rex/Kconfig b/src/mainboard/google/rex/Kconfig index 62ece95..528a2dc 100644 --- a/src/mainboard/google/rex/Kconfig +++ b/src/mainboard/google/rex/Kconfig @@ -27,6 +27,7 @@ select PMC_IPC_ACPI_INTERFACE select SOC_INTEL_CSE_LITE_SKU select SOC_INTEL_CSE_PRE_CPU_RESET_TELEMETRY_V2 + select SOC_INTEL_CSE_SEND_EOP_ASYNC config BOARD_GOOGLE_BASEBOARD_REX def_bool n diff --git a/src/soc/intel/meteorlake/Kconfig b/src/soc/intel/meteorlake/Kconfig index c34f255..7762d5f 100644 --- a/src/soc/intel/meteorlake/Kconfig +++ b/src/soc/intel/meteorlake/Kconfig @@ -89,7 +89,7 @@ select SOC_INTEL_COMMON_PCH_CLIENT select SOC_INTEL_COMMON_RESET select SOC_INTEL_COMMON_BLOCK_IOC - select SOC_INTEL_CSE_SEND_EOP_LATE + select SOC_INTEL_CSE_SEND_EOP_LATE if !MAINBOARD_HAS_CHROMEOS select SOC_INTEL_CSE_SET_EOP select SOC_INTEL_GFX_NON_PREFETCHABLE_MMIO select SOC_INTEL_MEM_MAPPED_PM_CONFIGURATION -- To view, visit https://review.coreboot.org/c/coreboot/+/74562 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I27b540eeddcada521eba91fcc51504831d6dc855 Gerrit-Change-Number: 74562 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: mb/google/rex: Enable asynchronous End-Of-Post
by Subrata Banik (Code Review) 22 Apr '23

22 Apr '23

Attention is currently required from: Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74562 ) Change subject: mb/google/rex: Enable asynchronous End-Of-Post ...................................................................... Patch Set 2: (1 comment) File src/soc/intel/meteorlake/Kconfig: https://review.coreboot.org/c/coreboot/+/74562/comment/13e7b16a_a574f343 PS1, Line 92: if !BOARD_GOOGLE_REX_COMMON > that looks a bit better to me; might be good to see if it's possible to have a soc-defined default and being able to select an override in the mainboard kconfig. > since i'm not a maintainer of this subtree, i don't have a too strong opinion on this; just some suggestions about how i'd probably do this Thanks! noted your feedback, i will come back with some improvement in future (sooner enough) as applicable -- To view, visit https://review.coreboot.org/c/coreboot/+/74562 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I27b540eeddcada521eba91fcc51504831d6dc855 Gerrit-Change-Number: 74562 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sat, 22 Apr 2023 05:37:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: include/cpu/amd/mtrr: fix typo in get_top_of_mem_above_4gb
by Felix Held (Code Review) 22 Apr '23

22 Apr '23

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/74710 ) Change subject: include/cpu/amd/mtrr: fix typo in get_top_of_mem_above_4gb ...................................................................... include/cpu/amd/mtrr: fix typo in get_top_of_mem_above_4gb Add the missing 'g' to the 4gb so that get_top_of_mem_above_4gb is in line with get_top_of_mem_below_4gb. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ic9170372d8b0c27d7de3bd04d822c95e2015cb10 --- M src/include/cpu/amd/mtrr.h M src/northbridge/amd/pi/00730F01/northbridge.c M src/soc/amd/common/block/acpi/tables.c M src/soc/amd/stoneyridge/northbridge.c 4 files changed, 18 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/74710/1 diff --git a/src/include/cpu/amd/mtrr.h b/src/include/cpu/amd/mtrr.h index e96dc90..04d775c 100644 --- a/src/include/cpu/amd/mtrr.h +++ b/src/include/cpu/amd/mtrr.h @@ -70,7 +70,7 @@ return rdmsr(TOP_MEM).lo; } -static inline uint64_t get_top_of_mem_above_4g(void) +static inline uint64_t get_top_of_mem_above_4gb(void) { msr_t msr = rdmsr(TOP_MEM2); return (uint64_t)msr.hi << 32 | msr.lo; diff --git a/src/northbridge/amd/pi/00730F01/northbridge.c b/src/northbridge/amd/pi/00730F01/northbridge.c index cba57d8..b9ac022 100644 --- a/src/northbridge/amd/pi/00730F01/northbridge.c +++ b/src/northbridge/amd/pi/00730F01/northbridge.c @@ -550,7 +550,7 @@ * Shift value right by 20 bit to make it fit into 32bit, * giving us 1MB granularity and a limit of almost 4Exabyte of memory. */ - acpigen_write_name_dword("TOM2", get_top_of_mem_above_4g() >> 20); + acpigen_write_name_dword("TOM2", get_top_of_mem_above_4gb() >> 20); acpigen_pop_len(); } @@ -834,7 +834,7 @@ sizek = 0; } else { - uint64_t topmem2 = get_top_of_mem_above_4g(); + uint64_t topmem2 = get_top_of_mem_above_4gb(); basek = 4 * 1024 * 1024; sizek = topmem2 / 1024 - basek; } diff --git a/src/soc/amd/common/block/acpi/tables.c b/src/soc/amd/common/block/acpi/tables.c index 17db421..3ba9af6 100644 --- a/src/soc/amd/common/block/acpi/tables.c +++ b/src/soc/amd/common/block/acpi/tables.c @@ -58,6 +58,6 @@ * Shift value right by 20 bit to make it fit into 32bit, * giving us 1MB granularity and a limit of almost 4Exabyte of memory. */ - acpigen_write_name_dword("TOM2", get_top_of_mem_above_4g() >> 20); + acpigen_write_name_dword("TOM2", get_top_of_mem_above_4gb() >> 20); acpigen_pop_len(); } diff --git a/src/soc/amd/stoneyridge/northbridge.c b/src/soc/amd/stoneyridge/northbridge.c index bafeabb..75fdfa4 100644 --- a/src/soc/amd/stoneyridge/northbridge.c +++ b/src/soc/amd/stoneyridge/northbridge.c @@ -328,7 +328,7 @@ uint32_t uma_size = get_uma_size(); uint32_t mem_useable = (uintptr_t)cbmem_top(); uint32_t tom = get_top_of_mem_below_4gb(); - uint64_t high_tom = get_top_of_mem_above_4g(); + uint64_t high_tom = get_top_of_mem_above_4gb(); uint64_t high_mem_useable; int idx = 0x10; -- To view, visit https://review.coreboot.org/c/coreboot/+/74710 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic9170372d8b0c27d7de3bd04d822c95e2015cb10 Gerrit-Change-Number: 74710 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[master]: mb/google/skyrim: Hide unused GPP port
by Felix Held (Code Review) 22 Apr '23

22 Apr '23

Attention is currently required from: Martin L Roth, Jason Nien, Eric Lai, Karthik Ramasubramanian. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73862 ) Change subject: mb/google/skyrim: Hide unused GPP port ...................................................................... Patch Set 1: -Code-Review (1 comment) Patchset: PS1: since CB:74526 got submitted, this is probably no longer needed -- To view, visit https://review.coreboot.org/c/coreboot/+/73862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9c1e4c21649610b856469b164ce5b0221f01b7f9 Gerrit-Change-Number: 73862 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Sat, 22 Apr 2023 02:47:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: mb/google/myst: Enable PCIe devices in devicetree
by Felix Held (Code Review) 22 Apr '23

22 Apr '23

Attention is currently required from: Raul Rangel, Martin L Roth, Jon Murphy, Tim Van Patten, Karthik Ramasubramanian, Mark Hasemeyer. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74112 ) Change subject: mb/google/myst: Enable PCIe devices in devicetree ...................................................................... Patch Set 40: -Code-Review (1 comment) File src/mainboard/google/myst/port_descriptors.c: https://review.coreboot.org/c/coreboot/+/74112/comment/1112ed72_14e05dea PS40, Line 51: GEN3 > `lspci -v` should show you the speed. i'm still working on the dxio descriptor data structure update; want to do all upd-related changes in one patch train to only have one point in time where we have an incompatible upd data structure change where we need to make sure to use the correct coreboot + fsp combination. the scope of the upd cleanup/improvement is rather large and not the only thing i work on right now, but i try to have that done before the bringup. the link speed parameter is mainly meant to limit the link speed -- To view, visit https://review.coreboot.org/c/coreboot/+/74112 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icdad785bcb90de036095bcc4219c15f55f4277fe Gerrit-Change-Number: 74112 Gerrit-PatchSet: 40 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Mark Hasemeyer <markhas(a)google.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Mark Hasemeyer <markhas(a)google.com> Gerrit-Comment-Date: Sat, 22 Apr 2023 02:39:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Tim Van Patten <timvp(a)google.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: mb/google/skyrim: Add mainboard chip.h
by Jon Murphy (Code Review) 22 Apr '23

22 Apr '23

Attention is currently required from: Raul Rangel, Jason Nien, Arthur Heymans, Martin Roth, Tim Van Patten, Karthik Ramasubramanian, Mark Hasemeyer. Jon Murphy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74609 ) Change subject: mb/google/skyrim: Add mainboard chip.h ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: > > adding the mainboard as a chip allows me to have a mainboard specific header to expose additional […] it's still describing the hardware, it's just describing it in the context of the board. It's providing additional descriptive information about the hardware. -- To view, visit https://review.coreboot.org/c/coreboot/+/74609 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic609d294135e2982064cb1c1fe6eac98dc24086f Gerrit-Change-Number: 74609 Gerrit-PatchSet: 5 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Mark Hasemeyer <markhas(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Mark Hasemeyer <markhas(a)google.com> Gerrit-Comment-Date: Sat, 22 Apr 2023 00:51:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jon Murphy <jpmurphy(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: mb/google/skyrim: Add mainboard chip.h
by Arthur Heymans (Code Review) 22 Apr '23

22 Apr '23

Attention is currently required from: Raul Rangel, Jason Nien, Jon Murphy, Martin Roth, Tim Van Patten, Karthik Ramasubramanian, Mark Hasemeyer. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74609 ) Change subject: mb/google/skyrim: Add mainboard chip.h ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: > adding the mainboard as a chip allows me to have a mainboard specific header to expose additional constants to the device tree. if you look at the CL above this, I've added net names in place of the #defined GPIOs. > > I'd be open to updating the parsing script to add said functionality for board definitions, but I expect that to affect more of the community and require more discussion and engagement Does this make sense? The devicetree is there to describe the hardware. Abstracting it again in a header C file with alias defeats that purpose. -- To view, visit https://review.coreboot.org/c/coreboot/+/74609 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic609d294135e2982064cb1c1fe6eac98dc24086f Gerrit-Change-Number: 74609 Gerrit-PatchSet: 5 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Mark Hasemeyer <markhas(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Mark Hasemeyer <markhas(a)google.com> Gerrit-Comment-Date: Sat, 22 Apr 2023 00:05:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jon Murphy <jpmurphy(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: Revert "util/ifdtool: Add option to create FMAP template"
by Matt DeVillier (Code Review) 22 Apr '23

22 Apr '23

1 0

[M] Change in coreboot[master]: util/ifdtool: Add option to create FMAP template
by Matt DeVillier (Code Review) 22 Apr '23

22 Apr '23

Matt DeVillier has created a revert of this change. ( https://review.coreboot.org/c/coreboot/+/73487 ) Change subject: util/ifdtool: Add option to create FMAP template ...................................................................... -- To view, visit https://review.coreboot.org/c/coreboot/+/73487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I82cb252fff456773af69943e188480a4998736fd Gerrit-Change-Number: 73487 Gerrit-PatchSet: 9 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-MessageType: revert

1 0

[M] Change in coreboot[master]: Revert "util/ifdtool: Add option to create FMAP template"
by Matt DeVillier (Code Review) 22 Apr '23

22 Apr '23

Attention is currently required from: Martin L Roth, Patrick Rudolph, Subrata Banik, Stefan Reinauer, Kapil Porwal, Maximilian Brune, Angel Pons, Lean Sheng Tan. Hello build bot (Jenkins), Martin L Roth, Patrick Rudolph, Subrata Banik, Kapil Porwal, Stefan Reinauer, Maximilian Brune, Angel Pons, Lean Sheng Tan, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/74692 to review the following change. Change subject: Revert "util/ifdtool: Add option to create FMAP template" ...................................................................... Revert "util/ifdtool: Add option to create FMAP template" This reverts commit 347596ae6ee5ec50526f5dbf9300c183e1d48cd0. Above commit inserts multiple IFD regions not present into the default FMAP template, some of which overlap, resulting in far less space available for BIOS region / CBFS. Change-Id: I7cbcbe16d508787f5c8aae4889199185890f0dc7 --- M Makefile.inc M util/ifdtool/ifdtool.c M util/ifdtool/ifdtool.h 3 files changed, 22 insertions(+), 118 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/74692/1 diff --git a/Makefile.inc b/Makefile.inc index d2d3f88..ba4f6fa 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -934,22 +934,11 @@ $(call sort-files,$(call placed-files-in-region,$(region))), \ $(call cbfs-add-cmd,$(file),$(region),$(CONFIG_UPDATE_IMAGE)))) -# If no FMD file (Flashmap) is supplied by mainboard, fall back to a default ifeq ($(CONFIG_FMDFILE),) - +# For a description of the flash layout described by these variables, check +# the $(DEFAULT_FLASHMAP) .fmd files. ifeq ($(CONFIG_ARCH_X86),y) - DEFAULT_FLASHMAP:=$(top)/util/cbfstool/default-x86.fmd -# check if IFD_CHIPSET is set and if yes generate a FMAP template from IFD descriptor -ifneq ($(CONFIG_IFD_CHIPSET),) -ifeq ($(CONFIG_HAVE_IFD_BIN),y) -DEFAULT_FLASHMAP:=$(obj)/fmap-template.fmd -$(DEFAULT_FLASHMAP): $(call strip_quotes,$(CONFIG_IFD_BIN_PATH)) $(IFDTOOL) - echo " IFDTOOL -p $(CONFIG_IFD_CHIPSET) -F $@ $<" - $(IFDTOOL) -p $(CONFIG_IFD_CHIPSET) -F $@ $< -endif # ifeq($(CONFIG_HAVE_IFD_BIN),y) -endif # ifneq($(CONFIG_IFD_CHIPSET),) - # entire flash FMAP_ROM_ADDR := $(call int-subtract, 0x100000000 $(CONFIG_ROM_SIZE)) FMAP_ROM_SIZE := $(CONFIG_ROM_SIZE) diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c index 625b4cf..c2dde3c 100644 --- a/util/ifdtool/ifdtool.c +++ b/util/ifdtool/ifdtool.c @@ -331,7 +331,6 @@ region.base = (flreg & base_mask) << 12; region.limit = ((flreg & limit_mask) >> 4) | 0xfff; region.size = region.limit - region.base + 1; - region.type = region_type; if (region.size < 0) region.size = 0; @@ -996,91 +995,6 @@ } } -/* Takes an image containing an IFD and creates a Flashmap .fmd file template. - * This flashmap will contain all IFD regions except the BIOS region. - * The BIOS region is created by coreboot itself and 'should' match the IFD region - * anyway (CONFIG_VALIDATE_INTEL_DESCRIPTOR should make sure). coreboot built system will use - * this template to generate the final Flashmap file. - */ -static void create_fmap_template(char *image, int size, const char *layout_fname) -{ - const struct frba *frba = find_frba(image, size); - if (!frba) - exit(EXIT_FAILURE); - - int layout_fd = open(layout_fname, O_WRONLY | O_CREAT | O_TRUNC, 0644); - if (layout_fd == -1) { - perror("Could not open file"); - exit(EXIT_FAILURE); - } - - char *bbuf = "FLASH@##ROM_BASE## ##ROM_SIZE## {\n"; - if (write(layout_fd, bbuf, strlen(bbuf)) < 0) { - perror("Could not write to file"); - exit(EXIT_FAILURE); - } - - /* fmaptool requires regions in .fmd to be sorted. - * => We need to sort the regions by base address before writing them in .fmd File - */ - int count_regions = 0; - struct region sorted_regions[MAX_REGIONS] = { 0 }; - for (unsigned int i = 0; i < max_regions; i++) { - struct region region = get_region(frba, i); - /* is region invalid? */ - if (region.size < 1) - continue; - - /* Here we decide to use the coreboot generated FMAP BIOS region, instead of - * the one specified in the IFD. The case when IFD and FMAP BIOS region do not - * match cannot be caught here, therefore one should still validate IFD and - * FMAP via CONFIG_VALIDATE_INTEL_DESCRIPTOR - */ - if (i == REGION_BIOS) - continue; - - sorted_regions[count_regions] = region; - // basically insertion sort - for (int i = count_regions-1; i >= 0 ; i--) { - if (sorted_regions[i].base > sorted_regions[i+1].base) { - struct region tmp = sorted_regions[i]; - sorted_regions[i] = sorted_regions[i+1]; - sorted_regions[i+1] = tmp; - } - } - count_regions++; - } - - // Now write regions sorted by base address in the fmap file - for (int i = 0; i < count_regions; i++) { - struct region region = sorted_regions[i]; - char buf[LAYOUT_LINELEN]; - snprintf(buf, LAYOUT_LINELEN, "\t%s@0x%X 0x%X\n", region_names[region.type].fmapname, region.base, region.size); - if (write(layout_fd, buf, strlen(buf)) < 0) { - perror("Could not write to file"); - exit(EXIT_FAILURE); - } - } - - char *ebuf = "\tSI_BIOS@##BIOS_BASE## ##BIOS_SIZE## {\n" - "\t\t##CONSOLE_ENTRY##\n" - "\t\t##MRC_CACHE_ENTRY##\n" - "\t\t##SMMSTORE_ENTRY##\n" - "\t\t##SPD_CACHE_ENTRY##\n" - "\t\t##VPD_ENTRY##\n" - "\t\tFMAP@##FMAP_BASE## ##FMAP_SIZE##\n" - "\t\tCOREBOOT(CBFS)@##CBFS_BASE## ##CBFS_SIZE##\n" - "\t}\n" - "}\n"; - if (write(layout_fd, ebuf, strlen(ebuf)) < 0) { - perror("Could not write to file"); - exit(EXIT_FAILURE); - } - - close(layout_fd); - printf("Wrote layout to %s\n", layout_fname); -} - static void write_regions(char *image, int size) { unsigned int i; @@ -1775,7 +1689,6 @@ printf("\n" " -d | --dump: dump intel firmware descriptor\n" " -f | --layout <filename> dump regions into a flashrom layout file\n" - " -F | --fmap-layout <filename> dump IFD regions into a fmap layout template (.fmd) file\n" " -t | --validate Validate that the firmware descriptor layout matches the fmap layout\n" " -x | --extract: extract intel fd modules\n" " -i | --inject <region>:<module> inject file <module> into region <region>\n" @@ -1822,7 +1735,7 @@ int mode_dump = 0, mode_extract = 0, mode_inject = 0, mode_spifreq = 0; int mode_em100 = 0, mode_locked = 0, mode_unlocked = 0, mode_validate = 0; int mode_layout = 0, mode_newlayout = 0, mode_density = 0, mode_setstrap = 0; - int mode_read = 0, mode_altmedisable = 0, altmedisable = 0, mode_fmap_template = 0; + int mode_read = 0, mode_altmedisable = 0, altmedisable = 0; char *region_type_string = NULL, *region_fname = NULL; const char *layout_fname = NULL; char *new_filename = NULL; @@ -1835,7 +1748,6 @@ static const struct option long_options[] = { {"dump", 0, NULL, 'd'}, {"layout", 1, NULL, 'f'}, - {"fmap-template", 1, NULL, 'F'}, {"extract", 0, NULL, 'x'}, {"inject", 1, NULL, 'i'}, {"newlayout", 1, NULL, 'n'}, @@ -1857,7 +1769,7 @@ {0, 0, 0, 0} }; - while ((opt = getopt_long(argc, argv, "S:V:df:F:D:C:M:xi:n:O:s:p:elruvth?", + while ((opt = getopt_long(argc, argv, "S:V:df:D:C:M:xi:n:O:s:p:elruvth?", long_options, &option_index)) != EOF) { switch (opt) { case 'd': @@ -1879,15 +1791,6 @@ exit(EXIT_FAILURE); } break; - case 'F': - mode_fmap_template = 1; - layout_fname = strdup(optarg); - if (!layout_fname) { - fprintf(stderr, "No layout file specified\n"); - fprintf(stderr, "run '%s -h' for usage\n", argv[0]); - exit(EXIT_FAILURE); - } - break; case 'x': mode_extract = 1; break; @@ -2110,7 +2013,7 @@ } } - if ((mode_dump + mode_layout + mode_fmap_template + mode_extract + mode_inject + + if ((mode_dump + mode_layout + mode_extract + mode_inject + mode_setstrap + mode_newlayout + (mode_spifreq | mode_em100 | mode_unlocked | mode_locked) + mode_altmedisable + mode_validate) > 1) { fprintf(stderr, "You may not specify more than one mode.\n\n"); @@ -2118,7 +2021,7 @@ exit(EXIT_FAILURE); } - if ((mode_dump + mode_layout + mode_fmap_template + mode_extract + mode_inject + + if ((mode_dump + mode_layout + mode_extract + mode_inject + mode_setstrap + mode_newlayout + mode_spifreq + mode_em100 + mode_locked + mode_unlocked + mode_density + mode_altmedisable + mode_validate) == 0) { fprintf(stderr, "You need to specify a mode.\n\n"); @@ -2183,9 +2086,6 @@ if (mode_layout) dump_flashrom_layout(image, size, layout_fname); - if (mode_fmap_template) - create_fmap_template(image, size, layout_fname); - if (mode_extract) write_regions(image, size); diff --git a/util/ifdtool/ifdtool.h b/util/ifdtool/ifdtool.h index 5717a0a..6029c66 100644 --- a/util/ifdtool/ifdtool.h +++ b/util/ifdtool/ifdtool.h @@ -191,7 +191,7 @@ }; struct region { - int base, limit, size, type; + int base, limit, size; }; struct region_name { -- To view, visit https://review.coreboot.org/c/coreboot/+/74692 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7cbcbe16d508787f5c8aae4889199185890f0dc7 Gerrit-Change-Number: 74692 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-MessageType: newchange

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2023