From fb4a8761fc04cad7d259f6563c85460f8aeb6a7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Desbiens?= Date: Wed, 30 Sep 2026 09:55:51 -0400 Subject: [PATCH] Fixed Erbium interrupt masking and PLIC update races Polling UART output held machine interrupts off for an entire string, which could lose ThreadX ticks. PLIC enable-word changes could overwrite an update made by an interrupt handler. The UART now polls with interrupts enabled and protects only the final status check and byte write. PLIC enable and disable changes are protected across their read-modify-write. The README explains the new simulator tests. Both simulator tests failed on the original code and passed with these fixes. The 100-million-cycle demo run reported all eight threads and five thread 0 wakeups. AI disclosure and port consistency checks passed. No silicon run. Assisted-by: Codex (GPT-6-Sol) --- .../gnu/example_build/erbium/README.md | 15 ++++++++ .../risc-v64/gnu/example_build/erbium/plic.c | 6 ++++ .../risc-v64/gnu/example_build/erbium/uart.c | 35 ++++++++++--------- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/ports/risc-v64/gnu/example_build/erbium/README.md b/ports/risc-v64/gnu/example_build/erbium/README.md index 826d7cab..8cfa2828 100644 --- a/ports/risc-v64/gnu/example_build/erbium/README.md +++ b/ports/risc-v64/gnu/example_build/erbium/README.md @@ -99,6 +99,20 @@ simulated cycles, so the 100,000,000-cycle run shows `thread_0` five times. The simulator verifies register accesses, the trap flow and the console. It does not verify baud timing or the timer rate of real silicon. +## Simulator regression tests + +With `erbium_emu` on `PATH`, run: + +```bash +ERBIUM_EMU=erbium_emu ./test/run_simulator_tests.sh +``` + +The UART test checks that a long polling write does not lose ThreadX ticks. +The PLIC test changes one source in timer context while a thread changes +another, and checks that neither update is lost. The runner builds both +images with CMake and Ninja and reports a failure if either check fails. +These tests use the simulator's UART drain rate, not physical baud timing. + ## Files | File | Purpose | @@ -114,6 +128,7 @@ It does not verify baud timing or the timer rate of real silicon. | `link.lds` | MRAM layout | | `erbium_gnu.cmake` | Toolchain file: `rv64imc_zicsr_zifencei`, `lp64` | | `CMakeLists.txt`, `build.sh` | Build of the library and the demo | +| `test/` | Simulator tests for UART timekeeping and PLIC updates | ## References diff --git a/ports/risc-v64/gnu/example_build/erbium/plic.c b/ports/risc-v64/gnu/example_build/erbium/plic.c index 93b1f317..9ca25f1b 100644 --- a/ports/risc-v64/gnu/example_build/erbium/plic.c +++ b/ports/risc-v64/gnu/example_build/erbium/plic.c @@ -29,21 +29,27 @@ static int plic_source_valid(int irqno) void plic_irq_enable(int irqno) { volatile uint32_t *reg = (volatile uint32_t *)PLIC_MENABLE(riscv_get_core()); + int intr_enable; if (!plic_source_valid(irqno)) return; + intr_enable = riscv_mintr_save(); *reg = *reg | (1u << (unsigned int)irqno); + riscv_mintr_restore(intr_enable); } void plic_irq_disable(int irqno) { volatile uint32_t *reg = (volatile uint32_t *)PLIC_MENABLE(riscv_get_core()); + int intr_enable; if (!plic_source_valid(irqno)) return; + intr_enable = riscv_mintr_save(); *reg = *reg & ~(1u << (unsigned int)irqno); + riscv_mintr_restore(intr_enable); } int plic_register_callback(int irqno, irq_callback callback) diff --git a/ports/risc-v64/gnu/example_build/erbium/uart.c b/ports/risc-v64/gnu/example_build/erbium/uart.c index ee065e8a..1b8f2dfc 100644 --- a/ports/risc-v64/gnu/example_build/erbium/uart.c +++ b/ports/risc-v64/gnu/example_build/erbium/uart.c @@ -78,29 +78,32 @@ int uart_init(void) return 0; } -static inline void uart_putc_nolock(int ch) -{ - while ((UART_REG(UART_STATUS) & STATUS_TX_FULL) != 0u) - ; - UART_REG(UART_TX) = (uint32_t)(ch & 0xFF); -} - int uart_putc(int ch) { - int intr_enable = riscv_mintr_save(); - uart_putc_nolock(ch); - riscv_mintr_restore(intr_enable); - return 1; + int intr_enable; + + for (;;) + { + while ((UART_REG(UART_STATUS) & STATUS_TX_FULL) != 0u) + ; + + intr_enable = riscv_mintr_save(); + if ((UART_REG(UART_STATUS) & STATUS_TX_FULL) == 0u) + { + UART_REG(UART_TX) = (uint32_t)(ch & 0xFF); + riscv_mintr_restore(intr_enable); + return 1; + } + riscv_mintr_restore(intr_enable); + } } int uart_puts(const char *str) { int i; - int intr_enable = riscv_mintr_save(); for (i = 0; str[i] != 0; i++) - uart_putc_nolock(str[i]); - uart_putc_nolock('\r'); - uart_putc_nolock('\n'); - riscv_mintr_restore(intr_enable); + uart_putc(str[i]); + uart_putc('\r'); + uart_putc('\n'); return i; }