coreboot-gerrit March 2022

coreboot-gerrit@coreboot.org

1 participants
6700 discussions

Change in coreboot[master]: Move common EFI and FSP definitions to commonlib
by build bot (Jenkins) (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Arthur Heymans, Patrick Rudolph. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63004 ) Change subject: Move common EFI and FSP definitions to commonlib ...................................................................... Patch Set 3: (4 comments) File src/commonlib/include/commonlib/uefi_types.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144483): https://review.coreboot.org/c/coreboot/+/63004/comment/765fa55b_5dcf57d6 PS3, Line 9: #define SIGNATURE_32(A, B, C, D) (SIGNATURE_16 (A, B) | (SIGNATURE_16 (C, D) << 16)) space prohibited between function name and open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144483): https://review.coreboot.org/c/coreboot/+/63004/comment/37e00b1d_444625b4 PS3, Line 9: #define SIGNATURE_32(A, B, C, D) (SIGNATURE_16 (A, B) | (SIGNATURE_16 (C, D) << 16)) space prohibited between function name and open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144483): https://review.coreboot.org/c/coreboot/+/63004/comment/129a6ef9_1637b8a0 PS3, Line 13: #define EFI_FVH_SIGNATURE SIGNATURE_32 ('_', 'F', 'V', 'H') space prohibited between function name and open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144483): https://review.coreboot.org/c/coreboot/+/63004/comment/07d53882_6c133624 PS3, Line 13: #define EFI_FVH_SIGNATURE SIGNATURE_32 ('_', 'F', 'V', 'H') Macros with complex values should be enclosed in parentheses -- To view, visit https://review.coreboot.org/c/coreboot/+/63004 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia2e4051cf31789a48e63b9cee6cdf2b67a6e475f Gerrit-Change-Number: 63004 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 22 Mar 2022 20:09:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Move common EFI and FSP definitions to commonlib
by Arthur Heymans (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Arthur Heymans, Patrick Rudolph. Hello build bot (Jenkins), Patrick Rudolph, Arthur Heymans, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63004 to look at the new patch set (#3). Change subject: Move common EFI and FSP definitions to commonlib ...................................................................... Move common EFI and FSP definitions to commonlib The original implementation of fsp_component_relocate() was very lazy in that it indiscriminately pulled in all EFI headers. Because that is used in commonlib code, it essentially created an EFI decendency on both the firmware components, as well as the userspace components of the codebase. Resolve this by pulling in the needed definitions into commonlib, and passing them through the semantic patches from FSP 2.0 to remove the deep dependency on EFI definitions. It turns out that very little code is needed to make things function. Change-Id: Ia2e4051cf31789a48e63b9cee6cdf2b67a6e475f Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/commonlib/fsp_relocate.c A src/commonlib/include/commonlib/fsp_info_header.h A src/commonlib/include/commonlib/uefi_types.h M util/cbfstool/Makefile.inc 4 files changed, 179 insertions(+), 46 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/63004/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/63004 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia2e4051cf31789a48e63b9cee6cdf2b67a6e475f Gerrit-Change-Number: 63004 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/tpm/cr50: Use cr50_get_firmware_version in get_board_cfg
by Tim Wawrzynczak (Code Review) 22 Mar '22

22 Mar '22

Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/62964 ) Change subject: drivers/tpm/cr50: Use cr50_get_firmware_version in get_board_cfg ...................................................................... drivers/tpm/cr50: Use cr50_get_firmware_version in get_board_cfg cr50_get_board_cfg() may be called in ramstage for some mainboards in order to determine the BOARD_CFG register's value. The code was written assuming that the firmware version was already retrieved, but for boards calling this in ramstage, this is not the case. Therefore, instead of using the cached cr50_firmware_version (which is all 0s in ramstage at that time), use the cr50_get_firmware_version function instead. BUG=b:225206079, b:220685274 BRANCH=firmware-brya-14505.B TEST=boot on brya0 and see: [INFO ] Firmware version: B2-C:0 RO_B:0.0.11/4d655eab RW_B:0.6.93/cr50_v3.94 [INFO ] Enabling GPIO PM b/c CR50 has long IRQ pulse support in the logs. Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: Ia6e5f4965a8852793d2f95e6eb21ea87860335a9 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62964 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Julius Werner <jwerner(a)chromium.org> Reviewed-by: Subrata Banik <subratabanik(a)google.com> --- M src/drivers/tpm/cr50.c 1 file changed, 13 insertions(+), 5 deletions(-) Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved Subrata Banik: Looks good to me, approved diff --git a/src/drivers/tpm/cr50.c b/src/drivers/tpm/cr50.c index f96e174..1d6c691 100644 --- a/src/drivers/tpm/cr50.c +++ b/src/drivers/tpm/cr50.c @@ -13,8 +13,6 @@ (CONFIG(CR50_USE_LONG_INTERRUPT_PULSES) \ ? CR50_BOARD_CFG_100US_READY_PULSE : 0) -static struct cr50_firmware_version cr50_firmware_version; - enum cr50_register { CR50_FW_VER_REG, CR50_BOARD_CFG_REG, @@ -74,9 +72,13 @@ */ static uint32_t cr50_get_board_cfg(void) { + struct cr50_firmware_version ver; uint32_t value; - if (!cr50_fw_supports_board_cfg(&cr50_firmware_version)) + if (cr50_get_firmware_version(&ver) != CB_SUCCESS) + return 0; + + if (!cr50_fw_supports_board_cfg(&ver)) return 0; const enum cb_err ret = tis_vendor_read(get_reg_addr(CR50_BOARD_CFG_REG), &value, @@ -94,10 +96,14 @@ */ enum cb_err cr50_set_board_cfg(void) { - uint32_t value; + struct cr50_firmware_version ver; enum cb_err ret; + uint32_t value; - if (!cr50_fw_supports_board_cfg(&cr50_firmware_version)) + if (cr50_get_firmware_version(&ver) != CB_SUCCESS) + return CB_ERR; + + if (!cr50_fw_supports_board_cfg(&ver)) return CB_ERR; /* Set the CR50_BOARD_CFG register, for e.g. asking cr50 to use longer ready pulses. */ @@ -167,6 +173,8 @@ enum cb_err cr50_get_firmware_version(struct cr50_firmware_version *version) { + static struct cr50_firmware_version cr50_firmware_version; + if (cr50_firmware_version.epoch || cr50_firmware_version.major || cr50_firmware_version.minor) goto success; -- To view, visit https://review.coreboot.org/c/coreboot/+/62964 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia6e5f4965a8852793d2f95e6eb21ea87860335a9 Gerrit-Change-Number: 62964 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: mb/google/hatch: split up hatch and puff baseboards
by Tim Wawrzynczak (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Shelley Chen, Subrata Banik, Matt DeVillier, Edward O'Callaghan, Angel Pons, Nick Vaccaro. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62970 ) Change subject: mb/google/hatch: split up hatch and puff baseboards ...................................................................... Patch Set 2: (2 comments) Patchset: PS2: I like this, but it's a big change, let's let it bake a for just a bit File src/mainboard/google/puff/Kconfig: https://review.coreboot.org/c/coreboot/+/62970/comment/244045d4_41dcbf9c PS2, Line 95: select GBB_FLAG_FORCE_DEV_SWITCH_ON : select GBB_FLAG_FORCE_DEV_BOOT_USB : select GBB_FLAG_FORCE_DEV_BOOT_ALTFW nit: not alphabetized -- To view, visit https://review.coreboot.org/c/coreboot/+/62970 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8c7350f3afcff3ddefc6fa14054a3f9257568cd3 Gerrit-Change-Number: 62970 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Comment-Date: Tue, 22 Mar 2022 19:56:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Move common EFI and FSP definitions to commonlib
by build bot (Jenkins) (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Arthur Heymans, Patrick Rudolph. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63004 ) Change subject: Move common EFI and FSP definitions to commonlib ...................................................................... Patch Set 2: (5 comments) File src/commonlib/include/commonlib/fsp_info_header.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144481): https://review.coreboot.org/c/coreboot/+/63004/comment/7a795648_888d98d6 PS2, Line 37: struct FSP_EXTENTED_HEADER { 'EXTENTED' may be misspelled - perhaps 'EXTENDED'? File src/commonlib/include/commonlib/uefi_types.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144481): https://review.coreboot.org/c/coreboot/+/63004/comment/e2814643_a71ce939 PS2, Line 9: #define SIGNATURE_32(A, B, C, D) (SIGNATURE_16 (A, B) | (SIGNATURE_16 (C, D) << 16)) space prohibited between function name and open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144481): https://review.coreboot.org/c/coreboot/+/63004/comment/39aa7952_dfd1589d PS2, Line 9: #define SIGNATURE_32(A, B, C, D) (SIGNATURE_16 (A, B) | (SIGNATURE_16 (C, D) << 16)) space prohibited between function name and open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144481): https://review.coreboot.org/c/coreboot/+/63004/comment/1e163d07_62ba0a7c PS2, Line 13: #define EFI_FVH_SIGNATURE SIGNATURE_32 ('_', 'F', 'V', 'H') space prohibited between function name and open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144481): https://review.coreboot.org/c/coreboot/+/63004/comment/eed2abfa_57e5e396 PS2, Line 13: #define EFI_FVH_SIGNATURE SIGNATURE_32 ('_', 'F', 'V', 'H') Macros with complex values should be enclosed in parentheses -- To view, visit https://review.coreboot.org/c/coreboot/+/63004 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia2e4051cf31789a48e63b9cee6cdf2b67a6e475f Gerrit-Change-Number: 63004 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Comment-Date: Tue, 22 Mar 2022 19:53:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Move common EFI and FSP definitions to commonlib
by Arthur Heymans (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Arthur Heymans, Patrick Rudolph. Hello build bot (Jenkins), Patrick Rudolph, Arthur Heymans, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63004 to look at the new patch set (#2). Change subject: Move common EFI and FSP definitions to commonlib ...................................................................... Move common EFI and FSP definitions to commonlib The original implementation of fsp_component_relocate() was very lazy in that it indiscriminately pulled in all EFI headers. Because that is used in commonlib code, it essentially created an EFI decendency on both the firmware components, as well as the userspace components of the codebase. Resolve this by pulling in the needed definitions into commonlib, and passing them through the semantic patches from FSP 2.0 to remove the deep dependency on EFI definitions. It turns out that very little code is needed to make things function. Change-Id: Ia2e4051cf31789a48e63b9cee6cdf2b67a6e475f Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/commonlib/fsp_relocate.c A src/commonlib/include/commonlib/fsp_info_header.h A src/commonlib/include/commonlib/uefi_types.h M util/cbfstool/Makefile.inc 4 files changed, 188 insertions(+), 46 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/63004/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/63004 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia2e4051cf31789a48e63b9cee6cdf2b67a6e475f Gerrit-Change-Number: 63004 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: Move common EFI and FSP definitions to commonlib
by build bot (Jenkins) (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Arthur Heymans. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63004 ) Change subject: Move common EFI and FSP definitions to commonlib ...................................................................... Patch Set 1: (5 comments) File src/commonlib/include/commonlib/fsp_info_header.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144480): https://review.coreboot.org/c/coreboot/+/63004/comment/8a3545ee_022a1f56 PS1, Line 51: struct FSP_EXTENTED_HEADER { 'EXTENTED' may be misspelled - perhaps 'EXTENDED'? File src/commonlib/include/commonlib/uefi_types.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144480): https://review.coreboot.org/c/coreboot/+/63004/comment/e5055803_51814766 PS1, Line 23: #define SIGNATURE_32(A, B, C, D) (SIGNATURE_16 (A, B) | (SIGNATURE_16 (C, D) << 16)) space prohibited between function name and open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144480): https://review.coreboot.org/c/coreboot/+/63004/comment/2030a5ac_a7e59373 PS1, Line 23: #define SIGNATURE_32(A, B, C, D) (SIGNATURE_16 (A, B) | (SIGNATURE_16 (C, D) << 16)) space prohibited between function name and open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144480): https://review.coreboot.org/c/coreboot/+/63004/comment/bb1b6b78_4d6814c1 PS1, Line 27: #define EFI_FVH_SIGNATURE SIGNATURE_32 ('_', 'F', 'V', 'H') space prohibited between function name and open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144480): https://review.coreboot.org/c/coreboot/+/63004/comment/38c8bf00_75a44497 PS1, Line 27: #define EFI_FVH_SIGNATURE SIGNATURE_32 ('_', 'F', 'V', 'H') Macros with complex values should be enclosed in parentheses -- To view, visit https://review.coreboot.org/c/coreboot/+/63004 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia2e4051cf31789a48e63b9cee6cdf2b67a6e475f Gerrit-Change-Number: 63004 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 22 Mar 2022 19:50:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Move common EFI and FSP definitions to commonlib
by Arthur Heymans (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Arthur Heymans. Hello Arthur Heymans, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/63004 to review the following change. Change subject: Move common EFI and FSP definitions to commonlib ...................................................................... Move common EFI and FSP definitions to commonlib The original implementation of fsp_component_relocate() was very lazy in that it indiscriminately pulled in all EFI headers. Because that is used in commonlib code, it essentially created an EFI decendency on both the firmware components, as well as the userspace components of the codebase. Resolve this by pulling in the needed definitions into commonlib, and passing them through the semantic patches from FSP 2.0 to remove the deep dependency on EFI definitions. It turns out that very little code is needed to make things function. Change-Id: Ia2e4051cf31789a48e63b9cee6cdf2b67a6e475f Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/commonlib/fsp_relocate.c A src/commonlib/include/commonlib/fsp_info_header.h A src/commonlib/include/commonlib/uefi_types.h M util/cbfstool/Makefile.inc 4 files changed, 216 insertions(+), 46 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/63004/1 diff --git a/src/commonlib/fsp_relocate.c b/src/commonlib/fsp_relocate.c index 5d326b6..c7c9a4b 100644 --- a/src/commonlib/fsp_relocate.c +++ b/src/commonlib/fsp_relocate.c @@ -3,19 +3,8 @@ #include <console/console.h> #include <commonlib/endian.h> #include <commonlib/fsp.h> -/* - * Intel's code does not have a handle on changing global packing state. - * Therefore, one needs to protect against packing policies that are set - * globally for a compilation unit just by including a header file. - */ -#pragma pack(push) - -/* Default bind FSP 1.1 API to edk2 UEFI 2.4 types. */ -#include <vendorcode/intel/edk2/uefi_2.4/uefi_types.h> -#include <vendorcode/intel/fsp/fsp1_1/IntelFspPkg/Include/FspInfoHeader.h> - -/* Restore original packing policy. */ -#pragma pack(pop) +#include <commonlib/uefi_types.h> +#include <commonlib/fsp_info_header.h> #include <commonlib/helpers.h> #include <stdlib.h> @@ -32,7 +21,8 @@ */ /* Return 0 if equal. Non-zero if not equal. */ -static int guid_compare(const EFI_GUID *le_guid, const EFI_GUID *native_guid) +static int guid_compare(const struct EFI_GUID *le_guid, + const struct EFI_GUID *native_guid) { if (read_le32(&le_guid->Data1) != native_guid->Data1) return 1; @@ -44,8 +34,8 @@ ARRAY_SIZE(le_guid->Data4)); } -static const EFI_GUID ffs2_guid = EFI_FIRMWARE_FILE_SYSTEM2_GUID; -static const EFI_GUID fih_guid = FSP_INFO_HEADER_GUID; +static const struct EFI_GUID ffs2_guid = EFI_FIRMWARE_FILE_SYSTEM2_GUID; +static const struct EFI_GUID fih_guid = FSP_INFO_HEADER_GUID; struct fsp_patch_table { uint32_t signature; @@ -102,9 +92,9 @@ static int te_relocate(uintptr_t new_addr, void *te) { - EFI_TE_IMAGE_HEADER *teih; - EFI_IMAGE_DATA_DIRECTORY *relocd; - EFI_IMAGE_BASE_RELOCATION *relocb; + struct EFI_TE_IMAGE_HEADER *teih; + struct EFI_IMAGE_DATA_DIRECTORY *relocd; + struct EFI_IMAGE_BASE_RELOCATION *relocb; uintptr_t image_base; size_t fixup_offset; size_t num_relocs; @@ -130,7 +120,7 @@ * program is found by adding the fixup_offset to the ImageBase. */ fixup_offset = read_le16(&teih->StrippedSize); - fixup_offset -= sizeof(EFI_TE_IMAGE_HEADER); + fixup_offset -= sizeof(struct EFI_TE_IMAGE_HEADER); /* Keep track of a base that is correctly adjusted so that offsets * can be used directly. */ te_base = te; @@ -200,7 +190,7 @@ return 0; } -static size_t csh_size(const EFI_COMMON_SECTION_HEADER *csh) +static size_t csh_size(const struct EFI_COMMON_SECTION_HEADER *csh) { size_t size; @@ -213,46 +203,46 @@ return size; } -static size_t section_data_offset(const EFI_COMMON_SECTION_HEADER *csh) +static size_t section_data_offset(const struct EFI_COMMON_SECTION_HEADER *csh) { if (csh_size(csh) == 0x00ffffff) - return sizeof(EFI_COMMON_SECTION_HEADER2); + return sizeof(struct EFI_COMMON_SECTION_HEADER2); else - return sizeof(EFI_COMMON_SECTION_HEADER); + return sizeof(struct EFI_COMMON_SECTION_HEADER); } -static size_t section_data_size(const EFI_COMMON_SECTION_HEADER *csh) +static size_t section_data_size(const struct EFI_COMMON_SECTION_HEADER *csh) { size_t section_size; + const struct EFI_COMMON_SECTION_HEADER2 *csh2 = (const void *)csh; if (csh_size(csh) == 0x00ffffff) - section_size = read_le32(&SECTION2_SIZE(csh)); + section_size = read_le32(&csh2->ExtendedSize); else section_size = csh_size(csh); return section_size - section_data_offset(csh); } -static size_t file_section_offset(const EFI_FFS_FILE_HEADER *ffsfh) +static size_t file_section_offset(const struct EFI_FFS_FILE_HEADER *ffsfh) { if (IS_FFS_FILE2(ffsfh)) - return sizeof(EFI_FFS_FILE_HEADER2); + return sizeof(struct EFI_FFS_FILE_HEADER2); else - return sizeof(EFI_FFS_FILE_HEADER); + return sizeof(struct EFI_FFS_FILE_HEADER); } -static size_t ffs_file_size(const EFI_FFS_FILE_HEADER *ffsfh) +static size_t ffs_file_size(const struct EFI_FFS_FILE_HEADER *ffsfh) { size_t size; + const struct EFI_FFS_FILE_HEADER2 *ffsfh2 = (const void *)ffsfh; if (IS_FFS_FILE2(ffsfh)) { /* - * this cast is needed with UEFI 2.6 headers in order - * to read the UINT32 value that FFS_FILE2_SIZE converts - * the return into + * Field is 64-bit in newer EFI headers, but reading as 32-bit + * works for now because field is little-endian. */ - uint32_t file2_size = FFS_FILE2_SIZE(ffsfh); - size = read_le32(&file2_size); + size = read_le32(&ffsfh2->ExtendedSize); } else { size = read_le8(&ffsfh->Size[0]) << 0; size |= read_le8(&ffsfh->Size[1]) << 8; @@ -306,9 +296,9 @@ static ssize_t relocate_remaining_items(void *fsp, size_t size, uintptr_t new_addr, size_t fih_offset) { - EFI_FFS_FILE_HEADER *ffsfh; - EFI_COMMON_SECTION_HEADER *csh; - FSP_INFO_HEADER *fih; + struct EFI_FFS_FILE_HEADER *ffsfh; + struct EFI_COMMON_SECTION_HEADER *csh; + struct FSP_INFO_HEADER *fih; ssize_t adjustment; size_t offset; @@ -381,9 +371,9 @@ static ssize_t relocate_fvh(uintptr_t new_addr, void *fsp, size_t fsp_size, size_t fvh_offset, size_t *fih_offset) { - EFI_FIRMWARE_VOLUME_HEADER *fvh; - EFI_FFS_FILE_HEADER *ffsfh; - EFI_COMMON_SECTION_HEADER *csh; + struct EFI_FIRMWARE_VOLUME_HEADER *fvh; + struct EFI_FFS_FILE_HEADER *ffsfh; + struct EFI_COMMON_SECTION_HEADER *csh; size_t offset; size_t file_offset; size_t size; @@ -413,7 +403,7 @@ } if (read_le16(&fvh->ExtHeaderOffset) != 0) { - EFI_FIRMWARE_VOLUME_EXT_HEADER *fveh; + struct EFI_FIRMWARE_VOLUME_EXT_HEADER *fveh; offset += read_le16(&fvh->ExtHeaderOffset); fveh = relative_offset(fsp, offset); diff --git a/src/commonlib/include/commonlib/fsp_info_header.h b/src/commonlib/include/commonlib/fsp_info_header.h new file mode 100644 index 0000000..553e026 --- /dev/null +++ b/src/commonlib/include/commonlib/fsp_info_header.h @@ -0,0 +1,60 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2017 Adurb Akhbar <aakhbar(a)mail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _FSP_INFO_HEADER_H_ +#define _FSP_INFO_HEADER_H_ + +#include <stdint.h> + +#define FSP_SIG 0x48505346 /* 'FSPH' */ + +#define FSP_INFO_HEADER_GUID \ +{ \ + 0x912740BE, 0x2284, 0x4734, \ + {0xB9, 0x71, 0x84, 0xB0, 0x27, 0x35, 0x3F, 0x0C} \ +} + +struct FSP_INFO_HEADER { + uint32_t Signature; + uint32_t HeaderLength; + uint8_t Reserved1[3]; + uint8_t HeaderRevision; + uint32_t ImageRevision; + char ImageId[8]; + uint32_t ImageSize; + uint32_t ImageBase; + uint32_t ImageAttribute; + uint32_t CfgRegionOffset; + uint32_t CfgRegionSize; + uint32_t ApiEntryNum; + uint32_t TempRamInitEntryOffset; + uint32_t FspInitEntryOffset; + uint32_t NotifyPhaseEntryOffset; + uint32_t FspMemoryInitEntryOffset; + uint32_t TempRamExitEntryOffset; + uint32_t FspSiliconInitEntryOffset; +} __packed; + +struct FSP_EXTENTED_HEADER { + uint32_t Signature; + uint32_t HeaderLength; + uint8_t Revision; + uint8_t Reserved; + char OemId[6]; + uint32_t OemRevision; +} __packed; + +#endif diff --git a/src/commonlib/include/commonlib/uefi_types.h b/src/commonlib/include/commonlib/uefi_types.h new file mode 100644 index 0000000..c3a20dd --- /dev/null +++ b/src/commonlib/include/commonlib/uefi_types.h @@ -0,0 +1,124 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2017 Adurb Akhbar <aakhbar(a)mail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __UEFI_TYPES_H__ +#define __UEFI_TYPES_H__ + +#include <stdint.h> + +#define SIGNATURE_16(A, B) ((A) | (B << 8)) +#define SIGNATURE_32(A, B, C, D) (SIGNATURE_16 (A, B) | (SIGNATURE_16 (C, D) << 16)) + +#define EFI_TE_IMAGE_HEADER_SIGNATURE SIGNATURE_16('V', 'Z') +#define EFI_TE_IMAGE_DIRECTORY_ENTRY_BASERELOC 0 +#define EFI_FVH_SIGNATURE SIGNATURE_32 ('_', 'F', 'V', 'H') + +#define EFI_IMAGE_REL_BASED_ABSOLUTE 0 +#define EFI_IMAGE_REL_BASED_HIGHLOW 3 +#define EFI_SECTION_TE 0x12 +#define EFI_SECTION_RAW 0x19 +#define FFS_ATTRIB_LARGE_FILE 0x01 +#define EFI_FV_FILETYPE_FFS_MAX 0xff +#define EFI_FV_FILETYPE_FFS_PAD 0xf0 + +#define EFI_FIRMWARE_FILE_SYSTEM2_GUID \ +{ 0x8c8ce578, 0x8a3d, 0x4f1c, { 0x99, 0x35, 0x89, 0x61, 0x85, 0xc3, 0x2d, 0xd3 } } + +struct EFI_GUID { + uint32_t Data1; + uint16_t Data2; + uint16_t Data3; + uint8_t Data4[8]; +} __packed; + +struct EFI_IMAGE_DATA_DIRECTORY { + uint32_t VirtualAddress; + uint32_t Size; +} __packed; + +struct EFI_IMAGE_BASE_RELOCATION { + uint32_t VirtualAddress; + uint32_t SizeOfBlock; +} __packed; + +struct EFI_TE_IMAGE_HEADER { + uint16_t Signature; + uint16_t Machine; + uint8_t NumberOfSections; + uint8_t Subsystem; + uint16_t StrippedSize; + uint32_t AddressOfEntryPoint; + uint32_t BaseOfCode; + uint64_t ImageBase; + struct EFI_IMAGE_DATA_DIRECTORY DataDirectory[2]; +} __packed; + + +struct EFI_COMMON_SECTION_HEADER { + uint8_t Size[3]; + uint8_t Type; +} __packed; + +struct EFI_COMMON_SECTION_HEADER2 { + uint8_t Size[3]; + uint8_t Type; + uint32_t ExtendedSize; +} __packed; + +struct EFI_FFS_FILE_HEADER { + struct EFI_GUID Name; + uint16_t IntegrityCheck; + uint8_t Type; + uint8_t Attributes; + uint8_t Size[3]; + uint8_t State; +} __packed; + +struct EFI_FFS_FILE_HEADER2 { + struct EFI_GUID Name; + uint8_t IntegrityCheck; + uint8_t Type; + uint8_t Attributes; + uint8_t Size[3]; + uint8_t State; + uint32_t ExtendedSize; /* 64-bit in newer EFI headers */ +} __packed; + +static inline int IS_FFS_FILE2(const struct EFI_FFS_FILE_HEADER *ffsfh) +{ + return !!(ffsfh->Attributes & FFS_ATTRIB_LARGE_FILE); +} + +struct EFI_FIRMWARE_VOLUME_HEADER { + uint8_t ZeroVector[16]; + struct EFI_GUID FileSystemGuid; + uint64_t FvLength; + uint32_t Signature; + uint32_t Attributes; + uint16_t HeaderLength; + uint16_t Checksum; + uint16_t ExtHeaderOffset; + uint8_t Reserved[1]; + uint8_t Revision; + uint32_t BlockMap[1]; +} __packed; + +struct EFI_FIRMWARE_VOLUME_EXT_HEADER { + struct EFI_GUID FvName; + uint32_t ExtHeaderSize; +} __packed; + +#endif /* __UEFI_TYPES_H__*/ diff --git a/util/cbfstool/Makefile.inc b/util/cbfstool/Makefile.inc index 3787a56..0bf59b8 100644 --- a/util/cbfstool/Makefile.inc +++ b/util/cbfstool/Makefile.inc @@ -126,10 +126,6 @@ TOOLCPPFLAGS += -I$(VBOOT_SOURCE)/firmware/2lib/include TOOLCPPFLAGS += -I$(VBOOT_SOURCE)/host/include TOOLCPPFLAGS += -I$(VBOOT_SOURCE)/host/lib/include -# UEFI header file support. It's not pretty, but that's what we currently -# have right now. -TOOLCPPFLAGS += -I$(top)/src -TOOLCPPFLAGS += -I$(top)/src/vendorcode/intel/edk2/uefi_2.4/MdePkg/Include TOOLLDFLAGS ?= HOSTCFLAGS += -fms-extensions -- To view, visit https://review.coreboot.org/c/coreboot/+/63004 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia2e4051cf31789a48e63b9cee6cdf2b67a6e475f Gerrit-Change-Number: 63004 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: drivers/intel/fsp2_0: Add provision to extract FSP Performance Data
by Tim Wawrzynczak (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Subrata Banik, Maulik V Vaghela, Paul Menzel, Nick Vaccaro, Eric Lai, Andrey Petrov. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62942 ) Change subject: drivers/intel/fsp2_0: Add provision to extract FSP Performance Data ...................................................................... Patch Set 4: (1 comment) File src/drivers/intel/fsp2_0/fsp_timestamp.c: https://review.coreboot.org/c/coreboot/+/62942/comment/e6f15c41_12547bc2 PS4, Line 111: : BOOT_STATE_INIT_ENTRY(BS_PAYLOAD_LOAD, BS_ON_EXIT, fsp_get_timestamp, NULL); : BOOT_STATE_INIT_ENTRY(BS_OS_RESUME, BS_ON_ENTRY, fsp_get_timestamp, NULL); Instead of boot state entries, can we just add call fsp_print_timestamps() from somewhere in the flow of the fsp2_0 driver, e.g. end of do_silicon_init() ? I realize that you will not capture any timestamp emitted by Notify APIs that way, but we are also moving towards eliminating the need for those too... -- To view, visit https://review.coreboot.org/c/coreboot/+/62942 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia1b7f6b98bafeec0afe843f0f78c99c2f34f50b3 Gerrit-Change-Number: 62942 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Tue, 22 Mar 2022 19:39:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/tpm/cr50: Use cr50_get_firmware_version in get_board_cfg
by Subrata Banik (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Tim Wawrzynczak, Nick Vaccaro. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62964 ) Change subject: drivers/tpm/cr50: Use cr50_get_firmware_version in get_board_cfg ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/62964 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia6e5f4965a8852793d2f95e6eb21ea87860335a9 Gerrit-Change-Number: 62964 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Comment-Date: Tue, 22 Mar 2022 19:38:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2022