coreboot-gerrit November 2022

coreboot-gerrit@coreboot.org

1 participants
7286 discussions

[M] Change in coreboot[master]: soc/intel/common: Add support to read CPU and PCH Trace Hub modes
by Felix Held (Code Review) 21 Nov '22

21 Nov '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/64437 ) Change subject: soc/intel/common: Add support to read CPU and PCH Trace Hub modes ...................................................................... soc/intel/common: Add support to read CPU and PCH Trace Hub modes The patch parses CPU and PCH Trace Hub modes from the debug area in the Descriptor Region. The modes can be updated in the debug area in order to configure the CPU and PCH Trace Hub modes. The debug area's offset starts from the SPI Flash offset:0xf00. For runtime debugging, the OEM Section in the Descriptor Region is being used as debug area. The OEM Section details are documented in the SPI Programmer Guide of CSE Lite kit. TEST=Build code for Gimble Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Change-Id: I61241c5c1981ddc4b21581bb3ed9f531da5f41b2 Reviewed-on: https://review.coreboot.org/c/coreboot/+/64437 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Tim Wawrzynczak <inforichland(a)gmail.com> --- M src/soc/intel/common/basecode/debug/debug_feature.c M src/soc/intel/common/basecode/include/intelbasecode/debug_feature.h 2 files changed, 53 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved diff --git a/src/soc/intel/common/basecode/debug/debug_feature.c b/src/soc/intel/common/basecode/debug/debug_feature.c index 6491934..d048abf 100644 --- a/src/soc/intel/common/basecode/debug/debug_feature.c +++ b/src/soc/intel/common/basecode/debug/debug_feature.c @@ -9,9 +9,22 @@ #define DEBUG_FEATURE_CTRL_SZ 64 #define SI_DESC_REGION_SZ 4096 +#define DEBUG_FEATURE_UNDEFINED 0xff struct debug_feature_cntrl { uint8_t cse_fw_update_disable; /* Byte location: 0xF00 */ - uint8_t reserved[63]; + + /* + * Supported CPU Trace Hub modes: + * 0: Disable, 1: Target Debugger Mode, 2: Host Debugger Mode + */ + uint8_t cpu_tracehub_mode; /* Byte location: 0xF01 */ + + /* + * Supported PCH Trace Hub modes: + * 0: Disable, 1:Target Debugger Mode, 2:Host Debugger Mode + */ + uint8_t pch_tracehub_mode; /* Byte location: 0xF02 */ + uint8_t reserved[61]; }; static struct debug_feature_cntrl dbg_feature_cntrl; @@ -20,6 +33,18 @@ && sizeof(struct debug_feature_cntrl) <= 256, "sizeof(struct debug_feature_cntrl) must be a multiple of 64 bytes and up to 256 bytes"); +void debug_get_pch_cpu_tracehub_modes(uint8_t *cpu_tracehub_mode, uint8_t *pch_tracehub_mode) +{ + if (dbg_feature_cntrl.pch_tracehub_mode != DEBUG_FEATURE_UNDEFINED) + *pch_tracehub_mode = dbg_feature_cntrl.pch_tracehub_mode; + + if (dbg_feature_cntrl.cpu_tracehub_mode != DEBUG_FEATURE_UNDEFINED) + *cpu_tracehub_mode = dbg_feature_cntrl.cpu_tracehub_mode; + + printk(BIOS_DEBUG, "rt_debug: CPU Trace Hub Mode: %d PCH Trace Hub Mode: %d\n", + *pch_tracehub_mode, *cpu_tracehub_mode); +} + bool is_debug_cse_fw_update_disable(void) { printk(BIOS_DEBUG, "rt_debug: dbg_feature_cntrl.cse_fw_update_disable=%d\n", diff --git a/src/soc/intel/common/basecode/include/intelbasecode/debug_feature.h b/src/soc/intel/common/basecode/include/intelbasecode/debug_feature.h index c8e2382..9a01590f 100644 --- a/src/soc/intel/common/basecode/include/intelbasecode/debug_feature.h +++ b/src/soc/intel/common/basecode/include/intelbasecode/debug_feature.h @@ -5,6 +5,9 @@ #include <types.h> +/* Get cpu and pch tracehub modes defined in the OEM Section of descriptor region */ +void debug_get_pch_cpu_tracehub_modes(uint8_t *cpu_tracehub_mode, uint8_t *pch_trachub_mode); + /* Check if CSE firmware update is enabled or not */ bool is_debug_cse_fw_update_disable(void); -- To view, visit https://review.coreboot.org/c/coreboot/+/64437 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I61241c5c1981ddc4b21581bb3ed9f531da5f41b2 Gerrit-Change-Number: 64437 Gerrit-PatchSet: 9 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Maulik Vaghela <maulikvaghela(a)google.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: vboot: Fix hash digest size, padding and comparison
by Jakub Czapiga (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Jakub Czapiga, Caveh Jalali, Julius Werner, Boris Mittelberg. Hello build bot (Jenkins), Caveh Jalali, Julius Werner, Yu-Ping Wu, Boris Mittelberg, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/69762 to look at the new patch set (#4). Change subject: vboot: Fix hash digest size, padding and comparison ...................................................................... vboot: Fix hash digest size, padding and comparison Allow for saving hash shorter than slot size by padding too short data with zeros. Eliminate possible prefix-only comparisons. Signed-off-by: Jakub Czapiga <jacz(a)semihalf.com> Change-Id: If6d46e0b58dbca86af56221b7ff2606ab2d1799a --- M src/ec/google/chromeec/vboot_storage.c M src/security/vboot/vboot_logic.c 2 files changed, 25 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/69762/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/69762 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If6d46e0b58dbca86af56221b7ff2606ab2d1799a Gerrit-Change-Number: 69762 Gerrit-PatchSet: 4 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Boris Mittelberg <bmbm(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: soc/intel/meteorlake: Select X86_INIT_NEED_1_SIPI Kconfig
by build bot (Jenkins) (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Tarun Tuli, Kapil Porwal. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69851 ) Change subject: soc/intel/meteorlake: Select X86_INIT_NEED_1_SIPI Kconfig ...................................................................... Patch Set 3: (1 comment) Commit Message: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-164178): https://review.coreboot.org/c/coreboot/+/69851/comment/6b3cb87c_62ff4433 PS3, Line 8: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 6526e7896727 ("soc/intel/alderlake: Select X86_INIT_NEED_1_SIPI Kconfig for RPL")' -- To view, visit https://review.coreboot.org/c/coreboot/+/69851 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iec21470b9b34514169789c39bdc3be4e4ff6c7b5 Gerrit-Change-Number: 69851 Gerrit-PatchSet: 3 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-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Mon, 21 Nov 2022 14:00:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/meteorlake: Select X86_INIT_NEED_1_SIPI Kconfig
by Subrata Banik (Code Review) 21 Nov '22

21 Nov '22

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69851 ) Change subject: soc/intel/meteorlake: Select X86_INIT_NEED_1_SIPI Kconfig ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/69851 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iec21470b9b34514169789c39bdc3be4e4ff6c7b5 Gerrit-Change-Number: 69851 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 21 Nov 2022 14:00:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: util/amdfwtool: Deal with psp position in flash offset directly
by build bot (Jenkins) (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Jason Nien, Matt DeVillier, Martin Roth, Fred Reitberger, Felix Held. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69852 ) Change subject: util/amdfwtool: Deal with psp position in flash offset directly ...................................................................... Patch Set 2: (6 comments) File util/amdfwtool/amdfwtool.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-164177): https://review.coreboot.org/c/coreboot/+/69852/comment/75717f0b_5d605fd4 PS2, Line 2328: case 0xFA0000: /* Fall through */ Prefer 'fallthrough;' over fallthrough comment Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-164177): https://review.coreboot.org/c/coreboot/+/69852/comment/3b022495_aca5eabf PS2, Line 2329: case 0xF20000: /* Fall through */ Prefer 'fallthrough;' over fallthrough comment Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-164177): https://review.coreboot.org/c/coreboot/+/69852/comment/947c43f3_9e2e5cb7 PS2, Line 2330: case 0xE20000: /* Fall through */ Prefer 'fallthrough;' over fallthrough comment Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-164177): https://review.coreboot.org/c/coreboot/+/69852/comment/99df1de8_ea3da53f PS2, Line 2331: case 0xC20000: /* Fall through */ Prefer 'fallthrough;' over fallthrough comment Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-164177): https://review.coreboot.org/c/coreboot/+/69852/comment/bfb31a55_ee070fcb PS2, Line 2332: case 0x820000: /* Fall through */ Prefer 'fallthrough;' over fallthrough comment Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-164177): https://review.coreboot.org/c/coreboot/+/69852/comment/cd149461_4a16cbf2 PS2, Line 2333: case 0x20000: /* Fall through */ Prefer 'fallthrough;' over fallthrough comment -- To view, visit https://review.coreboot.org/c/coreboot/+/69852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I89c9e73e7db748379c97e3c0ad69af3faedc8d66 Gerrit-Change-Number: 69852 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 21 Nov 2022 13:45:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: util/amdfwtool: Deal with psp position in flash offset directly
by Arthur Heymans (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Jason Nien, Matt DeVillier, Martin Roth, Fred Reitberger, Felix Held. Hello Jason Glenesk, Raul Rangel, Jason Nien, Matt DeVillier, Martin Roth, Fred Reitberger, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/69852 to look at the new patch set (#2). Change subject: util/amdfwtool: Deal with psp position in flash offset directly ...................................................................... util/amdfwtool: Deal with psp position in flash offset directly Using a memory mapped offset is simply wrong to specify the PSP base as it depends on the flash size. Setting the flash offset directly in Kconfig allows to remove a lot of unnecessary aritmetics in both amdfwtool and Makefile. It also gets rid of the weird index semantics. This does not result in identical files as the aritmetics for setting the amdfwtool is cbfs was simply wrong and broken. TESTED: google/vilboz still boots. Change-Id: I89c9e73e7db748379c97e3c0ad69af3faedc8d66 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/mainboard/amd/birman/Kconfig M src/mainboard/amd/chausie/Kconfig M src/mainboard/amd/majolica/Kconfig M src/mainboard/amd/mandolin/Kconfig M src/mainboard/google/guybrush/Kconfig M src/mainboard/google/kahlee/Kconfig M src/mainboard/google/skyrim/Kconfig M src/mainboard/google/zork/Kconfig M src/soc/amd/cezanne/Kconfig M src/soc/amd/cezanne/Makefile.inc M src/soc/amd/common/Makefile.inc M src/soc/amd/common/block/include/amdblocks/psp_efs.h M src/soc/amd/common/block/psp/Kconfig M src/soc/amd/glinda/Kconfig M src/soc/amd/glinda/Makefile.inc M src/soc/amd/mendocino/Kconfig M src/soc/amd/mendocino/Makefile.inc M src/soc/amd/morgana/Kconfig M src/soc/amd/morgana/Makefile.inc M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/stoneyridge/Kconfig M src/soc/amd/stoneyridge/Makefile.inc M src/soc/amd/stoneyridge/fch.c M util/amdfwtool/amdfwtool.c 25 files changed, 109 insertions(+), 324 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/69852/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/69852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I89c9e73e7db748379c97e3c0ad69af3faedc8d66 Gerrit-Change-Number: 69852 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[L] Change in coreboot[master]: util/amdfwtool: Deal psp position in flash offset directly
by build bot (Jenkins) (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Jason Nien, Matt DeVillier, Martin Roth, Fred Reitberger, Felix Held. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69852 ) Change subject: util/amdfwtool: Deal psp position in flash offset directly ...................................................................... Patch Set 1: (6 comments) File util/amdfwtool/amdfwtool.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-164176): https://review.coreboot.org/c/coreboot/+/69852/comment/526272a7_0db4dd38 PS1, Line 2328: case 0xFA0000: /* Fall through */ Prefer 'fallthrough;' over fallthrough comment Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-164176): https://review.coreboot.org/c/coreboot/+/69852/comment/b1416905_0b488103 PS1, Line 2329: case 0xF20000: /* Fall through */ Prefer 'fallthrough;' over fallthrough comment Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-164176): https://review.coreboot.org/c/coreboot/+/69852/comment/506ee3f1_85b9132b PS1, Line 2330: case 0xE20000: /* Fall through */ Prefer 'fallthrough;' over fallthrough comment Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-164176): https://review.coreboot.org/c/coreboot/+/69852/comment/47c872cd_8cc3b8aa PS1, Line 2331: case 0xC20000: /* Fall through */ Prefer 'fallthrough;' over fallthrough comment Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-164176): https://review.coreboot.org/c/coreboot/+/69852/comment/b9d01792_2469bb94 PS1, Line 2332: case 0x820000: /* Fall through */ Prefer 'fallthrough;' over fallthrough comment Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-164176): https://review.coreboot.org/c/coreboot/+/69852/comment/5eeaeda3_fa8ef2d1 PS1, Line 2333: case 0x20000: /* Fall through */ Prefer 'fallthrough;' over fallthrough comment -- To view, visit https://review.coreboot.org/c/coreboot/+/69852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I89c9e73e7db748379c97e3c0ad69af3faedc8d66 Gerrit-Change-Number: 69852 Gerrit-PatchSet: 1 Gerrit-Owner: 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: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 21 Nov 2022 13:38:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: util/amdfwtool: Deal psp position in flash offset directly
by Arthur Heymans (Code Review) 21 Nov '22

21 Nov '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/69852 ) Change subject: util/amdfwtool: Deal psp position in flash offset directly ...................................................................... util/amdfwtool: Deal psp position in flash offset directly Using a memory mapped offset is simply wrong to specify the PSP base is simply wrong as it depends on the flash size. Setting the flash offset directly in Kconfig allows to remove a lot of unnecessary aritmetics in both amdfwtool and Makefile. This does not result in identical files as the aritmetics for setting the amdfwtool is cbfs was simply wrong and broken. Change-Id: I89c9e73e7db748379c97e3c0ad69af3faedc8d66 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/mainboard/amd/birman/Kconfig M src/mainboard/amd/chausie/Kconfig M src/mainboard/amd/majolica/Kconfig M src/mainboard/amd/mandolin/Kconfig M src/mainboard/google/guybrush/Kconfig M src/mainboard/google/kahlee/Kconfig M src/mainboard/google/skyrim/Kconfig M src/mainboard/google/zork/Kconfig M src/soc/amd/cezanne/Kconfig M src/soc/amd/cezanne/Makefile.inc M src/soc/amd/common/Makefile.inc M src/soc/amd/common/block/include/amdblocks/psp_efs.h M src/soc/amd/common/block/psp/Kconfig M src/soc/amd/glinda/Kconfig M src/soc/amd/glinda/Makefile.inc M src/soc/amd/mendocino/Kconfig M src/soc/amd/mendocino/Makefile.inc M src/soc/amd/morgana/Kconfig M src/soc/amd/morgana/Makefile.inc M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/stoneyridge/Kconfig M src/soc/amd/stoneyridge/Makefile.inc M src/soc/amd/stoneyridge/fch.c M util/amdfwtool/amdfwtool.c 25 files changed, 107 insertions(+), 324 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/69852/1 diff --git a/src/mainboard/amd/birman/Kconfig b/src/mainboard/amd/birman/Kconfig index 412300d..c6cae09 100644 --- a/src/mainboard/amd/birman/Kconfig +++ b/src/mainboard/amd/birman/Kconfig @@ -14,6 +14,7 @@ select PCIEXP_COMMON_CLOCK select PCIEXP_L1_SUB_STATE select SOC_AMD_COMMON_BLOCK_ESPI_RETAIN_PORT80_EN + select AMD_FWM_POSITION_E20000_DEFAULT if CHROMEOS config FMDFILE default "src/mainboard/amd/birman/chromeos.fmd" if CHROMEOS @@ -30,12 +31,6 @@ default "devicetree_glinda.cb" if BOARD_AMD_BIRMAN_GLINDA default "devicetree_morgana.cb" -config AMD_FWM_POSITION_INDEX - int - default 3 if CHROMEOS - help - TODO: might need to be adapted for better placement of files in cbfs - config BIRMAN_HAVE_MCHP_FW bool "Have Microchip EC firmware?" default n diff --git a/src/mainboard/amd/chausie/Kconfig b/src/mainboard/amd/chausie/Kconfig index da0e0c4..b2dd2f9 100644 --- a/src/mainboard/amd/chausie/Kconfig +++ b/src/mainboard/amd/chausie/Kconfig @@ -15,6 +15,7 @@ select PCIEXP_COMMON_CLOCK select PCIEXP_L1_SUB_STATE select SOC_AMD_COMMON_BLOCK_ESPI_RETAIN_PORT80_EN + select AMD_FWM_POSITION_E20000_DEFAULT if CHROMEOS config FMDFILE default "src/mainboard/amd/chausie/chromeos.fmd" if CHROMEOS @@ -26,12 +27,6 @@ config MAINBOARD_PART_NUMBER default "CHAUSIE" -config AMD_FWM_POSITION_INDEX - int - default 3 if CHROMEOS - help - TODO: might need to be adapted for better placement of files in cbfs - config CHAUSIE_HAVE_MCHP_FW bool "Have Microchip EC firmware?" default n diff --git a/src/mainboard/amd/majolica/Kconfig b/src/mainboard/amd/majolica/Kconfig index 7095e01..83b4a3f 100644 --- a/src/mainboard/amd/majolica/Kconfig +++ b/src/mainboard/amd/majolica/Kconfig @@ -9,6 +9,8 @@ select SOC_AMD_COMMON_BLOCK_USE_ESPI select AMD_SOC_CONSOLE_UART select MAINBOARD_HAS_CHROMEOS + select AMD_FWM_POSITION_E20000_DEFAULT if CHROMEOS + select AMD_FWM_POSITION_F20000_DEFAULT if !CHROMEOS config FMDFILE default "src/mainboard/amd/majolica/chromeos.fmd" if CHROMEOS @@ -20,13 +22,6 @@ config MAINBOARD_PART_NUMBER default "MAJOLICA" -config AMD_FWM_POSITION_INDEX - int - default 3 if CHROMEOS - default 4 - help - TODO: might need to be adapted for better placement of files in cbfs - config MAJOLICA_HAVE_MCHP_FW bool "Have Microchip EC firmware?" default n diff --git a/src/mainboard/amd/mandolin/Kconfig b/src/mainboard/amd/mandolin/Kconfig index 50b3e5d..3425391 100644 --- a/src/mainboard/amd/mandolin/Kconfig +++ b/src/mainboard/amd/mandolin/Kconfig @@ -12,6 +12,8 @@ select HAVE_ACPI_RESUME select DRIVERS_UART_ACPI select AMD_SOC_CONSOLE_UART if !AMD_LPC_DEBUG_CARD + select AMD_FWM_POSITION_E20000_DEFAULT if BOARD_AMD_MANDOLIN + select AMD_FWM_POSITION_F20000_DEFAULT if BOARD_AMD_CEREME config FMDFILE default "src/mainboard/amd/mandolin/variants/\$(CONFIG_VARIANT_DIR)/board.fmd" @@ -67,13 +69,6 @@ bool default y -config AMD_FWM_POSITION_INDEX - int - default 3 if BOARD_AMD_MANDOLIN - default 4 if BOARD_AMD_CEREME - help - TODO: might need to be adapted for better placement of files in cbfs - config MANDOLIN_HAVE_MCHP_FW bool "Have Microchip EC firmware?" default n diff --git a/src/mainboard/google/guybrush/Kconfig b/src/mainboard/google/guybrush/Kconfig index 50bcfa9..2777c1d 100644 --- a/src/mainboard/google/guybrush/Kconfig +++ b/src/mainboard/google/guybrush/Kconfig @@ -49,6 +49,7 @@ select SOC_AMD_COMMON_BLOCK_PSP_FUSE_SPL select SYSTEM_TYPE_LAPTOP select TPM_GOOGLE_CR50 + select AMD_FWM_POSITION_E20000_DEFAULT config VBOOT select EC_GOOGLE_CHROMEEC_SWITCHES @@ -81,11 +82,6 @@ default "Nipperkin" if BOARD_GOOGLE_NIPPERKIN default "Dewatt" if BOARD_GOOGLE_DEWATT -config AMD_FWM_POSITION_INDEX - int - default 3 - help - TODO: might need to be adapted for better placement of files in cbfs config DRIVER_TPM_I2C_BUS hex diff --git a/src/mainboard/google/kahlee/Kconfig b/src/mainboard/google/kahlee/Kconfig index 8c8486e..2fe702c 100644 --- a/src/mainboard/google/kahlee/Kconfig +++ b/src/mainboard/google/kahlee/Kconfig @@ -35,6 +35,7 @@ select HAVE_EM100_SUPPORT select SYSTEM_TYPE_LAPTOP select TPM_GOOGLE_CR50 + select AMD_FWM_POSITION_820000_DEFAULT if BOARD_GOOGLE_BASEBOARD_KAHLEE @@ -104,10 +105,6 @@ config CHROMEOS select LP_DEFCONFIG_OVERRIDE if PAYLOAD_DEPTHCHARGE -config AMD_FWM_POSITION_INDEX - int - default 1 - config DRIVER_TPM_I2C_BUS hex default 0x01 diff --git a/src/mainboard/google/skyrim/Kconfig b/src/mainboard/google/skyrim/Kconfig index ff80c26..d6458f8 100644 --- a/src/mainboard/google/skyrim/Kconfig +++ b/src/mainboard/google/skyrim/Kconfig @@ -5,12 +5,6 @@ if BOARD_GOOGLE_BASEBOARD_SKYRIM -config AMD_FWM_POSITION_INDEX - int - default 3 - help - TODO: might need to be adapted for better placement of files in cbfs - config BOARD_SPECIFIC_OPTIONS def_bool y select AMD_SOC_CONSOLE_UART @@ -44,6 +38,7 @@ select PCIEXP_CLK_PM select PCIEXP_COMMON_CLOCK select PCIEXP_L1_SUB_STATE + select AMD_FWM_POSITION_E20000_DEFAULT config DEVICETREE default "variants/baseboard/devicetree.cb" diff --git a/src/mainboard/google/zork/Kconfig b/src/mainboard/google/zork/Kconfig index d483999..d6e889e 100644 --- a/src/mainboard/google/zork/Kconfig +++ b/src/mainboard/google/zork/Kconfig @@ -51,6 +51,7 @@ select DRIVERS_USB_ACPI select DRIVERS_UART_ACPI select DRIVERS_GENERIC_BAYHUB_LV2 + select AMD_FWM_POSITION_C20000_DEFAULT config ELOG_BOOT_COUNT_CMOS_OFFSET int @@ -119,10 +120,6 @@ # Use default libpayload config select LP_DEFCONFIG_OVERRIDE if PAYLOAD_DEPTHCHARGE -config AMD_FWM_POSITION_INDEX - int - default 2 - config DRIVER_TPM_I2C_BUS hex default 0x03 diff --git a/src/soc/amd/cezanne/Kconfig b/src/soc/amd/cezanne/Kconfig index 30e3e73..fd9ed5e 100644 --- a/src/soc/amd/cezanne/Kconfig +++ b/src/soc/amd/cezanne/Kconfig @@ -316,39 +316,6 @@ menu "PSP Configuration Options" -config AMD_FWM_POSITION_INDEX - int "Firmware Directory Table location (0 to 5)" - range 0 5 - default 0 if BOARD_ROMSIZE_KB_512 - default 1 if BOARD_ROMSIZE_KB_1024 - default 2 if BOARD_ROMSIZE_KB_2048 - default 3 if BOARD_ROMSIZE_KB_4096 - default 4 if BOARD_ROMSIZE_KB_8192 - default 5 if BOARD_ROMSIZE_KB_16384 - help - Typically this is calculated by the ROM size, but there may - be situations where you want to put the firmware directory - table in a different location. - 0: 512 KB - 0xFFFA0000 - 1: 1 MB - 0xFFF20000 - 2: 2 MB - 0xFFE20000 - 3: 4 MB - 0xFFC20000 - 4: 8 MB - 0xFF820000 - 5: 16 MB - 0xFF020000 - -comment "AMD Firmware Directory Table set to location for 512KB ROM" - depends on AMD_FWM_POSITION_INDEX = 0 -comment "AMD Firmware Directory Table set to location for 1MB ROM" - depends on AMD_FWM_POSITION_INDEX = 1 -comment "AMD Firmware Directory Table set to location for 2MB ROM" - depends on AMD_FWM_POSITION_INDEX = 2 -comment "AMD Firmware Directory Table set to location for 4MB ROM" - depends on AMD_FWM_POSITION_INDEX = 3 -comment "AMD Firmware Directory Table set to location for 8MB ROM" - depends on AMD_FWM_POSITION_INDEX = 4 -comment "AMD Firmware Directory Table set to location for 16MB ROM" - depends on AMD_FWM_POSITION_INDEX = 5 - config AMDFW_CONFIG_FILE string default "src/soc/amd/cezanne/fw.cfg" diff --git a/src/soc/amd/cezanne/Makefile.inc b/src/soc/amd/cezanne/Makefile.inc index 1f2bb6d..ca633ed 100644 --- a/src/soc/amd/cezanne/Makefile.inc +++ b/src/soc/amd/cezanne/Makefile.inc @@ -55,21 +55,12 @@ MAINBOARD_BLOBS_DIR:=$(top)/3rdparty/blobs/mainboard/$(MAINBOARDDIR) # ROMSIG Normally At ROMBASE + 0x20000 -# Overridden by CONFIG_AMD_FWM_POSITION_INDEX # +-----------+---------------+----------------+------------+ # |0x55AA55AA | | | | # +-----------+---------------+----------------+------------+ # | | PSPDIR ADDR | BIOSDIR ADDR | # +-----------+---------------+----------------+ -$(if $(CONFIG_AMD_FWM_POSITION_INDEX), ,\ - $(error Invalid AMD firmware position index. Check if the board sets a valid ROM size)) - -CEZANNE_FWM_POSITION=$(call int-add, \ - $(call int-subtract, 0xffffffff \ - $(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 # Building the cbfs image will fail if the offset isn't large enough AMD_FW_AB_POSITION := 0x40 diff --git a/src/soc/amd/common/Makefile.inc b/src/soc/amd/common/Makefile.inc index f047a59..3fa9461 100644 --- a/src/soc/amd/common/Makefile.inc +++ b/src/soc/amd/common/Makefile.inc @@ -22,9 +22,8 @@ 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) + $(call int-subtract, 0xffffffff $(CONFIG_ROM_SIZE)) 1 $(CONFIG_AMD_FWM_POSITION)) + endif # ifeq ($(CONFIG_RESET_VECTOR_IN_RAM),y) ifeq ($(CONFIG_VBOOT_GSCVD),y) diff --git a/src/soc/amd/common/block/include/amdblocks/psp_efs.h b/src/soc/amd/common/block/include/amdblocks/psp_efs.h index 51b06a6..05c1644 100644 --- a/src/soc/amd/common/block/include/amdblocks/psp_efs.h +++ b/src/soc/amd/common/block/include/amdblocks/psp_efs.h @@ -6,7 +6,7 @@ #include <types.h> -#define EFS_OFFSET (0xffffff - (0x80000 << CONFIG_AMD_FWM_POSITION_INDEX) + 0x20000 + 1) +#define EFS_OFFSET (0xffffffff - CONFIG_ROM_SIZE + 1 - CONFIG_AMD_FWW_POSITION) #define EMBEDDED_FW_SIGNATURE 0x55aa55aa diff --git a/src/soc/amd/common/block/psp/Kconfig b/src/soc/amd/common/block/psp/Kconfig index 84e0215..b9c77a5 100644 --- a/src/soc/amd/common/block/psp/Kconfig +++ b/src/soc/amd/common/block/psp/Kconfig @@ -48,3 +48,61 @@ coreboot ROM is properly signed and can not be disabled once fused. Refer AMD PSB user guide doc# 56654, Revision# 1.00, this document is only available with NDA customers. + +config AMD_FWM_POSITION_20000_DEFAULT + bool + +config AMD_FWM_POSITION_820000_DEFAULT + bool + +config AMD_FWM_POSITION_C20000_DEFAULT + bool + +config AMD_FWM_POSITION_E20000_DEFAULT + bool + +config AMD_FWM_POSITION_F20000_DEFAULT + bool + +config AMD_FWM_POSITION_FA0000_DEFAULT + bool + +choice AMD_FWM_POSITION_CHOICE + prompt "AMD FW position" + default AMD_FWM_POSITION_20000 if AMD_FWM_POSITION_20000_DEFAULT + default AMD_FWM_POSITION_820000 if AMD_FWM_POSITION_820000_DEFAULT + default AMD_FWM_POSITION_C20000 if AMD_FWM_POSITION_C20000_DEFAULT + default AMD_FWM_POSITION_E20000 if AMD_FWM_POSITION_E20000_DEFAULT + default AMD_FWM_POSITION_F20000 if AMD_FWM_POSITION_F20000_DEFAULT + default AMD_FWM_POSITION_FA0000 if AMD_FWM_POSITION_FA0000_DEFAULT + help + Set the position on flash offset where the AMD FW needs to be. + +config AMD_FWM_POSITION_20000 + bool "0x20000" + +config AMD_FWM_POSITION_820000 + bool "0x820000" + +config AMD_FWM_POSITION_C20000 + bool "0xC20000" + +config AMD_FWM_POSITION_E20000 + bool "0xE20000" + +config AMD_FWM_POSITION_F20000 + bool "0xF20000" + +config AMD_FWM_POSITION_FA0000 + bool "0xFA0000" + +endchoice + +config AMD_FWM_POSITION + hex + default 0x20000 if AMD_FWM_POSITION_20000 + default 0x820000 if AMD_FWM_POSITION_820000 + default 0xc20000 if AMD_FWM_POSITION_C20000 + default 0xe20000 if AMD_FWM_POSITION_E20000 + default 0xf20000 if AMD_FWM_POSITION_F20000 + default 0xfa0000 if AMD_FWM_POSITION_FA0000 diff --git a/src/soc/amd/glinda/Kconfig b/src/soc/amd/glinda/Kconfig index 5bf1d5f..8111855 100644 --- a/src/soc/amd/glinda/Kconfig +++ b/src/soc/amd/glinda/Kconfig @@ -311,39 +311,6 @@ menu "PSP Configuration Options" -config AMD_FWM_POSITION_INDEX - int "Firmware Directory Table location (0 to 5)" - range 0 5 - default 0 if BOARD_ROMSIZE_KB_512 - default 1 if BOARD_ROMSIZE_KB_1024 - default 2 if BOARD_ROMSIZE_KB_2048 - default 3 if BOARD_ROMSIZE_KB_4096 - default 4 if BOARD_ROMSIZE_KB_8192 - default 5 if BOARD_ROMSIZE_KB_16384 - help - Typically this is calculated by the ROM size, but there may - be situations where you want to put the firmware directory - table in a different location. - 0: 512 KB - 0xFFFA0000 - 1: 1 MB - 0xFFF20000 - 2: 2 MB - 0xFFE20000 - 3: 4 MB - 0xFFC20000 - 4: 8 MB - 0xFF820000 - 5: 16 MB - 0xFF020000 - -comment "AMD Firmware Directory Table set to location for 512KB ROM" - depends on AMD_FWM_POSITION_INDEX = 0 -comment "AMD Firmware Directory Table set to location for 1MB ROM" - depends on AMD_FWM_POSITION_INDEX = 1 -comment "AMD Firmware Directory Table set to location for 2MB ROM" - depends on AMD_FWM_POSITION_INDEX = 2 -comment "AMD Firmware Directory Table set to location for 4MB ROM" - depends on AMD_FWM_POSITION_INDEX = 3 -comment "AMD Firmware Directory Table set to location for 8MB ROM" - depends on AMD_FWM_POSITION_INDEX = 4 -comment "AMD Firmware Directory Table set to location for 16MB ROM" - depends on AMD_FWM_POSITION_INDEX = 5 - config AMDFW_CONFIG_FILE string "AMD PSP Firmware config file" default "src/soc/amd/glinda/fw.cfg" diff --git a/src/soc/amd/glinda/Makefile.inc b/src/soc/amd/glinda/Makefile.inc index e00421d..bca7276 100644 --- a/src/soc/amd/glinda/Makefile.inc +++ b/src/soc/amd/glinda/Makefile.inc @@ -59,21 +59,12 @@ MAINBOARD_BLOBS_DIR:=$(top)/3rdparty/blobs/mainboard/$(MAINBOARDDIR) # ROMSIG Normally At ROMBASE + 0x20000 -# Overridden by CONFIG_AMD_FWM_POSITION_INDEX # +-----------+---------------+----------------+------------+ # |0x55AA55AA | | | | # +-----------+---------------+----------------+------------+ # | | PSPDIR ADDR | BIOSDIR ADDR | # +-----------+---------------+----------------+ -$(if $(CONFIG_AMD_FWM_POSITION_INDEX), ,\ - $(error Invalid AMD firmware position index. Check if the board sets a valid ROM size)) - -GLINDA_FWM_POSITION=$(call int-add, \ - $(call int-subtract, 0xffffffff \ - $(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 # Building the cbfs image will fail if the offset isn't large enough AMD_FW_AB_POSITION := 0x40 @@ -259,7 +250,7 @@ $(OPT_VERSTAGE_FILE) \ $(OPT_VERSTAGE_SIG_FILE) \ $(OPT_SPL_TABLE_FILE) \ - --location $(shell printf "%#x" $(GLINDA_FWM_POSITION)) \ + --location $(CONFIG_AMD_FWM_POSITION) \ --output $@ $(PSP_BIOSBIN_FILE): $(PSP_ELF_FILE) $(AMDCOMPRESS) diff --git a/src/soc/amd/mendocino/Kconfig b/src/soc/amd/mendocino/Kconfig index 05c349f..9d396b9 100644 --- a/src/soc/amd/mendocino/Kconfig +++ b/src/soc/amd/mendocino/Kconfig @@ -325,39 +325,6 @@ menu "PSP Configuration Options" -config AMD_FWM_POSITION_INDEX - int "Firmware Directory Table location (0 to 5)" - range 0 5 - default 0 if BOARD_ROMSIZE_KB_512 - default 1 if BOARD_ROMSIZE_KB_1024 - default 2 if BOARD_ROMSIZE_KB_2048 - default 3 if BOARD_ROMSIZE_KB_4096 - default 4 if BOARD_ROMSIZE_KB_8192 - default 5 if BOARD_ROMSIZE_KB_16384 - help - Typically this is calculated by the ROM size, but there may - be situations where you want to put the firmware directory - table in a different location. - 0: 512 KB - 0xFFFA0000 - 1: 1 MB - 0xFFF20000 - 2: 2 MB - 0xFFE20000 - 3: 4 MB - 0xFFC20000 - 4: 8 MB - 0xFF820000 - 5: 16 MB - 0xFF020000 - -comment "AMD Firmware Directory Table set to location for 512KB ROM" - depends on AMD_FWM_POSITION_INDEX = 0 -comment "AMD Firmware Directory Table set to location for 1MB ROM" - depends on AMD_FWM_POSITION_INDEX = 1 -comment "AMD Firmware Directory Table set to location for 2MB ROM" - depends on AMD_FWM_POSITION_INDEX = 2 -comment "AMD Firmware Directory Table set to location for 4MB ROM" - depends on AMD_FWM_POSITION_INDEX = 3 -comment "AMD Firmware Directory Table set to location for 8MB ROM" - depends on AMD_FWM_POSITION_INDEX = 4 -comment "AMD Firmware Directory Table set to location for 16MB ROM" - depends on AMD_FWM_POSITION_INDEX = 5 - config AMDFW_CONFIG_FILE string "AMD PSP Firmware config file" default "src/soc/amd/mendocino/fw.cfg" diff --git a/src/soc/amd/mendocino/Makefile.inc b/src/soc/amd/mendocino/Makefile.inc index 92c75f8..0f22d48 100644 --- a/src/soc/amd/mendocino/Makefile.inc +++ b/src/soc/amd/mendocino/Makefile.inc @@ -60,21 +60,12 @@ endif # ROMSIG Normally At ROMBASE + 0x20000 -# Overridden by CONFIG_AMD_FWM_POSITION_INDEX # +-----------+---------------+----------------+------------+ # |0x55AA55AA | | | | # +-----------+---------------+----------------+------------+ # | | PSPDIR ADDR | BIOSDIR ADDR | # +-----------+---------------+----------------+ -$(if $(CONFIG_AMD_FWM_POSITION_INDEX), ,\ - $(error Invalid AMD firmware position index. Check if the board sets a valid ROM size)) - -MENDOCINO_FWM_POSITION=$(call int-add, \ - $(call int-subtract, 0xffffffff \ - $(call int-shift-left, \ - 0x80000 $(CONFIG_AMD_FWM_POSITION_INDEX))) 0x20000 1) - # 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 := 0x80 @@ -260,7 +251,7 @@ $(OPT_VERSTAGE_FILE) \ $(OPT_VERSTAGE_SIG_FILE) \ $(OPT_SPL_TABLE_FILE) \ - --location $(shell printf "%#x" $(MENDOCINO_FWM_POSITION)) \ + --location $(CONFIG_AMD_FWM_POSITION) \ --output $@ ifeq ($(CONFIG_CBFS_VERIFICATION)$(CONFIG_VBOOT_STARTS_IN_BOOTBLOCK),yy) diff --git a/src/soc/amd/morgana/Kconfig b/src/soc/amd/morgana/Kconfig index 69b6959..832e907 100644 --- a/src/soc/amd/morgana/Kconfig +++ b/src/soc/amd/morgana/Kconfig @@ -311,39 +311,6 @@ menu "PSP Configuration Options" -config AMD_FWM_POSITION_INDEX - int "Firmware Directory Table location (0 to 5)" - range 0 5 - default 0 if BOARD_ROMSIZE_KB_512 - default 1 if BOARD_ROMSIZE_KB_1024 - default 2 if BOARD_ROMSIZE_KB_2048 - default 3 if BOARD_ROMSIZE_KB_4096 - default 4 if BOARD_ROMSIZE_KB_8192 - default 5 if BOARD_ROMSIZE_KB_16384 - help - Typically this is calculated by the ROM size, but there may - be situations where you want to put the firmware directory - table in a different location. - 0: 512 KB - 0xFFFA0000 - 1: 1 MB - 0xFFF20000 - 2: 2 MB - 0xFFE20000 - 3: 4 MB - 0xFFC20000 - 4: 8 MB - 0xFF820000 - 5: 16 MB - 0xFF020000 - -comment "AMD Firmware Directory Table set to location for 512KB ROM" - depends on AMD_FWM_POSITION_INDEX = 0 -comment "AMD Firmware Directory Table set to location for 1MB ROM" - depends on AMD_FWM_POSITION_INDEX = 1 -comment "AMD Firmware Directory Table set to location for 2MB ROM" - depends on AMD_FWM_POSITION_INDEX = 2 -comment "AMD Firmware Directory Table set to location for 4MB ROM" - depends on AMD_FWM_POSITION_INDEX = 3 -comment "AMD Firmware Directory Table set to location for 8MB ROM" - depends on AMD_FWM_POSITION_INDEX = 4 -comment "AMD Firmware Directory Table set to location for 16MB ROM" - depends on AMD_FWM_POSITION_INDEX = 5 - config AMDFW_CONFIG_FILE string "AMD PSP Firmware config file" default "src/soc/amd/morgana/fw.cfg" diff --git a/src/soc/amd/morgana/Makefile.inc b/src/soc/amd/morgana/Makefile.inc index a88e1a5..9fa22df 100644 --- a/src/soc/amd/morgana/Makefile.inc +++ b/src/soc/amd/morgana/Makefile.inc @@ -59,21 +59,12 @@ MAINBOARD_BLOBS_DIR:=$(top)/3rdparty/blobs/mainboard/$(MAINBOARDDIR) # ROMSIG Normally At ROMBASE + 0x20000 -# Overridden by CONFIG_AMD_FWM_POSITION_INDEX # +-----------+---------------+----------------+------------+ # |0x55AA55AA | | | | # +-----------+---------------+----------------+------------+ # | | PSPDIR ADDR | BIOSDIR ADDR | # +-----------+---------------+----------------+ -$(if $(CONFIG_AMD_FWM_POSITION_INDEX), ,\ - $(error Invalid AMD firmware position index. Check if the board sets a valid ROM size)) - -MORGANA_FWM_POSITION=$(call int-add, \ - $(call int-subtract, 0xffffffff \ - $(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 # Building the cbfs image will fail if the offset isn't large enough AMD_FW_AB_POSITION := 0x40 @@ -259,7 +250,7 @@ $(OPT_VERSTAGE_FILE) \ $(OPT_VERSTAGE_SIG_FILE) \ $(OPT_SPL_TABLE_FILE) \ - --location $(shell printf "%#x" $(MORGANA_FWM_POSITION)) \ + --location $(CONFIG_AMD_FWM_POSITION) \ --output $@ $(PSP_BIOSBIN_FILE): $(PSP_ELF_FILE) $(AMDCOMPRESS) diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 765ac4c..12a3281 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -360,39 +360,6 @@ menu "PSP Configuration Options" -config AMD_FWM_POSITION_INDEX - int "Firmware Directory Table location (0 to 5)" - range 0 5 - default 0 if BOARD_ROMSIZE_KB_512 - default 1 if BOARD_ROMSIZE_KB_1024 - default 2 if BOARD_ROMSIZE_KB_2048 - default 3 if BOARD_ROMSIZE_KB_4096 - default 4 if BOARD_ROMSIZE_KB_8192 - default 5 if BOARD_ROMSIZE_KB_16384 - help - Typically this is calculated by the ROM size, but there may - be situations where you want to put the firmware directory - table in a different location. - 0: 512 KB - 0xFFFA0000 - 1: 1 MB - 0xFFF20000 - 2: 2 MB - 0xFFE20000 - 3: 4 MB - 0xFFC20000 - 4: 8 MB - 0xFF820000 - 5: 16 MB - 0xFF020000 - -comment "AMD Firmware Directory Table set to location for 512KB ROM" - depends on AMD_FWM_POSITION_INDEX = 0 -comment "AMD Firmware Directory Table set to location for 1MB ROM" - depends on AMD_FWM_POSITION_INDEX = 1 -comment "AMD Firmware Directory Table set to location for 2MB ROM" - depends on AMD_FWM_POSITION_INDEX = 2 -comment "AMD Firmware Directory Table set to location for 4MB ROM" - depends on AMD_FWM_POSITION_INDEX = 3 -comment "AMD Firmware Directory Table set to location for 8MB ROM" - depends on AMD_FWM_POSITION_INDEX = 4 -comment "AMD Firmware Directory Table set to location for 16MB ROM" - depends on AMD_FWM_POSITION_INDEX = 5 - config AMDFW_CONFIG_FILE string default "src/soc/amd/picasso/fw.cfg" diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index 504a40e..626bc61 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -60,21 +60,12 @@ MAINBOARD_BLOBS_DIR:=$(top)/3rdparty/blobs/mainboard/$(MAINBOARDDIR) # ROMSIG Normally At ROMBASE + 0x20000 -# Overridden by CONFIG_AMD_FWM_POSITION_INDEX # +-----------+---------------+----------------+------------+ # |0x55AA55AA | | | | # +-----------+---------------+----------------+------------+ # | | PSPDIR ADDR | BIOSDIR ADDR | # +-----------+---------------+----------------+ -$(if $(CONFIG_AMD_FWM_POSITION_INDEX), ,\ - $(error Invalid AMD firmware position index. Check if the board sets a valid ROM size)) - -PICASSO_FWM_POSITION=$(call int-add, \ - $(call int-subtract, 0xffffffff \ - $(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 # Building the cbfs image will fail if the offset isn't large enough AMD_FW_AB_POSITION := 0x40 @@ -254,7 +245,7 @@ $(OPT_APOB0_NV_BASE) \ $(OPT_VERSTAGE_FILE) \ $(OPT_VERSTAGE_SIG_FILE) \ - --location $(shell printf "%#x" $(PICASSO_FWM_POSITION)) \ + --location $(CONFIG_AMD_FWM_POSITION) \ --output $@ $(PSP_BIOSBIN_FILE): $(PSP_ELF_FILE) $(AMDCOMPRESS) diff --git a/src/soc/amd/stoneyridge/Kconfig b/src/soc/amd/stoneyridge/Kconfig index 01800c1..77eb099 100644 --- a/src/soc/amd/stoneyridge/Kconfig +++ b/src/soc/amd/stoneyridge/Kconfig @@ -330,39 +330,6 @@ option to manually attach the generated amdfw.rom outside of cbfs. The location is selected by the FWM position. -config AMD_FWM_POSITION_INDEX - int "Firmware Directory Table location (0 to 5)" - range 0 5 - default 0 if BOARD_ROMSIZE_KB_512 - default 1 if BOARD_ROMSIZE_KB_1024 - default 2 if BOARD_ROMSIZE_KB_2048 - default 3 if BOARD_ROMSIZE_KB_4096 - default 4 if BOARD_ROMSIZE_KB_8192 - default 5 if BOARD_ROMSIZE_KB_16384 - help - Typically this is calculated by the ROM size, but there may - be situations where you want to put the firmware directory - table in a different location. - 0: 512 KB - 0xFFFA0000 - 1: 1 MB - 0xFFF20000 - 2: 2 MB - 0xFFE20000 - 3: 4 MB - 0xFFC20000 - 4: 8 MB - 0xFF820000 - 5: 16 MB - 0xFF020000 - -comment "AMD Firmware Directory Table set to location for 512KB ROM" - depends on AMD_FWM_POSITION_INDEX = 0 -comment "AMD Firmware Directory Table set to location for 1MB ROM" - depends on AMD_FWM_POSITION_INDEX = 1 -comment "AMD Firmware Directory Table set to location for 2MB ROM" - depends on AMD_FWM_POSITION_INDEX = 2 -comment "AMD Firmware Directory Table set to location for 4MB ROM" - depends on AMD_FWM_POSITION_INDEX = 3 -comment "AMD Firmware Directory Table set to location for 8MB ROM" - depends on AMD_FWM_POSITION_INDEX = 4 -comment "AMD Firmware Directory Table set to location for 16MB ROM" - depends on AMD_FWM_POSITION_INDEX = 5 - config DIMM_SPD_SIZE default 512 # DDR4 diff --git a/src/soc/amd/stoneyridge/Makefile.inc b/src/soc/amd/stoneyridge/Makefile.inc index 078a566..1267cee 100644 --- a/src/soc/amd/stoneyridge/Makefile.inc +++ b/src/soc/amd/stoneyridge/Makefile.inc @@ -72,7 +72,6 @@ CPPFLAGS_common += -I$(src)/soc/amd/stoneyridge/acpi # ROMSIG Normally At ROMBASE + 0x20000 -# Overridden by CONFIG_AMD_FWM_POSITION_INDEX # +-----------+---------------+----------------+------------+ # |0x55AA55AA |EC ROM Address |GEC ROM Address |USB3 ROM | # +-----------+---------------+----------------+------------+ @@ -80,11 +79,6 @@ # +-----------+ # # EC ROM should be 64K aligned. -STONEYRIDGE_FWM_POSITION=$(call int-add, \ - $(call int-subtract, 0xffffffff \ - $(call int-shift-left, \ - 0x80000 $(CONFIG_AMD_FWM_POSITION_INDEX))) 0x20000 1) - ### 0 FIRMWARE_LOCATION=$(shell grep -e FIRMWARE_LOCATION $(CONFIG_AMDFW_CONFIG_FILE) | awk '{print $$2}') @@ -152,23 +146,17 @@ --config $(CONFIG_AMDFW_CONFIG_FILE) \ $(OPT_SOCNAME) \ --flashsize $(CONFIG_ROM_SIZE) \ - --location $(shell printf "0x%x" $(STONEYRIDGE_FWM_POSITION)) \ + --location $(CONFIG_AMD_FWM_POSITION) \ --output $@ ifeq ($(CONFIG_AMDFW_OUTSIDE_CBFS),y) -# Calculate firmware position inside the ROM -STONEYRIDGE_FWM_ROM_POSITION=$(call int-add, \ - $(call int-subtract, $(CONFIG_ROM_SIZE) \ - $(call int-shift-left, \ - 0x80000 $(CONFIG_AMD_FWM_POSITION_INDEX))) 0x20000) - $(call add_intermediate, add_amdfw, $(obj)/amdfw.rom) printf " DD Adding AMD Firmware at ROM offset 0x%x\n" \ - "$(STONEYRIDGE_FWM_ROM_POSITION)" + "$(CONFIG_AMD_FWM_POSITION)" dd if=$(obj)/amdfw.rom \ of=$< conv=notrunc bs=1 \ - seek=$(STONEYRIDGE_FWM_ROM_POSITION) >/dev/null 2>&1 + seek=$(CONFIG_AMD_FWM_POSITION) >/dev/null 2>&1 else # ifeq ($(CONFIG_AMDFW_OUTSIDE_CBFS),y) diff --git a/src/soc/amd/stoneyridge/fch.c b/src/soc/amd/stoneyridge/fch.c index 9680fb2..4afff15 100644 --- a/src/soc/amd/stoneyridge/fch.c +++ b/src/soc/amd/stoneyridge/fch.c @@ -151,7 +151,7 @@ uintptr_t fwaddr; size_t fwsize; - amdfw_rom = 0x20000 - (0x80000 << CONFIG_AMD_FWM_POSITION_INDEX); + amdfw_rom = CONFIG_AMD_FWM_POSITION; xhci_fw = read32p(amdfw_rom + XHCI_FW_SIG_OFFSET); fwaddr = 2 + read16p(xhci_fw + XHCI_FW_ADDR_OFFSET + XHCI_FW_BOOTRAM_SIZE); diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index e28d9c4..98a422e 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -2018,7 +2018,6 @@ uint32_t dir_location = 0; bool any_location = 0; uint32_t romsig_offset; - uint32_t rom_base_address; uint8_t soc_id = PLATFORM_UNKNOWN; uint8_t efs_spi_readmode = 0xff; uint8_t efs_spi_speed = 0xff; @@ -2312,8 +2311,7 @@ printf(" AMDFWTOOL Using ROM size of %dKB\n", ctx.rom_size / 1024); - rom_base_address = 0xFFFFFFFF - ctx.rom_size + 1; - if (dir_location && (dir_location < rom_base_address)) { + if (dir_location && (dir_location >= ctx.rom_size)) { fprintf(stderr, "Error: Directory location outside of ROM.\n\n"); return 1; } @@ -2327,17 +2325,17 @@ } else { switch (dir_location) { case 0: /* Fall through */ - case 0xFFFA0000: /* Fall through */ - case 0xFFF20000: /* Fall through */ - case 0xFFE20000: /* Fall through */ - case 0xFFC20000: /* Fall through */ - case 0xFF820000: /* Fall through */ - case 0xFF020000: /* Fall through */ + case 0xFA0000: /* Fall through */ + case 0xF20000: /* Fall through */ + case 0xE20000: /* Fall through */ + case 0xC20000: /* Fall through */ + case 0x820000: /* Fall through */ + case 0x20000: /* Fall through */ break; default: fprintf(stderr, "Error: Invalid Directory location.\n"); - fprintf(stderr, " Valid locations are 0xFFFA0000, 0xFFF20000,\n"); - fprintf(stderr, " 0xFFE20000, 0xFFC20000, 0xFF820000, 0xFF020000\n"); + fprintf(stderr, " Valid locations are 0xFA0000, 0xF20000,\n"); + fprintf(stderr, " 0xE20000, 0xC20000, 0x820000, 0x20000\n"); return 1; } } @@ -2349,7 +2347,7 @@ memset(ctx.rom, 0xFF, ctx.rom_size); if (dir_location) - romsig_offset = ctx.current = dir_location - rom_base_address; + romsig_offset = ctx.current = dir_location; else romsig_offset = ctx.current = AMD_ROMSIG_OFFSET; -- To view, visit https://review.coreboot.org/c/coreboot/+/69852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I89c9e73e7db748379c97e3c0ad69af3faedc8d66 Gerrit-Change-Number: 69852 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[master]: src/mainboard: Remove unnecessary space after casts
by Jonathon Hall (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Philipp Hug, Jonathan Zhang, Arthur Heymans, Piotr Król, Tarun Tuli, Michał Żygowski, Johnny Lin, Christian Walter, Morgan Jang, Werner Zeh, ron minnich, Elyes Haouas, Tim Chu. Jonathon Hall has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69815 ) Change subject: src/mainboard: Remove unnecessary space after casts ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/69815 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id8e1a52279e6a606441eefe30e24bcd44e006aad Gerrit-Change-Number: 69815 Gerrit-PatchSet: 2 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Mon, 21 Nov 2022 13:33:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: Makefile.inc: fix multiple jobs build issue
by Michał Żygowski (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Martin L Roth. Hello build bot (Jenkins), Martin L Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/69819 to look at the new patch set (#2). Change subject: Makefile.inc: fix multiple jobs build issue ...................................................................... Makefile.inc: fix multiple jobs build issue Certain C source files for coreboot stages require fmap_config.h to be present. When building coreboot using multiple jobs the dependency is not always satisfied due to race condition and results in make error. Workaround it by adding fmap_config.h to stage C deps. Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Change-Id: I3a70beedf2eb1c018c5ff98163904253f9a87a61 --- M Makefile.inc 1 file changed, 23 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/69819/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/69819 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3a70beedf2eb1c018c5ff98163904253f9a87a61 Gerrit-Change-Number: 69819 Gerrit-PatchSet: 2 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2022