From 430059d27b557232e586bb1cbc574bbd6706b295 Mon Sep 17 00:00:00 2001 From: Ben Levinsky Date: Tue, 8 Sep 2026 10:49:33 -0700 Subject: [PATCH 1/2] openamp: resolve Versal2 TCMs by SCMI power domain Versal2 now describes RPU cores and TCM banks with SCMI power-domain IDs. The remoteproc path used those IDs to index an address table keyed by legacy firmware IDs, causing generation to fail with a KeyError. Translate supported Versal2 SCMI TCM IDs through the common mapping table while preserving SCMI tuples in generated remoteproc nodes. Keep direct legacy power-domain IDs and xlnx,power-domain as fallback paths for existing ZynqMP, Versal, and Versal Net inputs. Rename the common address table from memory_nodes to legacy_memory_nodes to make its key space explicit. The only in-tree consumer is updated here. Because the symbol is reachable through wildcard imports, out-of-tree users referencing memory_nodes must migrate to the new name. Validate missing or unsupported IDs with explicit diagnostics. Add coverage for SCMI-only, legacy, and transitional device trees. Signed-off-by: Ben Levinsky --- lopper/assists/openamp_xlnx.py | 56 ++++++++++-- lopper/assists/openamp_xlnx_common.py | 43 ++++++++- tests/test_openamp.py | 126 ++++++++++++++++++++++++++ 3 files changed, 215 insertions(+), 10 deletions(-) diff --git a/lopper/assists/openamp_xlnx.py b/lopper/assists/openamp_xlnx.py index c65b1b15..34fbb998 100644 --- a/lopper/assists/openamp_xlnx.py +++ b/lopper/assists/openamp_xlnx.py @@ -1189,8 +1189,10 @@ def xlnx_remoteproc_v2_construct_cluster(tree, openamp_channel_info, channel_elf Algorithm: Validates platform support, merges power-domain data from carveouts, derives - ranges for TCM and DDR nodes, ensures the cluster node exists with correct - configuration, tracks newly added DDR regions, and finally inserts the core + ranges for TCM and DDR nodes, and ensures the cluster node exists with the + correct configuration. Versal2 TCM addresses are selected by their SCMI + power-domain IDs; older firmware IDs and ``xlnx,power-domain`` remain + compatibility paths. Finally, tracks new DDR regions and inserts the core node using ``xlnx_remoteproc_v2_add_core``. """ print(" -> xlnx_remoteproc_v2_construct_cluster") @@ -1204,8 +1206,6 @@ def xlnx_remoteproc_v2_construct_cluster(tree, openamp_channel_info, channel_elf power_domains = [] core_reg_val = [] - global memory_nodes - if not platform_validate(platform): return False @@ -1219,10 +1219,52 @@ def xlnx_remoteproc_v2_construct_cluster(tree, openamp_channel_info, channel_elf # loop through TCM nodes for n in [ n for n in channel_elfload_nodes if n.propval("xlnx,ip-name") != [''] ]: - pd = n.propval("power-domains") + # Preserve the complete provider/specifier tuple for the generated + # remoteproc node. It is also the primary source for selecting the TCM + # address mapping below. + pd = n.propval("power-domains", list) + node_path = getattr(n, "abs_path", n.name) + if not pd or pd == [""] or len(pd) < 2: + print(f"ERROR: TCM node {node_path} is missing a valid " + "power-domains property") + return False power_domains.extend(pd) - core_reg_val.extend(memory_nodes[pd[1]]["rpu_view"]) - cluster_ranges_val.extend(memory_nodes[pd[1]]["system_view"]) + + pd_id = pd[1] + legacy_pd_id = None + + # Versal2 uses SCMI IDs in power-domains. Translate a supported SCMI + # TCM ID to the existing legacy address-table key. + if platform == SOC_TYPE.VERSAL2: + legacy_pd_id = versal2_scmi_to_legacy_pd.get(pd_id) + + # Older ZynqMP and Versal SDTs, plus some Versal NET TCMs, put the + # legacy address-table ID directly in power-domains. + if legacy_pd_id is None and pd_id in legacy_memory_nodes: + legacy_pd_id = pd_id + + # Transitional SDTs may carry the legacy key separately. This remains + # a compatibility fallback; modern Versal2 mapping does not require it. + legacy_pd = n.propval("xlnx,power-domain", list) + if (legacy_pd_id is None and legacy_pd and + legacy_pd != [""]): + legacy_pd_id = legacy_pd[0] + + mapping = legacy_memory_nodes.get(legacy_pd_id) + if mapping is None: + pd_id_string = hex(pd_id) if isinstance(pd_id, int) else str(pd_id) + legacy_id_string = "" + if legacy_pd and legacy_pd != [""]: + legacy_id = legacy_pd[0] + if isinstance(legacy_id, int): + legacy_id = hex(legacy_id) + legacy_id_string = f", legacy ID {legacy_id}" + print(f"ERROR: TCM node {node_path} has no address mapping for " + f"power-domains ID {pd_id_string}{legacy_id_string}") + return False + + core_reg_val.extend(mapping["rpu_view"]) + cluster_ranges_val.extend(mapping["system_view"]) # map TCM node name to binding compliant TCM name if not any(tcm_name_substr in n.name.lower() for tcm_name_substr in core_reg_names_mappings): diff --git a/lopper/assists/openamp_xlnx_common.py b/lopper/assists/openamp_xlnx_common.py index 27aaac75..8fbed919 100644 --- a/lopper/assists/openamp_xlnx_common.py +++ b/lopper/assists/openamp_xlnx_common.py @@ -300,7 +300,7 @@ class CLUSTER_CONFIG(Enum): RPU_0 = 1 RPU_1 = 2 -memory_nodes = { +legacy_memory_nodes = { 15: { # psu_r5_0_atcm_global "system_view": [0x0, 0x0, 0x0, 0xffe00000, 0x0, 0x10000], "rpu_view": [0x0, 0x0, 0x0, 0x10000] @@ -406,8 +406,45 @@ class CLUSTER_CONFIG(Enum): "rpu_view": [0x1, 0x18000, 0x0, 0x8000] }, } -"""dict[int, dict[str, list[int]]]: Mapping of power-domain identifiers to -system-view and RPU-view memory descriptors used for remoteproc construction.""" +"""dict[int, dict[str, list[int]]]: Legacy firmware power-domain IDs mapped to +system-view and RPU-view memory descriptors used for remoteproc construction. + +Older SDTs place these IDs directly in ``power-domains``. Transitional Versal2 +SDTs may expose them separately through ``xlnx,power-domain``. +""" + +# The direct legacy-ID fallback relies on the SCMI and legacy key spaces +# remaining disjoint. The supported Versal2 SCMI TCM IDs fall within +# 0x44-0x61, while legacy_memory_nodes uses 0x0f-0x12 and 0x1831xxxx. +# Nothing enforces this separation: if a future SCMI ID overlaps a legacy +# key, the fallback will silently select the wrong address descriptor. +# Reassess or explicitly guard the fallback whenever SCMI IDs are extended. +# +# Versal2's SCMI binding assigns new power-domain IDs to the TCM banks. Keep +# this translation separate from the legacy address table so ``power-domains`` +# can be the authoritative input without duplicating the address descriptors. +# Only TCMs already supported by legacy_memory_nodes are listed here. +versal2_scmi_to_legacy_pd = { + 0x44: 0x183180cb, # r52_0a_atcm_global + 0x45: 0x183180cc, # r52_0a_btcm_global + 0x46: 0x183180cd, # r52_0a_ctcm_global + 0x4a: 0x183180ce, # r52_0b_atcm_global + 0x4b: 0x183180cf, # r52_0b_btcm_global + 0x4c: 0x183180d0, # r52_0b_ctcm_global + 0x56: 0x18318106, # r52_0d_atcm_global + 0x57: 0x18318107, # r52_0d_btcm_global + 0x58: 0x18318108, # r52_0d_ctcm_global + 0x59: 0x18318109, # r52_1d_atcm_global + 0x5a: 0x1831810a, # r52_1d_btcm_global + 0x5b: 0x1831810b, # r52_1d_ctcm_global + 0x5c: 0x1831810c, # r52_0e_atcm_global + 0x5d: 0x1831810d, # r52_0e_btcm_global + 0x5e: 0x1831810e, # r52_0e_ctcm_global + 0x5f: 0x1831810f, # r52_1e_atcm_global + 0x60: 0x18318110, # r52_1e_btcm_global + 0x61: 0x18318111, # r52_1e_ctcm_global +} +"""dict[int, int]: Versal2 SCMI TCM IDs mapped to legacy address-table IDs.""" openamp_linux_hosts = [ "psv_cortexa72_0", "psx_cortexa78_0", "psu_cortexa53_0", "cortexa78_0" ] """list[str]: Names of processor nodes recognized as OpenAMP Linux hosts.""" diff --git a/tests/test_openamp.py b/tests/test_openamp.py index a3480886..ef8c8bf0 100644 --- a/tests/test_openamp.py +++ b/tests/test_openamp.py @@ -90,6 +90,30 @@ def pnode(self, phandle): return self._phandles.get(phandle) +def _remoteproc_v2_fixture( + pd_id=0x44, legacy_pd=None, + node_name="r52_0a_atcm_global@eba00000"): + """Build the minimum channel data needed for R52 TCM construction.""" + tcm = LopperNode( + -1, + f"/{node_name}", + name=node_name, + ) + tcm["xlnx,ip-name"] = ["r52_0a_atcm_global"] + if pd_id is not None: + tcm["power-domains"] = [0xA5, pd_id] + if legacy_pd is not None: + tcm["xlnx,power-domain"] = [legacy_pd] + + pd_property = type("PowerDomainProperty", (), {"value": [0xA5, 0]})() + channel_info = { + "cpu_config": openamp_xlnx.CPU_CONFIG.RPU_SPLIT, + "remote_node": object(), + "rpu_core_pd_prop": pd_property, + } + return channel_info, tcm + + def _cpu_selection_fixture(mask, cpu_count=1, first_reg=0): cpus = [ _FakeNode( @@ -417,6 +441,108 @@ def test_openamp_enables_only_selected_linux_uio_timer(monkeypatch): assert unrelated.propval("compatible", list) == ["cdns,ttc"] +@pytest.mark.parametrize( + "platform, pd_id, legacy_pd, node_name, expected_legacy_id", + [ + # Current Versal2 SDTs use an SCMI ID and need no legacy property. + ( + openamp_xlnx.SOC_TYPE.VERSAL2, + 0x44, + None, + "r52_0a_atcm_global@eba00000", + 0x183180CB, + ), + # Older platforms put the address-table ID in power-domains itself. + ( + openamp_xlnx.SOC_TYPE.ZYNQMP, + 15, + None, + "psu_r5_0_atcm_global@ffe00000", + 15, + ), + # Transitional SDTs can fall back to xlnx,power-domain. + ( + openamp_xlnx.SOC_TYPE.VERSAL2, + 0xDEADBEEF, + 0x183180CB, + "r52_0a_atcm_global@eba00000", + 0x183180CB, + ), + ], +) +def test_remoteproc_v2_resolves_current_and_legacy_tcm_ids( + monkeypatch, platform, pd_id, legacy_pd, node_name, + expected_legacy_id): + """Modern, legacy, and transitional TCM IDs resolve address mappings.""" + channel_info, tcm = _remoteproc_v2_fixture( + pd_id, legacy_pd, node_name) + captured = {} + + monkeypatch.setattr( + openamp_xlnx, "determinte_rpu_core", + lambda tree, cpu_config, remote_node: openamp_xlnx.RPU_CORE.RPU_0) + monkeypatch.setattr( + openamp_xlnx, "get_platform", + lambda tree, verbose=0: platform) + + def capture_cluster(tree, platform, cpu_config, ranges, path): + captured["ranges"] = ranges + return True + + def capture_core(tree, info, power_domains, reg, reg_names, path, + platform): + captured["power_domains"] = power_domains + captured["reg"] = reg + captured["reg_names"] = reg_names + return "core" + + monkeypatch.setattr( + openamp_xlnx, "xlnx_remoteproc_v2_add_cluster", capture_cluster) + monkeypatch.setattr( + openamp_xlnx, "xlnx_remoteproc_v2_add_core", capture_core) + + result = openamp_xlnx.xlnx_remoteproc_v2_construct_cluster( + object(), channel_info, [tcm]) + + mapping = openamp_xlnx.legacy_memory_nodes[expected_legacy_id] + assert result == "core" + assert captured["power_domains"] == [0xA5, 0, 0xA5, pd_id] + assert captured["reg"] == mapping["rpu_view"] + assert captured["ranges"] == mapping["system_view"] + assert captured["reg_names"] == ["atcm0"] + + +@pytest.mark.parametrize( + "pd_id, legacy_pd, expected_error", + [ + (None, None, "missing a valid power-domains property"), + (0xDEADBEEF, None, + "no address mapping for power-domains ID 0xdeadbeef"), + (0xDEADBEEF, 0xFEEDFACE, + "power-domains ID 0xdeadbeef, legacy ID 0xfeedface"), + ], +) +def test_remoteproc_v2_reports_invalid_tcm_mapping( + monkeypatch, capsys, pd_id, legacy_pd, expected_error): + """Missing and unknown TCM power-domain IDs have useful diagnostics.""" + channel_info, tcm = _remoteproc_v2_fixture(pd_id, legacy_pd) + + monkeypatch.setattr( + openamp_xlnx, "determinte_rpu_core", + lambda tree, cpu_config, remote_node: openamp_xlnx.RPU_CORE.RPU_0) + monkeypatch.setattr( + openamp_xlnx, "get_platform", + lambda tree, verbose=0: openamp_xlnx.SOC_TYPE.VERSAL2) + + result = openamp_xlnx.xlnx_remoteproc_v2_construct_cluster( + object(), channel_info, [tcm]) + + assert result is False + diagnostic = capsys.readouterr().out + assert tcm.abs_path in diagnostic + assert expected_error in diagnostic + + def test_zephyr_ipc_shm_replaces_domain_carveout_references(): """Consolidated IPC memory replaces deleted domain phandles.""" tree = LopperTree() From 7c767ef71708c706521ba4994434b550e0bb9bcd Mon Sep 17 00:00:00 2001 From: Ben Levinsky Date: Tue, 8 Sep 2026 11:01:28 -0700 Subject: [PATCH 2/2] openamp: fail when relation processing fails The OpenAMP assist logs relation-processing errors but returns False to the generic dispatcher. The dispatcher warns and still exits zero, so builds can accept output without the requested remoteproc nodes. Exit with status 1 at the OpenAMP entry point after relation processing fails. Keep lower-level helpers returning False and add coverage for the fatal conversion. Signed-off-by: Ben Levinsky --- lopper/assists/openamp_xlnx.py | 13 +++++++++++-- tests/test_openamp.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/lopper/assists/openamp_xlnx.py b/lopper/assists/openamp_xlnx.py index 34fbb998..ed658e32 100644 --- a/lopper/assists/openamp_xlnx.py +++ b/lopper/assists/openamp_xlnx.py @@ -1682,9 +1682,13 @@ def xlnx_openamp_parse(sdt, options, verbose = 0 ): Returns: bool: True when processing succeeds or no domains exist, False on errors. + Raises: + SystemExit: If a requested OpenAMP relation cannot be processed. + Algorithm: Parses assist arguments, checks for OpenAMP-compatible domains, delegates - relation handling when appropriate. + relation handling when appropriate. Relation-processing failures are + fatal because continuing would write an incomplete OpenAMP device tree. """ # Xilinx OpenAMP subroutine to parse OpenAMP Channel # information and generate Device Tree information. @@ -1714,6 +1718,11 @@ def xlnx_openamp_parse(sdt, options, verbose = 0 ): if openamp_args["dt_type"] in ["zephyr_dt", "linux_dt"] or openamp_args["openamp_output_filename"]: # if find_only is False, then processing will also occur. if not xlnx_handle_relations(sdt, machine, False, openamp_args["dt_type"]): - return False + _error( + "openamp_xlnx: failed to process OpenAMP relations for " + "processor '%s' and OS '%s'" % + (machine, openamp_args["dt_type"]), + 1, + ) return True diff --git a/tests/test_openamp.py b/tests/test_openamp.py index ef8c8bf0..76674bae 100644 --- a/tests/test_openamp.py +++ b/tests/test_openamp.py @@ -628,3 +628,34 @@ def test_libmetal_missing_processor_lists_supported_targets(monkeypatch, caplog) assert "processor 'psu_cortexr5_0'" in caplog.text assert "APU_Linux (os=linux, processor=cpus_a53)" in caplog.text assert "R5_1_BAREMETAL (os=baremetal, processor=psu_cortexr5_1)" in caplog.text + + +def test_openamp_relation_failure_exits_nonzero(monkeypatch, caplog): + """A requested tree transformation must not report successful output.""" + config = { + "machine": "cortexa78_0", + "dt_type": "linux_dt", + "openamp_output_filename": None, + "report_valid_ipis": False, + } + sdt = type("FakeSdt", (), {"tree": object()})() + + monkeypatch.setattr( + openamp_xlnx, "parse_openamp_args", lambda args: config) + monkeypatch.setattr( + openamp_xlnx, "xlnx_openamp_find_compat_domains", + lambda tree: True) + monkeypatch.setattr( + openamp_xlnx, "xlnx_openamp_update_relation_timers", + lambda sdt, dt_type, machine: True) + monkeypatch.setattr( + openamp_xlnx, "xlnx_handle_relations", + lambda sdt, machine, find_only, os: False) + + with pytest.raises(SystemExit) as error: + openamp_xlnx.xlnx_openamp_parse( + sdt, {"args": ["cortexa78_0", "linux_dt"]}) + + assert error.value.code == 1 + assert "failed to process OpenAMP relations" in caplog.text + assert "processor 'cortexa78_0' and OS 'linux_dt'" in caplog.text