From 63e0abdb519347cb00683d6e141463f9a94deb50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Ma=C4=87kowski?= Date: Mon, 19 Oct 2026 14:16:06 +0000 Subject: [PATCH] feat(standard)!: add Tree and TreeStandard traits This allows to use the `standard` module wrappers (cpu, memory, etc.) with the `model` API and unifies the `find_node` method between `fdt` and `model` APIs. --- fuzz/fuzz_targets/parse.rs | 2 +- src/fdt.rs | 26 +++-- src/lib.rs | 35 +++++++ src/model.rs | 24 +++-- src/standard.rs | 52 +++++++++- src/standard/chosen.rs | 14 +-- src/standard/cpus.rs | 21 +---- src/standard/memory.rs | 35 ++----- tests/fdt.rs | 2 +- tests/standard.rs | 189 ++++++++++++++++++++++++++++++------- 10 files changed, 280 insertions(+), 120 deletions(-) diff --git a/fuzz/fuzz_targets/parse.rs b/fuzz/fuzz_targets/parse.rs index 558b951..006e3ba 100644 --- a/fuzz/fuzz_targets/parse.rs +++ b/fuzz/fuzz_targets/parse.rs @@ -14,7 +14,7 @@ use dtoolkit::{ Node, Property, fdt::{Fdt, FdtNode}, - standard::NodeStandard, + standard::{NodeStandard, TreeStandard}, }; use libfuzzer_sys::fuzz_target; diff --git a/src/fdt.rs b/src/fdt.rs index 5ef9119..4087160 100644 --- a/src/fdt.rs +++ b/src/fdt.rs @@ -33,9 +33,9 @@ use zerocopy_derive::*; pub use self::node::FdtNode; pub use self::property::FdtProperty; -use crate::Node; use crate::error::{FdtErrorKind, FdtParseError}; use crate::memreserve::MemoryReservation; +use crate::{Node, Tree}; /// Version of the FDT specification supported by this library. const FDT_VERSION: u32 = 17; @@ -185,6 +185,15 @@ impl TryFrom for FdtToken { } } +impl<'a> Tree<'a> for Fdt<'a> { + type Node = FdtNode<'a>; + + fn root(self) -> FdtNode<'a> { + let offset = self.header().off_dt_struct() as usize; + FdtNode::new(self, offset) + } +} + impl<'a> Fdt<'a> { /// Creates a new `Fdt` from the given byte slice. /// @@ -664,8 +673,7 @@ impl<'a> Fdt<'a> { /// ``` #[must_use] pub fn root(self) -> FdtNode<'a> { - let offset = self.header().off_dt_struct() as usize; - FdtNode::new(self, offset) + Tree::root(self) } /// Finds a node by its path. @@ -719,17 +727,7 @@ impl<'a> Fdt<'a> { /// ``` #[must_use] pub fn find_node(self, path: &str) -> Option> { - if !path.starts_with('/') { - return None; - } - let mut current_node = self.root(); - if path == "/" { - return Some(current_node); - } - for component in path.split('/').filter(|s| !s.is_empty()) { - current_node = current_node.child(component)?; - } - Some(current_node) + Tree::find_node(self, path) } /// Returns the node at the given byte offset relative to the start of the diff --git a/src/lib.rs b/src/lib.rs index 62203cb..e4d5af6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -144,6 +144,41 @@ pub use crate::values::{ FdtStringListIterator, FromPropertyValue, PropEncodedArrayIterator, ToPropertyValue, }; +/// A device tree. +pub trait Tree<'a>: Copy { + /// The type used for nodes of the device tree. + type Node: Node<'a>; + + /// Returns the root node of the device tree. + #[must_use] + fn root(self) -> Self::Node; + + /// Finds a node by its path. + /// + /// If a name in the given path contains a _unit-address_ (the part after + /// the `@` sign) then both the _node-name_ and _unit-address_ must + /// match. If it doesn't have a _unit-address_, then nodes with any + /// _unit-address_ or none will be allowed. + /// + /// For example, searching for `/cpus/cpu` would match either `/cpus/cpu`, + /// `/cpus/cpu@0` or `/cpus/cpu@1`, while `/cpus/cpu@1` would match only the + /// latter. + #[must_use] + fn find_node(self, path: &str) -> Option { + if !path.starts_with('/') { + return None; + } + let mut current_node = self.root(); + if path == "/" { + return Some(current_node); + } + for component in path.split('/').filter(|s| !s.is_empty()) { + current_node = current_node.child(component)?; + } + Some(current_node) + } +} + /// A device tree node. pub trait Node<'a>: Copy { /// The type used for properties of the node. diff --git a/src/model.rs b/src/model.rs index 7889b8b..bc12cfb 100644 --- a/src/model.rs +++ b/src/model.rs @@ -25,7 +25,7 @@ use core::fmt::Display; pub use node::{DeviceTreeNode, DeviceTreeNodeBuilder, DeviceTreeNodeRef}; pub use property::DeviceTreeProperty; -use crate::Node; +use crate::Tree; use crate::fdt::Fdt; use crate::memreserve::MemoryReservation; @@ -51,6 +51,14 @@ pub struct DeviceTree { pub memory_reservations: Vec, } +impl<'a> Tree<'a> for &'a DeviceTree { + type Node = DeviceTreeNodeRef<'a>; + + fn root(self) -> DeviceTreeNodeRef<'a> { + self.root.as_read_only() + } +} + impl DeviceTree { /// Creates a new `DeviceTree` with the given root node. /// @@ -95,7 +103,7 @@ impl DeviceTree { /// Returns a read-only view of the root node of the device tree. #[must_use] pub fn root(&self) -> DeviceTreeNodeRef<'_> { - self.root.as_read_only() + Tree::root(self) } /// Finds a node by its path and returns a read-only view of it. @@ -119,17 +127,7 @@ impl DeviceTree { /// ``` #[must_use] pub fn find_node(&self, path: &str) -> Option> { - if !path.starts_with('/') { - return None; - } - let mut current_node = self.root(); - if path == "/" { - return Some(current_node); - } - for component in path.split('/').filter(|s| !s.is_empty()) { - current_node = current_node.child(component)?; - } - Some(current_node) + Tree::find_node(self, path) } /// Finds a node by its path and returns a mutable reference to it. diff --git a/src/standard.rs b/src/standard.rs index f838c6e..f1506d4 100644 --- a/src/standard.rs +++ b/src/standard.rs @@ -22,11 +22,61 @@ pub use self::ranges::Range; pub use self::reg::Reg; pub use self::status::Status; use crate::error::{PropertyError, StandardError}; -use crate::{Node, Property}; +use crate::{Node, Property, Tree}; pub(crate) const DEFAULT_ADDRESS_CELLS: u32 = 2; pub(crate) const DEFAULT_SIZE_CELLS: u32 = 1; +/// Methods to access standard nodes on a device tree. +pub trait TreeStandard<'a>: Tree<'a> { + /// Returns the `/cpus` node. + /// + /// This should always be included in a valid device tree. + /// + /// # Errors + /// + /// Returns [`StandardError::CpusMissing`] if the CPUs node is missing. + fn cpus(self) -> Result, StandardError> { + let node = self.find_node("/cpus").ok_or(StandardError::CpusMissing)?; + Ok(Cpus::new(node)) + } + + /// Returns the `/memory` node. + /// + /// This should always be included in a valid device tree. + /// + /// # Errors + /// + /// Returns [`StandardError::MemoryMissing`] if the memory node is missing. + fn memory(self) -> Result, StandardError> { + let node = self + .find_node("/memory") + .ok_or(StandardError::MemoryMissing)?; + Ok(Memory::new(node)) + } + + /// Returns the `/reserved-memory/*` nodes, if any. + #[must_use] + fn reserved_memory( + self, + ) -> Option> + use<'a, Self>> { + Some( + self.find_node("/reserved-memory")? + .children() + .map(ReservedMemory::new), + ) + } + + /// Returns the `/chosen` node, if it exists. + #[must_use] + fn chosen(self) -> Option> { + let node = self.find_node("/chosen")?; + Some(Chosen::new(node)) + } +} + +impl<'a, T: Tree<'a>> TreeStandard<'a> for T {} + /// Methods to access standard properties on FDT nodes. pub trait NodeStandard<'a>: Node<'a> { /// Returns the value of the standard `compatible` property. diff --git a/src/standard/chosen.rs b/src/standard/chosen.rs index 2037936..31ac3d3 100644 --- a/src/standard/chosen.rs +++ b/src/standard/chosen.rs @@ -10,18 +10,8 @@ use core::fmt::{self, Display, Formatter}; use core::ops::Deref; use crate::error::PropertyError; -use crate::fdt::{Fdt, FdtNode}; use crate::{Node, Property}; -impl<'a> Fdt<'a> { - /// Returns the `/chosen` node, if it exists. - #[must_use] - pub fn chosen(self) -> Option>> { - let node = self.find_node("/chosen")?; - Some(Chosen { node }) - } -} - /// Typed wrapper for a `/chosen` node. #[derive(Debug, Clone, Copy)] pub struct Chosen { @@ -43,6 +33,10 @@ impl Display for Chosen { } impl<'a, N: Node<'a>> Chosen { + pub(super) const fn new(node: N) -> Self { + Self { node } + } + /// Returns the value of the standard `bootargs` property. /// /// # Errors diff --git a/src/standard/cpus.rs b/src/standard/cpus.rs index ea805d8..1ba06aa 100644 --- a/src/standard/cpus.rs +++ b/src/standard/cpus.rs @@ -10,27 +10,10 @@ use core::fmt::{self, Display, Formatter}; use core::ops::Deref; use crate::error::{PropertyError, StandardError}; -use crate::fdt::{Fdt, FdtNode}; use crate::standard::NodeStandard; use crate::values::FdtStringListIterator; use crate::{Cells, Node, Property}; -impl<'a> Fdt<'a> { - /// Returns the `/cpus` node. - /// - /// This should always be included in a valid device tree. - /// - /// # Errors - /// - /// Returns a parse error if there was a problem reading the FDT structure - /// to find the node, or [`StandardError::CpusMissing`] if the CPUs node is - /// missing. - pub fn cpus(self) -> Result>, StandardError> { - let node = self.find_node("/cpus").ok_or(StandardError::CpusMissing)?; - Ok(Cpus { node }) - } -} - /// Typed wrapper for a `/cpus` node. #[derive(Debug, Clone, Copy)] pub struct Cpus { @@ -52,6 +35,10 @@ impl Display for Cpus { } impl<'a, N: Node<'a>> Cpus { + pub(super) const fn new(node: N) -> Self { + Self { node } + } + /// Returns an iterator over the `/cpus/cpu@*` nodes. pub fn cpus(self) -> impl Iterator> + use<'a, N> { self.node.children().filter_map(|child| { diff --git a/src/standard/memory.rs b/src/standard/memory.rs index 814be29..b1146df 100644 --- a/src/standard/memory.rs +++ b/src/standard/memory.rs @@ -10,36 +10,9 @@ use core::fmt::{self, Display, Formatter}; use core::ops::Deref; use crate::error::{PropertyError, StandardError}; -use crate::fdt::{Fdt, FdtNode}; use crate::standard::Reg; use crate::{Cells, Node, Property}; -impl<'a> Fdt<'a> { - /// Returns the `/memory` node. - /// - /// This should always be included in a valid device tree. - /// - /// # Errors - /// - /// Returns [`StandardError::MemoryMissing`] if the memory node is missing. - pub fn memory(self) -> Result>, StandardError> { - let node = self - .find_node("/memory") - .ok_or(StandardError::MemoryMissing)?; - Ok(Memory { node }) - } - - /// Returns the `/reserved-memory/*` nodes, if any. - #[must_use] - pub fn reserved_memory(self) -> Option>>> { - Some( - self.find_node("/reserved-memory")? - .children() - .map(|node| ReservedMemory { node }), - ) - } -} - /// Typed wrapper for a `/memory` node. #[derive(Debug, Clone, Copy)] pub struct Memory { @@ -61,6 +34,10 @@ impl Display for Memory { } impl<'a, N: Node<'a>> Memory { + pub(super) const fn new(node: N) -> Self { + Self { node } + } + /// Returns the value of the standard `initial-mapped-area` property of the /// memory node. /// @@ -140,6 +117,10 @@ impl Display for ReservedMemory { } impl<'a, N: Node<'a>> ReservedMemory { + pub(super) const fn new(node: N) -> Self { + Self { node } + } + /// Returns the value of the standard `size` property of the reserved memory /// node, if it is present. /// diff --git a/tests/fdt.rs b/tests/fdt.rs index 8719dbc..a62a215 100644 --- a/tests/fdt.rs +++ b/tests/fdt.rs @@ -11,7 +11,7 @@ use dtoolkit::fdt::{Fdt, FdtNode}; use dtoolkit::fdt_mut::FdtMut; #[cfg(feature = "write")] use dtoolkit::model::{DeviceTree, DeviceTreeNode, DeviceTreeProperty}; -use dtoolkit::standard::{InitialMappedArea, NodeStandard, Status}; +use dtoolkit::standard::{InitialMappedArea, NodeStandard, Status, TreeStandard}; use dtoolkit::{Node, Property}; #[test] diff --git a/tests/standard.rs b/tests/standard.rs index 604f5ec..12ec4e3 100644 --- a/tests/standard.rs +++ b/tests/standard.rs @@ -6,16 +6,30 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use dtoolkit::Node; use dtoolkit::fdt::Fdt; -use dtoolkit::standard::NodeStandard; +#[cfg(feature = "write")] +use dtoolkit::model::DeviceTree; +use dtoolkit::standard::{NodeStandard, TreeStandard}; +use dtoolkit::{Node, Tree}; #[test] -fn cpus() { +fn cpus_fdt() { let dtb = include_bytes!("dtb/test_standard.dtb"); let fdt = Fdt::new(dtb).unwrap(); + cpus(fdt); +} - let all_cpus = fdt.cpus().unwrap(); +#[cfg(feature = "write")] +#[test] +fn cpus_model() { + let dtb = include_bytes!("dtb/test_standard.dtb"); + let fdt = Fdt::new(dtb).unwrap(); + let tree = DeviceTree::from_fdt(&fdt); + cpus(&tree); +} + +fn cpus<'a, T: Tree<'a>>(tree: T) { + let all_cpus = tree.cpus().expect("/cpus should exist"); assert_eq!(all_cpus.name(), "cpus"); let cpu_list: Vec<_> = all_cpus.cpus().collect(); @@ -24,21 +38,31 @@ fn cpus() { let first_cpu = &cpu_list[0]; assert_eq!(first_cpu.name(), "cpu@0"); assert_eq!(first_cpu.name_without_address(), "cpu"); - let enable_method: Vec<_> = first_cpu.enable_method().unwrap().collect(); + let enable_method: Vec<_> = first_cpu + .enable_method() + .expect("enable-method should exist") + .collect(); assert_eq!(enable_method, vec!["spin-table"]); assert_eq!( - first_cpu.cpu_release_addr().unwrap(), + first_cpu + .cpu_release_addr() + .expect("cpu-release-addr should be valid"), Some(0x1234_5678_9abc_def0) ); - let ids: Vec<_> = first_cpu.ids().unwrap().collect(); + let ids: Vec<_> = first_cpu.ids().expect("ids should be valid").collect(); assert_eq!(ids.len(), 1); - assert_eq!(ids[0].to_int::().unwrap(), 0); + assert_eq!(ids[0].to_int::().expect("id should fit in u32"), 0); let second_cpu = &cpu_list[1]; assert_eq!(second_cpu.name(), "cpu@1"); assert!(second_cpu.enable_method().is_none()); - assert_eq!(second_cpu.cpu_release_addr().unwrap(), None); + assert_eq!( + second_cpu + .cpu_release_addr() + .expect("cpu-release-addr should be valid"), + None + ); assert!(matches!( second_cpu.ids(), Err(dtoolkit::error::StandardError::CpuMissingReg) @@ -46,96 +70,189 @@ fn cpus() { } #[test] -fn cpus_missing() { +fn cpus_missing_fdt() { let dtb_no_cpus = include_bytes!("dtb/test.dtb"); let fdt_no_cpus = Fdt::new(dtb_no_cpus).unwrap(); + cpus_missing(fdt_no_cpus); +} + +#[cfg(feature = "write")] +#[test] +fn cpus_missing_model() { + let dtb_no_cpus = include_bytes!("dtb/test.dtb"); + let fdt_no_cpus = Fdt::new(dtb_no_cpus).unwrap(); + let tree_no_cpus = DeviceTree::from_fdt(&fdt_no_cpus); + cpus_missing(&tree_no_cpus); +} +fn cpus_missing<'a, T: Tree<'a>>(tree: T) { assert!(matches!( - fdt_no_cpus.cpus(), + tree.cpus(), Err(dtoolkit::error::StandardError::CpusMissing) )); } #[test] -fn chosen() { +fn chosen_fdt() { let dtb = include_bytes!("dtb/test_standard.dtb"); let fdt = Fdt::new(dtb).unwrap(); + chosen(fdt); +} - let chosen = fdt.chosen().unwrap(); +#[cfg(feature = "write")] +#[test] +fn chosen_model() { + let dtb = include_bytes!("dtb/test_standard.dtb"); + let fdt = Fdt::new(dtb).unwrap(); + let tree = DeviceTree::from_fdt(&fdt); + chosen(&tree); +} + +fn chosen<'a, T: Tree<'a>>(tree: T) { + let chosen = tree.chosen().expect("/chosen should exist"); assert_eq!(chosen.name(), "chosen"); assert_eq!( - chosen.bootargs().unwrap(), + chosen.bootargs().expect("bootargs should be valid"), Some("console=ttyS0 root=/dev/ram0") ); - assert_eq!(chosen.stdout_path().unwrap(), Some("/soc/serial@1000")); - assert_eq!(chosen.stdin_path().unwrap(), Some("/soc/serial@1000")); + assert_eq!( + chosen.stdout_path().expect("stdout-path should be valid"), + Some("/soc/serial@1000") + ); + assert_eq!( + chosen.stdin_path().expect("stdin-path should be valid"), + Some("/soc/serial@1000") + ); +} + +#[test] +fn chosen_missing_fdt() { + let dtb_no_chosen = include_bytes!("dtb/test.dtb"); + let fdt_no_chosen = Fdt::new(dtb_no_chosen).unwrap(); + chosen_missing(fdt_no_chosen); } +#[cfg(feature = "write")] #[test] -fn chosen_missing() { +fn chosen_missing_model() { let dtb_no_chosen = include_bytes!("dtb/test.dtb"); let fdt_no_chosen = Fdt::new(dtb_no_chosen).unwrap(); + let tree_no_chosen = DeviceTree::from_fdt(&fdt_no_chosen); + chosen_missing(&tree_no_chosen); +} - assert!(fdt_no_chosen.chosen().is_none()); +fn chosen_missing<'a, T: Tree<'a>>(tree: T) { + assert!(tree.chosen().is_none()); } #[test] -fn memory() { +fn memory_fdt() { let dtb = include_bytes!("dtb/test_pretty_print.dtb"); let fdt = Fdt::new(dtb).unwrap(); + memory(fdt); let memory = fdt.memory().unwrap(); + assert!(format!("{memory}").starts_with("memory@80000000 {")); + let shared = fdt.reserved_memory().unwrap().next().unwrap(); + assert!(format!("{shared}").starts_with("shared {")); +} + +#[cfg(feature = "write")] +#[test] +fn memory_model() { + let dtb = include_bytes!("dtb/test_pretty_print.dtb"); + let fdt = Fdt::new(dtb).unwrap(); + let tree = DeviceTree::from_fdt(&fdt); + memory(&tree); +} + +fn memory<'a, T: Tree<'a>>(tree: T) { + let memory = tree.memory().expect("/memory should exist"); assert_eq!(memory.name(), "memory@80000000"); assert!(memory.hotpluggable()); - assert!(format!("{memory}").starts_with("memory@80000000 {")); let ima = memory .initial_mapped_area() - .unwrap() - .unwrap() + .expect("initial-mapped-area should be valid") + .expect("initial-mapped-area should exist") .next() - .unwrap(); + .expect("initial-mapped-area should have an entry"); assert_eq!(ima.effective_address, 0x1234); assert_eq!(ima.physical_address, 0x4321); assert_eq!(ima.size, 0x1000); - let mut reserved = fdt.reserved_memory().unwrap(); - let shared = reserved.next().unwrap(); - assert!(format!("{shared}").starts_with("shared {")); + let mut reserved = tree + .reserved_memory() + .expect("/reserved-memory should exist"); + let shared = reserved + .next() + .expect("/reserved-memory should have a child"); assert!(shared.reusable()); assert!(!shared.no_map()); assert!(!shared.no_map_fixup()); - assert!(shared.alloc_ranges().unwrap().is_none()); + assert!( + shared + .alloc_ranges() + .expect("alloc-ranges should be valid") + .is_none() + ); } #[test] -fn memory_missing() { +fn memory_missing_fdt() { let dtb_no_mem = include_bytes!("dtb/test_standard.dtb"); let fdt_no_mem = Fdt::new(dtb_no_mem).unwrap(); + memory_missing(fdt_no_mem); +} + +#[cfg(feature = "write")] +#[test] +fn memory_missing_model() { + let dtb_no_mem = include_bytes!("dtb/test_standard.dtb"); + let fdt_no_mem = Fdt::new(dtb_no_mem).unwrap(); + let tree_no_mem = DeviceTree::from_fdt(&fdt_no_mem); + memory_missing(&tree_no_mem); +} +fn memory_missing<'a, T: Tree<'a>>(tree: T) { assert!(matches!( - fdt_no_mem.memory(), + tree.memory(), Err(dtoolkit::error::StandardError::MemoryMissing) )); - assert!(fdt_no_mem.reserved_memory().is_none()); + assert!(tree.reserved_memory().is_none()); } #[test] -fn node_standard() { +fn node_standard_fdt() { let dtb = include_bytes!("dtb/test_standard.dtb"); let fdt = Fdt::new(dtb).unwrap(); - let root = fdt.root(); - let cpus_container = root.child("cpus").unwrap(); + node_standard(fdt); +} + +#[cfg(feature = "write")] +#[test] +fn node_standard_model() { + let dtb = include_bytes!("dtb/test_standard.dtb"); + let fdt = Fdt::new(dtb).unwrap(); + let tree = DeviceTree::from_fdt(&fdt); + node_standard(&tree); +} + +fn node_standard<'a, T: Tree<'a>>(tree: T) { + let root = tree.root(); + let cpus_container = root.child("cpus").expect("cpus should exist"); assert_eq!(cpus_container.find_compatible("arm,cortex-a9").count(), 2); assert_eq!(cpus_container.find_compatible("non-existent").count(), 0); - let cpu0 = cpus_container.child("cpu@0").unwrap(); + let cpu0 = cpus_container.child("cpu@0").expect("cpu@0 should exist"); assert!(cpu0.is_compatible("arm,cortex-a9")); assert!(!cpu0.is_compatible("non-existent")); - let l2 = cpus_container.child("l2-cache").unwrap(); + let l2 = cpus_container + .child("l2-cache") + .expect("l2-cache should exist"); assert!(!l2.is_compatible("arm,cortex-a9")); assert!(l2.compatible().is_none()); - assert_eq!(l2.model().unwrap(), None); + assert_eq!(l2.model().expect("model should be valid"), None); }