Peer review fixes

Author: dgarske

.github/workflows/test-configs.yml

@@ -654,12 +654,6 @@ jobs:
       arch: arm
       config-file: ./config/examples/zynq7000.config
 
-  zynq7000_linux_test:
-    uses: ./.github/workflows/test-build.yml
-    with:
-      arch: arm
-      config-file: ./config/examples/zynq7000_linux.config
-
   zc702_sdcard_test:
     uses: ./.github/workflows/test-build.yml
     with:

arch.mk

@@ -310,10 +310,19 @@ ifeq ($(ARCH),ARM)
      UPDATE_OBJS:=src/update_ram.o
      CFLAGS+=-DWOLFBOOT_DUALBOOT -fno-builtin -ffreestanding
      # Do NOT define WOLFBOOT_USE_STDLIBC: newlib's memcpy uses unaligned
-     # LDRs which fault on Cortex-A9 when MMU is off (FSBL leaves MMU off
-     # on Zynq-7000). Use wolfBoot's own aligned-safe memcpy from src/string.c.
-     # U-Boot legacy header detection for Linux/U-Boot payloads (Milestone 5)
-     CFLAGS+=-DWOLFBOOT_UBOOT_LEGACY
+     # LDRs which fault on ARMv7-A whenever the active mapping treats memory
+     # as Strongly-Ordered (typically MMU off, but also some boot-stage
+     # configurations). wolfBoot's startup keeps FSBL's MMU + flat 1:1
+     # mapping enabled to avoid that, but we still link against the
+     # aligned-safe memcpy in src/string.c so unaligned loads can never
+     # surprise us regardless of MMU state.
+     # U-Boot legacy 64-byte header strip is only meaningful for the
+     # Linux/U-Boot payload variant (zynq7000_linux.config). Bare-metal
+     # payloads (zynq7000.config, zc702_sdcard.config) shouldn't risk a
+     # ~1-in-2^32 false-positive collision with UBOOT_IMG_HDR_MAGIC.
+     ifeq ($(LINUX_PAYLOAD),1)
+       CFLAGS+=-DWOLFBOOT_UBOOT_LEGACY
+     endif
   endif
 
   ifeq ($(TARGET),va416x0)

config/examples/zc702_sdcard.config

@@ -20,12 +20,15 @@ NO_XIP=1
 # Stage payload at low DDR (clear of wolfBoot at 0x04000000-0x040FFFFF).
 WOLFBOOT_LOAD_ADDRESS=0x10000000
 
-# GPT/MBR partition layout on the SD card.
-#   GPT partition 0 (idx 0): FAT32 - holds BOOT.BIN for the BootROM.
-#   GPT partition 1 (idx 1): raw   - signed boot image (BOOT_PART_A).
-#   GPT partition 2 (idx 2): raw   - signed update image (BOOT_PART_B).
+# MBR partition layout on the SD card. Pure MBR (no GPT) - the Zynq-7000
+# BootROM (UG821 ch.6.3) only accepts MBR with the first partition as
+# FAT32 and the Active flag set. wolfBoot's src/disk.c falls back to MBR
+# parsing when no protective-GPT entry is present.
+#   MBR p1 (wolfBoot idx 0): FAT32-LBA Active - holds BOOT.BIN for BootROM.
+#   MBR p2 (wolfBoot idx 1): Linux raw (0x83) - signed boot image.
+#   MBR p3 (wolfBoot idx 2): Linux raw (0x83) - signed update image.
 # tools/scripts/zc702/prepare_sdcard.sh lays this out; BOOT_PART_A/B tell
-# update_disk.c which GPT entries to use for boot/update.
+# update_disk.c which MBR entries (0-indexed) to use for boot/update.
 CFLAGS_EXTRA+=-DBOOT_PART_A=1 -DBOOT_PART_B=2
 
 # Arasan SDHCI v2.0 on Zynq-7000 is 3.3V-only, no UHS-I. The generic

config/examples/zynq7000.config

@@ -3,40 +3,63 @@ TARGET?=zynq7000
 SIGN?=ECC256
 HASH?=SHA256
 
-# Cortex-A9 (Zynq-7000) - selected automatically via TARGET=zynq7000 in arch.mk
+# Cortex-A9 (Zynq-7000) - selected automatically via TARGET=zynq7000 in arch.mk.
+# This single config supports bare-metal, U-Boot, and Linux payloads from QSPI.
 DEBUG?=0
 DEBUG_UART?=1
 V?=0
 SPMATH?=1
 
-# wolfBoot itself is loaded by Xilinx FSBL to DDR at 0x04000000 (hal/zynq7000.ld).
-# WOLFBOOT_LOAD_ADDRESS is the *app* staging address: where wolfBoot copies
-# the verified signed image before do_boot. Must NOT overlap wolfBoot itself
-# AND src/update_ram.c expects dst > wolfBoot's _end - so place it above the
-# wolfBoot region (0x04000000-0x040FFFFF) at 16 MB.
+# Linux/U-Boot payload support (no-op for plain bare-metal payloads):
+#   LINUX_PAYLOAD=1 -> do_boot uses ARM Linux boot ABI (r0=0, r1=~0,
+#                      r2=DTB_phys, r3=0). Bare-metal apps don't read these
+#                      registers, so the same ABI is fine for them.
+#   MMU=1           -> enables update_ram.c DTB-load codepath and pulls in
+#                      src/fdt.o. wolfBoot itself does NOT manage page
+#                      tables; it inherits FSBL's flat 1:1 DDR mapping.
+#   ELF=1           -> wolfBoot understands ELF inputs (vmlinux / u-boot.elf)
+#                      and loads only their LOAD segments. Flat binaries
+#                      (zImage, bare-metal .bin) fall through to raw-binary
+#                      boot.
+# Cost vs. a strictly bare-metal-only build: ~5 KB extra wolfBoot binary
+# from the FDT/MMU/ELF support, in exchange for one config that covers all
+# three payload types.
+LINUX_PAYLOAD=1
+MMU=1
+ELF=1
+
+# wolfBoot itself is staged by FSBL to DDR at 0x04000000 (hal/zynq7000.ld);
+# the verified payload (kernel/U-Boot/bare-metal app) is staged at
+# WOLFBOOT_LOAD_ADDRESS, well clear of wolfBoot. 1 GB DDR3 on ZC702
+# starts at 0x00000000.
 WOLFBOOT_LOAD_ADDRESS=0x10000000
 
+# DTB load address (Linux only). Kernel reads it from r2. 16 MB clear of
+# WOLFBOOT_LOAD_ADDRESS. Ignored for bare-metal payloads.
+WOLFBOOT_LOAD_DTS_ADDRESS=0x11000000
+
 # QSPI flash (16 MB N25Q128A on ZC702) via XQspiPs (hal/zynq7000.c).
 # Override EXT_FLASH=0 on the make command line for JTAG-only dev builds.
 EXT_FLASH?=1
 NO_XIP=1
 
-# QSPI partition layout (16 MB total):
-#   0x000000 - 0x0FFFFF  BOOT.BIN (FSBL + wolfboot)
-#   0x100000 - 0x6FFFFF  BOOT_A   (~6 MB primary)
-#   0x700000 - 0xCFFFFF  UPDATE_B (~6 MB update)
-#   0xD00000 - 0xD0FFFF  SWAP scratch (64 KB sector)
+# QSPI partition layout (16 MB total) - sized for a full Linux kernel + DTB
+# pair so the same layout also works for bare-metal payloads.
+#   0x000000 - 0x07FFFF  BOOT.BIN  (FSBL + wolfboot, 512 KB)
+#   0x080000 - 0x0FFFFF  DTS_BOOT  (signed DTB, 512 KB - Linux only)
+#   0x100000 - 0x6FFFFF  BOOT_A    (~6 MB primary)
+#   0x700000 - 0x77FFFF  DTS_UPD   (signed update DTB, 512 KB - Linux only)
+#   0x780000 - 0xDFFFFF  UPDATE_B  (~6.5 MB update)
+#   0xE00000 - 0xE0FFFF  SWAP      (64 KB scratch)
 WOLFBOOT_PARTITION_BOOT_ADDRESS=0x00100000
-WOLFBOOT_PARTITION_UPDATE_ADDRESS=0x00700000
-WOLFBOOT_PARTITION_SWAP_ADDRESS=0x00D00000
+WOLFBOOT_PARTITION_UPDATE_ADDRESS=0x00780000
+WOLFBOOT_PARTITION_SWAP_ADDRESS=0x00E00000
 WOLFBOOT_PARTITION_SIZE=0x00600000
 WOLFBOOT_SECTOR_SIZE=0x10000
 
-# DTS placeholders (used in Milestone 5 for Linux payload)
-WOLFBOOT_LOAD_DTS_ADDRESS=0x00100000
 WOLFBOOT_DTS_BOOT_ADDRESS=0x00080000
-WOLFBOOT_DTS_UPDATE_ADDRESS=0x00680000
+WOLFBOOT_DTS_UPDATE_ADDRESS=0x00700000
 
 IMAGE_HEADER_SIZE=1024
 
-CROSS_COMPILE=arm-none-eabi-
+CROSS_COMPILE?=arm-none-eabi-

config/examples/zynq7000_linux.config

@@ -3404,9 +3404,9 @@ BootROM -> FSBL -> wolfBoot -> signed app (or U-Boot/Linux)
 The FSBL handles all PS init (DDR, MIO, clocks, QSPI ref clock); wolfBoot only initializes UART, the QSPI controller, runs the verify/swap logic, and chain-loads the next stage.
 
 This target supports:
-- **QSPI boot** (primary): `config/examples/zynq7000.config`
-- **SD card boot** (Milestone 6, planned): `config/examples/zc702_sdcard.config`
-- **JTAG-loaded dev** via Platform Cable II + xsdb (no flash required)
+- **QSPI boot** (primary): `config/examples/zynq7000.config` -- one config for bare-metal, U-Boot, and Linux payloads (LINUX_PAYLOAD=1 + MMU=1 + ELF=1; bare-metal apps don't pay any runtime cost beyond ~5 KB of unused FDT/MMU code).
+- **SD card boot**: `config/examples/zc702_sdcard.config` -- bare-metal payload from MBR-partitioned SD via the generic SDHCI driver and the Arasan v2.0 translation in `hal/zynq7000.c`.
+- **JTAG-loaded dev** via Platform Cable II + xsdb (no flash required).
 
 ### Prerequisites
 
@@ -3438,8 +3438,10 @@ Key options in `config/examples/zynq7000.config`:
 - `ARCH=ARM` - 32-bit ARM
 - `TARGET=zynq7000` - selects `hal/zynq7000.{c,h,ld}` and the `CORTEX_A9` arch.mk block
 - `SIGN=ECC256` / `HASH=SHA256` - smaller and faster than RSA on Cortex-A9
+- `LINUX_PAYLOAD=1 MMU=1 ELF=1` - lets the same image boot Linux/U-Boot or bare-metal. `do_boot` switches to the ARM Linux boot ABI (`r0=0`, `r1=~0`, `r2=DTB_phys`, `r3=0`) which bare-metal apps simply ignore. `MMU=1` enables `update_ram.c`'s DTB-load codepath and pulls in `src/fdt.o`; wolfBoot does not manage page tables (it inherits FSBL's flat 1:1 DDR mapping). `ELF=1` lets wolfBoot understand `vmlinux` / `u-boot.elf` inputs and load only their LOAD segments. Cost over a strictly bare-metal-only build: ~5 KB extra wolfBoot binary (31 KB -> 36 KB).
 - `EXT_FLASH=1` - QSPI as external flash via `XQspiPs`
 - `WOLFBOOT_LOAD_ADDRESS=0x10000000` - DDR offset 256 MB, where the verified app is staged before `do_boot`. Must be **above** wolfBoot's own region (`0x04000000`-`0x040FFFFF`) because `src/update_ram.c` enforces `dst > _end`.
+- `WOLFBOOT_LOAD_DTS_ADDRESS=0x11000000` - DDR offset 272 MB, where a DTB read out of `PART_DTS_BOOT` would be relocated. Ignored for bare-metal payloads and for the appended-DTB Linux flow (where the DTB lives at the end of the signed kernel image).
 - `WOLFBOOT_PARTITION_BOOT_ADDRESS=0x00100000` - 16 MB QSPI layout below
 - `CROSS_COMPILE=arm-none-eabi-`
 
@@ -3483,11 +3485,31 @@ bootgen -arch zynq -image tools/scripts/zc702/zc702_qspi.bif -w -o BOOT.BIN
 
 ### Programming QSPI
 
-Set ZC702 boot mode straps to **JTAG** (SW16 all OFF) for programming, then either:
-- Vitis: `program_flash -f BOOT.BIN -flash_type qspi-x4-single -fsbl ${PREBUILT_DIR}/zynq_fsbl.elf -target_id <id>` (use `program_flash -jtagtargets` to get the `arm_dap` target ID for your Platform Cable; only needed when more than one cable is connected)
-- Vivado Hardware Manager: Tools -> Add Configuration Memory Device -> select N25Q128 -> program with BOOT.BIN at offset 0.
+The 16 MB QSPI flash holds two artifacts: `BOOT.BIN` at offset `0x0` (FSBL + wolfBoot, < 1 MB) and the signed payload at `WOLFBOOT_PARTITION_BOOT_ADDRESS` (default `0x100000` for the BOOT_A partition). Both go through `program_flash` over JTAG.
 
-After programming, set boot mode to **QSPI** by turning **SW16-4 ON** (the four-position dip mapping is `SW16-4 = MIO[5]` MSB of the boot device strap, with SW16-1..3 = MIO[2..4]; per UG850 ch.1.2.4). Power-cycle the board (cold) so the BootROM re-samples the strap. Console comes up on UART1 (J17 USB-UART), 115200 8N1.
+Set ZC702 boot mode straps to **JTAG** (SW16 all OFF) for programming. Then run two commands:
+
+```sh
+# 1. List JTAG targets and note the arm_dap target ID for the Xilinx Platform
+# Cable USB II (skip this step if you only have one JTAG cable connected).
+program_flash -jtagtargets -url TCP:127.0.0.1:3121
+
+# 2. Program BOOT.BIN at offset 0
+program_flash -f BOOT.BIN -offset 0 -flash_type qspi-x4-single \
+    -fsbl ${PREBUILT_DIR}/zynq_fsbl.elf \
+    -target_id <arm_dap_id> -url TCP:127.0.0.1:3121
+
+# 3. Program the signed payload at the BOOT_A partition offset
+program_flash -f test-app/image_v1_signed.bin -offset 0x100000 \
+    -flash_type qspi-x4-single -fsbl ${PREBUILT_DIR}/zynq_fsbl.elf \
+    -target_id <arm_dap_id> -url TCP:127.0.0.1:3121
+```
+
+`program_flash` ships with Vitis. The Vivado Hardware Manager UI works equivalently (Tools -> Add Configuration Memory Device -> select N25Q128 -> program two files at offsets 0 and 0x100000).
+
+After programming, set boot mode to **QSPI** by turning **SW16-4 ON** (the four-position dip mapping is `SW16-4 = MIO[5]` MSB of the boot device strap, with SW16-1..3 = MIO[2..4]; per UG850 ch.1.2.4). Power-cycle the board (cold) so the BootROM re-samples the strap. Console comes up on UART1 (J17 USB-UART), 115200 8N1, and you should see the wolfBoot banner followed by `=== ZC702 test-app: BOOT OK ===` (or, with `LINUX_PAYLOAD=1` and a signed kernel, the Linux boot log).
+
+`program_flash` may print a segmentation fault on exit ("Flash Operation Successful" precedes it) -- that's a Vitis tool quirk on cleanup, not a programming failure; the flash content is correct.
 
 ### JTAG-loaded development (no flash)
 

docs/Targets.md

@@ -251,11 +251,14 @@ static void qspi_linear_mode_setup(void)
     Z7_QSPI_ISR = Z7_QSPI_ISR_MASK;
 
     /* CR: IFMODE=1, FIFO=32-bit, MSTREN=1, SSFORCE=1, HOLD_B=1, /4 baud,
-     * MANSTRTEN=0 (auto-start), CPHA/CPOL=0, PCS bit 10 cleared (CS0
-     * asserted - in linear mode the controller still wants this). */
+     * MANSTRTEN=0 (auto-start), CPHA/CPOL=0, PCS=CS0 asserted explicitly
+     * (the linear-mode controller still drives PCS even with SSFORCE; we
+     * set the field deterministically rather than relying on the reset
+     * default). */
     Z7_QSPI_CR  = Z7_QSPI_CR_IFMODE
                 | Z7_QSPI_CR_HOLD_B
                 | Z7_QSPI_CR_SSFORCE
+                | Z7_QSPI_CR_PCS_CS0
                 | Z7_QSPI_CR_FIFO_WIDTH
                 | Z7_QSPI_CR_BAUD_DIV_4
                 | Z7_QSPI_CR_MSTREN;
@@ -461,18 +464,12 @@ static void qspi_print_hex32(uint32_t v)
 
 static void qspi_selftest(void)
 {
-    static const uint8_t pattern[SPI_NOR_PAGE_SIZE] = {
-        /* zero-initialized, filled in below */
-        0
-    };
+    uint8_t  patbuf[SPI_NOR_PAGE_SIZE];
     uint8_t  rdback[SPI_NOR_PAGE_SIZE];
     uint8_t  id[3] = { 0, 0, 0 };
     unsigned int i;
     int rc;
-    /* Local mutable pattern buffer */
-    uint8_t patbuf[SPI_NOR_PAGE_SIZE];
 
-    (void)pattern;
     for (i = 0; i < SPI_NOR_PAGE_SIZE; i++)
         patbuf[i] = (uint8_t)(i & 0xFFU);
 
@@ -600,7 +597,29 @@ void hal_init(void)
         const char banner[] = "wolfBoot Zynq-7000 (ZC702) hal_init\n";
         uart_write(banner, sizeof(banner) - 1);
     }
+    /* One-line boot config summary so the UART log identifies which
+     * config / storage / payload / clock plan the running wolfBoot was
+     * built for. Useful when juggling QSPI vs SD images. */
+#if defined(DISK_SDCARD) || defined(DISK_EMMC)
+    wolfBoot_printf("  storage : SDCard (Arasan SDHCI v2.0, %d MHz post-init)\n",
+        SDHCI_CLK_50MHZ / 1000);
+#elif defined(EXT_FLASH)
+    wolfBoot_printf("  storage : QSPI (XQspiPs N25Q128, IO_PLL/40 ~12.5 MHz)\n");
+#else
+    wolfBoot_printf("  storage : NONE (JTAG-loaded)\n");
+#endif
+#if defined(WOLFBOOT_LINUX_PAYLOAD)
+    wolfBoot_printf("  payload : ARM Linux ABI (r0=0, r1=~0, r2=DTB)\n");
+#else
+    wolfBoot_printf("  payload : bare-metal (r0=DTB, r1=r2=r3=0)\n");
 #endif
+    wolfBoot_printf("  partns  : load=0x%x boot=0x%x update=0x%x\n",
+        (unsigned int)WOLFBOOT_LOAD_ADDRESS,
+        (unsigned int)WOLFBOOT_PARTITION_BOOT_ADDRESS,
+        (unsigned int)WOLFBOOT_PARTITION_UPDATE_ADDRESS);
+    wolfBoot_printf("  cputmr  : Cortex-A9 GTimer @ %u Hz\n",
+        (unsigned int)Z7_GTIMER_FREQ_HZ);
+#endif /* DEBUG_UART */
 #ifdef EXT_FLASH
     qspi_init();
 #if defined(TEST_EXT_FLASH) || defined(TEST_QSPI)
@@ -814,10 +833,12 @@ void *hal_get_dts_update_address(void)
 #endif /* MMU */
 
 /* Microsecond timer using the Cortex-A9 Global Timer at PERIPHBASE+0x200.
- * 64-bit free-running counter, increments at PERIPHCLK (~166.67 MHz on
- * ZC702 with the default FSBL clock plan: CPU_6x4x = 666.67 MHz). The
- * Global Timer is started by the FSBL; if it isn't running yet we kick
- * it here. */
+ * 64-bit free-running counter, increments at PERIPHCLK = CPU_3x2x =
+ * 333.33 MHz on ZC702 with the default FSBL clock plan (ARM_PLL =
+ * 1.333 GHz, CPU_6x4x = 666.67 MHz). The actual divide constant lives
+ * in Z7_GTIMER_FREQ_HZ in hal/zynq7000.h - override there if you reclock
+ * the CPU. The Global Timer is started by the FSBL; if it isn't running
+ * yet we kick it here. */
 uint64_t hal_get_timer_us(void)
 {
     uint32_t hi1, lo, hi2;
@@ -1075,11 +1096,18 @@ void sdhci_platform_dma_prepare(void *buf, uint32_t sz, int is_write)
 void sdhci_platform_dma_complete(void *buf, uint32_t sz, int is_write)
 {
     if (!is_write) {
+        /* Post-DMA-read: invalidate-only (DCIMVAC, c7,c6,1). The DMA
+         * controller has just written fresh data into DRAM; we must
+         * discard any stale CPU lines (including ones the prefetcher
+         * may have pulled in between dma_prepare and dma_complete) so
+         * subsequent CPU reads see the new data. clean+invalidate
+         * (DCCIMVAC) would write back the stale lines first, partially
+         * overwriting the controller's data. */
         uintptr_t start = (uintptr_t)buf & ~31U;
         uintptr_t end   = ((uintptr_t)buf + sz + 31U) & ~31U;
         uintptr_t addr;
         for (addr = start; addr < end; addr += 32U) {
-            __asm__ volatile("mcr p15, 0, %0, c7, c14, 1" : : "r"(addr) : "memory");
+            __asm__ volatile("mcr p15, 0, %0, c7, c6, 1" : : "r"(addr) : "memory");
         }
         __asm__ volatile("dsb sy" : : : "memory");
     }