openamp: take RPU remoteproc topology from the SDT, and make OpenAMP failures fatal and consistent - #849
Conversation
The Zephyr assists keep the core-local address of each Cortex-R5 and Cortex-R52 TCM bank type in TCM_LOCAL_ORIGINS. Linux remoteproc nodes need the same addresses, and the two must agree for remoteproc to load Zephyr firmware into TCM. Move the table to xlnx_rpu_tcm.py, which has no other dependencies, so the OpenAMP assist can use it too. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
The remoteproc cluster and core compatibles, the core node name, and whether a cluster carries xlnx,tcm-mode were spread over separate per-platform dicts in three functions, and platform_validate kept its own list of platforms. Keep them in one RpuFamily record per platform, so adding a family means adding one record. No change in output. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
OpenAMP builds Linux remoteproc nodes from a domain tree from which domain_access has removed the RPU cluster nodes, so it cannot read the SDT's description of the remote core. YAML expansion still has it. When a domain runs on one Cortex-R5 or Cortex-R52 core, record on the domain: - rpu_core_num: the core's number across all RPU clusters, the unit address N of its cpus-r5@N or cpus-r52@N cluster, or the core's reg in an older SDT that puts both R5 cores in one cluster; - rpu_tcm_view: the TCM banks the cluster's address-map maps, as (power-domain ID, core address, size) triples. ZynqMP SDTs map each R5 core's banks at their core-local addresses this way; - rpu_cluster_base: the TCM base of the core's two-core RPU cluster, the (N / 2)th aligned TCM span among the SDT's global TCM banks. Each cluster's banks lie in one naturally aligned span (1 MB for R5, 512 KB for R52) that starts at its core 0 ATCM. Nothing reads them yet. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
determinte_rpu_core numbered the remote core by subtracting the platform's first RPU core power-domain ID from the core's ID. That needs a per-platform table of ID ranges, and on Versal2, whose SCMI RPU core IDs start at 0, any value from 0 to 9 in rpu_pd_val read as a core number. Use rpu_core_num, which YAML expansion now takes from the unit address of the core's cpus-r5@N or cpus-r52@N cluster, and keep core_num as the fallback for domains expanded without it. rpu_core_pd_ids is left only for the TCM ownership check, which a later commit removes. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
xlnx_remoteproc_v2_cluster_base_str named the cluster node from a per-platform table of every RPU core's cluster address, which had to grow with each family. Each RPU cluster's TCM banks lie in one naturally aligned span, 1 MB for R5 and 512 KB for R52, that starts at the cluster's core 0 ATCM. Name the cluster after the span holding the remote's banks, so a remote on either core lands in the same node. A remote without TCM uses the cluster base YAML expansion took from the SDT, and a remote whose banks are in two clusters fails. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
In R5 lockstep Linux maps the cluster's second ATCM and BTCM at 0xffe10000 and 0xffe30000, local 0x10000 and 0x30000, as in the xlnx,zynqmp-r5fss lockstep example. Lopper built them from core 1's split-mode banks at 0xffe90000 and 0xffeb0000, which needs it to know from hard-coded power-domain ranges which banks belong to core 1, and to move them. The SDT has nodes for the lockstep banks at their lockstep addresses. Have a lockstep remote list them with core 0's banks, and map every bank at its offset in the cluster's TCM span, named atcm0, btcm0, atcm1 and btcm1. The 2026.2 SDTs give these nodes core 0's power domains, where Linux expects core 1's, so the node then lists core 0's ATCM and BTCM power domains twice. A power domain listed twice was an error. Copy the SDT's values and warn that the output is malformed and the SDT's power-domains need fixing; once they are, the output is correct. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
legacy_memory_nodes gave the bank index and core-local address of every TCM bank, keyed by power-domain ID, with versal2_scmi_to_legacy_pd translating Versal2 SCMI IDs into it. An unknown ID fell back to xlnx,power-domain, which the Versal NET and Versal2 SDTs swap between r52_0b and r52_1a, and rpu_tcm_pd_ids rejected banks of another core by their ID range. Each new family or SDT power-domain scheme needed new table entries. Take each bank's values from the SDT: the type (ATCM, BTCM, CTCM) from its name, and its core-local address from the RPU cluster address-map where the SDT maps the bank there (ZynqMP SDTs map each R5 core's banks at their core-local addresses), or else from the CPU's TCM layout in xlnx_rpu_tcm.py, which the Zephyr assists also use. The bank index is the core's position in its cluster. Drop legacy_memory_nodes, versal2_scmi_to_legacy_pd, rpu_core_pd_ids, rpu_tcm_pd_ids, and the xlnx,power-domain fallback. A bank's owning core is no longer checked; a remote whose banks span two RPU clusters is still rejected. Linux remoteproc nodes generated from the 2026.2 ZCU102, VN-P-B2197 and VEK385 SDTs are unchanged. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
Add a section to the OpenAMP architecture document listing the SDT and YAML source of each value in a generated Linux remoteproc node, the domain properties YAML expansion records for it, and how lockstep clusters are handled, including the warning for the SDT's R5 lockstep power domains. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
openamp_linux_hosts and openamp_remotes are not read anywhere. They listed processor node names, which change between SDT releases (psx_cortexr52_0 became cortexr52_0), so they would also have gone stale. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
The OpenAMP assist reported problems four ways: bare print() with
"ERROR:", "OPENAMP: XLNX: ERROR:" or no prefix at all, Lopper's _error()
and _warning(), and ValueError messages that carried their own
"OPENAMP: XLNX:" prefix. Trace lines (" -> function") went to stdout on
every run, mixed in with the errors, and several messages did not say
which node they were about.
Use one convention:
- errors go through _error("openamp_xlnx: ..."), naming the node or
domain involved, and the function returns False as before;
- warnings go through _warning("openamp_xlnx: ...");
- trace and per-relation progress go through _debug();
- internal helpers keep raising ValueError, without a prefix, and their
callers log it.
Along the way:
- a reserved-memory overlap names both nodes instead of printing a list
of raw numbers;
- an unsupported platform names the root model and compatible, and the
libmetal and OpenAMP header outputs now say so instead of returning
False silently;
- xlnx_rpmsg_update_tree_linux checks for one vdev0buffer carveout once
instead of twice with two messages;
- the cluster-mode check in xlnx_remoteproc_v2_add_cluster is dropped;
xlnx_remoteproc_v2_construct_cluster rejects every mismatch first, so
it could not be reached;
- xlnx_remoteproc_rpu_parse no longer repeats the cpu_config error that
determine_cpus_config has just logged;
- "no OpenAMP domains found" is logged at info level whatever the
verbosity, instead of as a warning only at verbosity 2.
The report of valid IPIs keeps printing to stdout: that output is what
the user asked for. Which failures stop the build is unchanged.
Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
Lopper only warns when an assist returns False or raises, and exits zero. Relation processing and the libmetal CMake output called _error(..., 1) themselves, but other failures did not stop the build: - an OpenAMP header that could not be written (for example, without vring carveouts); - assist arguments that name no processor, or a libmetal output without a compatible string; - any exception, such as the AttributeError a remoteproc relation gets when its remote names no domain, which left the tree without remoteproc nodes; - non-contiguous Zephyr IPC carveouts, whose ValueError nothing caught. Make xlnx_openamp_parse the one place OpenAMP failures stop the build: it exits with status 1 when its steps return False, and on any exception, which it reports with the file, line and function that raised it. The steps log their reason and return False instead of exiting themselves, and the Zephyr IPC check is reported like the others. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
A remoteproc relation with no relation0, relation1, ... children, as in YAML written for the older list format, gave no relations to process, and the only error was the generic "failed to process OpenAMP relations". A remote or elfload entry that YAML expansion could not resolve stayed a string, and remoteproc processing then failed with an AttributeError far from the cause. Report a relation without children, and a remote or elfload entry that is not a node, naming the relation and the value. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
|
Thanks for doing the changes I requested on the last review! Full suite is clean here, 124 legacy and 1358 pytest, on current master with Which brings me to what I wanted to ask. You wrote:
I do not see those. Master is clean for me, and so is this branch .. zero One small thought, not a request. The wrapper catches bare |
This follows up on the review of the RPU fixes series (merged up to "openamp: map R5 lockstep TCM as the Linux binding describes").
Remoteproc: take the RPU topology from the SDT
Linux remoteproc construction relied on hard-coded tables:
When a power-domain ID wasn't in the tables, Lopper fell back to xlnx,power-domain, which the Versal NET and Versal2 SDTs swap between r52_0b and r52_1a.
Error handling
One logging convention. Errors go through _error("openamp_xlnx: …") and name the node or domain involved. Warnings use _warning. Trace and progress messages use _debug instead of going to
stdout. Internal helpers raise ValueError without a prefix, and callers log it. This covers the review note about print("ERROR …") vs _error().
One exit point. xlnx_openamp_parse is now the one place OpenAMP failures stop the build. It exits 1 whenever a step fails and on any exception, reporting the file, line and function that
raised it. These used to exit 0:
an OpenAMP header that couldn't be written;
invalid assist arguments;
non-contiguous Zephyr IPC carveouts;
exceptions such as the AttributeError from an unresolved remote, which left the tree without remoteproc nodes.
Clearer relation errors. A remoteproc relation with no relation0, relation1, … children (the old list format) is reported. So is a remote or elfload entry that isn't a node.
Testing
Unit tests: OpenAMP, libmetal, gen_domain and YAML tests pass (174 passed, 2 skipped), and each commit passes the OpenAMP tests on its own. The full suite passed before the three error-
handling commits, except for five SDT-assembly and external-assist tests that fail the same way on master.
New coverage: a cluster-construction matrix that checks every ranges, reg, reg-names and power-domains value. It covers:
split mode on every family, with both cores, core 0 only and core 1 only;
both Versal NET clusters, and all five Versal2 clusters;
R5 lockstep with today's and with fixed SDT power domains;
R52 lockstep on several clusters;
remotes without TCM;
YAML expansion of the SDT topology;
exit status and messages for the failure paths.
Real SDTs: 2026.2 ZCU102, K24c, VRK165, VN-P-B2197 and VEK385, plus VCK190 for the Versal CIPS overlay.
Generated split-mode remoteproc nodes are byte-identical to master's.
Lockstep produces the expected output on all three families tested (ZynqMP R5, Versal NET, Versal2).
Vitis and Yocto flows: the Vitis (openamp_sdt default domain YAMLs) and Yocto (meta-xilinx conf/domainyaml) flows were run step for step, including header, libmetal and linker outputs. Exit
status and outputs are identical to master in every flow. The Versal2 ISP overlay, converted to relation6–relation9, generates remoteproc for cores 6–9 (clusters D and E).