From 5684941f8bac16917df06d4dbe7816369319a685 Mon Sep 17 00:00:00 2001 From: Jonathon Hall Date: Tue, 2 Aug 2022 16:00:35 -0400 Subject: x86: Zero SMBIOS region before writing tables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clear the SMBIOS region before writing SMBIOS tables. On librem_mini and librem_mini_v2, CBMEM allocations are offset by 4K for reboots relative to the cold boot. This means the unused SMBIOS region could contain the first 4K of the ACPI tables from the last boot (including the signature), which prevents Linux from booting. The CBMEM 4K offset appears to be due to FSP allocating memory differently between cold boot and reboot, this appears to be normal and causes the CBMEM base address to change. It is not clear why Linux examines an ACPI signature found in this region, but boot logs over serial confirm that it sees the corrupt table. The table is supposed to be found just below 1M, and kernel source appears to look in this region, but it is definitely finding the corrupt table in CBMEM. Normal cold boot: [ 0.008615] ACPI: RSDP 0x00000000000F6190 000024 (v02 COREv4) [ 0.008619] ACPI: XSDT 0x0000000099B480E0 00005C (v01 COREv4 COREBOOT 00000000 CORE 20220331) [ 0.008624] ACPI: FACP 0x0000000099B4A2A0 000114 (v06 COREv4 COREBOOT 00000000 CORE 20220331) [ 0.008634] ACPI: DSDT 0x0000000099B48280 00201F (v02 COREv4 COREBOOT 20110725 INTL 20220331) ... Reboot with corrupt table: [ 0.008820] ACPI: RSDP 0x00000000000F6190 000024 (v02 COREv4) [ 0.008823] ACPI: XSDT 0x0000000099B480E0 00005C (v01 COREv4 COREBOOT 00000000 CORE 20220331) [ 0.008828] ACPI: ???G 0x0000000099B4A2A0 20002001 (v00 ?G?$ 47020100 ?, 47020100) [ 0.008831] ACPI: �y 0x0000000099B4A3C0 54523882 (v67 ?_HID? A�? 65520D4E al T 20656D69) ... There are no specific errors but it returns to the firmware soon after, presumably due to a fault. This appears to be so early in the boot that panic=0 on the kernel command line has no effect. Test: build/boot Librem Mini, Librem Mini v2 and reboot. Change-Id: Ia20d0b30160e89e8d96add34d7e0e881f070ec61 Signed-off-by: Jonathon Hall Reviewed-on: https://review.coreboot.org/c/coreboot/+/66377 Reviewed-by: Matt DeVillier Tested-by: build bot (Jenkins) Reviewed-by: Tim Wawrzynczak --- src/arch/x86/tables.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'src') diff --git a/src/arch/x86/tables.c b/src/arch/x86/tables.c index 8d2f7b6f20..5940c64eba 100644 --- a/src/arch/x86/tables.c +++ b/src/arch/x86/tables.c @@ -149,6 +149,13 @@ static unsigned long write_smbios_table(unsigned long rom_table_end) if (high_table_pointer) { unsigned long new_high_table_pointer; + /* + * Clear the entire region to ensure the unused space doesn't + * contain garbage from a previous boot, like stale table + * signatures that could be found by the OS. + */ + memset((void *)high_table_pointer, 0, MAX_SMBIOS_SIZE); + new_high_table_pointer = smbios_write_tables(high_table_pointer); rom_table_end = ALIGN_UP(rom_table_end, 16); -- cgit v1.2.3