mirror of https://github.com/wolfSSL/wolfBoot.git
F-7979: keep Zynq ext_flash_write Page Programs inside a NOR page
ext_flash_write() chunked the request by page index (address + page * FLASH_PAGE_SIZE, xferSz = min(len, FLASH_PAGE_SIZE)) without accounting for the start address's offset inside the device page. A write starting mid-page sent a full-page Page Program across the boundary - and NOR wraps the write pointer at the page start, so the excess bytes clobbered the beginning of the page. The page-index math also made the second transfer read up to FLASH_PAGE_SIZE bytes past the end of the caller's buffer for unaligned requests. Rewrite the loop position-based: each transfer is capped to the bytes remaining in the physical page holding the current address, and address/data/len advance by the actual chunk. Page-aligned requests behave as before; the failure-stop semantics (break on failed write-enable/transfer/wait, error propagated) are unchanged. Added tools/unit-tests/unit-zynq-ext-write: it awk-extracts the real ext_flash_write() from hal/zynq.c (same approach as the F-7980 erase test) and runs it against emulated qspi_* functions whose NOR models the real wrap (a crossing program overwrites the page start it wraps into). Tests: page-aligned multi-page write; a 300 byte write at offset 200 crossing two boundaries (pre-fix: a 200+256 program wraps and clobbers the page start; post-fix: 56 bytes to the edge, then the rest from the next page start); failed write-enable and failed transfer both stop the loop with -1. Verified: unit-zynq-ext-write 4/4 green post-fix (1/4 fails pre-fix), tools/unit-tests suite green (114 binaries).pull/862/head
Daniele Lacamera
parent
43890cfb39
commit
3b99e45081
27
hal/zynq.c
27
hal/zynq.c
|
|
@ -2370,7 +2370,7 @@ int RAMFUNCTION ext_flash_write(uintptr_t address, const uint8_t *data, int len)
|
|||
{
|
||||
int ret = 0;
|
||||
uint8_t cmd[8]; /* size multiple of uint32_t */
|
||||
uint32_t xferSz, page, pages, idx;
|
||||
uint32_t xferSz, page_room, idx;
|
||||
uintptr_t addr;
|
||||
|
||||
#if defined(DEBUG_ZYNQ) && DEBUG_ZYNQ >= 2
|
||||
|
|
@ -2378,18 +2378,21 @@ int RAMFUNCTION ext_flash_write(uintptr_t address, const uint8_t *data, int len)
|
|||
address, data, len);
|
||||
#endif
|
||||
|
||||
/* write by page */
|
||||
pages = ((len + (FLASH_PAGE_SIZE-1)) / FLASH_PAGE_SIZE);
|
||||
for (page = 0; page < pages; page++) {
|
||||
/* Write page by page. Each transfer must fit inside the physical
|
||||
* page holding its start address: NOR wraps the write pointer at
|
||||
* the page boundary, so a program crossing it clobbers the start
|
||||
* of the page. */
|
||||
while (len > 0) {
|
||||
page_room = FLASH_PAGE_SIZE - ((uint32_t)address % FLASH_PAGE_SIZE);
|
||||
xferSz = ((uint32_t)len > page_room) ? page_room
|
||||
: (uint32_t)len;
|
||||
|
||||
ret = qspi_write_enable(&mDev);
|
||||
if (ret != GQSPI_CODE_SUCCESS) {
|
||||
break;
|
||||
}
|
||||
xferSz = len;
|
||||
if (xferSz > FLASH_PAGE_SIZE)
|
||||
xferSz = FLASH_PAGE_SIZE;
|
||||
|
||||
addr = address + (page * FLASH_PAGE_SIZE);
|
||||
addr = address;
|
||||
if (mDev.stripe) {
|
||||
/* For dual parallel the address divide by 2 */
|
||||
addr /= 2;
|
||||
|
|
@ -2406,9 +2409,10 @@ int RAMFUNCTION ext_flash_write(uintptr_t address, const uint8_t *data, int len)
|
|||
cmd[idx++] = ((addr >> 8) & 0xFF);
|
||||
cmd[idx++] = ((addr >> 0) & 0xFF);
|
||||
ret = qspi_transfer(&mDev, cmd, idx,
|
||||
(const uint8_t*)(data + (page * FLASH_PAGE_SIZE)),
|
||||
(const uint8_t*)data,
|
||||
xferSz, NULL, 0, 0, GQSPI_GEN_FIFO_MODE_SPI);
|
||||
wolfBoot_printf("Flash Page %d Write: Ret %d\n", page, ret);
|
||||
wolfBoot_printf("Flash Page Write: Addr 0x%llx, Len %u, Ret %d\n",
|
||||
(unsigned long long)address, xferSz, ret);
|
||||
if (ret != GQSPI_CODE_SUCCESS)
|
||||
break;
|
||||
|
||||
|
|
@ -2417,6 +2421,9 @@ int RAMFUNCTION ext_flash_write(uintptr_t address, const uint8_t *data, int len)
|
|||
break;
|
||||
}
|
||||
qspi_write_disable(&mDev);
|
||||
|
||||
data += xferSz;
|
||||
address += xferSz;
|
||||
len -= xferSz;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -104,6 +104,7 @@ TESTS+=unit-flash-write-cc26x2
|
|||
TESTS+=unit-p1021-fcm-bytes
|
||||
TESTS+=unit-ls1028a-xspi-write
|
||||
TESTS+=unit-zynq-erase-loop
|
||||
TESTS+=unit-zynq-ext-write
|
||||
TESTS+=unit-versal-qspi-dma
|
||||
TESTS+=unit-sdhci-uhs-recover
|
||||
TESTS+=unit-ti-hercules-write
|
||||
|
|
@ -814,6 +815,18 @@ zynq_erase_extract.h: ../../hal/zynq.c
|
|||
unit-zynq-erase-loop: unit-zynq-erase-loop.c zynq_erase_extract.h
|
||||
gcc -o $@ unit-zynq-erase-loop.c $(CFLAGS) $(LDFLAGS)
|
||||
|
||||
# unit-zynq-ext-write runs the real ext_flash_write() from hal/zynq.c
|
||||
# (F-7979: page chunks crossed physical NOR page boundaries and the
|
||||
# wrap clobbered the page start). Same extraction approach as the
|
||||
# erase test; the emulated NOR models the wrap.
|
||||
# NB: gawk 5.x mis-lexes an action brace directly after a regex
|
||||
# literal, so each pattern is followed by a space.
|
||||
zynq_write_extract.h: ../../hal/zynq.c
|
||||
awk '/^int RAMFUNCTION ext_flash_write\(/ {f=1} f{print} f&&/^\}/{exit}' $< > $@
|
||||
|
||||
unit-zynq-ext-write: unit-zynq-ext-write.c zynq_write_extract.h
|
||||
gcc -o $@ unit-zynq-ext-write.c $(CFLAGS) $(LDFLAGS)
|
||||
|
||||
# unit-versal-qspi-dma drives the real DMA RX path of qspi_transfer() in
|
||||
# hal/versal.c (F-7982: an unaligned read larger than the 4096-byte
|
||||
# temp buffer copied the full requested length out of the buffer). The
|
||||
|
|
|
|||
|
|
@ -0,0 +1,309 @@
|
|||
/* unit-zynq-ext-write.c
|
||||
*
|
||||
* Regression test for F-7979: ext_flash_write() in hal/zynq.c divided
|
||||
* the request into length-based FLASH_PAGE_SIZE chunks (page 0, page
|
||||
* 1, ...) without accounting for the start address's offset inside
|
||||
* the device page. A write starting mid-page sent a full-page Page
|
||||
* Program across the boundary; NOR wraps the write pointer at the
|
||||
* page start, so the excess bytes clobbered the beginning of the
|
||||
* page.
|
||||
*
|
||||
* As in unit-zynq-erase-loop, hal/zynq.c cannot be compiled on the
|
||||
* host (Xilinx SDK headers), so the Makefile extracts ext_flash_write()
|
||||
* verbatim (zynq_write_extract.h) and runs it against emulated
|
||||
* qspi_* functions. The emulated NOR models the real wrap: a Page
|
||||
* Program crossing the boundary overwrites the page start it wraps
|
||||
* into, so pre-fix the corruption is observable in the flash image.
|
||||
*
|
||||
* 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 <stdio.h>
|
||||
#include <string.h>
|
||||
|
||||
/* The extracted function is a RAMFUNCTION; on the host that is nothing. */
|
||||
#define RAMFUNCTION
|
||||
|
||||
/* Xilinx / board constants the function uses (board build tree values).
|
||||
* QSPI NOR page program size is 256. */
|
||||
#define GQSPI_CODE_SUCCESS 0
|
||||
#define GQSPI_GEN_FIFO_MODE_SPI 0
|
||||
#define PAGE_PROG_CMD 0x02U
|
||||
#define FLASH_PAGE_SIZE 256
|
||||
#define GQPI_USE_4BYTE_ADDR 0
|
||||
#define wolfBoot_printf(...) do {} while (0)
|
||||
|
||||
/* The driver keeps its device state in a static mDev; the only member
|
||||
* the write loop reads is stripe (dual-parallel address scaling). */
|
||||
typedef struct {
|
||||
int stripe;
|
||||
} QspiDev_t;
|
||||
static QspiDev_t mDev;
|
||||
|
||||
/* Emulated QSPI device: records every operation and programs an
|
||||
* emulated NOR that wraps the write pointer at page boundaries, like
|
||||
* real NOR flash. */
|
||||
#define EMU_MAX_OPS 64
|
||||
enum {
|
||||
EMU_WEN,
|
||||
EMU_XFER,
|
||||
EMU_WAIT,
|
||||
EMU_WDIS,
|
||||
};
|
||||
struct emu_op {
|
||||
int kind;
|
||||
uint8_t cmd[16];
|
||||
uint32_t cmdsz;
|
||||
uint32_t txsz;
|
||||
};
|
||||
static struct emu_op g_ops[EMU_MAX_OPS];
|
||||
static int g_ops_n;
|
||||
/* Force the operation at this index to fail (one-shot, -1 = none). */
|
||||
static int g_fail_at_op = -1;
|
||||
|
||||
#define NOR_SIZE (4 * 1024)
|
||||
static uint8_t g_nor[NOR_SIZE];
|
||||
|
||||
static int emu_ret(void)
|
||||
{
|
||||
if (g_fail_at_op >= 0 && (g_ops_n - 1) == g_fail_at_op) {
|
||||
g_fail_at_op = -1;
|
||||
return -1;
|
||||
}
|
||||
return GQSPI_CODE_SUCCESS;
|
||||
}
|
||||
|
||||
static int qspi_write_enable(QspiDev_t *dev)
|
||||
{
|
||||
(void)dev;
|
||||
g_ops[g_ops_n].kind = EMU_WEN;
|
||||
g_ops_n++;
|
||||
return emu_ret();
|
||||
}
|
||||
|
||||
static int qspi_transfer(QspiDev_t *pDev,
|
||||
const uint8_t *cmdData, uint32_t cmdSz,
|
||||
const uint8_t *txData, uint32_t txSz,
|
||||
uint8_t *rxData, uint32_t rxSz, uint32_t dummySz,
|
||||
uint32_t mode)
|
||||
{
|
||||
(void)pDev; (void)rxData; (void)rxSz; (void)dummySz; (void)mode;
|
||||
|
||||
if (g_ops_n < EMU_MAX_OPS) {
|
||||
uint32_t n = cmdSz < sizeof(g_ops[0].cmd) ? cmdSz
|
||||
: sizeof(g_ops[0].cmd);
|
||||
|
||||
g_ops[g_ops_n].kind = EMU_XFER;
|
||||
memcpy(g_ops[g_ops_n].cmd, cmdData, n);
|
||||
g_ops[g_ops_n].cmdsz = cmdSz;
|
||||
g_ops[g_ops_n].txsz = txSz;
|
||||
|
||||
/* Model the Page Program: the write pointer starts at the
|
||||
* command address and wraps at the physical page boundary,
|
||||
* clobbering the page start for a crossing program. */
|
||||
if (cmdSz >= 4 && cmdData[0] == PAGE_PROG_CMD) {
|
||||
uint32_t addr = ((uint32_t)cmdData[1] << 16) |
|
||||
((uint32_t)cmdData[2] << 8) |
|
||||
(uint32_t)cmdData[3];
|
||||
uint32_t base = addr & ~(uint32_t)(FLASH_PAGE_SIZE - 1);
|
||||
uint32_t i;
|
||||
|
||||
for (i = 0; i < txSz; i++)
|
||||
g_nor[base + ((addr + i) & (FLASH_PAGE_SIZE - 1))] =
|
||||
txData[i];
|
||||
}
|
||||
}
|
||||
g_ops_n++;
|
||||
return emu_ret();
|
||||
}
|
||||
|
||||
static int qspi_wait_ready(QspiDev_t *dev)
|
||||
{
|
||||
(void)dev;
|
||||
g_ops[g_ops_n].kind = EMU_WAIT;
|
||||
g_ops_n++;
|
||||
return emu_ret();
|
||||
}
|
||||
|
||||
static void qspi_write_disable(QspiDev_t *dev)
|
||||
{
|
||||
(void)dev;
|
||||
if (g_ops_n < EMU_MAX_OPS)
|
||||
g_ops[g_ops_n].kind = EMU_WDIS;
|
||||
g_ops_n++;
|
||||
}
|
||||
|
||||
/* The real ext_flash_write() from hal/zynq.c (extracted by the
|
||||
* Makefile). */
|
||||
#include "zynq_write_extract.h"
|
||||
|
||||
static void setup(void)
|
||||
{
|
||||
memset(g_ops, 0, sizeof(g_ops));
|
||||
g_ops_n = 0;
|
||||
g_fail_at_op = -1;
|
||||
mDev.stripe = 0;
|
||||
memset(g_nor, 0xFF, sizeof(g_nor));
|
||||
}
|
||||
|
||||
static void teardown(void)
|
||||
{
|
||||
}
|
||||
|
||||
static void fill(uint8_t *buf, size_t len, uint8_t base)
|
||||
{
|
||||
size_t i;
|
||||
|
||||
for (i = 0; i < len; i++)
|
||||
buf[i] = (uint8_t)(base + i);
|
||||
}
|
||||
|
||||
/* Page-aligned multi-page write: three well-formed Page Programs,
|
||||
* data lands, nothing else is touched. */
|
||||
START_TEST(test_write_page_aligned)
|
||||
{
|
||||
uint8_t data[600];
|
||||
uint32_t xfers = 0;
|
||||
int i;
|
||||
|
||||
fill(data, sizeof(data), 0x10);
|
||||
|
||||
ck_assert_int_eq(ext_flash_write(0, data, 600), GQSPI_CODE_SUCCESS);
|
||||
|
||||
for (i = 0; i < g_ops_n; i++) {
|
||||
if (g_ops[i].kind == EMU_XFER) {
|
||||
xfers++;
|
||||
ck_assert_uint_eq(g_ops[i].cmd[0], PAGE_PROG_CMD);
|
||||
}
|
||||
}
|
||||
ck_assert_uint_eq(xfers, 3);
|
||||
ck_assert_uint_eq(g_nor[599], data[599]);
|
||||
for (i = 0; i < 600; i++)
|
||||
ck_assert_uint_eq(g_nor[i], data[i]);
|
||||
for (i = 600; i < NOR_SIZE; i++)
|
||||
ck_assert_uint_eq(g_nor[i], 0xFF);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
/* A write that crosses page boundaries must not clobber the starts
|
||||
* of the pages it wraps into. Pre-fix, the 256-byte program starting
|
||||
* 56 bytes before the boundary wrapped and overwrote g_nor[0..55]. */
|
||||
START_TEST(test_write_crossing_page_no_wrap)
|
||||
{
|
||||
uint8_t data[300];
|
||||
uint32_t xfers = 0;
|
||||
int i;
|
||||
|
||||
/* Pre-seed the whole flash with a pattern. */
|
||||
memset(g_nor, 0x5A, sizeof(g_nor));
|
||||
fill(data, sizeof(data), 0x60);
|
||||
|
||||
ck_assert_int_eq(ext_flash_write(200, data, 300), GQSPI_CODE_SUCCESS);
|
||||
|
||||
/* Every Page Program must fit in its physical page. */
|
||||
for (i = 0; i < g_ops_n; i++) {
|
||||
if (g_ops[i].kind == EMU_XFER) {
|
||||
uint32_t addr = ((uint32_t)g_ops[i].cmd[1] << 16) |
|
||||
((uint32_t)g_ops[i].cmd[2] << 8) |
|
||||
(uint32_t)g_ops[i].cmd[3];
|
||||
|
||||
xfers++;
|
||||
ck_assert_int_le((int)(addr % FLASH_PAGE_SIZE) +
|
||||
(int)g_ops[i].txsz, FLASH_PAGE_SIZE);
|
||||
}
|
||||
}
|
||||
/* Chunks: (200, 56) to the page edge, then the remaining 244
|
||||
* bytes in one program at the next page start. */
|
||||
ck_assert_uint_eq(xfers, 2);
|
||||
ck_assert_uint_eq(g_ops[1].txsz, 56);
|
||||
ck_assert_uint_eq(g_ops[5].txsz, 244);
|
||||
ck_assert_uint_eq((((uint32_t)g_ops[5].cmd[1] << 16) |
|
||||
((uint32_t)g_ops[5].cmd[2] << 8) | (uint32_t)g_ops[5].cmd[3]),
|
||||
256);
|
||||
|
||||
/* The written bytes. */
|
||||
for (i = 0; i < 300; i++)
|
||||
ck_assert_uint_eq(g_nor[200 + i], data[i]);
|
||||
|
||||
/* The page starts a wrapping program would clobber. */
|
||||
for (i = 0; i < 200; i++)
|
||||
ck_assert_uint_eq(g_nor[i], 0x5A);
|
||||
for (i = 500; i < NOR_SIZE; i++)
|
||||
ck_assert_uint_eq(g_nor[i], 0x5A);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
/* A failed write-enable must stop the loop and report the error. */
|
||||
START_TEST(test_write_stops_on_write_enable_failure)
|
||||
{
|
||||
uint8_t data[600];
|
||||
|
||||
/* ops: WEN(0) XFER(1) WAIT(2) WDIS(3) WEN(4) <- fails here */
|
||||
g_fail_at_op = 4;
|
||||
|
||||
ck_assert_int_eq(ext_flash_write(0, data, 600), -1);
|
||||
ck_assert_int_eq(g_ops_n, 5);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
/* A failed transfer must stop the loop and report the error. */
|
||||
START_TEST(test_write_stops_on_transfer_failure)
|
||||
{
|
||||
/* ops: WEN(0) XFER(1) <- fails here; the loop breaks without a
|
||||
* further operation (the next write re-issues Write Enable). */
|
||||
uint8_t data[600];
|
||||
|
||||
g_fail_at_op = 1;
|
||||
|
||||
ck_assert_int_eq(ext_flash_write(0, data, 600), -1);
|
||||
ck_assert_int_eq(g_ops_n, 2);
|
||||
ck_assert_int_eq(g_ops[0].kind, EMU_WEN);
|
||||
ck_assert_int_eq(g_ops[1].kind, EMU_XFER);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
Suite *zynq_ext_write_suite(void)
|
||||
{
|
||||
Suite *s = suite_create("zynq-ext-write");
|
||||
TCase *tc = tcase_create("zynq-ext-write");
|
||||
|
||||
tcase_add_checked_fixture(tc, setup, teardown);
|
||||
tcase_add_test(tc, test_write_page_aligned);
|
||||
tcase_add_test(tc, test_write_crossing_page_no_wrap);
|
||||
tcase_add_test(tc, test_write_stops_on_write_enable_failure);
|
||||
tcase_add_test(tc, test_write_stops_on_transfer_failure);
|
||||
suite_add_tcase(s, tc);
|
||||
|
||||
return s;
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
int fails;
|
||||
Suite *s = zynq_ext_write_suite();
|
||||
SRunner *sr = srunner_create(s);
|
||||
|
||||
srunner_run_all(sr, CK_NORMAL);
|
||||
fails = srunner_ntests_failed(sr);
|
||||
srunner_free(sr);
|
||||
|
||||
return fails;
|
||||
}
|
||||
Loading…
Reference in New Issue