EFI fixes for v5.7

The following changes since commit a088b858f16af85e3db359b6c6aaa92dd3bc0921:

efi/x86: Revert struct layout change to fix kexec boot regression (2020-04-14 08:32:17 +0200)

are available in the Git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-urgent

for you to fetch changes up to b4f1874c62168159fdb419ced4afc77c1b51c475:

tpm: check event log version before reading final events (2020-05-17 11:46:50 +0200)

----------------------------------------------------------------
EFI fixes for v5.7-rcX:
- fix EFI framebuffer earlycon for wide fonts
- avoid filling screen_info with garbage if the EFI framebuffer is not available
- fix a potential host tool build error due to a symbol clash on x86
- work around a EFI firmware bug regarding the binary format of the TPM final events table
- fix a missing memory free by reworking the E820 table sizing routine to not do the allocation in the first place
- add CPER parsing for firmware errors

----------------------------------------------------------------
Arvind Sankar (1):
x86/boot: Mark global variables as static

Benjamin Thiel (1):
efi: Pull up arch-specific prototype efi_systab_show_arch()

Dave Young (1):
efi/earlycon: Fix early printk for wider fonts

Heinrich Schuchardt (1):
efi/libstub: Avoid returning uninitialized data from setup_graphics()

Lenny Szubowicz (1):
efi/libstub/x86: Avoid EFI map buffer alloc in allocate_e820()

Loïc Yhuel (1):
tpm: check event log version before reading final events

Punit Agrawal (1):
efi: cper: Add support for printing Firmware Error Record Reference

 arch/x86/boot/tools/build.c             | 16 ++++-----
 drivers/firmware/efi/cper.c             | 62 +++++++++++++++++++++++++++++++++
 drivers/firmware/efi/earlycon.c         | 14 ++++----
 drivers/firmware/efi/efi.c              |  5 +--
 drivers/firmware/efi/libstub/arm-stub.c |  6 +++-
 drivers/firmware/efi/libstub/efistub.h  | 13 +++++++
 drivers/firmware/efi/libstub/mem.c      |  2 --
 drivers/firmware/efi/libstub/tpm.c      |  5 +--
 drivers/firmware/efi/libstub/x86-stub.c | 24 +++++--------
 drivers/firmware/efi/tpm.c              |  5 ++-
 include/linux/cper.h                    |  9 +++++
 include/linux/efi.h                     |  2 ++
 12 files changed, 124 insertions(+), 39 deletions(-)

From: Heinrich Schuchardt <[email protected]>

Currently, setup_graphics() ignores the return value of efi_setup_gop(). As AllocatePool() does not zero out memory, the screen information table will contain uninitialized data in this case.

We should free the screen information table if efi_setup_gop() returns an error code.

---
 drivers/firmware/efi/libstub/arm-stub.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

index 99a5cde7c2d8..48161b1dd098 100644
@@ -60,7 +60,11 @@ static struct screen_info *setup_graphics(void)
 		si = alloc_screen_info();
 		if (!si)
 			return NULL;
-		efi_setup_gop(si, &gop_proto, size);
+		status = efi_setup_gop(si, &gop_proto, size);
+		if (status != EFI_SUCCESS) {
+			free_screen_info(si);
+			return NULL;
+		}
 	}
 	return si;
 }

2.17.1

From: Dave Young <[email protected]>

When I play with terminus fonts I noticed the efi early printk does not work because the earlycon code assumes font width is 8.

Here add the code to adapt with larger fonts. Tested with all kinds of kernel built-in fonts on my laptop. Also tested with a local draft patch for 14x28 !bold terminus font.

---
 drivers/firmware/efi/earlycon.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

index 5d4f84781aa0..a52236e11e5f 100644
@@ -114,14 +114,16 @@ static void efi_earlycon_write_char(u32 *dst, unsigned char c, unsigned int h)
 	const u32 color_black = 0x00000000;
 	const u32 color_white = 0x00ffffff;
 	const u8 *src;
-	u8 s8;
-	int m;
+	int m, n, bytes;
+	u8 x;
 
-	src = font->data + c * font->height;
-	s8 = *(src + h);
+	bytes = BITS_TO_BYTES(font->width);
+	src = font->data + c * font->height * bytes + h * bytes;
 
-	for (m = 0; m < 8; m++) {
-		if ((s8 >> (7 - m)) & 1)
+	for (m = 0; m < font->width; m++) {
+		n = m % 8;
+		x = *(src + m / 8);
+		if ((x >> (7 - n)) & 1)
 			*dst = color_white;
 		else
 			*dst = color_black;

2.17.1

From: Lenny Szubowicz <[email protected]>

In allocate_e820(), call the EFI get_memory_map() service directly instead of indirectly via efi_get_memory_map(). This avoids allocation of a buffer and return of the full EFI memory map, which is not needed here and would otherwise need to be freed.

Routine allocate_e820() only needs to know how many EFI memory descriptors there are in the map to allocate an adequately sized e820ext buffer, if it's needed. Note that since efi_get_memory_map() returns a memory map buffer sized with extra headroom, allocate_e820() now needs to explicitly factor that into the e820ext size calculation.

---
 drivers/firmware/efi/libstub/efistub.h  | 13 +++++++++++++
 drivers/firmware/efi/libstub/mem.c      |  2 --
 drivers/firmware/efi/libstub/x86-stub.c | 24 +++++++++---------------
 3 files changed, 22 insertions(+), 17 deletions(-)

index 67d26949fd26..62943992f02f 100644
@@ -92,6 +92,19 @@ extern __pure efi_system_table_t  *efi_system_table(void);
 #define EFI_LOCATE_BY_REGISTER_NOTIFY		1
 #define EFI_LOCATE_BY_PROTOCOL			2
 
+/*
+ * An efi_boot_memmap is used by efi_get_memory_map() to return the
+ * EFI memory map in a dynamically allocated buffer.
+ *
+ * The buffer allocated for the EFI memory map includes extra room for
+ * a minimum of EFI_MMAP_NR_SLACK_SLOTS additional EFI memory descriptors.
+ * This facilitates the reuse of the EFI memory map buffer when a second
+ * call to ExitBootServices() is needed because of intervening changes to
+ * the EFI memory map. Other related structures, e.g. x86 e820ext, need
+ * to factor in this headroom requirement as well.
+ */
+#define EFI_MMAP_NR_SLACK_SLOTS	8
+
 struct efi_boot_memmap {
 	efi_memory_desc_t	**map;
 	unsigned long		*map_size;

index 869a79c8946f..09f4fa01914e 100644
@@ -5,8 +5,6 @@
 
 #include "efistub.h"
 
-#define EFI_MMAP_NR_SLACK_SLOTS	8
-
 static inline bool mmap_has_headroom(unsigned long buff_size,
 				     unsigned long map_size,
 				     unsigned long desc_size)

index 05ccb229fb45..f0339b5d3658 100644
@@ -606,24 +606,18 @@ static efi_status_t allocate_e820(struct boot_params *params,
 				  struct setup_data **e820ext,
 				  u32 *e820ext_size)
 {
-	unsigned long map_size, desc_size, buff_size;
-	struct efi_boot_memmap boot_map;
-	efi_memory_desc_t *map;
+	unsigned long map_size, desc_size, map_key;
 	efi_status_t status;
-	__u32 nr_desc;
+	__u32 nr_desc, desc_version;
 
-	boot_map.map		= ↦
-	boot_map.map_size	= &map_size;
-	boot_map.desc_size	= &desc_size;
-	boot_map.desc_ver	= NULL;
-	boot_map.key_ptr	= NULL;
-	boot_map.buff_size	= &buff_size;
+	/* Only need the size of the mem map and size of each mem descriptor */
+	map_size = 0;
+	status = efi_bs_call(get_memory_map, &map_size, NULL, &map_key,
+			     &desc_size, &desc_version);
+	if (status != EFI_BUFFER_TOO_SMALL)
+		return (status != EFI_SUCCESS) ? status : EFI_UNSUPPORTED;
 
-	status = efi_get_memory_map(&boot_map);
-	if (status != EFI_SUCCESS)
-		return status;
-
-	nr_desc = buff_size / desc_size;
+	nr_desc = map_size / desc_size + EFI_MMAP_NR_SLACK_SLOTS;
 
 	if (nr_desc > ARRAY_SIZE(params->e820_table)) {
 		u32 nr_e820ext = nr_desc - ARRAY_SIZE(params->e820_table);

2.17.1

From: Punit Agrawal <[email protected]>

While debugging a boot failure, the following unknown error record was seen in the boot logs.

    <...>
    BERT: Error records from previous boot:
    [Hardware Error]: event severity: fatal
    [Hardware Error]:  Error 0, type: fatal
    [Hardware Error]:   section type: unknown, 81212a96-09ed-4996-9471-8d729c8e69ed
    [Hardware Error]:   section length: 0x290
    [Hardware Error]:   00000000: 00000001 00000000 00000000 00020002  ................
    [Hardware Error]:   00000010: 00020002 0000001f 00000320 00000000  ........ .......
    [Hardware Error]:   00000020: 00000000 00000000 00000000 00000000  ................
    [Hardware Error]:   00000030: 00000000 00000000 00000000 00000000  ................
    <...>

On further investigation, it was found that the error record with UUID (81212a96-09ed-4996-9471-8d729c8e69ed) has been defined in the UEFI Specification at least since v2.4 and has recently had additional fields defined in v2.7 Section N.2.10 Firmware Error Record Reference.

Add support for parsing and printing the defined fields to give users a chance to figure out what went wrong.

---
 drivers/firmware/efi/cper.c | 62 +++++++++++++++++++++++++++++++++++++
 include/linux/cper.h        |  9 ++++++
 2 files changed, 71 insertions(+)

index 9d2512913d25..f564e15fbc7e 100644
@@ -407,6 +407,58 @@ static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
 	}
 }
 
+static const char * const fw_err_rec_type_strs[] = {
+	"IPF SAL Error Record",
+	"SOC Firmware Error Record Type1 (Legacy CrashLog Support)",
+	"SOC Firmware Error Record Type2",
+};
+
+static void cper_print_fw_err(const char *pfx,
+			      struct acpi_hest_generic_data *gdata,
+			      const struct cper_sec_fw_err_rec_ref *fw_err)
+{
+	void *buf = acpi_hest_get_payload(gdata);
+	u32 offset, length = gdata->error_data_length;
+
+	printk("%s""Firmware Error Record Type: %s\n", pfx,
+	       fw_err->record_type < ARRAY_SIZE(fw_err_rec_type_strs) ?
+	       fw_err_rec_type_strs[fw_err->record_type] : "unknown");
+	printk("%s""Revision: %d\n", pfx, fw_err->revision);
+
+	/* Record Type based on UEFI 2.7 */
+	if (fw_err->revision == 0) {
+		printk("%s""Record Identifier: %08llx\n", pfx,
+		       fw_err->record_identifier);
+	} else if (fw_err->revision == 2) {
+		printk("%s""Record Identifier: %pUl\n", pfx,
+		       &fw_err->record_identifier_guid);
+	}
+
+	/*
+	 * The FW error record may contain trailing data beyond the
+	 * structure defined by the specification. As the fields
+	 * defined (and hence the offset of any trailing data) vary
+	 * with the revision, set the offset to account for this
+	 * variation.
+	 */
+	if (fw_err->revision == 0) {
+		/* record_identifier_guid not defined */
+		offset = offsetof(struct cper_sec_fw_err_rec_ref,
+				  record_identifier_guid);
+	} else if (fw_err->revision == 1) {
+		/* record_identifier not defined */
+		offset = offsetof(struct cper_sec_fw_err_rec_ref,
+				  record_identifier);
+	} else {
+		offset = sizeof(*fw_err);
+	}
+
+	buf += offset;
+	length -= offset;
+
+	print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, buf, length, true);
+}
+
 static void cper_print_tstamp(const char *pfx,
 				   struct acpi_hest_generic_data_v300 *gdata)
 {

@@ -494,6 +546,16 @@ cper_estatus_print_section(const char *pfx, struct acpi_hest_generic_data *gdata
 		else
 			goto err_section_too_small;
 #endif
+	} else if (guid_equal(sec_type, &CPER_SEC_FW_ERR_REC_REF)) {
+		struct cper_sec_fw_err_rec_ref *fw_err = acpi_hest_get_payload(gdata);
+
+		printk("%ssection_type: Firmware Error Record Reference\n",
+		       newpfx);
+		/* The minimal FW Error Record contains 16 bytes */
+		if (gdata->error_data_length >= SZ_16)
+			cper_print_fw_err(newpfx, gdata, fw_err);
+		else
+			goto err_section_too_small;
 	} else {
 		const void *err = acpi_hest_get_payload(gdata);
 

index 4f005d95ce88..8537e9282a65 100644
@@ -521,6 +521,15 @@ struct cper_sec_pcie {
 	u8	aer_info[96];
 };
 
+/* Firmware Error Record Reference, UEFI v2.7 sec N.2.10  */
+struct cper_sec_fw_err_rec_ref {
+	u8 record_type;
+	u8 revision;
+	u8 reserved[6];
+	u64 record_identifier;
+	guid_t record_identifier_guid;
+};
+
 /* Reset to default packing */
 #pragma pack()
 

2.17.1

From: Arvind Sankar <[email protected]>

Mike Lothian reports that after commit
964124a97b97 ("efi/x86: Remove extra headroom for setup block")
gcc 10.1.0 fails with

HOSTCC arch/x86/boot/tools/build /usr/lib/gcc/x86_64-pc-linux-gnu/10.1.0/../../../../x86_64-pc-linux-gnu/bin/ld:
error: linker defined: multiple definition of '_end' /usr/lib/gcc/x86_64-pc-linux-gnu/10.1.0/../../../../x86_64-pc-linux-gnu/bin/ld:
/tmp/ccEkW0jM.o: previous definition here
collect2: error: ld returned 1 exit status
make[1]: *** [scripts/Makefile.host:103: arch/x86/boot/tools/build] Error 1
make: *** [arch/x86/Makefile:303: bzImage] Error 2

The issue is with the _end variable that was added, to hold the end of the compressed kernel from zoffsets.h (ZO__end). The name clashes with the linker-defined _end symbol that indicates the end of the build program itself.

Even when there is no compile-time error, this causes build to use memory past the end of its .bss section.

To solve this, mark _end as static, and for symmetry, mark the rest of the variables that keep track of symbols from the compressed kernel as static as well.

---
 arch/x86/boot/tools/build.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

index 8f8c8e386cea..c8b8c1a8d1fc 100644
@@ -59,14 +59,14 @@ u8 buf[SETUP_SECT_MAX*512];
 #define PECOFF_COMPAT_RESERVE 0x0
 #endif
 
-unsigned long efi32_stub_entry;
-unsigned long efi64_stub_entry;
-unsigned long efi_pe_entry;
-unsigned long efi32_pe_entry;
-unsigned long kernel_info;
-unsigned long startup_64;
-unsigned long _ehead;
-unsigned long _end;
+static unsigned long efi32_stub_entry;
+static unsigned long efi64_stub_entry;
+static unsigned long efi_pe_entry;
+static unsigned long efi32_pe_entry;
+static unsigned long kernel_info;
+static unsigned long startup_64;
+static unsigned long _ehead;
+static unsigned long _end;
 
 /*----------------------------------------------------------------------*/
 

2.17.1

From: Benjamin Thiel <[email protected]>

Pull up arch-specific prototype efi_systab_show_arch() in order to fix a -Wmissing-prototypes warning:

arch/x86/platform/efi/efi.c:957:7: warning: no previous prototype for ‘efi_systab_show_arch’ [-Wmissing-prototypes]
char *efi_systab_show_arch(char *str)

---
 drivers/firmware/efi/efi.c | 5 +----
 include/linux/efi.h        | 2 ++
 2 files changed, 3 insertions(+), 4 deletions(-)

index 911a2bd0f6b7..4e3055238f31 100644
@@ -130,11 +130,8 @@ static ssize_t systab_show(struct kobject *kobj,
 	if (efi.smbios != EFI_INVALID_TABLE_ADDR)
 		str += sprintf(str, "SMBIOS=0x%lx\n", efi.smbios);
 
-	if (IS_ENABLED(CONFIG_IA64) || IS_ENABLED(CONFIG_X86)) {
-		extern char *efi_systab_show_arch(char *str);
-
+	if (IS_ENABLED(CONFIG_IA64) || IS_ENABLED(CONFIG_X86))
 		str = efi_systab_show_arch(str);
-	}
 
 	return str - buf;
 }

index 251f1f783cdf..9430d01c0c3d 100644
@@ -1245,4 +1245,6 @@ struct linux_efi_memreserve {
 
 void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size);
 
+char *efi_systab_show_arch(char *str);
+
 #endif /* _LINUX_EFI_H */

2.17.1

From: Loic Yhuel <[email protected]>

This fixes the boot issues since 5.3 on several Dell models when the TPM is enabled. Depending on the exact grub binary, booting the kernel would freeze early, or just report an error parsing the final events log.

We get an event log in the SHA-1 format, which doesn't have a tcg_efi_specid_event_head in the first event, and there is a final events table which doesn't match the crypto agile format. __calc_tpm2_event_size reads bad "count" and "efispecid->num_algs", and either fails, or loops long enough for the machine to be appear frozen.

So we now only parse the final events table, which is per the spec always supposed to be in the crypto agile format, when we got a event log in this format.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1779611

---
 drivers/firmware/efi/libstub/tpm.c | 5 +++--
 drivers/firmware/efi/tpm.c         | 5 ++++-
 2 files changed, 7 insertions(+), 3 deletions(-)

index 1d59e103a2e3..e9a684637b70 100644
@@ -54,7 +54,7 @@ void efi_retrieve_tpm2_eventlog(void)
 	efi_status_t status;
 	efi_physical_addr_t log_location = 0, log_last_entry = 0;
 	struct linux_efi_tpm_eventlog *log_tbl = NULL;
-	struct efi_tcg2_final_events_table *final_events_table;
+	struct efi_tcg2_final_events_table *final_events_table = NULL;
 	unsigned long first_entry_addr, last_entry_addr;
 	size_t log_size, last_entry_size;
 	efi_bool_t truncated;

@@ -127,7 +127,8 @@ void efi_retrieve_tpm2_eventlog(void)
 	 * Figure out whether any events have already been logged to the
 	 * final events structure, and if so how much space they take up
 	 */
-	final_events_table = get_efi_config_table(LINUX_EFI_TPM_FINAL_LOG_GUID);
+	if (version == EFI_TCG2_EVENT_LOG_FORMAT_TCG_2)
+		final_events_table = get_efi_config_table(LINUX_EFI_TPM_FINAL_LOG_GUID);
 	if (final_events_table && final_events_table->nr_events) {
 		struct tcg_pcr_event2_head *header;
 		int offset;

index 31f9f0e369b9..0543fbf60222 100644
@@ -62,8 +62,11 @@ int __init efi_tpm_eventlog_init(void)
 	tbl_size = sizeof(*log_tbl) + log_tbl->size;
 	memblock_reserve(efi.tpm_log, tbl_size);
 
-	if (efi.tpm_final_log == EFI_INVALID_TABLE_ADDR)
+	if (efi.tpm_final_log == EFI_INVALID_TABLE_ADDR ||
+	    log_tbl->version != EFI_TCG2_EVENT_LOG_FORMAT_TCG_2) {
+		pr_warn(FW_BUG "TPM Final Events table missing or invalid\n");
 		goto out;
+	}
 
 	final_tbl = early_memremap(efi.tpm_final_log, sizeof(*final_tbl));
 

2.17.1

Hi,

can you please make your tags unique? For example call this one:

efi-urgent-for-v5.7-rc6

or so so that the git history can show for which -rc that urgent tag was meant?

I mean, technically one can do that already with git but it would be easier if it had the rc in the name. We do that already in tip - either -rc name or the date of the tag.

Also, using the same tag gives:

From git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi

 ! [rejected]                  efi-next           -> efi-next  (would clobber existing tag)
 ! [rejected]                  efi-urgent         -> efi-urgent  (would clobber existing tag)

when I try to fetch from you and I can delete the old tag but having unique tag names makes the history more user-friendly, I'd say.

Thx.

Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Sure

Ping?

Did you want to make your tags unique from the next pull request onwards and I were supposed to pull this one as is?

Thx.

Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

What usually happens is that Ingo applies the patches piecemeal, ignoring the tag altogether, so without any coordination between you as x86 maintainers or communication back to me, that is what i was expecting to happen this time as well.

Note that I have another PR pending since two weeks ago [0].

So if you want to start dealing with the EFI trees in a different way from now on, that is perfectly fine with me, but please align with Ingo and Thomas first.

Thanks,
Ard.

[0] https://lore.kernel.org/linux-efi/[email protected]/

Ard,

/me dons managerial hat

Yes, please. Your pull request really do not need any special handling.

Please add a unique signed tag to each pull request and stick the description, e.g.

" EFI fixes for v5.7-rcX:
- fix EFI framebuffer earlycon for wide fonts
- avoid filling screen_info with garbage...."

into the tag which gives us the merge commit message automagically.

Thanks

tglx

Sure.

In that case, please pull these changes from

efi-fixes-for-v5.7-rc6

instead, and disregard the PR for v5.8. I will respin that, include some patches that came in in the mean time, and tag it in the correct way before resending the PR.