Peer review feedback (thank you Copilot)

Author: dgarske

src/port/versal/Makefile

@@ -4,7 +4,7 @@
 #
 # Toolchain: ARM GNU aarch64-none-elf-gcc (tested with 14.3.rel1).
 #
-# UNTESTED ON HARDWARE -- structural scaffold mirroring src/port/zcu102/.
+# Brought up on a VMK180 (Cortex-A72): DHCP, ICMP ping and UDP echo work.
 
 CROSS_COMPILE ?= aarch64-none-elf-
 CC      := $(CROSS_COMPILE)gcc

src/port/versal/board.h

@@ -24,9 +24,8 @@
  * and the published `versal.dtsi` device tree. No Xilinx BSP header
  * (xparameters.h) or xilstandalone code is referenced.
  *
- * UNTESTED ON HARDWARE -- code-only scaffold while the lab board is
- * unavailable. The structure mirrors src/port/zcu102/. Key
- * differences from ZynqMP are:
+ * Brought up on a VMK180 (Cortex-A72). The structure mirrors
+ * src/port/zcu102/. Key differences from ZynqMP are:
  *   - Cortex-A72 (not A53), with PLM handoff at EL2
  *   - GICv3 distributor + redistributor (no GICv2 legacy GICC)
  *   - ARM PL011 UART (not Cadence)

src/port/versal/config.h

@@ -18,8 +18,8 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA
  *
- * wolfIP configuration for Xilinx ZCU102 (UltraScale+ MPSoC, A53-0 EL3
- * bare-metal). UDP-only profile aimed at deterministic DAL-C use.
+ * wolfIP configuration for Xilinx Versal Gen 1 / VMK180 (Cortex-A72
+ * EL3 bare-metal). UDP-only profile aimed at deterministic DAL-C use.
  */
 #ifndef WOLF_CONFIG_H
 #define WOLF_CONFIG_H

src/port/versal/entropy.c

@@ -18,12 +18,12 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA
  *
- * MemUse-pattern entropy source for the wolfIP ZCU102 port.
+ * MemUse-pattern entropy source for the wolfIP Versal port.
  *
- * The ZCU102's UltraScale+ PS does not ship a hardware TRNG that is
- * usable from EL3 bare-metal without the PMU firmware and CSU helpers.
+ * The Versal PS does not ship a hardware TRNG that is usable from EL3
+ * bare-metal without the PLM firmware and its helpers.
  * This source produces non-deterministic 32-bit words by sampling the
- * Cortex-A53 virtual count register (CNTVCT_EL0) before and after a
+ * Cortex-A72 virtual count register (CNTVCT_EL0) before and after a
  * memory-access loop that touches a state buffer larger than the L1
  * data cache. The cache-miss / line-fill / write-allocate timing
  * variance is the entropy source - the same primitive wolfCrypt's
@@ -35,19 +35,19 @@
  * ephemeral source ports, IP fragment ID) need unpredictable bits,
  * not uniformly-distributed cryptographic randomness. For crypto-
  * grade seeding the port should be rebuilt with the full wolfCrypt
- * wc_Entropy_Get() in place of zcu102_get_random32().
+ * wc_Entropy_Get() in place of versal_get_random32().
  *
  * Algorithm per call:
  *   1. t0 = CNTVCT_EL0
  *   2. Walk state[] performing read+xor+write; ~256 accesses spans
- *      multiple L1 cache lines on this 32 KB / 4-way A53 cache.
+ *      multiple L1 cache lines on this 32 KB / 2-way A72 cache.
  *   3. t1 = CNTVCT_EL0
  *   4. Fold (t1 - t0) into the rolling 64-bit accumulator and
  *      perturb state[] so the next call diverges.
  *   5. Apply a non-cryptographic finaliser (xorshift) and return
  *      the low 32 bits.
  *
- * The state buffer is 1024 bytes (sized to span the A53's 64-byte
+ * The state buffer is 1024 bytes (sized to span the A72's 64-byte
  * line size 16 times, ensuring at least a handful of cache misses
  * per call even on a warm cache).
  */
@@ -70,7 +70,7 @@ static inline uint64_t cntvct_el0(void)
 /* Return a 32-bit value with low predictability, suitable for
  * protocol identifiers (DHCP xid, DNS id, TCP ISN, ephemeral port,
  * IP fragment id). Not crypto-grade; see file header. */
-uint32_t zcu102_get_random32(void)
+uint32_t versal_get_random32(void)
 {
     uint64_t t0, t1, delta;
     uint64_t acc;

src/port/versal/gem.c

@@ -36,7 +36,7 @@
  * Register set per Versal ACAP TRM (AM011) chapter "Gigabit Ethernet
  * MAC" / Cadence GEM.
  *
- * UNTESTED ON HARDWARE -- structural scaffold.
+ * Brought up on a VMK180 (Cortex-A72 EL3): DHCP, ping and UDP echo.
  */
 #include <stdint.h>
 #include <string.h>
@@ -48,7 +48,7 @@
 #include "gem.h"
 #include "phy_dp83867.h"
 
-/* Cache maintenance helpers for GEM DMA coherency. Cortex-A53 cache
+/* Cache maintenance helpers for GEM DMA coherency. Cortex-A72 cache
  * line is 64 bytes. With D-cache enabled and BD/buffers in normal
  * cacheable memory, CPU writes may sit in L1 D-cache and not be
  * visible to the MAC's DMA path. cache_clean() writes back dirty
@@ -542,12 +542,12 @@ void gem_dump_state(void)
  * boots with networking up), so we deliberately do NOT touch CRL here.
  * The driver's per-MAC soft reset via GEM_NWCTRL is enough to put the
  * MAC into a known state on top of the PLM-configured clock. */
-static void gem3_set_ref_clk(int speed_mbps)
+static void gem0_set_ref_clk(int speed_mbps)
 {
     (void)speed_mbps;   /* clock owned by PLM; nothing to do on Versal */
 }
 
-static void gem3_hw_reset(void)
+static void gem0_hw_reset(void)
 {
 #ifdef DEBUG_GEM
     volatile uint32_t *gem0ref = (volatile uint32_t *)CRL_APB_GEM0_REF_CTRL;
@@ -564,7 +564,7 @@ static void gem3_hw_reset(void)
 /* ---------------------------------------------------------------------
  * Public init
  * ------------------------------------------------------------------- */
-int zcu102_eth_init(struct wolfIP_ll_dev *ll)
+int versal_eth_init(struct wolfIP_ll_dev *ll)
 {
     uint8_t addr;
     uint16_t id1;
@@ -573,7 +573,7 @@ int zcu102_eth_init(struct wolfIP_ll_dev *ll)
     int fd;
     int link_up;
 
-    gem3_hw_reset();
+    gem0_hw_reset();
 
     /* Disable everything before configuring. */
     GEM_NWCTRL = 0;
@@ -585,7 +585,7 @@ int zcu102_eth_init(struct wolfIP_ll_dev *ll)
 
     /* Initial NWCFG: gigabit, full duplex, MDC=/96, 1536-byte frames,
      * strip FCS from RX, accept broadcasts, multicast via hash,
-     * DWIDTH_64 because ZynqMP GEM hangs on a 64-bit AXI bus and
+     * DWIDTH_64 because the Cadence GEM hangs on a 64-bit AXI bus and
      * needs this bit for TX to actually transmit (matches U-Boot
      * ZYNQ_GEM_DBUS_WIDTH for CONFIG_ARM64).
      * COPYALL temporarily on for first-bring-up so we can confirm
@@ -645,7 +645,7 @@ int zcu102_eth_init(struct wolfIP_ll_dev *ll)
     cache_clean(&dummy_tx_bd, sizeof(dummy_tx_bd));
     cache_clean(&dummy_rx_bd, sizeof(dummy_rx_bd));
 
-    /* Clear any stale RX/TX packet classification screening. ZynqMP
+    /* Clear any stale RX/TX packet classification screening. The Cadence
      * GEM has SCREENING_TYPE_1 (TID match) at 0x500+ and SCREENING_TYPE_2
      * (compare) at 0x540+. If non-zero, frames may be routed to non-Q0
      * queues. Default 0 = all to Q0. */
@@ -660,9 +660,6 @@ int zcu102_eth_init(struct wolfIP_ll_dev *ll)
     /* Enable MDIO so we can talk to the PHY. */
     GEM_NWCTRL |= NWCTRL_MDEN;
 
-    /* Probe MDIO addresses 0..31 for a responsive PHY. ZCU102 routes
-     * DP83867 to MDIO address 0x0C, but probing makes the driver
-     * resilient to board variants. */
     /* Scan all 32 MDIO addresses, reporting each responsive PHY's ID and
      * link status (BMSR reg 1, bit 2). VMK180 can present more than one
      * DP83867 on the bus; prefer one that already has copper link so we
@@ -703,7 +700,7 @@ int zcu102_eth_init(struct wolfIP_ll_dev *ll)
     }
 
     /* If PHY ended up at 10/100, downshift the MAC speed. The GEM0
-     * reference clock is owned by the PLM (see gem3_set_ref_clk), so we
+     * reference clock is owned by the PLM (see gem0_set_ref_clk), so we
      * only adjust the MAC config here, not the CRL clock divider. */
     if (speed != 1000) {
         uint32_t cfg = GEM_NWCFG;
@@ -715,7 +712,7 @@ int zcu102_eth_init(struct wolfIP_ll_dev *ll)
         if (!fd)
             cfg &= ~NWCFG_FDEN;
         GEM_NWCFG = cfg;
-        gem3_set_ref_clk(speed);
+        gem0_set_ref_clk(speed);
     }
 
     /* Install RX ISR. */

src/port/versal/gem.h

@@ -4,23 +4,23 @@
  *
  * This file is part of wolfIP TCP/IP stack.
  *
- * Cadence GEM driver for Xilinx UltraScale+ MPSoC GEM3 (on-board RJ45
- * on ZCU102). Single-instance, RGMII, gigabit, polled TX, IRQ-driven
- * RX.
+ * Cadence GEM driver for Versal Gen 1 GEM0 (on-board RJ45 on VMK180).
+ * Single-instance, RGMII, gigabit, polled TX, IRQ-registered RX that
+ * is currently drained by polling (see gem.c / eth_poll).
  */
-#ifndef ZCU102_GEM_H
-#define ZCU102_GEM_H
+#ifndef VERSAL_GEM_H
+#define VERSAL_GEM_H
 
 #include <stdint.h>
 #include "../../../wolfip.h"
 
-/* Initialize GEM3, MMIO clock + reset, PHY, and populate the wolfIP
+/* Initialize GEM0, MMIO clock + reset, PHY, and populate the wolfIP
  * link-layer device. Returns:
  *   < 0          on error (negated TRM code)
  *   bits [7:0]   PHY MDIO address used
  *   bit  [8]     link_up flag (1 = link is up at end of init)
  */
-int zcu102_eth_init(struct wolfIP_ll_dev *ll);
+int versal_eth_init(struct wolfIP_ll_dev *ll);
 
 /* MDIO helpers exposed for the PHY driver (phy_dp83867.c). */
 int gem_mdio_read(uint8_t phy_addr, uint8_t reg, uint16_t *out);
@@ -32,4 +32,4 @@ uint32_t gem_irq_count(void);
 uint32_t gem_rx_frames(void);
 uint32_t gem_tx_sent(void);
 
-#endif /* ZCU102_GEM_H */
+#endif /* VERSAL_GEM_H */

src/port/versal/gic.c

@@ -32,10 +32,9 @@
  * the per-CPU redistributor for us; we only re-prime per-INTID config
  * for the SPIs we use and enable the CPU interface for our EL.
  *
- * UNTESTED ON HARDWARE -- structural scaffold mirroring the GICv2
- * driver under src/port/zcu102/gic.c. Bring-up steps and edge cases
- * (interrupt routing model, security state, GIC600 vs GIC500 quirks)
- * will need to be validated when the VMK180 lab board is available.
+ * Brought up on a VMK180 (Cortex-A72 EL3). Adapted from the GICv2
+ * driver under src/port/zcu102/gic.c for GICv3 system registers and
+ * the per-CPU redistributor.
  */
 #include <stdint.h>
 #include "board.h"

src/port/versal/gic.h

@@ -4,8 +4,8 @@
  *
  * This file is part of wolfIP TCP/IP stack.
  */
-#ifndef ZCU102_GIC_H
-#define ZCU102_GIC_H
+#ifndef VERSAL_GIC_H
+#define VERSAL_GIC_H
 
 #include <stdint.h>
 
@@ -32,10 +32,10 @@ uint32_t gic_last_intid(void);
  * by reading GICC_IAR, calling the registered handler, and EOI'ing.
  * Returns the number of interrupts dispatched in this call.
  *
- * Workaround: on this ZynqMP / Cortex-A53 / GIC-400 combination,
- * the GIC latches pending interrupts correctly but the CPU never
- * takes the IRQ exception (root cause not pinned). Calling this
- * function from the main loop is functionally equivalent. */
+ * Workaround: on this Versal / Cortex-A72 / GIC-600 (GICv3)
+ * combination, the GIC latches pending interrupts correctly but the
+ * CPU never takes the IRQ exception (root cause not pinned). Calling
+ * this function from the main loop is functionally equivalent. */
 uint32_t gic_poll_dispatch(void);
 
 /* Provided by startup.S, asm helpers. */
@@ -46,4 +46,4 @@ void irq_disable(void);
  * dispatches, and EOIs the current interrupt. */
 void irq_dispatch(void);
 
-#endif /* ZCU102_GIC_H */
+#endif /* VERSAL_GIC_H */

src/port/versal/main.c

@@ -21,8 +21,8 @@
  * wolfIP UDP echo + DHCP client demo for Xilinx Versal Gen 1 (VMK180
  * dev board, Cortex-A72 EL3 bare-metal, GEM0 -> on-board RJ45).
  *
- * UNTESTED ON HARDWARE -- structural scaffold mirroring the ZCU102
- * port. See src/port/versal/README.md.
+ * Brought up on a VMK180 (Cortex-A72 EL3): DHCP, ICMP ping and UDP
+ * echo all work. See src/port/versal/README.md.
  */
 #include <stdint.h>
 #include <string.h>
@@ -43,7 +43,7 @@ static uint8_t udp_rx_buf[RX_BUF_SIZE];
 
 /* Override newlib memset/memcpy with plain bytewise versions via
  * linker --wrap. The aarch64 newlib memset uses 'dc zva' which hangs
- * on this Cortex-A53 setup (DZE=1 in SCTLR_EL3 doesn't help; the
+ * on this Cortex-A72 setup (DZE=1 in SCTLR_EL3 doesn't help; the
  * instruction wedges before completing). The Makefile passes
  * -Wl,--wrap=memset -Wl,--wrap=memcpy so all calls get redirected
  * to these __wrap_ functions. */
@@ -98,11 +98,11 @@ void exception_report_serror(uint64_t esr, uint64_t elr, uint64_t far,
  * to the port-local memuse-pattern entropy source (entropy.c), which
  * follows the algorithm of wolfCrypt's wc_Entropy_Get() but is
  * self-contained for cert isolation. */
-extern uint32_t zcu102_get_random32(void);
+extern uint32_t versal_get_random32(void);
 
 uint32_t wolfIP_getrandom(void)
 {
-    return zcu102_get_random32();
+    return versal_get_random32();
 }
 
 static void udp_echo_cb(int fd, uint16_t event, void *arg)
@@ -147,9 +147,9 @@ int main(void)
 
     uart_puts("Bringing up GEM0 (RGMII, DP83867)...\n");
     ll = wolfIP_getdev(IPStack);
-    ret = zcu102_eth_init(ll);
+    ret = versal_eth_init(ll);
     if (ret < 0) {
-        uart_puts("ERROR: zcu102_eth_init failed: ");
+        uart_puts("ERROR: versal_eth_init failed: ");
         uart_puthex((uint32_t)ret);
         uart_puts("\n");
         while (1)
@@ -159,8 +159,8 @@ int main(void)
     uart_puts(", PHY=");      uart_puthex((uint32_t)(ret & 0xFF));
     uart_puts("\n");
 
-    /* Unmask IRQ at CPU now that GEM3 SPI is enabled at GICD. The CPU
-     * IRQ exception is currently not delivered on this A53/EL3 setup
+    /* Unmask IRQ at CPU now that GEM0 SPI is enabled at GICD. The CPU
+     * IRQ exception is currently not delivered on this A72/EL3 setup
      * (open issue - see README); eth_poll() drives gem_isr() from the
      * main loop instead. The SGI self-test instrumentation below is
      * left in place behind DEBUG_GIC for further investigation. */

src/port/versal/mmu.c

@@ -18,7 +18,7 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA
  *
- * Minimal MMU bring-up for A53 at EL3 with a 32-bit virtual address
+ * Minimal MMU bring-up for A72 at EL3 with a 32-bit virtual address
  * space (T0SZ=32, start level L1). Static tables map the full 4 GB VA
  * range:
  *