overlay: Resolve phandle fixups in child nodes#765
Open
arthokal wants to merge 27 commits into
Open
Conversation
Add new assist that scans the System Device Tree for devices under bus
nodes (e.g., simple-bus) and generates a YAML file containing a domain
with all devices in its access list. This enables glob patterns like
"dev: '*serial*'" to work without requiring a pre-existing parent domain.
The assist discovers all addressable devices (nodes with @ in name) under
bus-compatible nodes and generates a YAML structure:
domains:
sdt_all_devices:
compatible: lopper,sdt-devices-v1
access:
- dev: serial@ff000000
label: serial0
...
Usage:
lopper system.dts - -- sdt_devices -o /tmp/sdt-devices.yaml
lopper -f --permissive --enhanced \
-x '*.yaml' \
-i /tmp/sdt-devices.yaml \
-i my-domain.yaml \
system.dts output.dts
Options:
-b, --bus-types Comma-separated bus compatible strings (default: simple-bus)
-n, --domain-name Name for generated domain (default: sdt_all_devices)
-o Output file path
Also adds lop-sdt-devices.dts for automatic invocation and comprehensive
test coverage in tests/test_sdt_devices.py.
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
…nodes Extend the sdt_devices assist to discover devices across multiple categories, not just bus-attached devices. This enables comprehensive SDT device YAML generation for use as parent domains in glob-based device matching. Device Categories: - bus: Devices under simple-bus or other bus nodes (existing) - cpu: CPU clusters discovered via device_type="cpu" - memory: Memory nodes, reserved-memory, SRAM/TCM regions - firmware: /firmware/* nodes, IPI mailboxes - toplevel: Non-bus devices directly under root New Options: - -c, --categories: Comma-separated categories to include (default: all) - --exclude-categories: Categories to exclude from discovery - --include-pattern: Regex pattern for node names to include - --exclude-pattern: Regex pattern for node names to exclude The generated YAML now includes separate properties matching isospec format: - cpus: CPU cluster entries - memory: Main memory entries - sram: SRAM/TCM entries - access: Bus devices, firmware, toplevel nodes Examples: # All categories (default) lopper system.dts - -- sdt_devices -o /tmp/all-devices.yaml # Only CPUs and memory lopper system.dts - -- sdt_devices -c cpu,memory -o /tmp/cpu-mem.yaml # Exclude firmware lopper system.dts - -- sdt_devices --exclude-categories firmware -o out.yaml # Only serial devices lopper system.dts - -- sdt_devices --include-pattern "serial@.*" -o serial.yaml Backwards compatible: default behavior includes all categories. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Replace hardcoded node name patterns with a property-based approach for filtering structural nodes. Nodes without a 'compatible' property (like port@*, endpoint@*, channel@*) are now excluded because actual devices always have compatible strings identifying their driver. Also fix subnodes iteration to use child_nodes.values() directly since subnodes(children_only=True) still returns all descendants. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Major improvements to device filtering for domain partitioning: 1. Add INFRASTRUCTURE_COMPAT_PATTERNS list to exclude non-splittable devices that cannot be independently assigned to domains: - Clock nodes (fixed-clock, fixed-factor-clock, clock-controller) - Interrupt controllers (arm,gic, interrupt-controller) - IPI mailbox destinations (xlnx,versal-ipi-dest-mailbox) - System infrastructure (SMMU, IOMMU, reset-controller, etc.) 2. Update _is_actual_device() to check compatible strings against infrastructure patterns - this filters out nodes that have compatible properties but aren't assignable devices. 3. Apply _is_actual_device() filter to firmware and toplevel discovery, not just bus devices. 4. Improve memory/SRAM output with proper size parsing: - Parse reg property using parent #address-cells/#size-cells - Format sizes in human-readable form (64K, 2G, etc.) - Include both start address and size in output 5. Fix firmware IPI discovery to only include IPI controllers, not destination mailbox child nodes. Note: Clocks are currently filtered since they typically can't be shared between domains (frequency control conflicts). If exclusive clock assignment is needed, this could be made configurable. Result: Device count reduced from 601 to 326 meaningful devices that can actually be assigned to isolation domains. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Add --include-clocks option to optionally include clock nodes in the generated device list. By default, clocks are excluded since they typically can't be shared between domains (frequency control conflicts). Separate clock-related compatible patterns into CLOCK_COMPAT_PATTERNS list so they can be conditionally included based on the new option. Add documentation note that clock domain assignment is not currently implemented - when included, clocks appear in the device list but no special handling is performed. Future work could add exclusive clock assignment to prevent multiple domains from controlling the same clock frequency. Usage: # Default: clocks excluded lopper system.dts - -- sdt_devices -o devices.yaml # Include clocks lopper system.dts - -- sdt_devices --include-clocks -o devices.yaml Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Enhanced the sdt_devices assist with additional fields derived from SDT: CPU enhancements (matching reference domain YAML format): - One entry per CPU (not per cluster) - cluster: cluster name/label - cluster_cpu: individual CPU label/name - cpumask: hex format (0x1, 0x2, 0x4, etc.) based on CPU position - compatible: CPU type string Memory enhancements: - start: hex format address (e.g., 0xfffc0000) - size: hex format size (e.g., 0x40000) - removed human-readable format Device enhancements: - status: include status property for disabled devices (omitted when "okay") Reserved-memory enhancements: - no-map: boolean flag when present - reusable: boolean flag when present Note: Values output as hex strings due to LopperYAML's JSON intermediate conversion. Downstream processing handles both string and integer formats. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Modified LopperYAML.to_yaml() to preserve ruamel.yaml scalar types like HexInt when converting OrderedDicts to regular dicts: - Added _convert_ordered_dict() helper function that recursively converts OrderedDicts without using JSON serialization - Changed YAML dumper from 'safe' to 'rt' (round-trip) type to support HexInt representation - This allows values like cpumask, start, and size to be output as proper hex integers (0x1, 0xfffc0000) instead of decimal or strings Updated sdt_devices assist to use HexInt for: - cpumask: CPU position bitmask - start: memory region start address - size: memory region size Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Add categorized infrastructure device filtering with selective inclusion: - Reorganize flat INFRASTRUCTURE_COMPAT_PATTERNS into 15 named categories: interrupt, bus, ipi, smmu, power, syscon, phy, reset, pinctrl, misc, slcr, interconnect, protection, cpu-ctrl, platform - Add new patterns for non-PMU-controlled devices: - slcr: *slcr*, *_crf_*, *_crl_* (clock/reset control) - interconnect: *_gpv@*, *_cci_*, *_afi_* (fabric) - protection: *xmpu*, *xppu* (can't protect themselves) - cpu-ctrl: *_apu_*, *_rpu_* (cluster control registers) - platform: *_siou@*, *iouslcr* (platform config) - Add --include-infrastructure option to include specific categories - Add --list-infrastructure option to show available categories - Apply infrastructure filtering to SRAM/TCM discovery path - Add performance investigation TODO to agent documentation Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Add try/except handling for optional mode and cpumask fields in cpu_expand() to support the sdt_devices format which may not have these fields populated. Also support 'dev' key as alternative to 'cluster' for CPU entries generated by the sdt_devices assist. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Change sdt_devices assist to use the standard openamp,domain-v1,devices compatible string instead of the custom lopper,sdt-devices-v1 string. This allows the generated device inventory to be automatically detected as the parent domain for glob expansion via the ',devices' suffix. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Add render_cpu_access_map() and render_all_cpu_access_maps() functions
to provide ASCII visualization of CPU cluster device accessibility based
on address-map properties. Shows address ranges, sizes, and device names.
CPU clusters without address-map are shown as having unrestricted system
access, which is typical for application processors in heterogeneous SoCs.
New command-line options:
--cpumap=<file> Output CPU access map visualization (use - for stdout)
--cpumap-expand Expand bus nodes to show child devices
Example usage:
lopper --cpumap=- system-device-tree.dts
lopper --cpumap=- --cpumap-expand system-device-tree.dts
Sample output (with --cpumap-expand):
============================================================
CPU Cluster: cpus
Path: /cpus
============================================================
(no address-map - unrestricted system access)
============================================================
CPU Cluster: cpus_r5
Path: /cpus-cluster@0
============================================================
Address Range Size Device
------------------------------------ ------------ ------
0x00000000 - 0x7fffffff 2048 MB memory0
0xf1000000 - 0xffafffff 235 MB axi
├─ 0xfd000000 64 KB cci
├─ 0xff060000 24 KB can0
├─ 0xff070000 24 KB can1
└─ 0xffa80000 4 KB dma0
0xf9000000 - 0xf900ffff 64 KB rpu_bus
└─ 0xf9000000 4 KB gic
Total mappings: 4
Unique devices: 4
Signed-off-by: Bruce Ashfield <bruce.ashfield@gmail.com>
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
The existing schema system has two parallel implementations that don't share data structures: lopper/schema.py for type inference and lopper/audit/schema.py for constraint validation. This leads to duplicated type definitions and maintenance burden. Create lopper/schema/ package as Phase 1 of unification: - types.py: PropertyType enum and TypeDefinition dataclass - core.py: ConstraintType, PropertySpec, NodeSpec, SchemaRegistry - loader.py: Schema search path and YAML loading - learned.py: Renamed from schema.py (unchanged content) - dt-schema/: Moved from lopper/dt-schema/ for package cohesion The package supports user/vendor schema extensions via search path: 1. Built-in dt-schema (lopper/schema/dt-schema/schemas/) 2. User schemas (~/.config/lopper/schemas/) 3. LOPPER_SCHEMA_PATH environment variable 4. --schema-dir command-line option All existing exports are preserved via __init__.py re-exports for backwards compatibility. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
The learned schema system uses LopperFmt internally but the new unified schema system uses PropertyType and PropertySpec. To bridge these two systems without breaking existing code, add resolve_property_spec() to DTSPropertyTypeResolver. This method wraps get_property_type() and returns a PropertySpec with: - PropertyType (converted from LopperFmt) - TypeDefinition with source tracking - Confidence scores (0.9 path-specific, 0.85 compatible-specific, 0.8 learned, 0.7 heuristic-exact, 0.5 heuristic-suffix) - Preserved metadata (type_frequencies, phandle_pattern, context_lookups) The key invariant is that resolve_property_spec().type_def.property_type .to_lopper_fmt() == get_property_type() for all inputs. Also adds comprehensive test coverage: - test_schema_learned.py: 99 tests for learned schema behavior - test_schema_types.py: 65 tests for PropertyType/TypeDefinition Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Phase 3 of schema unification. The audit/schema.py module was defining its own ConstraintType enum and PropertyConstraint dataclass, duplicating what now exists in lopper/schema/core.py. This change imports ConstraintType and Constraint from the unified schema package and aliases PropertyConstraint to Constraint for backwards compatibility. All existing code using PropertyConstraint continues to work unchanged. The NodeConstraints dataclass is kept locally since it has audit-specific fields (schema_file) not present in the equivalent NodeSpec. Tests added to verify: - ConstraintType is the same object as lopper.schema.core.ConstraintType - PropertyConstraint is an alias for Constraint - lopper.audit exports both Constraint and PropertyConstraint - All 42 audit schema tests pass Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Connect the audit validation framework to the learned schema system.
When a learned schema is available, property values can now be checked
against the types learned from observing device trees.
New warning flags:
- schema_learned_types: Check property values match learned types
- schema_type_frequency: Detect minority type usage anomalies
- schema_learned: Meta-flag enabling both learned checks
New helper functions:
- _infer_type_from_value(): Infer PropertyType from value
- _types_compatible(): Check if two types are compatible
- _get_resolver(): Get the property type resolver
The checks use the unified PropertyType from lopper.schema.types,
completing the integration between the audit and schema packages.
High-confidence learned types (>0.8) are used to validate property
values, catching type mismatches early.
Usage:
lopper -W schema_learned input.dts output.dts
lopper -W schema_all input.dts output.dts # includes learned checks
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Bug fixes discovered during real device tree testing:
1. Fix resolver access: SchemaManager uses 'resolver' property, not
'_resolver' attribute. Try both for compatibility.
2. Fix property iteration: LopperTree nodes yield properties when
iterated, not property names. Use 'for prop in node' not
'for prop_name in node.props'.
3. Fix empty value handling: Empty properties like 'ranges;' produce
[''] (list with empty string), not []. Detect this and infer
EMPTY type correctly.
4. Expand type compatibility:
- All integer types (scalar and array) compatible with each other
- FLAG type compatible with integer values (ranges can be empty
or have values)
5. Add tests for empty string and [''] inference.
Tested against specification/system-device-tree-no-alias.dts:
- 81 nodes loaded
- 133 properties checked against learned schema
- 0 violations (down from 12 false positives)
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
The schema unification introduced new APIs (get_registry(), SchemaRegistry, resolve_property_spec()) but the legacy APIs (get_schema_manager(), SchemaManager, get_property_type()) were still being used without warning. Add deprecation warnings to guide users toward the new unified APIs: - get_schema_manager() warns to use get_registry() - SchemaManager class warns to use SchemaRegistry - DTSPropertyTypeResolver.get_property_type() docstring notes deprecation Internal code uses _get_schema_manager() directly to avoid spurious warnings in internal code paths while public API users get the warning. The _DEPRECATION_WARNINGS_ENABLED flag allows disabling warnings for testing scenarios. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
The tree_with_address_map fixture was missing from conftest.py, so 5 tests in TestAccessibleByMultipleClusters and TestAccessibleByWithSystemTree were always skipped rather than failing gracefully. Add a self-contained tree_with_address_map fixture directly in the test file. Phandles are set via the node property setter *after* tree.add() so the __pnodes__ index is populated correctly. Address-map properties are added after phandles are registered so phandle resolution works. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
…ference pruning
Add PATH_REF and ALIAS_REF to PropertyType and the schema learning phase so
that properties holding absolute node paths or alias names are tracked as
distinct types rather than generic strings.
Two new passes run inside the strict-mode pre-output block in LopperTree.print():
Pass A (path-ref): walks all nodes and removes any string property whose
value is an absolute node path ("/foo/bar@0") that no longer exists in the
tree. This cleans up stale /aliases entries left behind after assists like
domain_access delete nodes from the tree.
Pass B (alias-ref): checks properties registered in alias_ref_properties
(seeded with stdout-path and linux,stdout-path) and removes any whose
referenced alias name was itself pruned by pass A.
The learning phase detects path-ref (quoted string starting with '/') and
alias-ref (property name in alias_ref_properties hint list) in
_determine_property_type(), and the new types are registered in
DT_SCHEMA_TYPES with LopperFmt.STRING as their wire type.
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Add TestPathRefPruning class to test_glob_access.py with three tests:
- test_path_ref_pruning_removes_dangling_alias: pass A removes a /aliases
entry whose target node no longer exists in the tree.
- test_path_ref_pruning_preserves_valid_alias: pass A leaves intact aliases
that still resolve to live nodes.
- test_alias_ref_pruning_removes_dangling_stdout_path: pass B removes
stdout-path when the alias it references was pruned by pass A.
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
When a node is extracted into an overlay its lopper-comment-* children go with it, leaving dangling path-ref properties in the base tree that strict mode correctly drops. These are expected and not actionable, so suppress the warning for any property whose name starts with 'lopper-comment-' to avoid misleading noise in normal overlay workflows. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Add test_domain_access_phandle.py and its SDT fixture to guard against the two bugs fixed in domain_access step 2c: - carveout/mbox/timer nodes under /axi are preserved after domain_access when referenced by domain-to-domain relation0 phandles (depth regression) - the /axi simple-bus parent node itself is not deleted by filter devicetree-org#1 when its children are refcounted (parent-chain regression) Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
…ions _deserialize_overlay_node() calls append() on node.child_nodes, which is an OrderedDict, causing: AttributeError: 'collections.OrderedDict' object has no attribute 'append' The issue is observed in a two-pass flow where pass 1 runs xlnx_overlay_pl_dt with an overlay file containing child nodes (e.g., updated_zocl.dtsi with zyxclmm_drm under &amba_pl). Pass 1 embeds the overlay data into the output DTS via _serialize_overlay_subtrees. Pass 2 (gen_domain_dts) crashes at startup when _deserialize_overlay_subtrees tries to reconstruct the embedded child nodes. Fix by assigning node.parent = parent at the start of _deserialize_overlay_node so the parent backref is established before any property or child processing, and by using dict-style insertion (node.child_nodes[child.abs_path] = child) consistent with how child_nodes is managed throughout tree.py. Signed-off-by: Aravind Thokala <aravind.thokala@amd.com> Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Add TestDeserializeOverlayChildNodes to test_overlay_e2e.py to cover the bug where _deserialize_overlay_node() called append() on child_nodes (an OrderedDict), crashing on any overlay with nested child nodes. Tests verify: no AttributeError on round-trip, child_nodes stays an OrderedDict, child count and props survive serialization, parent backref is correctly set, and each child is keyed by abs_path. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
When a DTS with C-style comments is loaded via libfdt in enhanced mode, dtc encodes comments as lopper-comment-* properties whose values are raw bytes (ints) in the FDT dict. The resolve() comment branch assumed all values were already str, causing: TypeError: can only concatenate str (not "int") to str at outstring += s. Fix by decoding bytes values to str; plain str values pass through unchanged. This is a pre-existing bug first exposed by the test_domain_access_phandle tests, which are the first tests to run the full pipeline with libfdt=True against a DTS that has inline comments. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
The previous fix assumed libfdt returned comment prop values as bytes, but the apt-packaged dtc/libfdt version returns them as bare ints (Unicode codepoints), causing: AttributeError: 'int' object has no attribute 'decode' Handle all three types: int -> chr(), bytes/bytearray -> decode(), str -> pass through unchanged. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Phandle fixups (0xffffffff placeholders) were only resolved for direct properties of the __overlay__ node. Properties on child nodes (e.g., xlnx,memory-region in zyxclmm_drm) were deep-copied with unresolved values, causing them to be silently dropped at write time. Fix this by iterating label_to_fixups and patching nodes via ov_tree[path] directly, which resolves fixups across __overlay__ and all its children in one pass before any copying happens. Signed-off-by: Aravind Thokala <aravind.thokala@amd.com>
Contributor
Author
|
@onkarharsh, |
Collaborator
|
The current resolution architecture was on purpose and should only be done on write time. I don't have access to my full setup to test this, as I'm on vacation this week. Does this change preserve the phandle resolution at write time ? I can't tell from looking at the patch. Could you also attach some sample inputs and expected outputs ? So I can reproduce and test the issue myself. |
Contributor
Author
|
Hi @zeddii , Sorry for the delay in response. The original code already resolves phandle fixups at unwrap time. This patch only extends that same resolution to child nodes, which were being missed. I emailed you the artifacts to reproduce the issue. Please check. CC: @onkarharsh |
zeddii
pushed a commit
that referenced
this pull request
May 25, 2026
…fixups Previously _unwrap_overlay_tree() resolved 0xffffffff phandle placeholders at overlay registration time, looking up labels against the base tree snapshot. This was wrong for two reasons: 1. The base tree can still be modified by assists before overlay_tree() is called, making the resolved phandle values potentially stale. 2. The resolution only covered direct properties of the __overlay__ node; child node properties (e.g. zyxclmm_drm under amba_pl) were deep-copied with raw 0xffffffff values that were silently dropped at write time. PR #765 attempted to fix (2) by extending the early resolution to child nodes, but (1) remains a correctness problem regardless. The fix is a single resolution path at the right time: - _unwrap_overlay_tree() now copies props and child nodes as-is, leaving 0xffffffff placeholders intact. It rewrites the __fixups__ path refs from the dtc fragment layout (/fragment@N/__overlay__[/child]) to the real target abs_paths, and returns (result_nodes, rewritten_fixups). - The caller stores rewritten_fixups in overlay_fixups[stem] metadata. - _build_overlay_tree() already called _resolve_overlay_fixups() for the YAML overlay path; it now does so for DTS overlays too. Resolution runs against the fully merged result tree, so phandles are always correct and child nodes are covered automatically because _resolve_overlay_fixups() patches nodes by abs_path regardless of nesting depth. - _resolve_overlay_fixups() updated to accept the plain dict directly rather than a LopperNode wrapper. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
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
2 participants
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.
Issue:
When user overlay files (passed via
-i) contain child nodes with phandle references, those references are silently dropped in the generated pl.dtsiFor example,
updated_zocl.dtsicontains:The generated overlay has
zyxclmm_drmwithxlnx,memory-regionmissing.Root Cause:
_unwrap_overlay_tree()inlopper/__init__.pyresolves0xffffffffphandle placeholders (left by dtc plugin compilation) only for direct properties of the__overlay__node.Child node properties (like those in
zyxclmm_drm) are deep-copied as-is with unresolved0xffffffffvalues, which are silently dropped at write time.Fix:
Instead of iterating properties and searching for matching fixups (inside-out), iterate
label_to_fixupsand patch nodes directly viaov_tree[path] (outside-in). This resolves phandles acrossoverlay` and all its children in one pass before any copying happens.Example Lopper command: