F-11036: advance the page address in the SAMR21 erase loop

hal_flash_erase() used the length decrement as the unbraced body of the
NVMREADY wait loop. With the peripheral idle (NVMREADY set) the wait
body never ran, the length never shrank, and the outer loop re-erased
the first page of the range forever; whatever the wait duration, the
number of decrements tracked wait-loop iterations instead of completed
erases, and the address was never advanced, so later pages of the
requested range were never erased.

Brace the ready wait, and after a completed erase advance the address
by FLASH_PAGESIZE and decrement the length once, as the sibling
P1021 multi-block erase loop does (F-11034).

unit-samr21-erase-advance extracts the real function and register
macros and runs it against a host NVMCTRL window with NVMREADY preset
(an idle peripheral): a 128-byte range must end with page 0x1040
programmed, a 256-byte range with page 0xC0, and a single 64-byte
erase must complete. Pre-fix all three cases hang in the re-erase loop
and fail on the tcase timeout.

Verification:
- Built: arm-none-eabi-gcc -fsyntax-only -Wall -Wextra hal/samr21.c:
  clean.
- Tested: unit-samr21-erase-advance 3/3; pre-fix all three timed out
  (10 s tcase limit).
- Pitfalls: single-page erases and page-aligned ranges behave as
  before; a non-page-multiple len erases the final partial page's
  page, unchanged from the pre-existing decrement semantics.
- Style: cstyle-check.sh FMT diff on hal/samr21.c byte-identical to
  the pre-change file; the new test trips only the uncrustify
  START_TEST brace class the sibling unit tests trip.
- Unverified: no SAMR21 board execution.
- Message: F-11036: prefix, no co-author trailers.
pull/874/head
Daniele Lacamera 2026-08-26 17:22:17 +02:00
parent 7290d87657
commit 300e5db110
3 changed files with 156 additions and 1 deletions

View File

@ -210,7 +210,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))
while (!(NVMCTRL_INTFLAG & NVMCTRL_INTFLAG_NVMREADY)) { }
address += FLASH_PAGESIZE;
len -= FLASH_PAGESIZE;
}
return 0;

View File

@ -115,6 +115,7 @@ TESTS+=unit-versal-ext-write
TESTS+=unit-t10xx-qe-firmware
TESTS+=unit-t10xx-flash-status
TESTS+=unit-p1021-erase-advance
TESTS+=unit-samr21-erase-advance
TESTS+=unit-hifive1-flash-write
TESTS+=unit-fwtpm-rsp-overrun
TESTS+=unit-fwtpm-cmd-toctou
@ -1078,6 +1079,26 @@ unit-p1021-erase-advance: unit-p1021-erase-advance.c p1021_erase_extract.h \
p1021_erase_fn_extract.h
gcc -o $@ unit-p1021-erase-advance.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
# never advanced, so the loop re-erased the first page forever).
samr21_erase_extract.h: ../../hal/samr21.c
sed -n '/#define FLASH_PAGESIZE /p' $< > $@
sed -n '/#define NVMCTRLA_REG /p' $< >> $@
sed -n '/#define NVMCTRL_INTFLAG /p' $< >> $@
sed -n '/#define NVMCTRL_ADDR /p' $< >> $@
sed -n '/#define NVMCMD_KEY /p' $< >> $@
sed -n '/#define NVMCMD_ERASE /p' $< >> $@
sed -n '/#define NVMCTRL_INTFLAG_NVMREADY /p' $< >> $@
samr21_erase_fn_extract.h: ../../hal/samr21.c
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
gcc -o $@ unit-samr21-erase-advance.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

View File

@ -0,0 +1,133 @@
/* unit-samr21-erase-advance.c
*
* Regression test for F-11036: hal_flash_erase() in hal/samr21.c
* decremented the remaining length as the body of the NVMREADY wait
* loop instead of once per completed erase, and never advanced the
* address. With the peripheral idle (NVMREADY set) the wait body never
* ran, the length never shrank, and the loop re-erased the same page
* forever; the rest of the requested range was never reached.
*
* hal/samr21.c needs the SAMR21 SDK headers and cannot be built on the
* host, so the Makefile extracts the register macros and the function
* verbatim. The NVMCTRL register window is a host array with NVMREADY
* preset (an idle peripheral whose erases complete immediately), and
* the test checks the page left programmed by the loop.
*
* 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 <stdint.h>
#include <string.h>
/* The extracted function is a RAMFUNCTION; on the host that is nothing. */
#define RAMFUNCTION
/* Host mock of the NVMCTRL register window. The extracted macros index
* it through NVMCTRL_BASE: command @ 0x0, INTFLAG @ 0x14, ADDR @ 0x1c. */
static uint8_t g_nvm[0x20];
#define NVMCTRL_BASE ((uintptr_t)g_nvm)
/* NVMCTRL register macros + command codes + page size from
* hal/samr21.c (extracted by the Makefile). */
#include "samr21_erase_extract.h"
static void mock_reset(void)
{
memset(g_nvm, 0, sizeof(g_nvm));
/* Idle peripheral: the last erase completed, NVMREADY is set. */
g_nvm[0x14] = NVMCTRL_INTFLAG_NVMREADY;
}
/* The real hal_flash_erase() from hal/samr21.c (extracted). */
#include "samr21_erase_fn_extract.h"
/* A 64-byte page is one erase; a multi-page range must end with the
* last page of the range programmed, not the first. */
START_TEST (test_erase_multi_page_advances)
{
int ret;
mock_reset();
ret = hal_flash_erase(0x1000, 128);
ck_assert_int_eq(ret, 0);
/* Two 64-byte pages: 0x1000 then 0x1040. The programmed row
* register holds (byte address) >> 1; the final one must be the
* last page of the range. Pre-fix the loop re-erased page 0x1000
* forever (this case times out). */
ck_assert_uint_eq(NVMCTRL_ADDR, (0x1040 >> 1));
}
END_TEST
/* Four pages from the start of the flash must advance to the last one. */
START_TEST (test_erase_four_pages_advances)
{
int ret;
mock_reset();
ret = hal_flash_erase(0, 256);
ck_assert_int_eq(ret, 0);
ck_assert_uint_eq(NVMCTRL_ADDR, (192u >> 1));
}
END_TEST
/* A single-page erase completes and leaves its page programmed. */
START_TEST (test_erase_single_page)
{
int ret;
mock_reset();
ret = hal_flash_erase(0x2000, 64);
ck_assert_int_eq(ret, 0);
ck_assert_uint_eq(NVMCTRL_ADDR, (0x2000 >> 1));
}
END_TEST
Suite *samr21_erase_suite(void)
{
Suite *s = suite_create("samr21 erase advance");
TCase *tc = tcase_create("erase-address");
tcase_add_test(tc, test_erase_multi_page_advances);
tcase_add_test(tc, test_erase_four_pages_advances);
tcase_add_test(tc, test_erase_single_page);
/* Pre-fix the idle-peripheral model hangs in the re-erase loop;
* the tcase timeout is what turns that hang into a failure. */
tcase_set_timeout(tc, 10);
suite_add_tcase(s, tc);
return s;
}
int main(void)
{
int fails;
Suite *s = samr21_erase_suite();
SRunner *sr = srunner_create(s);
srunner_run_all(sr, CK_NORMAL);
fails = srunner_ntests_failed(sr);
srunner_free(sr);
return fails;
}