Skip to content

openamp: take RPU remoteproc topology from the SDT, and make OpenAMP failures fatal and consistent - #849

Merged
zeddii merged 12 commits into
devicetree-org:masterfrom
bentheredonethat:openamp-rpu-sdt-topology-fixes-oct-1-2026
Oct 2, 2026
Merged

zeddii merged 12 commits into
devicetree-org:masterfrom
bentheredonethat:openamp-rpu-sdt-topology-fixes-oct-1-2026

Conversation

@bentheredonethat

@bentheredonethat bentheredonethat commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

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:

  • legacy_memory_nodes: the bank index and core-local address of every TCM bank, keyed by power-domain ID;
  • versal2_scmi_to_legacy_pd: translated Versal2 SCMI IDs into that table;
  • rpu_core_pd_ids / rpu_tcm_pd_ids: per-SoC power-domain ID ranges used to number cores and assign banks;
  • a per-core table of cluster addresses.

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).

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>
@bentheredonethat bentheredonethat changed the title Openamp rpu sdt topology fixes oct 1 2026 openamp: take RPU remoteproc topology from the SDT, and make OpenAMP failures fatal and consistent Oct 1, 2026
@zeddii

zeddii commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

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
all twelve commits applied. The new tests assert concrete ranges, reg,
reg-names and power-domains values rather than just running, which for the
cluster matrix is the part that counts.

Which brings me to what I wanted to ask. You wrote:

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.

I do not see those. Master is clean for me, and so is this branch .. zero
failures in both halves, run the way CI runs them. So it is either something in
your environment, a different master, or a part of the suite I am invoking
differently. Worth pinning down before it settles into a known-bad that nobody
questions again. Which five are they, and what does the failure look like?

One small thought, not a request. The wrapper catches bare Exception and
reports a single origin frame, which is right for someone running a build. It
does drop the traceback, which is what you want the first time an internal
error turns up in a new flow. Keeping the full traceback behind -v would cost
very little.

@zeddii
zeddii merged commit 5390776 into devicetree-org:master Oct 2, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants