Skip to content

lopper: assist: zephyr: guard R52 reserved-memory ranges update #824

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zeddii merged 2 commits into from
Sep 2, 2026
Merged

lopper: assist: zephyr: guard R52 reserved-memory ranges update #824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
776618d
lopper: assist: zephyr: guard R52 reserved-memory ranges update
dbingi-amd Sep 1, 2026
31f90c7
tests: gen_domain: cover R52 reserved-memory ranges guard
dbingi-amd Sep 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lopper/assists/zephyr_domain_dts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,8 +488,9 @@ def xlnx_generate_zephyr_domain_dts_arm(tgt_node, sdt, options, machine):
if compatible == "arm,armv8-timer" and ('psv_cortexr5' in machine or 'psu_cortexr5' in machine):
sdt.tree.delete(node)
if node.name == 'reserved-memory' and 'r52' in machine:
node.delete('ranges')
node + LopperProp(name='ranges')
if node.propval('ranges') != ['']:
node.delete('ranges')
node + LopperProp(name='ranges')

@zeddii zeddii Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Design question rather than a defect, but worth a moment's thought since it's the behavioural half of the change.

With the add inside the guard, a /reserved-memory that arrives without ranges now leaves without one either. The reserved-memory binding wants an empty ranges present, so for the SCMI case you describe the output is arguably still not what a consumer expects ... it just no longer crashes on the way there.

If the intent of this block was "guarantee /reserved-memory ends up with ranges;", then only the delete needs guarding:

if node.props('^ranges$'):
    node.delete('ranges')
node + LopperProp(name='ranges')

That keeps the normalization you're preserving for bad ranges = <0x1> values, fixes the crash, and leaves the absent case standard-conformant.

If instead the deliberate call is "don't synthesize a property the SDT didn't ask for", that's a defensible position too. I'd just ask you to say so in the commit message, because the current wording ("leave the node as the SDT supplied it") reads as a consequence of the fix rather than a decision.

If you do go that way, please warn on it rather than passing silently. Emitting a /reserved-memory with no ranges is a notable outcome, and a silent skip is precisely how this class of problem stays invisible until someone hits it downstream ... which is the same shape as the bug you're fixing here. lopper.log._warning is the right API for it; the file has no log import today, so if you'd rather match local style it already does print(f"[WARNING] ...") at line 245.

@dbingi-amd dbingi-amd Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm keeping the guarded rewrite as-is we only touch ranges when it is already on /reserved-memory, and we do not add ranges; when the SDT omits it.

Why: this block was meant to normalize malformed ranges values (e.g. ranges = <0x1>), not to always materialize ranges; For the SCMI case that triggered the bug, the input has no ranges property, and that matches the Versal2 SDTs we see. Adding ranges; unconditionally would change output beyond what is needed to fix the crash.
The commit message now states that explicitly


board_symbol = board_symbol_for_machine(machine)
if board_symbol:
Expand Down
50 changes: 50 additions & 0 deletions tests/test_xlnx_gen_domain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,56 @@ def test_ttc_label_normalization_is_r5_platform_specific():
assert tree["/__symbols__"].propval("ttc0") == ['']


def _minimal_r52_tree(reserved_memory_props=None):
"""Minimal Versal2 R52 tree with SCMI-style /reserved-memory."""
tree = LopperTree()
root = tree["/"]
root["compatible"] = ["amd,versal2"]
for name in ("cpus", "chosen", "aliases", "axi"):
tree + LopperNode(-1, f"/{name}")
resmem = LopperNode(-1, "/reserved-memory")
resmem["#address-cells"] = [2]
resmem["#size-cells"] = [2]
if reserved_memory_props:
for prop_name, prop_value in reserved_memory_props.items():
resmem[prop_name] = prop_value
buffer_node = LopperNode(-1, "/reserved-memory/memory@20000000")
buffer_node["no-map"] = []
buffer_node["reg"] = [0, 0x20000000, 0, 0x20000]
resmem.add(buffer_node)
tree + resmem
tree.sync()
return tree


def test_r52_reserved_memory_without_ranges_does_not_raise(tmp_path):
"""SCMI /reserved-memory without ranges must not abort R52 domain generation."""
tree = _minimal_r52_tree()
sdt = SimpleNamespace(tree=tree, outdir=str(tmp_path))
options = {"args": ["cortexr52_0"], "verbose": 0}

assert gen_domain_dts.xlnx_generate_zephyr_domain_dts_arm(
"/", sdt, options, "cortexr52_0")

resmem = tree["/reserved-memory"]
assert resmem.propval("ranges") == ['']


def test_r52_reserved_memory_malformed_ranges_is_normalized(tmp_path):
"""Malformed ranges values are rewritten as an empty ranges flag."""
tree = _minimal_r52_tree(reserved_memory_props={"ranges": [1]})
sdt = SimpleNamespace(tree=tree, outdir=str(tmp_path))
options = {"args": ["cortexr52_0"], "verbose": 0}

gen_domain_dts.xlnx_generate_zephyr_domain_dts_arm(
"/", sdt, options, "cortexr52_0")

ranges_prop = tree["/reserved-memory"].props("ranges")[0]
ranges_prop.resolve()
assert ranges_prop.value in ([], [''])
assert "0x1" not in ranges_prop.string_val


def test_rpu_memory_rename_refreshes_path_and_phandle_references():
"""RPU local-view renames preserve chosen, symbol, and phandle refs."""
tree = LopperTree()
Expand Down
Loading