From 4c447f4bf9df60d94442914fd4e64d1a0a4b009b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 19:14:42 +0200 Subject: [PATCH] F-12104: Kontron TGL: apply the SPI BIOS-region lock through the BAR tgl_lock_bios_region() wrote the protected range and the FLOCKDN value through PCI configuration space (offsets 0x48 and 0x04, the status/command dword) instead of the SPI controller's memory-mapped registers at the BAR0 base, and took the range from FREG1 (non-BIOS) instead of FREG0 (BIOS). The lock now writes FPR0 and BIOS/H SFSTS/CTL through mmio_write32(), verifies both by readback, and returns an error if the bits do not stick. The helper had no callers: no hal_flash_protect() override existed, so the weak no-op default ran before handoff and the BIOS region stayed writable. Add the override routing to tgl_lock_bios_region(). Including for the hook signature also exposes the hal_flash_write/hal_flash_erase stubs as mismatching the HAL contract; fix their address parameter to haladdr_t. Add unit-kontron-tgl-spi: runs the extracted tgl_lock_bios_region() and hal_flash_protect() against mocked PCI config space and an MMIO array at the BAR address (build fails pre-fix - hal_flash_protect undefined - 4/4 pass post-fix). --- hal/kontron_vx3060_s2.c | 47 +++-- tools/unit-tests/Makefile | 18 ++ tools/unit-tests/unit-kontron-tgl-spi.c | 229 ++++++++++++++++++++++++ 3 files changed, 282 insertions(+), 12 deletions(-) create mode 100644 tools/unit-tests/unit-kontron-tgl-spi.c diff --git a/hal/kontron_vx3060_s2.c b/hal/kontron_vx3060_s2.c index 1ca9752c..22116629 100644 --- a/hal/kontron_vx3060_s2.c +++ b/hal/kontron_vx3060_s2.c @@ -20,6 +20,7 @@ */ #include +#include #include #include #include @@ -33,7 +34,10 @@ #define SPI_PCI_DEV 31 #define SPI_PCI_FUN 5 #define SPI_BAR_OFF 0x10 -#define SPI_FREG1 0x58 +/* Tiger Lake SPI controller register offsets, memory-mapped at the + * BAR0 base. FREG0 holds the BIOS flash region base/limit; FPR0 is + * the protected range register with the same base/limit layout. */ +#define SPI_FREG0 0x50 #define SPI_FREG_BASE_MASK (0x7fffU << 0) #define SPI_FREG_LIMIT_MASK (0x7fffU << 16) #define SPI_FREG_LIMIT_SHIFT (16) @@ -48,6 +52,7 @@ int tgl_lock_bios_region() { uint32_t spi_bar, spi_cmd; uint32_t reg; + int ret = 0; #if defined(DEBUG) uint32_t bios_reg_base, bios_reg_lim; @@ -60,7 +65,12 @@ int tgl_lock_bios_region() pci_config_write32(0, SPI_PCI_DEV, SPI_PCI_FUN, PCI_COMMAND_OFFSET, spi_cmd | PCI_COMMAND_MEM_SPACE); - reg = mmio_read32(spi_bar + SPI_FREG1); + /* The Flash Protected Range register has the same base/limit + * layout as the Flash Region register: take the BIOS region + * (FREG0) and enable read and write protection on it. The SPI + * registers live in the BAR's memory-mapped space, not in PCI + * configuration space. */ + reg = mmio_read32(spi_bar + SPI_FREG0); #if defined(DEBUG) bios_reg_base = (reg & SPI_FREG_BASE_MASK) << SPI_FREG_ADDR_SHIFT; bios_reg_lim = ((reg & SPI_FREG_LIMIT_MASK) >> SPI_FREG_LIMIT_SHIFT) @@ -68,21 +78,34 @@ int tgl_lock_bios_region() wolfBoot_printf("Bios reg base: 0x%x lim: 0x%x\r\n", bios_reg_base, bios_reg_lim); #endif - /* Flash Protected Range register has very similar layout of the Flash - * Region Register, so we can reuse it and just enable read and write - * protection - */ reg |= (SPI_FPR_RPE) | (SPI_FPR_WPE); - pci_config_write32(0, SPI_PCI_DEV, SPI_PCI_FUN, SPI_FPR0, reg); + mmio_write32(spi_bar + SPI_FPR0, reg); + if ((mmio_read32(spi_bar + SPI_FPR0) & + (SPI_FPR_RPE | SPI_FPR_WPE)) != (SPI_FPR_RPE | SPI_FPR_WPE)) { + ret = -1; + } /* lock down BIOS register configuration */ - reg = pci_config_read32(0, SPI_PCI_DEV, SPI_PCI_FUN, SPI_BIOS_HSFSTS_CTL); + reg = mmio_read32(spi_bar + SPI_BIOS_HSFSTS_CTL); reg |= SPI_FLOCKDN; - pci_config_write32(0, SPI_PCI_DEV, SPI_PCI_FUN, SPI_BIOS_HSFSTS_CTL, reg); + mmio_write32(spi_bar + SPI_BIOS_HSFSTS_CTL, reg); + if ((mmio_read32(spi_bar + SPI_BIOS_HSFSTS_CTL) & SPI_FLOCKDN) == 0) { + ret = -1; + } /* restore original cmd */ pci_config_write32(0, SPI_PCI_DEV, SPI_PCI_FUN, PCI_COMMAND_OFFSET, spi_cmd); - return 0; + return ret; +} + +int hal_flash_protect(haladdr_t address, int len) +{ + (void)address; + (void)len; + + /* The TGL BIOS region covers the bootloader partition, so the + * hook's address/len are the same range FREG0 describes. */ + return tgl_lock_bios_region(); } void hal_init(void) @@ -97,7 +120,7 @@ void hal_prepare_boot(void) } #endif -int hal_flash_write(uint32_t address, const uint8_t *data, int len) +int hal_flash_write(haladdr_t address, const uint8_t *data, int len) { return 0; } @@ -110,7 +133,7 @@ void hal_flash_lock(void) { } -int hal_flash_erase(uint32_t address, int len) +int hal_flash_erase(haladdr_t address, int len) { return 0; } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index b3fd5b07..e2fca006 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -116,6 +116,7 @@ TESTS+=unit-t10xx-qe-firmware TESTS+=unit-t10xx-flash-status TESTS+=unit-p1021-erase-advance TESTS+=unit-p1021-read-badblock +TESTS+=unit-kontron-tgl-spi TESTS+=unit-samr21-erase-advance TESTS+=unit-hifive1-flash-write TESTS+=unit-rp2350-flash-write @@ -1259,6 +1260,22 @@ unit-p1021-read-badblock: unit-p1021-read-badblock.c p1021_read_extract.h \ p1021_read_fn_extract.h gcc -o $@ unit-p1021-read-badblock.c $(CFLAGS) $(LDFLAGS) +# unit-kontron-tgl-spi runs the real tgl_lock_bios_region() from +# hal/kontron_vx3060_s2.c against mocked PCI config space and an +# MMIO array at the SPI BAR address (F-12104: the lock was never +# applied - no hal_flash_protect() override - and targeted the +# wrong register space). +kontron_spi_extract.h: ../../hal/kontron_vx3060_s2.c + sed -n '/^#define SPI_PCI_DEV/,/^#define SPI_FLOCKDN/p' $< > $@ + +kontron_spi_fn_extract.h: ../../hal/kontron_vx3060_s2.c + sed -n '/^int tgl_lock_bios_region/,/^}/p' $< > $@ + sed -n '/^int hal_flash_protect/,/^}/p' $< >> $@ + +unit-kontron-tgl-spi: unit-kontron-tgl-spi.c kontron_spi_extract.h \ + kontron_spi_fn_extract.h + gcc -o $@ unit-kontron-tgl-spi.c $(CFLAGS) $(LDFLAGS) + # unit-samr21-erase-advance runs the real hal_flash_erase() from # hal/samr21.c against a host NVMCTRL register window (F-11036: the # length decrement was the body of the NVMREADY wait and the address @@ -1536,6 +1553,7 @@ GENERATED_SRC:=aurix_erased_extract.h \ nxp_ls1028a_host.c nxp_p1021_host.c nxp_t10xx_fixup_extract.h \ p1021_erase_extract.h p1021_erase_fn_extract.h \ p1021_read_extract.h p1021_read_fn_extract.h \ + kontron_spi_extract.h kontron_spi_fn_extract.h \ rp2350_flash_write_extract.h \ sdhci_host.c \ stm32g4_write_extract.h stm32l4_write_extract.h stm32l5_write_extract.h \ diff --git a/tools/unit-tests/unit-kontron-tgl-spi.c b/tools/unit-tests/unit-kontron-tgl-spi.c new file mode 100644 index 00000000..b1ae0c65 --- /dev/null +++ b/tools/unit-tests/unit-kontron-tgl-spi.c @@ -0,0 +1,229 @@ +/* unit-kontron-tgl-spi.c + * + * Regression test for F-12104: the Kontron VX3060 S2 (Tiger Lake) + * SPI BIOS-region lock was never applied - no hal_flash_protect() + * override existed, so the weak no-op default ran before handoff - + * and the only helper, tgl_lock_bios_region(), wrote the protected + * range and lock values through PCI configuration space (offsets + * 0x48/0x04) instead of the SPI BAR's memory-mapped register space, + * and took the range from FREG1 (non-BIOS) instead of FREG0 (BIOS). + * + * The real function is extracted by the Makefile and run against + * mocked PCI config space and an MMIO array at the BAR address. + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot 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; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot 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. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include +#include +#include + +typedef uintptr_t haladdr_t; + +#define MMIO_BASE 0xFED40000UL + +/* SPI PCI device constants + register offsets from + * hal/kontron_vx3060_s2.c (extracted by the Makefile). */ +#include "kontron_spi_extract.h" + +/* TGL register offsets used by the model. */ +#define TGL_FREG0_OFF 0x50 +#define TGL_FREG1_OFF 0x58 +#define TGL_FPR0_OFF 0x48 +#define TGL_SFSTS_CTL_OFF 0x04 + +/* FREG0: BIOS region, base/limit fields (14 bits each, shifted 12). + * FREG1: a different range, so a test that compares the FPR0 value + * against FREG0 also proves FREG1 was not the source. */ +#define FREG0_INIT 0x7FFF7400U +#define FREG1_INIT 0x3FFF0000U + +static uint32_t g_pci_cfg[16]; /* 64 bytes of config space */ +static uint8_t g_mmio[256]; +static uint8_t g_cfg_write_off[16]; +static int g_cfg_write_count; +static int g_fail_fpr_readback; +static int g_fail_lockdn_readback; + +static void sim_reset(void) +{ + memset(g_mmio, 0, sizeof(g_mmio)); + memset(g_pci_cfg, 0, sizeof(g_pci_cfg)); + g_pci_cfg[PCI_COMMAND_OFFSET / 4] = 0x0001; /* IO space only */ + /* memory BAR, read-write (type bit 0 set), 32-bit: the code's + * PCI_BAR_MASK strips the type bits */ + g_pci_cfg[PCI_BAR0_OFFSET / 4] = (uint32_t)(MMIO_BASE) | 0x1; + g_mmio[TGL_FREG0_OFF] = 0; + *(uint32_t *)&g_mmio[TGL_FREG0_OFF] = FREG0_INIT; + *(uint32_t *)&g_mmio[TGL_FREG1_OFF] = FREG1_INIT; + g_cfg_write_count = 0; + g_fail_fpr_readback = 0; + g_fail_lockdn_readback = 0; +} + +/* Mocks for the PCI config accessors (src/pci.c). */ +uint32_t pci_config_read32(uint8_t bus, uint8_t dev, uint8_t fun, + uint8_t off) +{ + (void)bus; + (void)dev; + (void)fun; + + return g_pci_cfg[off / 4]; +} + +void pci_config_write32(uint8_t bus, uint8_t dev, uint8_t fun, + uint8_t off, uint32_t val) +{ + (void)bus; + (void)dev; + (void)fun; + + if (g_cfg_write_count < (int)(sizeof(g_cfg_write_off) / + sizeof(g_cfg_write_off[0]))) + g_cfg_write_off[g_cfg_write_count] = off; + g_cfg_write_count++; + if (off == PCI_COMMAND_OFFSET) + g_pci_cfg[off / 4] = val; +} + +/* Mocks for the MMIO accessors (src/x86/common.c). */ +static void mmio_write32(uintptr_t address, uint32_t value) +{ + uint32_t *slot = (uint32_t *)&g_mmio[address - MMIO_BASE]; + + *slot = value; +} + +static uint32_t mmio_read32(uintptr_t address) +{ + uint32_t val = *(uint32_t *)&g_mmio[address - MMIO_BASE]; + + if (g_fail_fpr_readback && (address - MMIO_BASE) == TGL_FPR0_OFF) + val &= ~SPI_FPR_WPE; + if (g_fail_lockdn_readback && + (address - MMIO_BASE) == TGL_SFSTS_CTL_OFF) + val &= ~SPI_FLOCKDN; + return val; +} + +/* The real tgl_lock_bios_region() + hal_flash_protect() from + * hal/kontron_vx3060_s2.c (extracted). */ +#include "kontron_spi_fn_extract.h" + +/* The lock must land in the MMIO space: FPR0 carries the FREG0 + * (BIOS region) base/limit with RPE/WPE set, FLOCKDN is set in + * BIOS/H SFSTS/CTL, and PCI config space sees only the COMMAND + * enable/restore. Pre-fix the values went to config offsets 0x48 + * and 0x04 and FPR0 was never written. */ +START_TEST (test_lock_written_to_mmio){ + uint32_t expected_fpr0 = FREG0_INIT | SPI_FPR_RPE | SPI_FPR_WPE; + int i, ret; + int cmd_restored = 1; + + sim_reset(); + ret = tgl_lock_bios_region(); + + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_FPR0_OFF], expected_fpr0); + ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_SFSTS_CTL_OFF] & + SPI_FLOCKDN, SPI_FLOCKDN); + /* FREG0 itself is not modified */ + ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_FREG0_OFF], FREG0_INIT); + /* config space: only the COMMAND register is written, and the + * final write restores the original value */ + for (i = 0; i < g_cfg_write_count; i++) + ck_assert_int_eq(g_cfg_write_off[i], PCI_COMMAND_OFFSET); + if (g_cfg_write_count > 0) + cmd_restored = (g_pci_cfg[PCI_COMMAND_OFFSET / 4] == 0x0001); + ck_assert_int_eq(cmd_restored, 1); +} +END_TEST + +/* The hal_flash_protect() override must route to the TGL lock with + * the hook's address/len, so the update paths' + * hal_flash_protect(WOLFBOOT_ORIGIN, BOOTLOADER_PARTITION_SIZE) + * call actually establishes protection. */ +START_TEST(test_hal_flash_protect_wires_lock) +{ + uint32_t expected_fpr0 = FREG0_INIT | SPI_FPR_RPE | SPI_FPR_WPE; + int ret; + + sim_reset(); + ret = hal_flash_protect(0xFFF00000, 0x600000); + + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_FPR0_OFF], expected_fpr0); + ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_SFSTS_CTL_OFF] & + SPI_FLOCKDN, SPI_FLOCKDN); +} +END_TEST + +/* A protected range that does not stick (readback missing WPE) + * must be reported as an error, not silently accepted. */ +START_TEST(test_fpr_readback_mismatch) +{ + int ret; + + sim_reset(); + g_fail_fpr_readback = 1; + ret = tgl_lock_bios_region(); + + ck_assert_int_lt(ret, 0); +} +END_TEST + +/* Same for the FLOCKDN bit. */ +START_TEST(test_flockdn_readback_mismatch) +{ + int ret; + + sim_reset(); + g_fail_lockdn_readback = 1; + ret = tgl_lock_bios_region(); + + ck_assert_int_lt(ret, 0); +} +END_TEST + +Suite *kontron_tgl_spi_suite(void) +{ + Suite *s = suite_create("kontron tgl spi"); + TCase *tc = tcase_create("bios-region lock"); + + tcase_add_test(tc, test_lock_written_to_mmio); + tcase_add_test(tc, test_hal_flash_protect_wires_lock); + tcase_add_test(tc, test_fpr_readback_mismatch); + tcase_add_test(tc, test_flockdn_readback_mismatch); + tcase_set_timeout(tc, 10); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = kontron_tgl_spi_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +}