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 <hal.h> 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).
pull/880/head
Daniele Lacamera 2026-09-01 19:14:42 +02:00
parent 20c9031965
commit 4c447f4bf9
3 changed files with 282 additions and 12 deletions

View File

@ -20,6 +20,7 @@
*/
#include <wolfboot/wolfboot.h>
#include <hal.h>
#include <stdint.h>
#include <uart_drv.h>
#include <printf.h>
@ -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;
}

View File

@ -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 \

View File

@ -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 <check.h>
#include <pci.h>
#include <stdint.h>
#include <string.h>
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;
}