Add Agilex 5 FCS and wolfBoot integration - #846
aidangarske wants to merge 10 commits into
Conversation
7e310c9 to
d67e834
Compare
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds an Altera Agilex 5 (013B) port and integration points to boot wolfBoot as BL33 in the GSRD SD-card flow, including SDHCI/platform init, memory mapping, example FIT artifacts, and CI build coverage.
Changes:
- Adds Agilex 5 HAL (C + header), linker script, FIT example, and a BL33 smoke-test app.
- Updates AArch64 startup/MMU mappings and SDHCI behavior to support Agilex 5 boot/SD timing.
- Updates disk-boot logging and adds docs/config + GitHub Actions build coverage for the new target.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test-app/app_agilex5.c | Adds a BL33 smoke-test app for Agilex 5 (timer + EL print). |
| src/update_disk.c | Improves disk-boot debug output (block size + pointer-format prints). |
| src/sdhci.c | Adds command-line reset, inhibit timeouts, optional post-command delay, and CMD8 retry logic. |
| src/boot_aarch64_start.S | Adds Agilex 5 include, adjusts CNTFRQ/RVBAR handling, and adds Agilex 5 MMU mappings. |
| src/boot_aarch64.c | Adds Agilex 5 HAL include selection for AArch64 boot code. |
| include/sdhci.h | Adds reset bit definitions and new SDHCI tuning/behavior macros. |
| hal/agilex5.ld | New linker script matching BL33 placement at 0x80200000. |
| hal/agilex5.its | Example FIT description for Agilex 5 Linux payload (kernel + DTB). |
| hal/agilex5.h | New Agilex 5 target configuration and platform constants. |
| hal/agilex5.c | New Agilex 5 HAL implementation (timer, UART, DT fixups, SDHCI PHY + DMA cache ops). |
| docs/Targets.md | Documents the new Agilex 5 target and references Agilex5 bring-up doc. |
| docs/Agilex5.md | Adds bring-up and integration documentation (GSRD/FIT/WIC/test order/CI). |
| config/examples/agilex5_013b_sdcard.config | Adds a buildable example config for Agilex 5 SD-card boot flow. |
| arch.mk | Adds Agilex 5 AArch64 flags + target-specific bootloader responsibilities. |
| .github/workflows/test-configs.yml | Adds Agilex 5 config to existing CI matrix via reusable workflow. |
| .github/workflows/test-build-agilex5.yml | Adds a dedicated Agilex 5 build workflow (path-filtered). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #846
Scan targets checked: wolfboot-bugs, wolfboot-src
Findings: 4
4 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
|
@aidangarske please rebase. |
058516e to
5525e40
Compare
8549302 to
16b9c2a
Compare
16b9c2a to
b3ad8dc
Compare
b3ad8dc to
58a0b88
Compare
| #define UART_TIMEOUT 1000000U | ||
| #define UART_REG(_n) \ | ||
| (*(volatile uint32_t *)(AGILEX5_UART0_BASE + ((uintptr_t)(_n) << 2))) | ||
|
|
There was a problem hiding this comment.
wolfBoot always prints to UART0 and always tells Linux to print its early messages there, and there is no setting to change it. Which UART a board uses for its console is a board design choice, so the port should let the config pick any of them. On any board that does not use UART0, wolfBoot runs with no visible output and an early Linux crash cannot be seen.
The same UART0 address is also written into the Linux command line at hal/agilex5.c line 42 (earlycon=uart8250,mmio32,0x10c02000).
What I did: I used a board whose console is not on UART0 (a DE25-Nano, whose U-Boot reports serial addr = 0x0000000010c02100). I built wolfboot.bin from this PR, put it on a TFTP server, and started it from the U-Boot prompt:
setenv autoload no; dhcp
dcache off; icache off
setenv serverip <server>; tftp 0x80200000 wolfboot.bin
go 0x80200000
What I found: nothing after the jump for 90 seconds.
Bytes transferred = 115096 (1c198 hex)
SOCFPGA_AGILEX5 #
go 0x80200000
## Starting application at 0x80200000 ...
Then I pointed UART_REG at that board's console UART, changed nothing else, ran the same steps, and found:
## Starting application at 0x80200000 ...
wolfBoot Secure Boot - Altera Agilex 5
TF-A BL33 entry, current EL: 2
Reading MBR...
Found MBR partition table
To check that an early Linux crash really cannot be seen, I then built the PR with only wolfBoot's own printing moved to the board's console UART, leaving the PR's earlycon on UART0 and the PR's 1792 MiB DDR size, which crashes Linux at once on this 1 GiB board. I booted a signed Linux FIT and found that Linux printed nothing at all after wolfBoot handed over:
FDT: Set memory, start=0x80000000, size=0x70000000
FDT: Set leds (21556), status=disabled
FDT: Set chosen (21296), bootargs=earlycon=uart8250,mmio32,0x10c02000 console=ttyS0,115200 root=/dev/mmcblk0p2 rootwait
do_boot: flushing caches, disabling MMU
(nothing more for 5 minutes)
With earlycon on the board's console UART and everything else the same, the same crash is visible:
[ 0.000000] earlycon: uart8250 at MMIO32 0x0000000010c02100 (options '')
[ 0.000000] SError Interrupt on CPU0, code 0x00000000be000011 -- SError
[ 0.000000] Kernel panic - not syncing: Asynchronous SError Interrupt
wolfBoot already has a convention for this. ZynqMP and Versal pick their console with DEBUG_UART_NUM, mapped to DEBUG_UART_BASE in the HAL header (hal/zynq.h lines 375 to 384, hal/versal.h lines 156 to 160), and the Versal example configs set it with CFLAGS_EXTRA+=-DDEBUG_UART_NUM=0. Following the same pattern, with UART0 as the default, keeps the prebuilt dev kit config unchanged and lets any design choose whichever UART it routes its console to. Feeding earlycon from the same value keeps wolfBoot and Linux on the same port:
/* hal/agilex5.h, same pattern as hal/zynq.h and hal/versal.h */
#ifndef DEBUG_UART_BASE
#if defined(DEBUG_UART_NUM) && DEBUG_UART_NUM == 1
#define DEBUG_UART_BASE 0x10C02100
#else
#define DEBUG_UART_BASE 0x10C02000
#endif
#endif
/* hal/agilex5.c */
#define AGILEX5_STR2(x) #x
#define AGILEX5_STR(x) AGILEX5_STR2(x)
#define LINUX_BOOTARGS \
"earlycon=uart8250,mmio32," AGILEX5_STR(DEBUG_UART_BASE) \
" console=ttyS0,115200 " \
"root=" LINUX_BOOTARGS_ROOT " rootwait"
#define UART_REG(_n) \
(*(volatile uint32_t *)((uintptr_t)DEBUG_UART_BASE + \
((uintptr_t)(_n) << 2)))The base is written without a UL suffix because it is also turned into the earlycon string. A board then sets CFLAGS_EXTRA+=-DDEBUG_UART_NUM=1, or -DDEBUG_UART_BASE=<address> for any other UART. I tested the same mechanism under a different macro name: with no override a clean build still produces earlycon=uart8250,mmio32,0x10C02000, so the dev kit build is unchanged, and with the override set to the board's UART both wolfBoot and Linux early output appear on that port (earlycon: uart8250 at MMIO32 0x0000000010c02100 on the board I used).
| /* The DK-A5E013BM16AEA is fitted with 1792 MiB of LPDDR4. The SPL/U-Boot | ||
| * device tree normally fills this in at runtime; wolfBoot hands the DTB | ||
| * directly to Linux, so the fixed board size must be supplied here. */ | ||
| #define AGILEX5_DDR_SIZE 0x70000000UL |
There was a problem hiding this comment.
wolfBoot always tells Linux the board has 1792 MB of memory, the dev kit's amount, and there is no setting to change it. How much DDR a board fits is a board design choice, so the port should let the config state whatever size the board has. On any board with less memory, Linux tries to use memory that is not there and crashes at once.
The value is written into the DTB memory node by hal_dts_fixup() at hal/agilex5.c lines 181 to 183.
What I did: I used a board with less than 1792 MiB (my DE25-Nano has 1 GiB), with the console set to its UART so I could see the output. I built a FIT from the GSRD Image and the board's DTB using the layout of hal/agilex5.its (kernel at 0x82000000, DTB at 0x8f000000), signed it with the build's key, wrote it to the A partition, and started wolfBoot.
./tools/keytools/sign --rsa4096 --sha3 linux.itb wolfboot_signing_private_key.der 1
dd if=linux_v1_signed.bin of=/dev/mmcblk0p3 bs=1M && sync
What I found with the PR's value:
Firmware Valid.
Booting at 82000000
FDT: Set memory, start=0x80000000, size=0x70000000
[ 0.000000] earlycon: uart8250 at MMIO32 0x0000000010c02100 (options '')
[ 0.000000] SError Interrupt on CPU0, code 0x00000000be000011 -- SError
[ 0.000000] Kernel panic - not syncing: Asynchronous SError Interrupt
Then I rebuilt with only the size set to the board's real amount, booted the same kernel, DTB and card, and found:
FDT: Set memory, start=0x80000000, size=0x40000000
[ 0.274605] Memory: 909276K/1048576K available
de25-nano login:
The size cannot simply be taken from the board DTB instead. In the normal flow U-Boot corrects the memory node after SPL has measured the DDR, and without U-Boot the node holds whatever the DTB author wrote, which need not match the fitted memory (the DTB on the board I used claims 2 GiB). A build option with the dev kit value as the default lets each design state its own size; hal/nxp_ppc.h lines 35 to 37 already make its DDR size overridable the same way:
#ifndef AGILEX5_DDR_SIZE
#define AGILEX5_DDR_SIZE 0x70000000UL
#endifand each board sets its size in its config, for example CFLAGS_EXTRA+=-DAGILEX5_DDR_SIZE=<its DDR size>. I confirmed in the built wolfboot.elf that hal_dts_fixup() loads 0x70000000 with no override and the board's value with it.
| /* The board DT contains an optional FPGA-backed gpio-leds node. The | ||
| * normal GSRD path programs the fabric in U-Boot before probing it; the | ||
| * direct wolfBoot handoff does not yet program that optional design. | ||
| * Keep Linux from touching an unconfigured fabric register while the | ||
| * HPS, FCS and storage paths remain available. */ | ||
| off = fdt_find_node_offset(&ctx, -1, "leds"); | ||
| if (off >= 0) { | ||
| ret = fdt_fixup_str(&ctx, off, "leds", "status", "disabled"); | ||
| if (ret != 0) { | ||
| wolfBoot_printf("FDT: failed to disable FPGA LEDs (%d)\n", ret); | ||
| return ret; | ||
| } | ||
| } |
There was a problem hiding this comment.
To stop Linux touching GPIO routed through the unprogrammed FPGA, wolfBoot marks the first device tree node called leds as disabled. It picks the node by its name, not by what its pins are wired to, so on another board it can switch off pins that work fine, and it leaves every other node that uses FPGA routed GPIO (a relay, a button, an enable line) switched on. wolfBoot never drives a pin itself, but hiding the wrong node means Linux never sets that line, and missing the right one means Linux can still reach the unprogrammed fabric the comment is trying to protect.
How a board routes its GPIO is a board design choice, so the port should not assume the dev kit's wiring or its node names.
What I did: I used a board whose leds node is driven by HPS GPIO (on my DE25-Nano it uses the HPS GPIO controller at gpio@10C03300, with no FPGA in the path):
leds {
compatible = "gpio-leds";
hps0 {
label = "hps_led0";
gpios = <0x21 0x11 0x01>;
};
};
First, as a control, I booted the board normally through its own U-Boot, with the same FPGA image, kernel and DTB, and checked the LED exists and Linux can drive it:
ls /sys/class/leds/
echo STATUS=$(cat /proc/device-tree/leds/status 2>/dev/null || echo no-status-property)
cat /sys/class/leds/hps_led0/brightness
echo 1 > /sys/class/leds/hps_led0/brightness; echo SET1_EXIT=$?
cat /sys/class/leds/hps_led0/brightness
echo 0 > /sys/class/leds/hps_led0/brightness; echo SET0_EXIT=$?
cat /sys/class/leds/hps_led0/brightness
What I found with the normal boot:
hps_led0 mmc0::
STATUS=no-status-property
0
SET1_EXIT=0
1
SET0_EXIT=0
0
Then I booted a signed Linux FIT through wolfBoot, with nothing else changed, and ran ls /sys/class/leds/; cat /proc/device-tree/leds/status.
What I found with wolfBoot:
FDT: Set leds (21556), status=disabled
...
root@de25-nano:~# ls /sys/class/leds/; cat /proc/device-tree/leds/status
mmc0::
disabled
The line that Linux could drive under the normal boot is gone, and the only difference is wolfBoot's leds fixup.
To show the fixup chooses by name and stops at the first match, I added two nodes to the same DTB that have no driver and no pins, so they cannot affect the hardware: a second node named leds under test-second, and a node named relay. I booted that DTB through wolfBoot and listed the status of every node named leds or relay:
find /proc/device-tree/ -name status | grep -E 'leds|relay' | while read f; do echo "NODE_STATUS $f $(cat $f)"; done
What I found:
FDT: Set leds (21556), status=disabled
NODE_STATUS /proc/device-tree/relay/status okay
NODE_STATUS /proc/device-tree/test-second/leds/status okay
NODE_STATUS /proc/device-tree/leds/status disabled
Only the first leds node was disabled. The second leds node and the relay node were left enabled, so a board that drives a relay or any other line through the FPGA, under any other node name, gets no protection from this fixup.
If unprogrammed fabric GPIO needs protecting, it is better decided by the board's config than by a node name: either a build option listing the nodes to disable, which the dev kit config sets, or disabling the nodes whose GPIO controller sits behind the FPGA bridges. Either way, designs whose pins are on the HPS are left as their DTB describes them.
| int hal_dts_fixup(void *dts_addr, uint32_t capacity) | ||
| { | ||
| fdt_ctx ctx; | ||
| int off; | ||
| int ret; | ||
|
|
||
| /* Validate the blob against the window it actually occupies. */ | ||
| ret = fdt_open(&ctx, dts_addr, capacity); |
There was a problem hiding this comment.
Before starting a payload, wolfBoot edits the device tree it hands to Linux, but it does this even when the payload is a bare-metal program or an RTOS that comes with no device tree, and then it tries to read one from address zero. The payload still boots, so nothing fails, but every such boot prints a false FDT: invalid header error that points the user at a problem that does not exist. It also means wolfBoot reads a peripheral address it has no reason to touch, which only works because that region happens to be mapped on this target.
Some background on what the fixup is for. hal_dts_fixup() is the Agilex 5 step that prepares the device tree for Linux: it sets the memory size, disables the leds node, and writes the kernel command line. A signed FIT carries a DTB, and wolfBoot places it at 0x8F000000 and passes that address in. A plain ELF or raw binary, such as a bare-metal application or an RTOS image, carries no DTB, so the address passed in is zero.
do_boot() in src/boot_aarch64.c line 166 calls the fixup in both cases, and the Agilex 5 fixup does not check for a missing DTB before parsing. On this target src/boot_aarch64_start.S lines 735 to 738 map 0x0000_0000 to 0x7FFF_FFFF as device memory, so the read at zero does not fault; it just finds no valid header.
What I did: I signed the PR's own bare-metal test application (test-app/image.elf, built from test-app/app_agilex5.c) as a wolfBoot image, wrote it to the A partition and booted it.
What I found, the false error and then the application running normally:
Booting at 90000000
do_boot: entry=0x90000000, EL=2
do_boot: dts=0x00000000
FDT: invalid header (-5)
do_boot: flushing caches, disabling MMU
Agilex 5 BL33 smoke test
Current EL: 2 (expected 2)
Generic timer advanced by 1003 us
AGILEX5_BL33_SMOKE_PASS
As a control, I booted the same wolfBoot build with a signed Linux FIT that carries a DTB, and found the fixup working on the real DTB with no error:
do_boot: entry=0x82000000, EL=2
do_boot: dts=0x8F000000
FDT: Set memory, start=0x80000000, size=0x40000000
So the error appears only when there is no DTB, and it comes from parsing address zero. Other targets already guard this: hal/mpfs250.c returns early from its fixup when dts_addr == NULL. The same check at the top of the Agilex 5 fixup, returning 0 because there is nothing to fix, would remove the false error and the stray read:
int hal_dts_fixup(void *dts_addr, uint32_t capacity)
{
...
if (dts_addr == NULL)
return 0; /* bare-metal or RTOS payload, no DTB to fix up */| 5. Build the complete WIC, inspect all four partitions, flash the whole card, | ||
| compare the complete image span, cold boot, and test A-to-B fallback by | ||
| corrupting only a disposable copy of A. |
There was a problem hiding this comment.
The docs ask for a test where slot A is damaged and the board falls back to slot B. That only works when both slots hold the same version, because with the default anti-rollback setting wolfBoot will not fall back to an older image and stops instead. Someone following the steps with two different versions will see the board stop and may think fallback is broken.
This is wolfBoot's intended behaviour with ALLOW_DOWNGRADE?=0 (config/examples/agilex5_013b_sdcard.config line 26), and the docs for other disk boot targets already explain it. docs/Targets.md line 9761, in the i.MX 8QuadMax section, says: "Failover cannot cross a version boundary downwards. This is the anti-rollback policy rather than a gap: the retry is refused by the ALLOW_DOWNGRADE guard (Rollback to lower version not allowed) whenever the fallback slot carries a lower version, so equal-version slots are what make failover complete." The Agilex 5 docs added here have no equivalent note.
What I did: I wrote a damaged version 2 image to A and a good version 1 image to B, and booted. Then I repeated it with a damaged version 1 in A and a good version 1 in B.
What I found, A version 2 damaged, B version 1 good:
Versions, A:2 B:1
Attempting boot from P:A
Checking image integrity...Error validating integrity for P:A
Rollback to lower version not allowed
wolfBoot: PANIC!
What I found, A version 1 damaged, B version 1 good:
Versions, A:1 B:1
Attempting boot from P:A
Checking image integrity...Error validating integrity for P:A
Attempting boot from P:B
Firmware Valid.
A sentence in step 5 saying both slots must hold the same version for the fallback test, or a pointer to the existing explanation in docs/Targets.md, would cover it.
Description
partner pr: wolfSSL/meta-wolfssl#177