Peer review fixes

pull/759/head
David Garske 2026-04-13 15:53:36 -07:00 committed by Daniele Lacamera
parent 5d71feb4ef
commit 3c11bf3f55
8 changed files with 29 additions and 14 deletions

View File

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

View File

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

View File

@ -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));

View File

@ -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);

View File

@ -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*/

View File

@ -38,11 +38,12 @@ SECTIONS
}
/* collect all uninitialized .bss sections */
.bss (NOLOAD) : {
.bss (NOLOAD) : {
. = ALIGN(4);
_start_bss = .;
*(.bss)
_end_bss = .;
_end = .;
}
}

View File

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

View File

@ -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()