Peer review fixes

Author: dgarske

config/examples/zynqmp_sdcard.config

@@ -84,8 +84,11 @@ CFLAGS_EXTRA+=-DBOOT_PART_A=1
 CFLAGS_EXTRA+=-DBOOT_PART_B=2
 
 # Disk read chunk size for firmware loading (update_disk.c). 512KB gives the
-# best throughput (~1.4s for 32MB). The SDMA engine handles boundary crossings
-# every 4KB (SDHCI_DMA_THRESHOLD default) within each 512KB chunk.
+# best throughput (~1.4s for 32MB). The SDMA engine handles SDMA buffer
+# boundary crossings within each 512KB chunk; this boundary is 4KB by default
+# (auto-derived from SDHCI_DMA_THRESHOLD). To reduce boundary IRQs, override
+# SDHCI_DMA_BUFF_BOUNDARY independently, e.g.:
+#   CFLAGS_EXTRA+=-DSDHCI_DMA_BUFF_BOUNDARY=SDHCI_SRS01_DMA_BUFF_512KB
 CFLAGS_EXTRA+=-DDISK_BLOCK_SIZE=0x80000
 
 # Linux rootfs is on partition 4. Device naming depends on whether both

docs/Targets.md

@@ -2599,7 +2599,7 @@ qemu-system-aarch64 -machine xlnx-zcu102 -cpu cortex-a53 -serial stdio -display
 
 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.
 
-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.
+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.
 
 **Partition layout**
 | Partition | Name   | Size      | Type                          | Contents                                   |
@@ -2705,8 +2705,11 @@ The ZynqMP uses an Arasan SDHCI v3.0 controller. Key considerations:
   level. `SDHCI_FORCE_CARD_DETECT` is set in the config since FSBL already booted from
   the same SD card.
 - **`DISK_BLOCK_SIZE`**: Controls the firmware read chunk size in `update_disk.c` (default
-  64KB). This determines the per-read size passed to the SDHCI driver. Must be less than
-  the SDMA buffer boundary (4KB with the default threshold).
+  64KB). This determines the per-read size passed to the SDHCI driver. It does not need
+  to be smaller than `SDHCI_DMA_BUFF_BOUNDARY`; if a read crosses one or more SDMA buffer
+  boundaries, the SDHCI driver handles that via the normal SDMA boundary interrupt path.
+  In practice, this setting is a tradeoff: larger reads may trigger boundary IRQs more
+  often, while smaller reads reduce crossings but increase request overhead.
 
 **Debug**
 

hal/zynq.c

@@ -504,7 +504,6 @@ static int csu_dma_config(int ch, int doSwap)
 int csu_aes(int enc, const uint8_t* iv, const uint8_t* in, uint8_t* out, uint32_t sz)
 {
     int ret;
-    uint32_t reg;
 
     /* Flush data cache for variables used */
     flush_dcache_range((unsigned long)iv,  (unsigned long)iv + AES_GCM_TAG_SZ);
@@ -601,7 +600,6 @@ int csu_init(void)
 #endif
     uint32_t reg1 = pmu_mmio_read(CSU_IDCODE);
     uint32_t reg2 = pmu_mmio_read(CSU_VERSION);
-    uint64_t ms;
 
     wolfBoot_printf("CSU ID 0x%08x, Ver 0x%08x\n",
         reg1, reg2 & CSU_VERSION_MASK);
@@ -1444,13 +1442,29 @@ void qspi_init(void)
 #if (GQSPI_CLK_REF / (2 << GQSPI_CLK_DIV)) <= 40000000 /* 40MHz */
     /* At <40 MHz, the Quad-SPI controller should be in non-loopback mode with
      * the clock and data tap delays bypassed. */
-    IOU_TAPDLY_BYPASS |= IOU_TAPDLY_BYPASS_LQSPI_RX;
+    /* IOU_TAPDLY_BYPASS is not writable from EL2/EL1 without going through PMU. */
+    if (current_el() <= 2) {
+        pmu_request(PM_MMIO_WRITE, IOU_TAPDLY_BYPASS_ADDR,
+                    IOU_TAPDLY_BYPASS_LQSPI_RX, IOU_TAPDLY_BYPASS_LQSPI_RX,
+                    0, NULL);
+    }
+    else {
+        IOU_TAPDLY_BYPASS |= IOU_TAPDLY_BYPASS_LQSPI_RX;
+    }
     GQSPI_LPBK_DLY_ADJ = 0;
     GQSPI_DATA_DLY_ADJ = 0;
 #elif (GQSPI_CLK_REF / (2 << GQSPI_CLK_DIV)) <= 100000000 /* 100MHz */
     /* At <100 MHz, the Quad-SPI controller should be in clock loopback mode
      * with the clock tap delay bypassed, but the data tap delay enabled. */
-    IOU_TAPDLY_BYPASS |= IOU_TAPDLY_BYPASS_LQSPI_RX;
+    /* IOU_TAPDLY_BYPASS is not writable from EL2/EL1 without going through PMU. */
+    if (current_el() <= 2) {
+        pmu_request(PM_MMIO_WRITE, IOU_TAPDLY_BYPASS_ADDR,
+                    IOU_TAPDLY_BYPASS_LQSPI_RX, IOU_TAPDLY_BYPASS_LQSPI_RX,
+                    0, NULL);
+    }
+    else {
+        IOU_TAPDLY_BYPASS |= IOU_TAPDLY_BYPASS_LQSPI_RX;
+    }
     GQSPI_LPBK_DLY_ADJ = GQSPI_LPBK_DLY_ADJ_USE_LPBK;
     GQSPI_DATA_DLY_ADJ = (GQSPI_DATA_DLY_ADJ_USE_DATA_DLY |
                           GQSPI_DATA_DLY_ADJ_DATA_DLY_ADJ(2));
@@ -1810,14 +1824,23 @@ void RAMFUNCTION ext_flash_unlock(void)
 }
 
 #if defined(MMU) && defined(__WOLFBOOT)
+/* Fallback timer frequency if CNTFRQ_EL0 is not configured (e.g. boot path
+ * that did not run ATF/BL31). ZynqMP system counter is 100 MHz. */
+#ifndef ZYNQMP_TIMER_CLK_FREQ
+#define ZYNQMP_TIMER_CLK_FREQ 100000000ULL
+#endif
+
 /* Get current time in microseconds using ARMv8 generic timer */
 uint64_t hal_get_timer_us(void)
 {
     uint64_t count, freq;
     __asm__ volatile("mrs %0, CNTPCT_EL0" : "=r"(count));
     __asm__ volatile("mrs %0, CNTFRQ_EL0" : "=r"(freq));
+    /* Fall back to a known frequency rather than returning 0, so udelay()
+     * callers that spin on hal_get_timer_us() advancing remain monotonic
+     * (matches hal/versal.c). */
     if (freq == 0)
-        return 0;
+        freq = ZYNQMP_TIMER_CLK_FREQ;
     /* Use __uint128_t to avoid overflow of (count * 1e6) at long uptimes
      * (would overflow uint64_t after ~51h at 100MHz). */
     return (uint64_t)(((__uint128_t)count * 1000000ULL) / freq);

include/sdhci.h

@@ -57,7 +57,13 @@
 #define DISK_TEST_BLOCK_ADDR    149504  /* ~76MB offset */
 #endif
 
-/* Auto-select DMA buffer boundary based on threshold */
+/* DMA buffer boundary: how often the SDMA engine pauses to refresh its
+ * address pointer (handled by sdhci_irq_handler() via SDHCI_SRS12_DMAINT).
+ * This is a throughput knob and is independent of SDHCI_DMA_THRESHOLD
+ * (which controls when to switch from PIO to SDMA). Override in target
+ * .config to match the largest expected single transfer for fewer
+ * boundary IRQs; otherwise auto-select based on the threshold. */
+#ifndef SDHCI_DMA_BUFF_BOUNDARY
 #if (SDHCI_DMA_THRESHOLD > (256U * 1024U))
     #define SDHCI_DMA_BUFF_BOUNDARY   SDHCI_SRS01_DMA_BUFF_512KB
     #if (SDHCI_DMA_THRESHOLD != (512U * 1024U))
@@ -99,6 +105,7 @@
         #warning "SDHCI_DMA_THRESHOLD rounded up to 4KB (minimum)"
     #endif
 #endif
+#endif /* !SDHCI_DMA_BUFF_BOUNDARY */
 
 /* Timeouts */
 #ifndef SDHCI_INIT_TIMEOUT_US

src/boot_aarch64.c

@@ -34,7 +34,7 @@
 #include "hal/versal.h"
 #elif defined(TARGET_zynq)
 #include "hal/zynq.h"
-#elif defined(TARGET_ls1028a)
+#elif defined(TARGET_nxp_ls1028a)
 #include "hal/nxp_ls1028a.h"
 #endif