From 3c11bf3f5514c9eac4b8bbf4871d6eee24cdab58 Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 13 Apr 2026 15:53:36 -0700 Subject: [PATCH] Peer review fixes --- config/examples/polarfire_mpfs250_m_qspi.config | 3 ++- docs/Targets.md | 7 ++++--- hal/mpfs250-m.ld | 4 +++- hal/mpfs250.c | 16 ++++++++++------ hal/mpfs250.h | 1 + hal/sama5d3.ld | 3 ++- src/boot_riscv_start.S | 1 - tools/scripts/mpfs_qspi_prog.py | 8 +++++++- 8 files changed, 29 insertions(+), 14 deletions(-) diff --git a/config/examples/polarfire_mpfs250_m_qspi.config b/config/examples/polarfire_mpfs250_m_qspi.config index 192daa47..be18785c 100644 --- a/config/examples/polarfire_mpfs250_m_qspi.config +++ b/config/examples/polarfire_mpfs250_m_qspi.config @@ -59,7 +59,8 @@ OPTIMIZATION_LEVEL=1 RISCV_MMODE?=1 # Stack size per hart: set to 0 for M-mode (only E51/hart 0 runs; -# secondary harts park in eNVM WFI loop and never use L2 Scratch stacks) +# secondary harts park in eNVM WFI loop and never use L2 Scratch stacks). +# The linker script (mpfs250-m.ld) uses STACK_SIZE_PER_HART = 0 to match. CFLAGS_EXTRA+=-DSTACK_SIZE_PER_HART=0 # E51 core lacks RISC-V crypto extensions (Zknh), use portable C implementations diff --git a/docs/Targets.md b/docs/Targets.md index 83b06fbb..200fd182 100644 --- a/docs/Targets.md +++ b/docs/Targets.md @@ -839,11 +839,12 @@ These flags apply to `polarfire_mpfs250_m_qspi.config` and are added via `CFLAGS #### Stack overflow detection -The trap handler in `src/boot_riscv.c` automatically detects stack overflow on synchronous exceptions. When a trap fires with `SP < _main_hart_stack_bottom`, it prints: +The trap handler in `src/boot_riscv.c` automatically detects stack overflow on synchronous exceptions (requires `DEBUG_BOOT`). When a trap fires with `SP < _main_hart_stack_bottom`, it prints: ``` -TRAP: cause=2 epc=A000740 tval=0 sp=A02FFE8 -STACK OVERFLOW: sp=A02FFE8 < bottom=A030000 (under by 24) +TRAP: cause=2 epc=A000740 tval=0 + sp=A02FFE8 +STACK OVERFLOW: under by 24 ``` This is helpful for diagnosing illegal-instruction TRAPs at random valid `.text` addresses, which are the classic signature of stack overflow corrupting the return address. diff --git a/hal/mpfs250-m.ld b/hal/mpfs250-m.ld index f0f8aa76..95de90ae 100644 --- a/hal/mpfs250-m.ld +++ b/hal/mpfs250-m.ld @@ -119,7 +119,9 @@ PROVIDE(_start_heap = _end); * * Total stack area: STACK_SIZE + 4 * STACK_SIZE_PER_HART */ -PROVIDE(STACK_SIZE_PER_HART = 8192); +/* M-mode: only E51 (hart 0) runs; secondary harts park in eNVM WFI loop. + * Set to 0 so no L2 Scratch is wasted on phantom secondary stacks. */ +PROVIDE(STACK_SIZE_PER_HART = 0); /* End of L2 scratchpad */ PROVIDE(_l2_scratch_end = ORIGIN(L2_SCRATCH) + LENGTH(L2_SCRATCH)); diff --git a/hal/mpfs250.c b/hal/mpfs250.c index 8499459f..461a0e10 100644 --- a/hal/mpfs250.c +++ b/hal/mpfs250.c @@ -122,8 +122,9 @@ extern uint8_t _main_hart_hls; /* linker-provided address symbol; typed as uint8 # define WATCHDOG_TIMEOUT_TICKS ((WATCHDOG_TIMEOUT_MS) * 300U) #endif -/* Saved boot ROM watchdog value, restored in hal_prepare_boot() */ +/* Saved boot ROM watchdog values, restored in hal_prepare_boot() */ static uint32_t mpfs_wdt_default_mvrp = 0; +static uint32_t mpfs_wdt_default_ctrl = 0; /* CLINT MSIP register for IPI delivery */ @@ -178,8 +179,9 @@ static void qspi_uart_program(void); void hal_init(void) { #ifdef WOLFBOOT_RISCV_MMODE - /* Capture boot ROM WDT default for restoration in hal_prepare_boot() */ + /* Capture boot ROM WDT defaults for restoration in hal_prepare_boot() */ mpfs_wdt_default_mvrp = MSS_WDT_MVRP(MSS_WDT_E51_BASE); + mpfs_wdt_default_ctrl = MSS_WDT_CONTROL(MSS_WDT_E51_BASE); #ifndef WATCHDOG /* WATCHDOG=0 (default): disable WDT for the duration of wolfBoot. @@ -191,6 +193,7 @@ void hal_init(void) * never have to pet the WDT during ECDSA verify. */ MSS_WDT_REFRESH(MSS_WDT_E51_BASE) = 0xDEADC0DEU; MSS_WDT_MVRP(MSS_WDT_E51_BASE) = WATCHDOG_TIMEOUT_TICKS; + MSS_WDT_CONTROL(MSS_WDT_E51_BASE) |= MSS_WDT_CTRL_ENABLE; #endif mpfs_config_l2_cache(); @@ -407,12 +410,13 @@ int hal_dts_fixup(void* dts_addr) void hal_prepare_boot(void) { #ifdef WOLFBOOT_RISCV_MMODE - /* Restore boot ROM WDT default so the application sees a normal WDT. + /* Restore boot ROM WDT defaults so the application sees a normal WDT. * Refresh first so the timer doesn't fire immediately after we apply - * the new MVRP. Re-enable in case it was disabled by hal_init(). */ + * the new MVRP. Restore the original CONTROL value (including the + * enable bit) rather than unconditionally enabling. */ MSS_WDT_REFRESH(MSS_WDT_E51_BASE) = 0xDEADC0DEU; MSS_WDT_MVRP(MSS_WDT_E51_BASE) = mpfs_wdt_default_mvrp; - MSS_WDT_CONTROL(MSS_WDT_E51_BASE) |= MSS_WDT_CTRL_ENABLE; + MSS_WDT_CONTROL(MSS_WDT_E51_BASE) = mpfs_wdt_default_ctrl; #endif /* reset the eMMC/SD card? */ } @@ -1339,7 +1343,7 @@ static void uart_init_base(unsigned long base) MMUART_IER(base) = 0u; MMUART_FCR(base) = CLEAR_RX_FIFO_MASK | CLEAR_TX_FIFO_MASK | RXRDY_TXRDYN_EN_MASK; MMUART_MCR(base) &= ~(LOOP_MASK | RLOOP_MASK); - MMUART_MCR(base) |= (1U << 1); /* Assert RTS — required for USB-UART bridge CTS */ + MMUART_MCR(base) |= RTS_MASK; /* Assert RTS — required for USB-UART bridge CTS */ MMUART_MM1(base) &= ~(E_MSB_TX_MASK | E_MSB_RX_MASK); MMUART_MM2(base) &= ~(EAFM_MASK | ESWM_MASK); MMUART_MM0(base) &= ~(ETTG_MASK | ERTO_MASK | EFBR_MASK); diff --git a/hal/mpfs250.h b/hal/mpfs250.h index ba8b9276..d2314d63 100644 --- a/hal/mpfs250.h +++ b/hal/mpfs250.h @@ -168,6 +168,7 @@ extern const unsigned long MSS_UART_BASE_ADDR[5]; #define CLEAR_RX_FIFO_MASK (1U << 1) /* Clear receiver FIFO */ #define CLEAR_TX_FIFO_MASK (1U << 2) /* Clear transmitter FIFO */ +#define RTS_MASK (1U << 1) /* Request To Send */ #define LOOP_MASK (1U << 4) /* Local loopback */ #define RLOOP_MASK (1U << 5) /* Remote loopback & Automatic echo*/ diff --git a/hal/sama5d3.ld b/hal/sama5d3.ld index 20316d05..fbf975a2 100644 --- a/hal/sama5d3.ld +++ b/hal/sama5d3.ld @@ -38,11 +38,12 @@ SECTIONS } /* collect all uninitialized .bss sections */ - .bss (NOLOAD) : { + .bss (NOLOAD) : { . = ALIGN(4); _start_bss = .; *(.bss) _end_bss = .; + _end = .; } } diff --git a/src/boot_riscv_start.S b/src/boot_riscv_start.S index 6bac729c..5fb1a5eb 100644 --- a/src/boot_riscv_start.S +++ b/src/boot_riscv_start.S @@ -274,7 +274,6 @@ _copy_params: j .L_bss_clear .L_bss_clear_done: - #ifndef TARGET_mpfs250 /* Clear SiFive bus error unit accrued registers (not present on MPFS) */ la a4,0x01700020UL diff --git a/tools/scripts/mpfs_qspi_prog.py b/tools/scripts/mpfs_qspi_prog.py index 2ff8ec15..47f03bb4 100755 --- a/tools/scripts/mpfs_qspi_prog.py +++ b/tools/scripts/mpfs_qspi_prog.py @@ -57,7 +57,13 @@ def wait_for(port, keyword, timeout_sec, label=""): while time.monotonic() < deadline: remaining = deadline - time.monotonic() port.timeout = min(1.0, remaining) - line = port.readline() + try: + line = port.readline() + except serial.SerialException: + # JTAG reset can cause a brief USB-UART glitch on PTY proxies. + # Tolerate the transient disconnect and keep trying. + time.sleep(0.5) + continue if not line: continue text = line.decode("ascii", errors="replace").rstrip()