Skip to content

Commit 8001203

Browse files
committed
Peer review fixes + ZynqMP robustness improvements
hal/zynq.c: - Route IOU_TAPDLY_BYPASS writes through pmu_request at EL<=2 in the <=40 MHz and <=100 MHz branches (previously only done at <=150 MHz); the register is equally unwritable from EL2/EL1 at lower clocks. - Add qspi_flash_reset() (RESET_ENABLE 0x66 + RESET_MEMORY 0x99), called per chip in qspi_init so the flash starts from a known state regardless of what FSBL/BootROM left behind (XIP, 4-byte addr, auto-boot). - Drop unused 'reg' in csu_aes and 'ms' in csu_init so -Werror=unused-variable builds (OPTIMIZATION_LEVEL=0 / DEBUG=1) pass. hal/zynq.ld: - Move wolfBoot ORIGIN from 0x08000000 to 0x10000000. Large FIT images (kernel load=0x00200000, payload >~126 MB) would sweep across 0x08000000 at handoff and overwrite wolfBoot's own code. tools/scripts/zcu102/zcu102-ca53-qspi.cmm: - Rewrite against the Lauterbach TRACE32 ZCU102 QSPI demo: PREPAREONLY entry mode, single/dual toggle, READ_ID_TEST, separate flash dialogs for BOOT.BIN (offset 0) and test-app/image_v1_signed.bin. - Document the ~128 MB TRACE32 temp-memory ceiling on FLASHFILE.Create: larger files must be split externally and loaded in chunks.
1 parent ee943dd commit 8001203

12 files changed

Lines changed: 389 additions & 258 deletions

File tree

config/examples/zynqmp_sdcard.config

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,12 @@ CFLAGS_EXTRA+=-DBOOT_PART_A=1
8484
CFLAGS_EXTRA+=-DBOOT_PART_B=2
8585

8686
# Disk read chunk size for firmware loading (update_disk.c). 512KB gives the
87-
# best throughput (~1.4s for 32MB). The SDMA engine handles boundary crossings
88-
# every 4KB (SDHCI_DMA_THRESHOLD default) within each 512KB chunk.
87+
# best throughput (~1.4s for 32MB). The SDMA engine handles SDMA buffer
88+
# boundary crossings within each 512KB chunk; this boundary is 4KB by default
89+
# (auto-derived from SDHCI_DMA_THRESHOLD). To reduce boundary IRQs, override
90+
# SDHCI_DMA_BUFF_BOUNDARY independently using the raw register value so the
91+
# override is safe to use in preprocessor #if expressions, e.g.:
92+
# CFLAGS_EXTRA+=-DSDHCI_DMA_BUFF_BOUNDARY=0x7000 # 512KB (SDHCI_SRS01_DMA_BUFF_512KB)
8993
CFLAGS_EXTRA+=-DDISK_BLOCK_SIZE=0x80000
9094

9195
# Linux rootfs is on partition 4. Device naming depends on whether both
@@ -99,8 +103,8 @@ CFLAGS_EXTRA+=-DLINUX_BOOTARGS_ROOT=\"/dev/mmcblk0p4\"
99103
# ============================================================================
100104
# Boot Memory Layout
101105
# ============================================================================
102-
# wolfBoot runs from DDR at 0x8000000 (same as U-Boot, loaded via BL31)
103-
WOLFBOOT_ORIGIN=0x8000000
106+
# wolfBoot runs from DDR at 0x10000000 (loaded by BL31; see hal/zynq.ld).
107+
WOLFBOOT_ORIGIN=0x10000000
104108

105109
# Load Partition to RAM Address (Linux kernel loads here)
106110
WOLFBOOT_LOAD_ADDRESS?=0x10000000

docs/Targets.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2599,7 +2599,7 @@ qemu-system-aarch64 -machine xlnx-zcu102 -cpu cortex-a53 -serial stdio -display
25992599

26002600
Use `config/examples/zynqmp_sdcard.config`. This uses the Arasan SDHCI controller (SD1 - external SD card slot on ZCU102) and an **MBR** partitioned SD card.
26012601

2602-
wolfBoot unconditionally flushes the EL2 D-cache/I-cache and disables the EL2 MMU before handoff (see `el2_flush_and_disable_mmu` in `src/boot_aarch64_start.S`), satisfying the ARM64 Linux boot protocol with no extra config flag required.
2602+
On the direct-jump handoff path, wolfBoot flushes the EL2 D-cache/I-cache and disables the EL2 MMU via `el2_flush_and_disable_mmu` in `src/boot_aarch64_start.S` when `BOOT_EL1` is not enabled and the current exception level is EL2. The ERET-to-EL1 handoff path is different, so this cleanup is not unconditional.
26032603

26042604
**Partition layout**
26052605
| Partition | Name | Size | Type | Contents |
@@ -2705,8 +2705,11 @@ The ZynqMP uses an Arasan SDHCI v3.0 controller. Key considerations:
27052705
level. `SDHCI_FORCE_CARD_DETECT` is set in the config since FSBL already booted from
27062706
the same SD card.
27072707
- **`DISK_BLOCK_SIZE`**: Controls the firmware read chunk size in `update_disk.c` (default
2708-
64KB). This determines the per-read size passed to the SDHCI driver. Must be less than
2709-
the SDMA buffer boundary (4KB with the default threshold).
2708+
64KB). This determines the per-read size passed to the SDHCI driver. It does not need
2709+
to be smaller than `SDHCI_DMA_BUFF_BOUNDARY`; if a read crosses one or more SDMA buffer
2710+
boundaries, the SDHCI driver handles that via the normal SDMA boundary interrupt path.
2711+
In practice, this setting is a tradeoff: larger reads may trigger boundary IRQs more
2712+
often, while smaller reads reduce crossings but increase request overhead.
27102713

27112714
**Debug**
27122715

hal/versal.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,11 @@
6666
/* Linux kernel command line arguments */
6767
#ifndef LINUX_BOOTARGS
6868
#ifndef LINUX_BOOTARGS_ROOT
69-
#define LINUX_BOOTARGS_ROOT "/dev/mmcblk0p4"
69+
/* Default Versal SD layout: rootfs on partition 2. Configurations that use
70+
* the 4-partition MBR layout with OFP_A/OFP_B slots
71+
* (boot / OFP_A / OFP_B / rootfs, e.g. config/examples/zynqmp_sdcard.config)
72+
* should override LINUX_BOOTARGS_ROOT to "/dev/mmcblk0p4". */
73+
#define LINUX_BOOTARGS_ROOT "/dev/mmcblk0p2"
7074
#endif
7175

7276
#define LINUX_BOOTARGS \
@@ -1275,10 +1279,11 @@ int hal_dts_fixup(void* dts_addr)
12751279
/* Expand total size to allow adding/modifying properties */
12761280
fdt_set_totalsize(fdt, fdt_totalsize(fdt) + 512);
12771281

1278-
/* Find /chosen node */
1282+
/* Find /chosen node; create it only if genuinely missing. Any other
1283+
* negative return (malformed FDT, etc.) is surfaced directly rather
1284+
* than masked by a follow-on fdt_add_subnode() failure. */
12791285
off = fdt_find_node_offset(fdt, -1, "chosen");
1280-
if (off < 0) {
1281-
/* Create /chosen node if it doesn't exist */
1286+
if (off == -FDT_ERR_NOTFOUND) {
12821287
off = fdt_add_subnode(fdt, 0, "chosen");
12831288
}
12841289

hal/zynq.c

Lines changed: 86 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,6 @@ static int csu_dma_config(int ch, int doSwap)
504504
int csu_aes(int enc, const uint8_t* iv, const uint8_t* in, uint8_t* out, uint32_t sz)
505505
{
506506
int ret;
507-
uint32_t reg;
508507

509508
/* Flush data cache for variables used */
510509
flush_dcache_range((unsigned long)iv, (unsigned long)iv + AES_GCM_TAG_SZ);
@@ -601,7 +600,6 @@ int csu_init(void)
601600
#endif
602601
uint32_t reg1 = pmu_mmio_read(CSU_IDCODE);
603602
uint32_t reg2 = pmu_mmio_read(CSU_VERSION);
604-
uint64_t ms;
605603

606604
wolfBoot_printf("CSU ID 0x%08x, Ver 0x%08x\n",
607605
reg1, reg2 & CSU_VERSION_MASK);
@@ -1361,6 +1359,39 @@ static int qspi_exit_4byte_addr(QspiDev_t* dev)
13611359
}
13621360
#endif
13631361

1362+
/* Soft-reset the flash to a known idle state.
1363+
* FSBL / BootROM may leave the flash in an unexpected mode (XIP enabled,
1364+
* 4-byte addr set, auto-boot probing, etc.). Issue RESET_ENABLE (0x66) +
1365+
* RESET_MEMORY (0x99) to bring it back to defaults before first transaction.
1366+
* Per Micron MT25Q datasheet: t_SHSL2 ~ 40 us max after RESET_MEMORY. */
1367+
static int qspi_flash_reset(QspiDev_t* dev)
1368+
{
1369+
int ret;
1370+
uint8_t cmd[4]; /* size multiple of uint32_t */
1371+
1372+
memset(cmd, 0, sizeof(cmd));
1373+
cmd[0] = RESET_ENABLE_CMD;
1374+
/* Reset commands are always issued in single-SPI mode regardless of
1375+
* dev->mode: the flash's current bus mode is unknown at reset time, and
1376+
* the single-SPI opcode is the universal-compatible form. */
1377+
ret = qspi_transfer(dev, cmd, 1, NULL, 0, NULL, 0, 0,
1378+
GQSPI_GEN_FIFO_MODE_SPI);
1379+
#if defined(DEBUG_ZYNQ) && DEBUG_ZYNQ >= 2
1380+
wolfBoot_printf("Flash Reset Enable: Ret %d\n", ret);
1381+
#endif
1382+
if (ret == GQSPI_CODE_SUCCESS) {
1383+
cmd[0] = RESET_MEMORY_CMD;
1384+
ret = qspi_transfer(dev, cmd, 1, NULL, 0, NULL, 0, 0,
1385+
GQSPI_GEN_FIFO_MODE_SPI);
1386+
#if defined(DEBUG_ZYNQ) && DEBUG_ZYNQ >= 2
1387+
wolfBoot_printf("Flash Reset Memory: Ret %d\n", ret);
1388+
#endif
1389+
}
1390+
/* Allow flash time to complete the reset and become ready. */
1391+
hal_delay_ms(1);
1392+
return ret;
1393+
}
1394+
13641395
/* QSPI functions */
13651396
void qspi_init(void)
13661397
{
@@ -1444,13 +1475,29 @@ void qspi_init(void)
14441475
#if (GQSPI_CLK_REF / (2 << GQSPI_CLK_DIV)) <= 40000000 /* 40MHz */
14451476
/* At <40 MHz, the Quad-SPI controller should be in non-loopback mode with
14461477
* the clock and data tap delays bypassed. */
1447-
IOU_TAPDLY_BYPASS |= IOU_TAPDLY_BYPASS_LQSPI_RX;
1478+
/* IOU_TAPDLY_BYPASS is not writable from EL2/EL1 without going through PMU. */
1479+
if (current_el() <= 2) {
1480+
pmu_request(PM_MMIO_WRITE, IOU_TAPDLY_BYPASS_ADDR,
1481+
IOU_TAPDLY_BYPASS_LQSPI_RX, IOU_TAPDLY_BYPASS_LQSPI_RX,
1482+
0, NULL);
1483+
}
1484+
else {
1485+
IOU_TAPDLY_BYPASS |= IOU_TAPDLY_BYPASS_LQSPI_RX;
1486+
}
14481487
GQSPI_LPBK_DLY_ADJ = 0;
14491488
GQSPI_DATA_DLY_ADJ = 0;
14501489
#elif (GQSPI_CLK_REF / (2 << GQSPI_CLK_DIV)) <= 100000000 /* 100MHz */
14511490
/* At <100 MHz, the Quad-SPI controller should be in clock loopback mode
14521491
* with the clock tap delay bypassed, but the data tap delay enabled. */
1453-
IOU_TAPDLY_BYPASS |= IOU_TAPDLY_BYPASS_LQSPI_RX;
1492+
/* IOU_TAPDLY_BYPASS is not writable from EL2/EL1 without going through PMU. */
1493+
if (current_el() <= 2) {
1494+
pmu_request(PM_MMIO_WRITE, IOU_TAPDLY_BYPASS_ADDR,
1495+
IOU_TAPDLY_BYPASS_LQSPI_RX, IOU_TAPDLY_BYPASS_LQSPI_RX,
1496+
0, NULL);
1497+
}
1498+
else {
1499+
IOU_TAPDLY_BYPASS |= IOU_TAPDLY_BYPASS_LQSPI_RX;
1500+
}
14541501
GQSPI_LPBK_DLY_ADJ = GQSPI_LPBK_DLY_ADJ_USE_LPBK;
14551502
GQSPI_DATA_DLY_ADJ = (GQSPI_DATA_DLY_ADJ_USE_DATA_DLY |
14561503
GQSPI_DATA_DLY_ADJ_DATA_DLY_ADJ(2));
@@ -1485,6 +1532,19 @@ void qspi_init(void)
14851532
(void)reg_cfg;
14861533
(void)reg_isr;
14871534

1535+
/* Issue flash soft reset so we start from a known state regardless of
1536+
* whatever mode FSBL/BootROM left the device in. Send to each chip in
1537+
* dual-parallel configurations by targeting both chip selects. */
1538+
mDev.mode = GQSPI_GEN_FIFO_MODE_SPI;
1539+
mDev.bus = GQSPI_GEN_FIFO_BUS_LOW;
1540+
mDev.cs = GQSPI_GEN_FIFO_CS_LOWER;
1541+
(void)qspi_flash_reset(&mDev);
1542+
#if GQPI_USE_DUAL_PARALLEL == 1
1543+
mDev.bus = GQSPI_GEN_FIFO_BUS_UP;
1544+
mDev.cs = GQSPI_GEN_FIFO_CS_UPPER;
1545+
(void)qspi_flash_reset(&mDev);
1546+
#endif
1547+
14881548
/* ------ Flash Read ID (retry) ------ */
14891549
timeout = 0;
14901550
while (++timeout < QSPI_FLASH_READY_TRIES) {
@@ -1577,6 +1637,10 @@ void hal_init(void)
15771637
wolfBoot_printf(bootMsg);
15781638
wolfBoot_printf("Current EL: %d\n", current_el());
15791639

1640+
#ifndef WOLFBOOT_REPRODUCIBLE_BUILD
1641+
wolfBoot_printf("Build: %s %s\n", __DATE__, __TIME__);
1642+
#endif
1643+
15801644
#if defined(EXT_FLASH) && (EXT_FLASH == 1)
15811645
qspi_init();
15821646
#endif
@@ -1809,15 +1873,28 @@ void RAMFUNCTION ext_flash_unlock(void)
18091873

18101874
}
18111875

1876+
/* The following helpers (hal_get_timer_us, hal_get_dts_address, hal_dts_fixup)
1877+
* are only compiled into the wolfBoot binary. The test-app build also links
1878+
* hal/zynq.o but must not pull in FDT/MMU-specific code, so __WOLFBOOT gates
1879+
* these symbols out of that build. */
18121880
#if defined(MMU) && defined(__WOLFBOOT)
1881+
/* Fallback timer frequency if CNTFRQ_EL0 is not configured (e.g. boot path
1882+
* that did not run ATF/BL31). ZynqMP system counter is 100 MHz. */
1883+
#ifndef ZYNQMP_TIMER_CLK_FREQ
1884+
#define ZYNQMP_TIMER_CLK_FREQ 100000000ULL
1885+
#endif
1886+
18131887
/* Get current time in microseconds using ARMv8 generic timer */
18141888
uint64_t hal_get_timer_us(void)
18151889
{
18161890
uint64_t count, freq;
18171891
__asm__ volatile("mrs %0, CNTPCT_EL0" : "=r"(count));
18181892
__asm__ volatile("mrs %0, CNTFRQ_EL0" : "=r"(freq));
1893+
/* Fall back to a known frequency rather than returning 0, so udelay()
1894+
* callers that spin on hal_get_timer_us() advancing remain monotonic
1895+
* (matches hal/versal.c). */
18191896
if (freq == 0)
1820-
return 0;
1897+
freq = ZYNQMP_TIMER_CLK_FREQ;
18211898
/* Use __uint128_t to avoid overflow of (count * 1e6) at long uptimes
18221899
* (would overflow uint64_t after ~51h at 100MHz). */
18231900
return (uint64_t)(((__uint128_t)count * 1000000ULL) / freq);
@@ -1856,10 +1933,11 @@ int hal_dts_fixup(void* dts_addr)
18561933
* the pattern used in hal/versal.c:hal_dts_fixup. */
18571934
fdt_set_totalsize(fdt, fdt_totalsize(fdt) + 512);
18581935

1859-
/* Find /chosen node */
1936+
/* Find /chosen node; create it only if genuinely missing. Any other
1937+
* negative return (malformed FDT, etc.) is surfaced directly rather
1938+
* than masked by a follow-on fdt_add_subnode() failure. */
18601939
off = fdt_find_node_offset(fdt, -1, "chosen");
1861-
if (off < 0) {
1862-
/* Create /chosen node if it doesn't exist */
1940+
if (off == -FDT_ERR_NOTFOUND) {
18631941
off = fdt_add_subnode(fdt, 0, "chosen");
18641942
}
18651943
if (off < 0) {

hal/zynq.ld

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,21 @@ MEMORY
1313
{
1414
/* psu_ddr_0_MEM_0 : ORIGIN = 0x0, LENGTH = 0x80000000 */
1515
/* wolfBoot DDR location (2MB reserved):
16-
* Loaded by FSBL/BL31 to DDR at 0x8000000 (128MB)
17-
* Same address used for both QSPI and SD card boot
16+
* Loaded by FSBL/BL31 to DDR at 0x10000000 (256MB). Must be above
17+
* the OS kernel load region: for FIT images whose "load" address is
18+
* 0x00200000 (typical for AArch64 kernels) and whose payload
19+
* approaches or exceeds ~126MB, the kernel memcpy into 0x00200000..
20+
* sweeps across 0x08000000 and would clobber wolfBoot if linked
21+
* there. Same address used for QSPI and SD card boot.
22+
*
23+
* WOLFBOOT_LOAD_ADDRESS (signed-image staging base) also defaults to
24+
* 0x10000000 on the SD-card config. The overlap with wolfBoot's own
25+
* 2MB text region is safe because wolfBoot completes the verify and
26+
* jump sequence from I-cache; the I-cache is only invalidated inside
27+
* el2_flush_and_disable_mmu, immediately before control transfers to
28+
* the loaded kernel (which no longer needs wolfBoot's .text).
1829
*/
19-
psu_ddr_0_MEM_0 : ORIGIN = 0x8000000, LENGTH = 0x200000
30+
psu_ddr_0_MEM_0 : ORIGIN = 0x10000000, LENGTH = 0x200000
2031
psu_ddr_1_MEM_0 : ORIGIN = 0x800000000, LENGTH = 0x80000000
2132
psu_ocm_ram_0_MEM_0 : ORIGIN = 0xFFFC0000, LENGTH = 0x40000
2233
psu_qspi_linear_0_MEM_0 : ORIGIN = 0xC0000000, LENGTH = 0x20000000

include/sdhci.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,13 @@
5757
#define DISK_TEST_BLOCK_ADDR 149504 /* ~76MB offset */
5858
#endif
5959

60-
/* Auto-select DMA buffer boundary based on threshold */
60+
/* DMA buffer boundary: how often the SDMA engine pauses to refresh its
61+
* address pointer (handled by sdhci_irq_handler() via SDHCI_SRS12_DMAINT).
62+
* This is a throughput knob and is independent of SDHCI_DMA_THRESHOLD
63+
* (which controls when to switch from PIO to SDMA). Override in target
64+
* .config to match the largest expected single transfer for fewer
65+
* boundary IRQs; otherwise auto-select based on the threshold. */
66+
#ifndef SDHCI_DMA_BUFF_BOUNDARY
6167
#if (SDHCI_DMA_THRESHOLD > (256U * 1024U))
6268
#define SDHCI_DMA_BUFF_BOUNDARY SDHCI_SRS01_DMA_BUFF_512KB
6369
#if (SDHCI_DMA_THRESHOLD != (512U * 1024U))
@@ -99,6 +105,7 @@
99105
#warning "SDHCI_DMA_THRESHOLD rounded up to 4KB (minimum)"
100106
#endif
101107
#endif
108+
#endif /* !SDHCI_DMA_BUFF_BOUNDARY */
102109

103110
/* Timeouts */
104111
#ifndef SDHCI_INIT_TIMEOUT_US

src/boot_aarch64.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
#include "hal/versal.h"
3535
#elif defined(TARGET_zynq)
3636
#include "hal/zynq.h"
37-
#elif defined(TARGET_ls1028a)
37+
#elif defined(TARGET_nxp_ls1028a)
3838
#include "hal/nxp_ls1028a.h"
3939
#endif
4040

src/boot_aarch64_start.S

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1362,7 +1362,7 @@ el2_to_el1_boot:
13621362
* at EL2, so instruction fetch keeps working after SCTLR_EL2.M is
13631363
* cleared.
13641364
*
1365-
* AAPCS64: clobbers x0-x11; x30 (LR) is preserved because the
1365+
* AAPCS64: clobbers x0-x7, x9-x11; x30 (LR) is preserved because the
13661366
* set/way loop body does not touch it.
13671367
*/
13681368
#if defined(EL2_HYPERVISOR) && EL2_HYPERVISOR == 1

tools/scripts/nxp_t1040/t1040_debug.cmm

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@
1818
; 3. wolfBoot runs from DDR (0x7FF00000)
1919
; ------------------------------------------------------------------------------
2020

21-
; Base directory for wolfBoot build output (adjust to match your build path)
22-
&basedir="/home/davidgarske/GitHub/wolfboot-alt"
21+
; Base directory for wolfBoot build output. "." means the TRACE32 current
22+
; working directory - set this to your wolfBoot checkout path if running
23+
; TRACE32 from elsewhere (e.g. "C:/src/wolfBoot" or "/home/user/wolfBoot").
24+
&basedir="."
2325

2426
PRINT "========================================"
2527
PRINT "T1040 wolfBoot Debug Session"

tools/scripts/nxp_t1040/t1040_flash.cmm

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,15 @@
2525
; 0xEFFFC000: Stage 1 loader (16 KB, includes reset vector)
2626
; ------------------------------------------------------------------------------
2727

28-
; Base directory for wolfBoot build output (adjust to match your build path)
29-
&basedir="/home/davidgarske/GitHub/wolfboot-alt"
30-
31-
; Persistent backup directory (survives make clean)
32-
&backupdir="/home/davidgarske/Projects/NXP/t1040rdb"
28+
; Base directory for wolfBoot build output. "." means the TRACE32 current
29+
; working directory - set this to your wolfBoot checkout path if running
30+
; TRACE32 from elsewhere (e.g. "C:/src/wolfBoot" or "/home/user/wolfBoot").
31+
&basedir="."
32+
33+
; Persistent backup directory (survives make clean). Leave as "." to keep
34+
; artifacts alongside the build tree, or point elsewhere to preserve
35+
; signed/backup images across source cleans.
36+
&backupdir="."
3337

3438
; FLASH Number of banks
3539
; The JS28F00AM29EWHA is a single 128MB NOR chip. CPLD virtual banking

0 commit comments

Comments
 (0)