Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The high-DDR recipe is currently overridden, and translation validation and FIT cache-cleaning gaps remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds Versal support for DDR apertures above 1 TB with configurable translation mappings and linker origin handling.
Changes:
- Adds optional high-DDR L0/L1 mappings and low-DDR omission support.
- Makes the Versal linker origin configurable.
- Adds configuration guidance and documentation.
File summaries
| File | Summary and review comments |
|---|---|
src/boot_aarch64_start.S |
Adds high-DDR translation entries and validation. nit (3 votes): the size guard accepts windows below the required 1 TB minimum. nit (1 vote): BOOT_EL1 should clean each FIT destination, including the ramdisk, before disabling the MMU. |
hal/versal.ld |
Uses configured WOLFBOOT_ORIGIN. |
hal/versal.h |
Adds high-DDR and low-window configuration controls. |
docs/Targets.md |
Documents high-aperture deployment. |
config/examples/versal_vmk180.config |
Adds a high-DDR recipe. moderate (2 votes): the unconditional low origin overrides the recipe, preventing it from selecting the high origin. |
Review details
Suppressed comments (1)
src/boot_aarch64_start.S:708
- The high-DDR flow is documented for FIT Linux, but the BOOT_EL1 handoff only cleans the fixed
WOLFBOOT_LOAD_ADDRESSwindow before callingel2_flush_and_disable_mmu(). That helper usesdc ciswset/way flushing, which does not guarantee that dirty kernel, DTB, and ramdisk lines in the newly mapped high window reach PoC on the A72/DSU; Linux can then fetch stale contents after MMU teardown. Clean each actual FIT destination by VA (including the ramdisk) before disabling the MMU.
/* 1 GB Normal-memory blocks covering the high DDR window. */
.balign 4096
MMUTableL1_HI:
.set SECT, VERSAL_DDR_HIGH_BASE
.rept (VERSAL_DDR_HIGH_SIZE / 0x40000000)
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
dgarske
force-pushed
the
versal_ddr_high
branch
from
September 18, 2026 15:23
02ec04a to
e00ba92
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Versal designs can place their DDR aperture above the default address map (a customer design puts it at 0x400_0000_0000), but wolfBoot's translation table only mapped the first 1 TB and
hal/versal.ldhardcoded the load origin. This addsVERSAL_DDR_HIGH_BASE/VERSAL_DDR_HIGH_SIZE(512 GB aligned, minimum 1 TB, enforced by assembler.errorguards) to map an additional Normal-memory window through new L0/L1 entries,VERSAL_NO_DDR_LOWto leave the low window unmapped when nothing remains at0x0(with a compile-time error if used without a high window), and makeshal/versal.ldhonorWOLFBOOT_ORIGINfrom the config; a commented recipe lands inconfig/examples/versal_vmk180.configand docs inTargets.md. Verified by building both Versal configs atWOLFBOOT_ORIGIN=0x40000800000with the window at0x400_0000_0000+ 1 TB and inspecting the generated tables in the ELF (correct L0 indices, 1 GB Normal blocks across the window, low DDR faulting), plus a stock build confirmed unchanged; not boot-tested, since no bench PDI routes a DDR aperture at that address.