From b6281e429cea44685eb6ab48a6d6f362bead9173 Mon Sep 17 00:00:00 2001 From: David Garske Date: Tue, 27 Oct 2026 17:54:59 -0700 Subject: [PATCH 1/2] samd21: add example configs, SERCOM console and NVMCTRL fixes --- .github/workflows/test-configs.yml | 12 ++ Makefile | 1 + arch.mk | 3 + config/examples/samd21.config | 41 ++++ config/examples/samd21_j18.config | 38 ++++ docs/Targets.md | 103 ++++++++++ hal/samr21.c | 64 +++++- hal/samr21.ld | 2 +- hal/uart/uart_drv_samr21.c | 222 +++++++++++++++++++++ test-app/app_samr21.c | 69 ++++++- tools/config.mk | 2 +- tools/unit-tests/Makefile | 7 +- tools/unit-tests/unit-flash-write-samr21.c | 69 ++++++- 13 files changed, 608 insertions(+), 25 deletions(-) create mode 100644 config/examples/samd21.config create mode 100644 config/examples/samd21_j18.config create mode 100644 hal/uart/uart_drv_samr21.c diff --git a/.github/workflows/test-configs.yml b/.github/workflows/test-configs.yml index 452ebc6a5f..3b6f9b1628 100644 --- a/.github/workflows/test-configs.yml +++ b/.github/workflows/test-configs.yml @@ -44,6 +44,18 @@ jobs: arch: arm config-file: ./config/examples/same51-dualbank.config + samd21_test: + uses: ./.github/workflows/test-build.yml + with: + arch: arm + config-file: ./config/examples/samd21.config + + samd21_j18_test: + uses: ./.github/workflows/test-build.yml + with: + arch: arm + config-file: ./config/examples/samd21_j18.config + imx_rt1040_test: uses: ./.github/workflows/test-build-mcux-sdk-manifests.yml with: diff --git a/Makefile b/Makefile index 95be9779a6..ed1ef4c00d 100644 --- a/Makefile +++ b/Makefile @@ -862,6 +862,7 @@ $(LSCRIPT): $(LSCRIPT_IN) FORCE sed -e "s/@STACK_SIZE_PER_HART@/$(STACK_SIZE_PER_HART)/g" | \ sed -e "s/@WOLFBOOT_L2SCRATCH_SIZE@/$(WOLFBOOT_L2SCRATCH_SIZE)/g" | \ sed -e "s/@STACK_SIZE@/$(STACK_SIZE)/g" | \ + sed -e "s/@WOLFBOOT_RAM_SIZE@/$(WOLFBOOT_RAM_SIZE)/g" | \ sed -e 's/@WOLFHAL_FLASH_EXCLUDE_TEXT@/$(WOLFHAL_FLASH_EXCLUDE_TEXT)/g' | \ sed -e 's/@WOLFHAL_FLASH_EXCLUDE_RODATA@/$(WOLFHAL_FLASH_EXCLUDE_RODATA)/g' | \ sed -e 's/@WOLFHAL_FLASH_RAM_SECTIONS@/$(WOLFHAL_FLASH_RAM_SECTIONS)/g' \ diff --git a/arch.mk b/arch.mk index 0da7cdd03f..b8e9a750cc 100644 --- a/arch.mk +++ b/arch.mk @@ -473,6 +473,9 @@ ifeq ($(ARCH),ARM) ## Target specific configuration ifeq ($(TARGET),samr21) CORTEX_M0=1 + # SAMD21x17 and SAMR21 have 16KB of SRAM, SAMD21x18 has 32KB. The + # linker script takes this as the length of its RAM region. + WOLFBOOT_RAM_SIZE?=0x4000 endif ifeq ($(TARGET),imx_rt) CORTEX_M7=1 diff --git a/config/examples/samd21.config b/config/examples/samd21.config new file mode 100644 index 0000000000..4496943c56 --- /dev/null +++ b/config/examples/samd21.config @@ -0,0 +1,41 @@ +ARCH?=ARM +TARGET?=samr21 +SIGN?=ECC256 +HASH?=SHA256 +DEBUG?=0 +DEBUG_UART?=0 +VTOR?=1 +CORTEX_M0?=1 +NO_ASM?=0 +NO_MPU?=1 +EXT_FLASH?=0 +SPI_FLASH?=0 +ALLOW_DOWNGRADE?=0 +NVM_FLASH_WRITEONCE?=1 +WOLFBOOT_VERSION?=0 +V?=0 +SPMATH?=1 +RAM_CODE?=0 +DUALBANK_SWAP?=0 + +# Cortex-M0+ doesn't support unaligned word accesses. WOLFSSL_USE_ALIGN +# routes word-sized hash operations through byte-safe code paths when the +# input/output buffers aren't 4-byte aligned. +CFLAGS_EXTRA+=-DWOLFSSL_USE_ALIGN + +# ATSAMD21J17A/G17A: 128KB flash, 16KB SRAM. +# NVMCTRL erases a 256-byte row, which is the smallest unit wolfBoot can swap. +WOLFBOOT_SECTOR_SIZE?=0x100 + +# 32KB bootloader, 2 * 47.75KB boot/update partitions, one row of swap. +# Largest signed application is 47KB: the partition less the 256-byte header +# and the two trailer rows NVM_FLASH_WRITEONCE uses. +# +# The bootloader region is 32KB because the NVMCTRL BOOTPROT fuse only +# protects 8KB, 16KB or 32KB. Any other boundary either leaves the +# bootloader writable or extends the protection into the boot partition, +# where it would block the swap. +WOLFBOOT_PARTITION_SIZE?=0xBF00 +WOLFBOOT_PARTITION_BOOT_ADDRESS?=0x8000 +WOLFBOOT_PARTITION_UPDATE_ADDRESS?=0x13F00 +WOLFBOOT_PARTITION_SWAP_ADDRESS?=0x1FE00 diff --git a/config/examples/samd21_j18.config b/config/examples/samd21_j18.config new file mode 100644 index 0000000000..2f157c22a7 --- /dev/null +++ b/config/examples/samd21_j18.config @@ -0,0 +1,38 @@ +ARCH?=ARM +TARGET?=samr21 +SIGN?=ECC256 +HASH?=SHA256 +DEBUG?=0 +DEBUG_UART?=0 +VTOR?=1 +CORTEX_M0?=1 +NO_ASM?=0 +NO_MPU?=1 +EXT_FLASH?=0 +SPI_FLASH?=0 +ALLOW_DOWNGRADE?=0 +NVM_FLASH_WRITEONCE?=1 +WOLFBOOT_VERSION?=0 +V?=0 +SPMATH?=1 +RAM_CODE?=0 +DUALBANK_SWAP?=0 + +# Cortex-M0+ doesn't support unaligned word accesses. WOLFSSL_USE_ALIGN +# routes word-sized hash operations through byte-safe code paths when the +# input/output buffers aren't 4-byte aligned. +CFLAGS_EXTRA+=-DWOLFSSL_USE_ALIGN + +# ATSAMD21J18A/G18A: 256KB flash, 32KB SRAM. +# NVMCTRL erases a 256-byte row, which is the smallest unit wolfBoot can swap. +WOLFBOOT_SECTOR_SIZE?=0x100 +WOLFBOOT_RAM_SIZE?=0x8000 + +# 32KB bootloader, 2 * 111.75KB boot/update partitions, one row of swap. +# 32KB is one of the three sizes the NVMCTRL BOOTPROT fuse can protect. +# Largest signed application is 111KB: the partition less the 256-byte header +# and the two trailer rows NVM_FLASH_WRITEONCE uses. +WOLFBOOT_PARTITION_SIZE?=0x1BF00 +WOLFBOOT_PARTITION_BOOT_ADDRESS?=0x8000 +WOLFBOOT_PARTITION_UPDATE_ADDRESS?=0x23F00 +WOLFBOOT_PARTITION_SWAP_ADDRESS?=0x3FE00 diff --git a/docs/Targets.md b/docs/Targets.md index 53026cd31c..9c513809ea 100644 --- a/docs/Targets.md +++ b/docs/Targets.md @@ -22,6 +22,7 @@ This README describes configuration of supported targets. * [Microchip PolarFire SoC](#microchip-polarfire-soc) * [Microchip SAMA5D3](#microchip-sama5d3) * [Microchip SAME51](#microchip-same51) +* [Microchip SAMD21 / SAMR21](#microchip-samd21--samr21) * [Nordic nRF52840](#nordic-nrf52840) * [Nordic nRF5340](#nordic-nrf5340) * [Nordic nRF54L15](#nordic-nrf54l15) @@ -5329,6 +5330,108 @@ The above is assuming the default configuration where the BOOT partition starts address `0x8000`. +## Microchip SAMD21 / SAMR21 + +SAM D21 and SAM R21 are Cortex-M0+ microcontrollers running at up to 48MHz. They +share one SYSCTRL/GCLK/NVMCTRL/PAC design, so a single HAL covers both and the +target name is `samr21` for either part. + +NVMCTRL programs a 64-byte page and erases a 256-byte row of four pages. The row +is the smallest unit wolfBoot can swap, so `WOLFBOOT_SECTOR_SIZE` is `0x100`. +That is also the floor for `IMAGE_HEADER_SIZE`, which keeps the practical +signature choice to ECC256 or ED25519: ECC384, ED448 and RSA need a 512-byte +header, and LMS/XMSS need 2KB or more, each forcing a logical sector of that +size and the same amount again of SRAM for the NVM cache. + +### Configurations + + * `config/examples/samd21.config` - ATSAMD21J17A/G17A, 128KB flash, 16KB SRAM + * `config/examples/samd21_j18.config` - ATSAMD21J18A/G18A, 256KB flash, 32KB SRAM + +`WOLFBOOT_RAM_SIZE` gives the linker script the length of the SRAM region, and +must match the part: `0x4000` for the x17 variants and SAMR21, `0x8000` for the +x18. The 128KB layout is a 32KB bootloader, two 47.75KB partitions and one row +of swap, which leaves 47KB for a signed application. The 256KB layout is a 32KB +bootloader, two 111.75KB partitions and one row of swap, leaving 111KB. + +Both layouts put the bootloader region at 32KB because the NVMCTRL `BOOTPROT` +fuse in the user row protects 8KB, 16KB or 32KB and nothing in between. A +boundary that is not one of those sizes either leaves the bootloader writable +or pushes the protected region into the boot partition, where it blocks the +swap. + +`WOLFSSL_USE_ALIGN` is set in both configs. Cortex-M0+ does not support +unaligned word accesses, and it routes word-sized hash operations through +byte-safe paths when a buffer is not 4-byte aligned. + +### Footprint and signature choice + +Measured with arm-none-eabi-gcc 13.2.1 at `-Os`, `SPMATH=1`, `DEBUG=0`, +`DEBUG_UART=0`. "Flash" is the sum of the allocated bootloader sections and +"stack" is the `STACK_USAGE` the build reports for the chosen algorithm: + +| Signature | Flash | Stack | Fits 16KB SRAM | +|---|---|---|---| +| ECC256 + SHA256 | 18320 | 7632 | yes | +| ECC256 + SHA256, `NO_ASM=1` | 15300 | 7632 | yes | +| ED25519 + SHA256 | 13316 | 5000 | yes | +| RSA2048 + SHA256 | 12940 | 17568 | no | +| LMS-8-10-1 + SHA256 | 9340 | 1320 | yes | + +RSA2048 is the smallest build and has by far the cheapest verification, because +a public-key operation with a small exponent costs much less than an ECDSA +verify. It needs more stack than a 16KB part has, so it is an option on the x18 +only. ED25519 gains nothing from `NO_ASM=0`: the Thumb-1 SP assembly accelerates +the integer math that ECC256 and RSA use, and ED25519 does not go through it. + +Cortex-M0+ is ARMv6-M, so `sp_armthumb.c` is the only assembly tier available; +`sp_cortexm.c` is Thumb-2 and will not assemble for this core. There is no +Thumb-1 per-algorithm assembly either, so SHA-256 is the portable C +implementation. `NO_ASM=1` drops the SP assembly as well, trading roughly a +factor of four in verification time for about 3KB of flash. + +### Uploading the bootloader and the firmware image + +The on-board EDBG debugger on an Xplained Pro board presents a CMSIS-DAP +interface that OpenOCD drives with its stock scripts: + +``` +$ openocd -f interface/cmsis-dap.cfg -f target/at91samdXX.cfg \ + -c "program wolfboot.bin 0x0 verify" \ + -c "program test-app/image_v1_signed.bin 0x8000 verify reset exit" +``` + +`0x8000` is `WOLFBOOT_PARTITION_BOOT_ADDRESS`, the same in both configs. A +J-Link or an MPLAB ICD 5 through MDB works equally well. + +### Console + +`hal/uart/uart_drv_samr21.c` drives a SERCOM in USART mode, and `hal_init()` +brings it up at 115200 8N1 when built with `DEBUG_UART=1`. That costs about +2.5KB of flash over a silent build. + +The defaults describe the SAM D21 Xplained Pro EDBG virtual COM port: SERCOM3, +TX on PA22 (pad 0), RX on PA23 (pad 1), both peripheral function C. A board that +wires its console differently overrides `UART_SERCOM`, `UART_PORT_GROUP`, +`UART_TX_PIN`, `UART_RX_PIN`, `UART_PIN_MUX`, `UART_TXPO` and `UART_RXPO`; +`UART_TXPO` and `UART_RXPO` are pad selections out of the SERCOM CTRLA field +tables rather than pin numbers. + +The driver sets the SERCOM core's generic clock channel itself, because +`hal_init()` attaches every peripheral channel to GCLK generator 7 and never +configures that generator - only generators 0 and 1 are set up. Nothing else in +wolfBoot needs a peripheral core clock, since NVMCTRL runs from the APB clock, +so the generator 7 assignment is left alone. + +### Status + +The HAL, both configurations, the console driver and the test application build +and are covered by CI and by the host unit tests `unit-samr21-erase-advance` and +`unit-flash-write-samr21`. **None of it has been validated on silicon yet**, and +the console pin assignment in particular is from the Xplained Pro schematic +rather than from a working board. + + ## NXP iMX-RT The NXP iMX-RT10xx family of devices contain a Cortex-M7 with a DCP coprocessor for SHA256 acceleration. diff --git a/hal/samr21.c b/hal/samr21.c index 4c116fa128..8bbfc14b00 100644 --- a/hal/samr21.c +++ b/hal/samr21.c @@ -21,6 +21,9 @@ #include #include "image.h" +#ifdef DEBUG_UART +#include "uart_drv.h" +#endif /* Clock settings for cpu samd21g18a @ 48MHz */ #define CPU_FREQ (48000000) @@ -59,12 +62,18 @@ #define NVMCTRLA_REG *((volatile uint16_t *)(NVMCTRL_BASE)) #define NVMCTRLB_REG *((volatile uint32_t *)(NVMCTRL_BASE + 4)) #define NVMCTRL_INTFLAG *((volatile uint8_t *)(NVMCTRL_BASE + 0x14)) +#define NVMCTRL_STATUS *((volatile uint16_t *)(NVMCTRL_BASE + 0x18)) #define NVMCTRL_ADDR *((volatile uint32_t *)(NVMCTRL_BASE + 0x1c)) #define NVMCMD_KEY (0xA500) #define NVMCMD_ERASE (0x02) #define NVMCMD_WP (0x04) #define NVMCMD_PBC (0x44) #define NVMCTRL_INTFLAG_NVMREADY (1) +/* CTRLB.MANW: commit each page with an explicit NVMCMD_WP instead of + * letting the controller write it out when the page buffer fills. */ +#define NVMCTRLB_MANW (1 << 7) +/* STATUS error flags, write-one-to-clear: PROGE, LOCKE, NVME */ +#define NVMSTATUS_ERR ((1 << 2) | (1 << 3) | (1 << 4)) #define GCLK_BASE (0x40000C00) @@ -100,26 +109,27 @@ #define PAC1_BASE (0x41000000) #define PAC1_WPCLR *((volatile uint32_t *)(PAC1_BASE)) /* Register to clear write protection (unlock flash) */ #define PAC1_WPSET *((volatile uint32_t *)(PAC1_BASE + 4)) /* Register to set write protection (lock flash) */ -#define PAC_WP_NVMCTL (1 << 1) /* Bit position for NVM in WPCLR/WPSET registers */ +#define PAC_WP_NVMCTL (1 << 2) /* Bit position for NVM in WPCLR/WPSET registers */ #define GCLK_WAITBUSY() { while (GCLK_STATUS & GCLK_STATUS_SYNCBUSY) {} } void hal_init(void) { + uint32_t i, reg; WDT_CTRL &= (~WDT_EN); #ifndef WOLFBOOT_UNIT_TEST_FLASH_WRITE __asm__ volatile ("cpsid i"); #endif - uint32_t i, reg; /* enable clocks for the power, sysctrl and gclk modules */ APBAMASK_REG = APBAMASK_PM_EN | APBAMASK_SYSCTRL_EN | APBAMASK_GCLK_EN; - /* set NVM wait states */ + /* Set NVM wait states and take manual control of the page write. The + * NVMCTRL APB clock is enabled out of reset and has to stay on: the + * flash write and erase paths below drive these same registers. */ APBBMASK_REG |= APBBMASK_NVM_EN; - NVMCTRLB_REG |= ((WAITSTATES & 0x0f) << 1); - APBBMASK_REG &= ~APBBMASK_NVM_EN; + NVMCTRLB_REG |= ((WAITSTATES & 0x0f) << 1) | NVMCTRLB_MANW; /* Set 8MHz oscillator */ @@ -145,15 +155,18 @@ void hal_init(void) GCLK_CLKCTRL = GCLK_CLKCTRL_GEN_CLK7 | i; GCLK_WAITBUSY(); } + +#ifdef DEBUG_UART + uart_init(115200, 8, 'N', 1); +#endif } void hal_prepare_boot(void) { #ifdef WOLFBOOT_RESTORE_CLOCK - /* Reset NVM wait states */ + /* Restore the NVM wait states and page write mode to their reset values */ APBBMASK_REG |= APBBMASK_NVM_EN; - NVMCTRLB_REG &= ~((WAITSTATES & 0x0f) << 1); - APBBMASK_REG &= ~APBBMASK_NVM_EN; + NVMCTRLB_REG &= ~(((WAITSTATES & 0x0f) << 1) | NVMCTRLB_MANW); /* Reset clock controller */ GCLK_CTRL = GCLK_CTRL_RESET; @@ -162,6 +175,21 @@ void hal_prepare_boot(void) } +/* Wait for the controller to go idle, then report and clear any error it + * latched. The STATUS error flags are write-one-to-clear. */ +static int RAMFUNCTION nvm_wait(void) +{ + uint16_t status; + + while (!(NVMCTRL_INTFLAG & NVMCTRL_INTFLAG_NVMREADY)) { } + status = (NVMCTRL_STATUS & NVMSTATUS_ERR); + if (status != 0) { + NVMCTRL_STATUS = status; + return -1; + } + return 0; +} + int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) { int i = 0; @@ -172,6 +200,8 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) /* Clear page buffer */ NVMCTRLA_REG = (NVMCMD_PBC | NVMCMD_KEY); + if (nvm_wait() != 0) + return -1; while (i < len) { if ((len - i > 3) && ((((address + i) & 0x03) == 0) && ((((uint32_t)data) + i) & 0x03) == 0)) { dst = (uint32_t *)(address + i); @@ -192,9 +222,20 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) } *dst = val; } + /* The page buffer holds one page, and MANW stops the controller + * from writing it out by itself, so commit at every page boundary + * and once more for a trailing partial page. */ + if ((i == len) || (((address + i) % FLASH_PAGESIZE) == 0)) { + NVMCTRLA_REG = (NVMCMD_WP | NVMCMD_KEY); + if (nvm_wait() != 0) + return -1; + if (i < len) { + NVMCTRLA_REG = (NVMCMD_PBC | NVMCMD_KEY); + if (nvm_wait() != 0) + return -1; + } + } } - /* Enable write protection */ - NVMCTRLA_REG = (NVMCMD_WP | NVMCMD_KEY); return 0; } @@ -213,7 +254,8 @@ int RAMFUNCTION hal_flash_erase(uint32_t address, int len) while (len > 0) { NVMCTRL_ADDR = (address >> 1); /* This register holds the address of a 16-bit row */ NVMCTRLA_REG = NVMCMD_ERASE | NVMCMD_KEY; - while (!(NVMCTRL_INTFLAG & NVMCTRL_INTFLAG_NVMREADY)) { } + if (nvm_wait() != 0) + return -1; address += FLASH_ROW_SIZE; len -= FLASH_ROW_SIZE; } diff --git a/hal/samr21.ld b/hal/samr21.ld index 66242fcb9e..82d0f566ef 100644 --- a/hal/samr21.ld +++ b/hal/samr21.ld @@ -1,7 +1,7 @@ MEMORY { FLASH (rx) : ORIGIN = 0x00000000, LENGTH = @BOOTLOADER_PARTITION_SIZE@ - RAM (rwx) : ORIGIN = 0x20000000, LENGTH = 0x4000 + RAM (rwx) : ORIGIN = 0x20000000, LENGTH = @WOLFBOOT_RAM_SIZE@ } SECTIONS diff --git a/hal/uart/uart_drv_samr21.c b/hal/uart/uart_drv_samr21.c new file mode 100644 index 0000000000..6ee4f71ed7 --- /dev/null +++ b/hal/uart/uart_drv_samr21.c @@ -0,0 +1,222 @@ +/* uart_drv_samr21.c + * + * Driver for the SERCOM USART on SAM D21 / SAM R21 + * + * 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 + */ + +#ifdef TARGET_samr21 + +#include + +#define CPU_FREQ (48000000) + +/* SERCOM instance, pins and pad routing. The defaults are the SAM D21 + * Xplained Pro EDBG virtual COM port: SERCOM3 with TX on PA22 (pad 0) and RX + * on PA23 (pad 1), both peripheral function C. A board that wires its console + * elsewhere overrides these; UART_TXPO and UART_RXPO are the pad selections + * from the CTRLA field tables, not pin numbers. */ +#ifndef UART_SERCOM +#define UART_SERCOM 3 +#endif +#ifndef UART_PORT_GROUP +#define UART_PORT_GROUP 0 +#endif +#ifndef UART_TX_PIN +#define UART_TX_PIN 22 +#endif +#ifndef UART_RX_PIN +#define UART_RX_PIN 23 +#endif +#ifndef UART_PIN_MUX +#define UART_PIN_MUX 2 +#endif +#ifndef UART_TXPO +#define UART_TXPO 0 +#endif +#ifndef UART_RXPO +#define UART_RXPO 1 +#endif + +/* The generic clock generator feeding the SERCOM core. hal_init() sets + * generator 0 to the 48MHz DPLL output. */ +#ifndef UART_GCLK_GEN +#define UART_GCLK_GEN 0 +#endif + +#define PM_BASE (0x40000400) +#define PM_APBCMASK *((volatile uint32_t *)(PM_BASE + 0x20)) +#define PM_APBCMASK_SERCOM(n) (1UL << (2 + (n))) + +#define GCLK_BASE (0x40000C00) +#define GCLK_STATUS *((volatile uint8_t *)(GCLK_BASE + 0x01)) +#define GCLK_CLKCTRL *((volatile uint16_t *)(GCLK_BASE + 0x02)) +#define GCLK_STATUS_SYNCBUSY (1 << 7) +#define GCLK_CLKCTRL_CLKEN (1 << 14) +#define GCLK_CLKCTRL_GEN_SHIFT (8) +/* GCLK_CLKCTRL.ID of SERCOMn_CORE: SERCOM0_CORE is 0x14 and the + * instances are consecutive. */ +#define GCLK_ID_SERCOM_CORE(n) (0x14 + (n)) + +#define PORT_BASE (0x41004400) +#define PORT_GROUP(g) (PORT_BASE + ((g) * 0x80)) +#define PORT_PINCFG(g, p) *((volatile uint8_t *)(PORT_GROUP(g) + 0x40 + (p))) +#define PORT_PMUX(g, p) *((volatile uint8_t *)(PORT_GROUP(g) + 0x30 + ((p) / 2))) +#define PINCFG_PMUXEN (1 << 0) + +#define SERCOM_BASE (0x42000800 + (UART_SERCOM * 0x400)) +#define SERCOM_CTRLA *((volatile uint32_t *)(SERCOM_BASE + 0x00)) +#define SERCOM_CTRLB *((volatile uint32_t *)(SERCOM_BASE + 0x04)) +#define SERCOM_BAUD *((volatile uint16_t *)(SERCOM_BASE + 0x0C)) +#define SERCOM_INTFLAG *((volatile uint8_t *)(SERCOM_BASE + 0x18)) +#define SERCOM_SYNCBUSY *((volatile uint32_t *)(SERCOM_BASE + 0x1C)) +#define SERCOM_DATA *((volatile uint16_t *)(SERCOM_BASE + 0x28)) + +#define CTRLA_SWRST (1UL << 0) +#define CTRLA_ENABLE (1UL << 1) +#define CTRLA_MODE_USART_INT (1UL << 2) /* USART with internal clock */ +#define CTRLA_FORM_PARITY (1UL << 24) +#define CTRLA_DORD_LSB (1UL << 30) +#define CTRLA_TXPO_SHIFT (16) +#define CTRLA_RXPO_SHIFT (20) + +#define CTRLB_CHSIZE_MASK (0x07UL) +#define CTRLB_SBMODE_2 (1UL << 6) +#define CTRLB_PMODE_ODD (1UL << 13) +#define CTRLB_TXEN (1UL << 16) +#define CTRLB_RXEN (1UL << 17) + +#define SYNCBUSY_SWRST (1UL << 0) +#define SYNCBUSY_ENABLE (1UL << 1) +#define SYNCBUSY_CTRLB (1UL << 2) + +#define INTFLAG_DRE (1 << 0) +#define INTFLAG_RXC (1 << 2) + +#define GCLK_WAITBUSY() { while (GCLK_STATUS & GCLK_STATUS_SYNCBUSY) {} } + +static void uart_pins_setup(void) +{ + uint8_t reg; + + PORT_PINCFG(UART_PORT_GROUP, UART_TX_PIN) |= PINCFG_PMUXEN; + PORT_PINCFG(UART_PORT_GROUP, UART_RX_PIN) |= PINCFG_PMUXEN; + + /* One PMUX register per pin pair: the even pin takes the low nibble and + * the odd pin the high one. */ + reg = PORT_PMUX(UART_PORT_GROUP, UART_TX_PIN); + if ((UART_TX_PIN & 1) == 0) + reg = (reg & 0xF0) | UART_PIN_MUX; + else + reg = (reg & 0x0F) | (UART_PIN_MUX << 4); + PORT_PMUX(UART_PORT_GROUP, UART_TX_PIN) = reg; + + reg = PORT_PMUX(UART_PORT_GROUP, UART_RX_PIN); + if ((UART_RX_PIN & 1) == 0) + reg = (reg & 0xF0) | UART_PIN_MUX; + else + reg = (reg & 0x0F) | (UART_PIN_MUX << 4); + PORT_PMUX(UART_PORT_GROUP, UART_RX_PIN) = reg; +} + +int uart_init(uint32_t bitrate, uint8_t data, char parity, uint8_t stop) +{ + uint32_t reg; + + /* Turn on the APB clock, then give the SERCOM core a generic clock. + * hal_init() leaves every peripheral channel pointed at generator 7, + * which it never configures, so the channel has to be set here. */ + PM_APBCMASK |= PM_APBCMASK_SERCOM(UART_SERCOM); + GCLK_CLKCTRL = GCLK_CLKCTRL_CLKEN | + (UART_GCLK_GEN << GCLK_CLKCTRL_GEN_SHIFT) | + GCLK_ID_SERCOM_CORE(UART_SERCOM); + GCLK_WAITBUSY(); + + uart_pins_setup(); + + /* Reset the peripheral so the register state is known */ + SERCOM_CTRLA = CTRLA_SWRST; + while ((SERCOM_SYNCBUSY & SYNCBUSY_SWRST) || + (SERCOM_CTRLA & CTRLA_SWRST)) + ; + + /* 16x oversampling in arithmetic mode: + * BAUD = 65536 * (1 - 16 * bitrate / CPU_FREQ) + * Scaling numerator and denominator by 1024 keeps the intermediate + * inside 32 bits, which matters on a core with no 64-bit divide. */ + SERCOM_BAUD = (uint16_t)(65536U - + ((bitrate * 1024U) / (CPU_FREQ / 1024U))); + + reg = CTRLA_MODE_USART_INT | CTRLA_DORD_LSB | + ((uint32_t)UART_TXPO << CTRLA_TXPO_SHIFT) | + ((uint32_t)UART_RXPO << CTRLA_RXPO_SHIFT); + if ((parity == 'O') || (parity == 'E')) + reg |= CTRLA_FORM_PARITY; + SERCOM_CTRLA = reg; + + reg = CTRLB_TXEN | CTRLB_RXEN; + /* CHSIZE encodes 8 bits as 0 and 9 as 1; 5 to 7 are their own values */ + if (data < 8) + reg |= (data & CTRLB_CHSIZE_MASK); + else if (data == 9) + reg |= 1; + if (parity == 'O') + reg |= CTRLB_PMODE_ODD; + if (stop > 1) + reg |= CTRLB_SBMODE_2; + SERCOM_CTRLB = reg; + while (SERCOM_SYNCBUSY & SYNCBUSY_CTRLB) + ; + + SERCOM_CTRLA |= CTRLA_ENABLE; + while (SERCOM_SYNCBUSY & SYNCBUSY_ENABLE) + ; + + return 0; +} + +int uart_tx(const uint8_t c) +{ + while ((SERCOM_INTFLAG & INTFLAG_DRE) == 0) + ; + SERCOM_DATA = c; + return 1; +} + +int uart_rx(uint8_t *c, int len) +{ + (void)len; + if (SERCOM_INTFLAG & INTFLAG_RXC) { + *c = (uint8_t)SERCOM_DATA; + return 1; + } + return 0; +} + +#ifdef DEBUG_UART +void uart_write(const char *buf, unsigned int len) +{ + while (len--) { + uart_tx((const uint8_t)*buf); + buf++; + } +} +#endif + +#endif /* TARGET_samr21 */ diff --git a/test-app/app_samr21.c b/test-app/app_samr21.c index 795438a545..d3102dd06d 100644 --- a/test-app/app_samr21.c +++ b/test-app/app_samr21.c @@ -1,4 +1,6 @@ /* samr21.c + * + * Test bare-metal application for SAM D21 / SAM R21 * * Copyright (C) 2026 wolfSSL Inc. * @@ -19,14 +21,67 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA */ -#include #include -#include -#include "hal.h" +#include "target.h" #include "wolfboot/wolfboot.h" -void main(void) { - asm volatile ("cpsie i"); - while(1) - asm volatile("WFI"); +/* SAM D21 PORT. One 0x80-byte register group per port, so group B is + * PORT_BASE + 0x80. The SAM E51 layout is the same at a different base. */ +#define PORT_BASE (0x41004400) +#define PORT_GROUP(g) (PORT_BASE + ((g) * 0x80)) +#define PORT_DIRSET(g) *((volatile uint32_t *)(PORT_GROUP(g) + 0x08)) +#define PORT_OUTCLR(g) *((volatile uint32_t *)(PORT_GROUP(g) + 0x14)) +#define PORT_OUTSET(g) *((volatile uint32_t *)(PORT_GROUP(g) + 0x18)) +#define PORT_OUTTGL(g) *((volatile uint32_t *)(PORT_GROUP(g) + 0x1C)) + +/* LED0 on the SAM D21 Xplained Pro is PB30, driven low to light. Override + * for a board that wires its LED elsewhere. */ +#ifndef LED_GROUP +#define LED_GROUP 1 +#endif +#ifndef LED_PIN +#define LED_PIN 30 +#endif +#ifndef LED_ACTIVE_LOW +#define LED_ACTIVE_LOW 1 +#endif + +#define LED_MASK (1U << LED_PIN) + +static void led_on(void) +{ +#if LED_ACTIVE_LOW + PORT_OUTCLR(LED_GROUP) = LED_MASK; +#else + PORT_OUTSET(LED_GROUP) = LED_MASK; +#endif +} + +static void delay(void) +{ + volatile uint32_t i; + for (i = 0; i < 1000000; i++) + ; +} + +void main(void) +{ + uint32_t version = wolfBoot_current_firmware_version(); + + PORT_DIRSET(LED_GROUP) = LED_MASK; + led_on(); + + /* A solid LED is the factory image; a blinking one is an update that + * has booted. Confirming it here is what stops the next boot from + * rolling the update back. */ + if (version > 1) { + wolfBoot_success(); + while (1) { + delay(); + PORT_OUTTGL(LED_GROUP) = LED_MASK; + } + } + + while (1) + __asm__ volatile ("WFI"); } diff --git a/tools/config.mk b/tools/config.mk index 3126a8744c..42ba159d8c 100644 --- a/tools/config.mk +++ b/tools/config.mk @@ -136,7 +136,7 @@ CONFIG_VARS:= ARCH TARGET SIGN HASH MCUXSDK MCUXPRESSO MCUXPRESSO_CPU MCUXPRESSO WOLFCRYPT_TZ_PSA \ WOLFBOOT_DICE_HW \ WOLFCRYPT_TZ_FWTPM \ - WOLFBOOT_PARTITION_SIZE WOLFBOOT_SECTOR_SIZE \ + WOLFBOOT_PARTITION_SIZE WOLFBOOT_SECTOR_SIZE WOLFBOOT_RAM_SIZE \ WOLFBOOT_PARTITION_BOOT_ADDRESS WOLFBOOT_PARTITION_UPDATE_ADDRESS \ WOLFBOOT_PARTITION_SWAP_ADDRESS WOLFBOOT_LOAD_ADDRESS \ WOLFBOOT_LOAD_DTS_ADDRESS WOLFBOOT_LOAD_RAMDISK_ADDRESS \ diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 9b25c10166..1744bd1e1d 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -2145,13 +2145,18 @@ samr21_erase_extract.h: ../../hal/samr21.c sed -n '/#define FLASH_ROW_SIZE /p' $< >> $@ sed -n '/#define NVMCTRLA_REG /p' $< >> $@ sed -n '/#define NVMCTRL_INTFLAG /p' $< >> $@ + sed -n '/#define NVMCTRL_STATUS /p' $< >> $@ sed -n '/#define NVMCTRL_ADDR /p' $< >> $@ sed -n '/#define NVMCMD_KEY /p' $< >> $@ sed -n '/#define NVMCMD_ERASE /p' $< >> $@ + sed -n '/#define NVMCMD_WP /p' $< >> $@ + sed -n '/#define NVMCMD_PBC /p' $< >> $@ sed -n '/#define NVMCTRL_INTFLAG_NVMREADY /p' $< >> $@ + sed -n '/#define NVMSTATUS_ERR /p' $< >> $@ samr21_erase_fn_extract.h: ../../hal/samr21.c - sed -n '/^int RAMFUNCTION hal_flash_erase/,/^}/p' $< > $@ + sed -n '/^static int RAMFUNCTION nvm_wait/,/^}/p' $< > $@ + sed -n '/^int RAMFUNCTION hal_flash_erase/,/^}/p' $< >> $@ unit-samr21-erase-advance: unit-samr21-erase-advance.c samr21_erase_extract.h \ samr21_erase_fn_extract.h diff --git a/tools/unit-tests/unit-flash-write-samr21.c b/tools/unit-tests/unit-flash-write-samr21.c index d13cf31169..c36d2cdee8 100644 --- a/tools/unit-tests/unit-flash-write-samr21.c +++ b/tools/unit-tests/unit-flash-write-samr21.c @@ -48,9 +48,15 @@ #include "../../hal/samr21.c" -/* hal_flash_write() pokes the NVMCTRL command register (NVMCTRLA_REG, fixed - * at NVMCTRL_BASE == 0x41004000) directly at entry/exit. Map that page so - * those writes don't fault; its contents are otherwise irrelevant here. */ +/* hal_flash_write() drives the NVMCTRL command register (NVMCTRLA_REG, fixed + * at NVMCTRL_BASE == 0x41004000) and waits on INTFLAG/STATUS. Map that page + * so those accesses don't fault, then present an idle, error-free controller + * so the waits fall straight through. + * + * Note this mock does not model CTRLB.MANW: a store to the flash mock lands + * immediately rather than sitting in a page buffer until NVMCMD_WP commits + * it. A host test therefore cannot observe a missing page-write command, only + * the data path and the return code. */ static void map_nvmctrl(void) { int flags = MAP_PRIVATE | MAP_ANONYMOUS; @@ -62,6 +68,8 @@ static void map_nvmctrl(void) void *p = mmap((void *)(uintptr_t)NVMCTRL_BASE, 4096, PROT_READ | PROT_WRITE, flags, -1, 0); ck_assert_ptr_eq(p, (void *)(uintptr_t)NVMCTRL_BASE); + NVMCTRL_INTFLAG = NVMCTRL_INTFLAG_NVMREADY; + NVMCTRL_STATUS = 0; } static void unmap_nvmctrl(void) @@ -72,7 +80,7 @@ static void unmap_nvmctrl(void) /* "address" is treated as a real pointer into memory-mapped flash. Keep the * mock flash buffer inside the 32-bit range, matching how "address" (a * uint32_t) is used by the real target. */ -#define MOCK_FLASH_SIZE 64 +#define MOCK_FLASH_SIZE 256 static uint8_t *mock_flash; static void setup(void) @@ -169,6 +177,56 @@ START_TEST(test_unaligned_write_single_word) } END_TEST +/* A write spanning more than one 64-byte page must program every byte and + * leave the controller with the final page committed. Because the mock does + * not model MANW (see map_nvmctrl()), this covers the data path and the last + * command only: a missing or miscounted intermediate page commit would still + * pass. Asserting one WP per page boundary needs a mock that logs the + * command sequence. */ +START_TEST(test_write_crosses_page_boundary) +{ + uint8_t data[FLASH_PAGESIZE * 2]; + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + int i; + + for (i = 0; i < (int)sizeof(data); i++) + data[i] = (uint8_t)i; + + ck_assert_int_eq(hal_flash_write(base, data, sizeof(data)), 0); + + for (i = 0; i < (int)sizeof(data); i++) + ck_assert_uint_eq(mock_flash[i], data[i]); + for (i = sizeof(data); i < MOCK_FLASH_SIZE; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); + /* The last command issued closes the final page. */ + ck_assert_uint_eq(NVMCTRLA_REG, (NVMCMD_WP | NVMCMD_KEY)); +} +END_TEST + +/* An error latched in NVMCTRL.STATUS must be reported to the caller rather + * than swallowed. The driver also writes the flags back to clear them, which + * this mock cannot check: STATUS is plain RAM here, so a write-one-to-clear + * store leaves the bits set instead of clearing them. */ +START_TEST(test_write_reports_nvm_error) +{ + uint8_t data[4] = { 1, 2, 3, 4 }; + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + + NVMCTRL_STATUS = NVMSTATUS_ERR; + ck_assert_int_eq(hal_flash_write(base, data, sizeof(data)), -1); +} +END_TEST + +/* Same contract on the erase path. */ +START_TEST(test_erase_reports_nvm_error) +{ + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + + NVMCTRL_STATUS = NVMSTATUS_ERR; + ck_assert_int_eq(hal_flash_erase(base, FLASH_ROW_SIZE), -1); +} +END_TEST + Suite *flash_write_suite(void) { Suite *s = suite_create("flash-write-samr21"); @@ -178,6 +236,9 @@ Suite *flash_write_suite(void) tcase_add_test(tc, test_unaligned_write_mismatched_alignment); tcase_add_test(tc, test_unaligned_write_matched_alignment); tcase_add_test(tc, test_unaligned_write_single_word); + tcase_add_test(tc, test_write_crosses_page_boundary); + tcase_add_test(tc, test_write_reports_nvm_error); + tcase_add_test(tc, test_erase_reports_nvm_error); suite_add_tcase(s, tc); return s; From 383ddb2d11f31b8b1fceedf14237578a231f6d1a Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 29 Oct 2026 18:33:38 -0700 Subject: [PATCH 2/2] samd21: protect the bootloader region with the NVMCTRL lock regions --- .gitignore | 2 + docs/Targets.md | 22 +- hal/samr21.c | 75 ++++ tools/unit-tests/Makefile | 34 ++ tools/unit-tests/unit-samr21-flash-protect.c | 347 +++++++++++++++++++ 5 files changed, 475 insertions(+), 5 deletions(-) create mode 100644 tools/unit-tests/unit-samr21-flash-protect.c diff --git a/.gitignore b/.gitignore index 93a59348c3..7be3b51268 100644 --- a/.gitignore +++ b/.gitignore @@ -546,6 +546,8 @@ tools/unit-tests/nxp_t10xx_fixup_extract.h tools/unit-tests/sama5d3_read_extract.h tools/unit-tests/samr21_erase_extract.h tools/unit-tests/samr21_erase_fn_extract.h +tools/unit-tests/samr21_protect_extract.h +tools/unit-tests/samr21_protect_fn_extract.h tools/unit-tests/sdhci_host.c tools/unit-tests/stm32l5_write_extract.h tools/unit-tests/stm32u5_write_extract.h diff --git a/docs/Targets.md b/docs/Targets.md index 9c513809ea..65d94751f4 100644 --- a/docs/Targets.md +++ b/docs/Targets.md @@ -5360,6 +5360,18 @@ boundary that is not one of those sizes either leaves the bootloader writable or pushes the protected region into the boot partition, where it blocks the swap. +`hal_flash_protect()` locks that region through the NVMCTRL lock regions +rather than the fuse. NVMCTRL splits the array into sixteen regions, so a +region is 8KB on a 128KB part and 16KB on a 256KB one, and a 32KB bootloader +is a whole number of regions either way. wolfBoot applies the lock on the way +out to the application, after any swap has finished writing; `NVMCTRL.LOCK` +reloads from the user row at every reset, so the bootloader itself always runs +with the region writable and re-locks it on each boot. + +Setting the persistent `BOOTPROT` fuse is left to whatever programs the part +in production. wolfBoot does not write it, because the user row would have to +be erased and rewritten on every boot to do so. + `WOLFSSL_USE_ALIGN` is set in both configs. Cortex-M0+ does not support unaligned word accesses, and it routes word-sized hash operations through byte-safe paths when a buffer is not 4-byte aligned. @@ -5372,11 +5384,11 @@ Measured with arm-none-eabi-gcc 13.2.1 at `-Os`, `SPMATH=1`, `DEBUG=0`, | Signature | Flash | Stack | Fits 16KB SRAM | |---|---|---|---| -| ECC256 + SHA256 | 18320 | 7632 | yes | -| ECC256 + SHA256, `NO_ASM=1` | 15300 | 7632 | yes | -| ED25519 + SHA256 | 13316 | 5000 | yes | -| RSA2048 + SHA256 | 12940 | 17568 | no | -| LMS-8-10-1 + SHA256 | 9340 | 1320 | yes | +| ECC256 + SHA256 | 18448 | 7632 | yes | +| ECC256 + SHA256, `NO_ASM=1` | 15428 | 7632 | yes | +| ED25519 + SHA256 | 13476 | 5000 | yes | +| RSA2048 + SHA256 | 13132 | 17568 | no | +| LMS-8-10-1 + SHA256 | 9500 | 1320 | yes | RSA2048 is the smallest build and has by far the cheapest verification, because a public-key operation with a small exponent costs much less than an ECDSA diff --git a/hal/samr21.c b/hal/samr21.c index 8bbfc14b00..b8185c2bc9 100644 --- a/hal/samr21.c +++ b/hal/samr21.c @@ -21,6 +21,7 @@ #include #include "image.h" +#include "hal.h" #ifdef DEBUG_UART #include "uart_drv.h" #endif @@ -61,12 +62,14 @@ #define NVMCTRL_BASE (0x41004000) #define NVMCTRLA_REG *((volatile uint16_t *)(NVMCTRL_BASE)) #define NVMCTRLB_REG *((volatile uint32_t *)(NVMCTRL_BASE + 4)) +#define NVMCTRL_PARAM *((volatile uint32_t *)(NVMCTRL_BASE + 0x08)) #define NVMCTRL_INTFLAG *((volatile uint8_t *)(NVMCTRL_BASE + 0x14)) #define NVMCTRL_STATUS *((volatile uint16_t *)(NVMCTRL_BASE + 0x18)) #define NVMCTRL_ADDR *((volatile uint32_t *)(NVMCTRL_BASE + 0x1c)) #define NVMCMD_KEY (0xA500) #define NVMCMD_ERASE (0x02) #define NVMCMD_WP (0x04) +#define NVMCMD_LR (0x40) #define NVMCMD_PBC (0x44) #define NVMCTRL_INTFLAG_NVMREADY (1) /* CTRLB.MANW: commit each page with an explicit NVMCMD_WP instead of @@ -74,6 +77,12 @@ #define NVMCTRLB_MANW (1 << 7) /* STATUS error flags, write-one-to-clear: PROGE, LOCKE, NVME */ #define NVMSTATUS_ERR ((1 << 2) | (1 << 3) | (1 << 4)) +/* PARAM reports the array geometry: NVMP pages of (8 << PSZ) bytes each. */ +#define NVMPARAM_NVMP (0xFFFF) +#define NVMPARAM_PSZ_SHIFT (16) +#define NVMPARAM_PSZ (7) +/* NVMCTRL divides the whole array into 16 lock regions, one per LOCK bit. */ +#define NVM_LOCK_REGIONS (16) #define GCLK_BASE (0x40000C00) @@ -262,3 +271,69 @@ int RAMFUNCTION hal_flash_erase(uint32_t address, int len) return 0; } +/* Lock the NVMCTRL regions covering [address, address+len). + * + * NVMCMD_LR only clears a bit in NVMCTRL.LOCK, which reloads from the user + * row at every reset, so the lock has to be re-applied on each boot. That is + * what the caller does: this runs on the way out to the application, after + * any swap or self-update has finished writing. The persistent BOOTPROT fuse + * is deliberately left to the production programmer, since setting it here + * would erase and rewrite the user row on every single boot. + * + * A region is one sixteenth of the array, so the geometry comes from PARAM + * rather than FLASH_SIZE: the 128KB parts have 8KB regions where the 256KB + * parts have 16KB. Protection is granted in whole regions, so a partial + * region at either end is locked whole - the locked range may come out wider + * than requested, never narrower. + */ +int RAMFUNCTION hal_flash_protect(haladdr_t address, int len) +{ + uint32_t param, flash_size, region_size, shift, start, end, i; + + if (len < 0) + return -1; + /* An empty range locks nothing. Return before the region math: the + * round-up below would otherwise widen it to one whole region. */ + if (len == 0) + return 0; + + param = NVMCTRL_PARAM; + flash_size = (param & NVMPARAM_NVMP) * + (8U << ((param >> NVMPARAM_PSZ_SHIFT) & NVMPARAM_PSZ)); + region_size = flash_size / NVM_LOCK_REGIONS; + /* Index the regions by shifting rather than dividing. Every SAMD21 array + * and page size is a power of two, so the region size is one too; the + * Cortex-M0+ has no divide instruction and a division by this runtime + * value would pull the soft-divide routine into the bootloader. The + * equality check below is what rules out a part this does not hold for, + * a PARAM of zero among them. */ + for (shift = 0; (region_size >> shift) > 1U; shift++) + ; + if ((1U << shift) != region_size) + return -1; + + start = (uint32_t)address; + if (start >= flash_size) + return -1; + /* Clamp a range that runs off the end of the array rather than failing: + * locking the part of it that exists is the safe reading. */ + if ((start + (uint32_t)len) > flash_size) + len = (int)(flash_size - start); + end = start + (uint32_t)len; + + /* The caller has already finished writing flash, so the PAC is guarding + * NVMCTRL again by now and the command register would not take a write. */ + hal_flash_unlock(); + for (i = (start >> shift); + i < ((end + region_size - 1) >> shift); i++) { + NVMCTRL_ADDR = ((i << shift) >> 1); + NVMCTRLA_REG = NVMCMD_LR | NVMCMD_KEY; + if (nvm_wait() != 0) { + hal_flash_lock(); + return -1; + } + } + hal_flash_lock(); + return 0; +} + diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 1744bd1e1d..0ec2f87de3 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -158,6 +158,7 @@ TESTS+=unit-kontron-tgl-spi TESTS+=unit-nrf5340-flash-protect TESTS+=unit-keygen-keystore TESTS+=unit-samr21-erase-advance +TESTS+=unit-samr21-flash-protect TESTS+=unit-hifive1-flash-write TESTS+=unit-hifive1-hfrosccfg TESTS+=unit-rp2350-flash-write @@ -2162,6 +2163,39 @@ unit-samr21-erase-advance: unit-samr21-erase-advance.c samr21_erase_extract.h \ samr21_erase_fn_extract.h gcc -o $@ unit-samr21-erase-advance.c $(CFLAGS) $(LDFLAGS) +# unit-samr21-flash-protect runs the real hal_flash_protect() from +# hal/samr21.c against a host NVMCTRL register window. NVMCTRL locks whole +# regions of a sixteenth of the array each, so the count has to round up and +# the region size has to come from PARAM: a truncated count leaves the last +# region writable, and a hardcoded 256KB array size under-locks a 128KB part. +samr21_protect_extract.h: ../../hal/samr21.c + sed -n '/#define NVMCTRLA_REG /p' $< > $@ + sed -n '/#define NVMCTRL_PARAM /p' $< >> $@ + sed -n '/#define NVMCTRL_INTFLAG /p' $< >> $@ + sed -n '/#define NVMCTRL_STATUS /p' $< >> $@ + sed -n '/#define NVMCTRL_ADDR /p' $< >> $@ + sed -n '/#define NVMCMD_KEY /p' $< >> $@ + sed -n '/#define NVMCMD_LR /p' $< >> $@ + sed -n '/#define NVMCTRL_INTFLAG_NVMREADY /p' $< >> $@ + sed -n '/#define NVMSTATUS_ERR /p' $< >> $@ + sed -n '/#define NVMPARAM_NVMP /p' $< >> $@ + sed -n '/#define NVMPARAM_PSZ_SHIFT /p' $< >> $@ + sed -n '/#define NVMPARAM_PSZ /p' $< >> $@ + sed -n '/#define NVM_LOCK_REGIONS /p' $< >> $@ + sed -n '/#define PAC1_WPCLR /p' $< >> $@ + sed -n '/#define PAC1_WPSET /p' $< >> $@ + sed -n '/#define PAC_WP_NVMCTL /p' $< >> $@ + +samr21_protect_fn_extract.h: ../../hal/samr21.c + sed -n '/^static int RAMFUNCTION nvm_wait/,/^}/p' $< > $@ + sed -n '/^void RAMFUNCTION hal_flash_unlock/,/^}/p' $< >> $@ + sed -n '/^void RAMFUNCTION hal_flash_lock/,/^}/p' $< >> $@ + sed -n '/^int RAMFUNCTION hal_flash_protect/,/^}/p' $< >> $@ + +unit-samr21-flash-protect: unit-samr21-flash-protect.c samr21_protect_extract.h \ + samr21_protect_fn_extract.h + gcc -o $@ unit-samr21-flash-protect.c $(CFLAGS) $(LDFLAGS) + # unit-hifive1-flash-write runs the real hal_flash_write() from # hal/hifive1.c against a mock fespi model (F-11035: the final partial # page of a multi-page write took the full-page branch, over-reading diff --git a/tools/unit-tests/unit-samr21-flash-protect.c b/tools/unit-tests/unit-samr21-flash-protect.c new file mode 100644 index 0000000000..35cc48a1bb --- /dev/null +++ b/tools/unit-tests/unit-samr21-flash-protect.c @@ -0,0 +1,347 @@ +/* unit-samr21-flash-protect.c + * + * Covers the lock-region arithmetic in hal_flash_protect() (hal/samr21.c). + * NVMCTRL grants write protection in whole lock regions, one sixteenth of the + * array each, so the region count has to be rounded up: a range ending inside + * a region leaves that region writable if the count is truncated, and the + * call still reports success. The region size comes from NVMCTRL.PARAM rather + * than a compile-time constant, because a 128KB part has 8KB regions where a + * 256KB part has 16KB, and sizing a 128KB part as if it were 256KB locks half + * the range asked for. + * + * 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 + +/* The extracted function is a RAMFUNCTION; on the host that is nothing. */ +#define RAMFUNCTION + +/* hal.h picks this by pointer width; the target is 32-bit. */ +typedef uint32_t haladdr_t; + +/* Host mock of the NVMCTRL register window. The extracted macros index it + * through NVMCTRL_BASE: command @ 0x0, PARAM @ 0x8, INTFLAG @ 0x14, + * STATUS @ 0x18, ADDR @ 0x1c. */ +static uint8_t g_nvm[0x20]; +#define NVMCTRL_BASE ((uintptr_t)g_nvm) + +/* Host mock of the PAC1 write-protect registers: WPCLR @ 0x0, WPSET @ 0x4. + * The hardware treats these as write-one-to-clear and write-one-to-set, so a + * plain memory mock only records that the bit was written, which is all the + * assertions below need. */ +static uint8_t g_pac[0x8]; +#define PAC1_BASE ((uintptr_t)g_pac) + +/* NVMCTRL register macros, command codes and the PARAM/region constants + * from hal/samr21.c (extracted by the Makefile). */ +#include "samr21_protect_extract.h" + +/* Every SAMD21 writes flash a 64-byte page at a time, so PSZ is 3 + * (8 << 3 == 64) and only the page count tells the parts apart. */ +#define PSZ_64 (3U << NVMPARAM_PSZ_SHIFT) +#define PARAM_256K ((4096U) | PSZ_64) /* J18A/G18A: 256KB, 16KB regions */ +#define PARAM_128K ((2048U) | PSZ_64) /* J17A/G17A: 128KB, 8KB regions */ + +/* ADDR holds a 16-bit (half-word) address, so a byte address is halved. */ +#define ADDR_OF(byte_addr) ((uint32_t)(byte_addr) >> 1) + +/* An address no region start can equal, so a test can tell "ADDR was never + * written" from "region 0 was locked". */ +#define ADDR_UNTOUCHED (0xA5A5A5A5) + +static void mock_reset(uint32_t param) +{ + memset(g_nvm, 0, sizeof(g_nvm)); + memset(g_pac, 0, sizeof(g_pac)); + NVMCTRL_PARAM = param; + NVMCTRL_ADDR = ADDR_UNTOUCHED; + /* Idle peripheral, no latched error: the waits fall straight through. */ + NVMCTRL_INTFLAG = NVMCTRL_INTFLAG_NVMREADY; + NVMCTRL_STATUS = 0; +} + +/* The real nvm_wait(), hal_flash_unlock(), hal_flash_lock() and + * hal_flash_protect() from hal/samr21.c. */ +#include "samr21_protect_fn_extract.h" + +/* The caller runs this after its last write, by which point hal_flash_lock() + * has handed NVMCTRL back to the PAC, so the command register will not take a + * write until the PAC is cleared again. Without the unlock the lock commands + * are silently refused and the bootloader boots unprotected, which no + * assertion on ADDR can see: the mock has no PAC enforcement. */ +START_TEST (test_protect_clears_pac_around_commands) +{ + int ret; + + mock_reset(PARAM_256K); + + ret = hal_flash_protect(0, 0x8000); + + ck_assert_int_eq(ret, 0); + ck_assert_uint_ne(PAC1_WPCLR & PAC_WP_NVMCTL, 0); + /* And handed back afterwards, so the application does not inherit an + * unguarded flash controller. */ + ck_assert_uint_ne(PAC1_WPSET & PAC_WP_NVMCTL, 0); +} +END_TEST + +/* The same on the error path: a failed lock must not leave the PAC open. */ +START_TEST (test_protect_restores_pac_on_error) +{ + int ret; + + mock_reset(PARAM_256K); + NVMCTRL_STATUS = NVMSTATUS_ERR; + + ret = hal_flash_protect(0, 0x8000); + + ck_assert_int_eq(ret, -1); + ck_assert_uint_ne(PAC1_WPSET & PAC_WP_NVMCTL, 0); +} +END_TEST + +/* The case the port actually relies on: a 32KB bootloader region on a 256KB + * part is regions 0 and 1 of 16KB, so the last region locked starts at + * 0x4000. */ +START_TEST (test_protect_bootloader_256k) +{ + int ret; + + mock_reset(PARAM_256K); + + ret = hal_flash_protect(0, 0x8000); + + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(NVMCTRL_ADDR, ADDR_OF(0x4000)); +} +END_TEST + +/* The same 32KB request on a 128KB part spans four 8KB regions, so the last + * one starts at 0x6000. Sizing the regions from a hardcoded 256KB would stop + * after 0x4000 and leave the top half of the bootloader writable. */ +START_TEST (test_protect_bootloader_128k) +{ + int ret; + + mock_reset(PARAM_128K); + + ret = hal_flash_protect(0, 0x8000); + + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(NVMCTRL_ADDR, ADDR_OF(0x6000)); +} +END_TEST + +/* A range ending inside a region must lock that region whole. 0x3000..0x5000 + * on 16KB regions covers regions 0 and 1, so the last is 0x4000. Truncating + * the count (len / region_size == 0 here) would lock region 0 alone, or + * nothing at all, and still return success. */ +START_TEST (test_protect_partial_region_rounds_up) +{ + int ret; + + mock_reset(PARAM_256K); + + ret = hal_flash_protect(0x3000, 0x2000); + + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(NVMCTRL_ADDR, ADDR_OF(0x4000)); +} +END_TEST + +/* A single byte still locks the whole region containing it. */ +START_TEST (test_protect_one_byte_locks_region) +{ + int ret; + + mock_reset(PARAM_256K); + + ret = hal_flash_protect(0, 1); + + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(NVMCTRL_ADDR, ADDR_OF(0)); +} +END_TEST + +/* The start region is the one containing the address, not always region 0: + * one region's worth at the second region boundary locks only that region. */ +START_TEST (test_protect_start_region_offset) +{ + int ret; + + mock_reset(PARAM_256K); + + ret = hal_flash_protect(0x4000, 0x4000); + + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(NVMCTRL_ADDR, ADDR_OF(0x4000)); +} +END_TEST + +/* The region the loop STARTS at, which the assertions above cannot see: they + * read ADDR after the call, so only the last region written survives. A + * latched error makes nvm_wait() fail on the first command and return + * straight out, leaving ADDR holding the first region instead of the last. + * Without this, starting the loop at region 0 regardless of the address + * passes every other test here, and over-locks everything below the range. */ +START_TEST (test_protect_first_region_is_start) +{ + int ret; + + mock_reset(PARAM_256K); + NVMCTRL_STATUS = NVMSTATUS_ERR; + + ret = hal_flash_protect(0x8000, 0x4000); + + ck_assert_int_eq(ret, -1); + ck_assert_uint_eq(NVMCTRL_ADDR, ADDR_OF(0x8000)); +} +END_TEST + +/* An empty range locks nothing and succeeds. The address here is deliberately + * not region-aligned: the round-up carries the start offset, so without an + * early return a zero length widens to the one region containing the address. + * A region-aligned address would pass either way and prove nothing. */ +START_TEST (test_protect_zero_len_locks_nothing) +{ + int ret; + + mock_reset(PARAM_256K); + + ret = hal_flash_protect(0x3000, 0); + + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(NVMCTRL_ADDR, ADDR_UNTOUCHED); +} +END_TEST + +/* A negative length is a caller bug, not an empty range: reject it rather + * than sign-extending it into a span of the whole array. */ +START_TEST (test_protect_negative_len_rejected) +{ + int ret; + + mock_reset(PARAM_256K); + + ret = hal_flash_protect(0, -1); + + ck_assert_int_eq(ret, -1); + ck_assert_uint_eq(NVMCTRL_ADDR, ADDR_UNTOUCHED); +} +END_TEST + +/* A start past the end of the array has no region to lock. */ +START_TEST (test_protect_start_past_end_rejected) +{ + int ret; + + mock_reset(PARAM_256K); + + ret = hal_flash_protect(0x40000, 0x100); + + ck_assert_int_eq(ret, -1); + ck_assert_uint_eq(NVMCTRL_ADDR, ADDR_UNTOUCHED); +} +END_TEST + +/* A range running off the end is clamped to the array rather than rejected, + * and must stop at the last region instead of walking past it. */ +START_TEST (test_protect_overlong_len_clamped) +{ + int ret; + + mock_reset(PARAM_256K); + + ret = hal_flash_protect(0x3C000, 0x10000); + + ck_assert_int_eq(ret, 0); + /* Last of the sixteen 16KB regions. */ + ck_assert_uint_eq(NVMCTRL_ADDR, ADDR_OF(0x3C000)); +} +END_TEST + +/* A latched controller error has to surface: hal_flash_protect() failing is + * what makes the caller panic instead of booting unprotected. */ +START_TEST (test_protect_reports_nvm_error) +{ + int ret; + + mock_reset(PARAM_256K); + NVMCTRL_STATUS = NVMSTATUS_ERR; + + ret = hal_flash_protect(0, 0x8000); + + ck_assert_int_eq(ret, -1); +} +END_TEST + +/* A PARAM of zero (no page count) would divide by zero working out the + * region size. Report it rather than fault. */ +START_TEST (test_protect_zero_param_rejected) +{ + int ret; + + mock_reset(0); + + ret = hal_flash_protect(0, 0x8000); + + ck_assert_int_eq(ret, -1); +} +END_TEST + +Suite *samr21_protect_suite(void) +{ + Suite *s = suite_create("samr21 flash protect"); + TCase *tc = tcase_create("lock-regions"); + + tcase_add_test(tc, test_protect_bootloader_256k); + tcase_add_test(tc, test_protect_bootloader_128k); + tcase_add_test(tc, test_protect_partial_region_rounds_up); + tcase_add_test(tc, test_protect_one_byte_locks_region); + tcase_add_test(tc, test_protect_start_region_offset); + tcase_add_test(tc, test_protect_first_region_is_start); + tcase_add_test(tc, test_protect_zero_len_locks_nothing); + tcase_add_test(tc, test_protect_negative_len_rejected); + tcase_add_test(tc, test_protect_start_past_end_rejected); + tcase_add_test(tc, test_protect_overlong_len_clamped); + tcase_add_test(tc, test_protect_reports_nvm_error); + tcase_add_test(tc, test_protect_zero_param_rejected); + tcase_add_test(tc, test_protect_clears_pac_around_commands); + tcase_add_test(tc, test_protect_restores_pac_on_error); + /* An unterminated region loop hangs rather than failing; the timeout is + * what turns that into a reported failure. */ + tcase_set_timeout(tc, 10); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = samr21_protect_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +}