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@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@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@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
-
Arthur Heymans (Code Review)