Merge "libfdt: Fix bug in delete_and_next_node()" into main
diff --git a/libs/libfdt/Android.bp b/libs/libfdt/Android.bp
index b5f7471..1bb5692 100644
--- a/libs/libfdt/Android.bp
+++ b/libs/libfdt/Android.bp
@@ -40,6 +40,7 @@
"libcstr",
"liblibfdt_bindgen",
"libmemoffset_nostd",
+ "libstatic_assertions",
"libzerocopy_nostd",
],
whole_static_libs: [
diff --git a/libs/libfdt/src/ctypes.rs b/libs/libfdt/src/ctypes.rs
deleted file mode 100644
index 640d447..0000000
--- a/libs/libfdt/src/ctypes.rs
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright 2024, The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-//! Safe zero-cost wrappers around integer values used by libfdt.
-
-use crate::{FdtError, Result};
-
-/// Wrapper guaranteed to contain a valid phandle.
-#[repr(transparent)]
-#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
-pub struct Phandle(u32);
-
-impl Phandle {
- /// Minimum valid value for device tree phandles.
- pub const MIN: Self = Self(1);
- /// Maximum valid value for device tree phandles.
- pub const MAX: Self = Self(libfdt_bindgen::FDT_MAX_PHANDLE);
-
- /// Creates a new Phandle
- pub const fn new(value: u32) -> Option<Self> {
- if Self::MIN.0 <= value && value <= Self::MAX.0 {
- Some(Self(value))
- } else {
- None
- }
- }
-}
-
-impl From<Phandle> for u32 {
- fn from(phandle: Phandle) -> u32 {
- phandle.0
- }
-}
-
-impl TryFrom<u32> for Phandle {
- type Error = FdtError;
-
- fn try_from(value: u32) -> Result<Self> {
- Self::new(value).ok_or(FdtError::BadPhandle)
- }
-}
diff --git a/libs/libfdt/src/lib.rs b/libs/libfdt/src/lib.rs
index 58b4463..3339262 100644
--- a/libs/libfdt/src/lib.rs
+++ b/libs/libfdt/src/lib.rs
@@ -17,23 +17,22 @@
#![no_std]
-mod ctypes;
mod iterators;
mod libfdt;
mod result;
+mod safe_types;
-pub use ctypes::Phandle;
pub use iterators::{
AddressRange, CellIterator, CompatibleIterator, DescendantsIterator, MemRegIterator,
PropertyIterator, RangesIterator, Reg, RegIterator, SubnodeIterator,
};
pub use result::{FdtError, Result};
+pub use safe_types::{FdtHeader, NodeOffset, Phandle, PropOffset, StringOffset};
-use core::ffi::{c_int, c_void, CStr};
+use core::ffi::{c_void, CStr};
use core::ops::Range;
use cstr::cstr;
use libfdt::get_slice_at_ptr;
-use result::{fdt_err, fdt_err_expect_zero, fdt_err_or_option};
use zerocopy::AsBytes as _;
use crate::libfdt::{Libfdt, LibfdtMut};
@@ -94,12 +93,12 @@
}
impl FdtPropertyStruct {
- fn from_offset(fdt: &Fdt, offset: c_int) -> Result<&Self> {
+ fn from_offset(fdt: &Fdt, offset: PropOffset) -> Result<&Self> {
Ok(fdt.get_property_by_offset(offset)?.as_ref())
}
- fn name_offset(&self) -> c_int {
- u32::from_be(self.0.nameoff).try_into().unwrap()
+ fn name_offset(&self) -> StringOffset {
+ StringOffset(u32::from_be(self.0.nameoff).try_into().unwrap())
}
fn data_len(&self) -> usize {
@@ -115,12 +114,12 @@
#[derive(Clone, Copy, Debug)]
pub struct FdtProperty<'a> {
fdt: &'a Fdt,
- offset: c_int,
+ offset: PropOffset,
property: &'a FdtPropertyStruct,
}
impl<'a> FdtProperty<'a> {
- fn new(fdt: &'a Fdt, offset: c_int) -> Result<Self> {
+ fn new(fdt: &'a Fdt, offset: PropOffset) -> Result<Self> {
let property = FdtPropertyStruct::from_offset(fdt, offset)?;
Ok(Self { fdt, offset, property })
}
@@ -148,7 +147,7 @@
#[derive(Clone, Copy, Debug)]
pub struct FdtNode<'a> {
fdt: &'a Fdt,
- offset: c_int,
+ offset: NodeOffset,
}
impl<'a> FdtNode<'a> {
@@ -356,7 +355,7 @@
#[derive(Debug)]
pub struct FdtNodeMut<'a> {
fdt: &'a mut Fdt,
- offset: c_int,
+ offset: NodeOffset,
}
impl<'a> FdtNodeMut<'a> {
@@ -434,7 +433,7 @@
}
/// Adds new subnodes to the given node.
- pub fn add_subnodes(&mut self, names: &[&CStr]) -> Result<()> {
+ pub fn add_subnodes(self, names: &[&CStr]) -> Result<()> {
for name in names {
self.fdt.add_subnode_namelen(self.offset, name.to_bytes())?;
}
@@ -442,7 +441,7 @@
}
/// Adds a new subnode to the given node and return it as a FdtNodeMut on success.
- pub fn add_subnode(&'a mut self, name: &CStr) -> Result<Self> {
+ pub fn add_subnode(self, name: &CStr) -> Result<Self> {
let name = name.to_bytes();
let offset = self.fdt.add_subnode_namelen(self.offset, name)?;
@@ -451,7 +450,7 @@
/// Adds a new subnode to the given node with name and namelen, and returns it as a FdtNodeMut
/// on success.
- pub fn add_subnode_with_namelen(&'a mut self, name: &CStr, namelen: usize) -> Result<Self> {
+ pub fn add_subnode_with_namelen(self, name: &CStr, namelen: usize) -> Result<Self> {
let name = &name.to_bytes()[..namelen];
let offset = self.fdt.add_subnode_namelen(self.offset, name)?;
@@ -527,7 +526,7 @@
self.delete_and_next(next_offset)
}
- fn delete_and_next(self, next_offset: Option<c_int>) -> Result<Option<Self>> {
+ fn delete_and_next(self, next_offset: Option<NodeOffset>) -> Result<Option<Self>> {
if Some(self.offset) == next_offset {
return Err(FdtError::Internal);
}
@@ -638,40 +637,30 @@
/// Unpacks the DT to cover the whole slice it is contained in.
pub fn unpack(&mut self) -> Result<()> {
- // SAFETY: "Opens" the DT in-place (supported use-case) by updating its header and
- // internal structures to make use of the whole self.fdt slice but performs no accesses
- // outside of it and leaves the DT in a state that will be detected by other functions.
- let ret = unsafe {
- libfdt_bindgen::fdt_open_into(
- self.as_ptr(),
- self.as_mut_ptr(),
- self.capacity().try_into().map_err(|_| FdtError::Internal)?,
- )
- };
- fdt_err_expect_zero(ret)
+ self.open_into_self()
}
/// Packs the DT to take a minimum amount of memory.
///
/// Doesn't shrink the underlying memory slice.
pub fn pack(&mut self) -> Result<()> {
- // SAFETY: "Closes" the DT in-place by updating its header and relocating its structs.
- let ret = unsafe { libfdt_bindgen::fdt_pack(self.as_mut_ptr()) };
- fdt_err_expect_zero(ret)
+ LibfdtMut::pack(self)
}
/// Applies a DT overlay on the base DT.
///
/// # Safety
///
- /// On failure, the library corrupts the DT and overlay so both must be discarded.
- pub unsafe fn apply_overlay<'a>(&'a mut self, overlay: &'a mut Fdt) -> Result<&'a mut Self> {
- let ret =
- // SAFETY: Both pointers are valid because they come from references, and fdt_overlay_apply
- // doesn't keep them after it returns. It may corrupt their contents if there is an error,
- // but that's our caller's responsibility.
- unsafe { libfdt_bindgen::fdt_overlay_apply(self.as_mut_ptr(), overlay.as_mut_ptr()) };
- fdt_err_expect_zero(ret)?;
+ /// As libfdt corrupts the input DT on failure, `self` should be discarded on error:
+ ///
+ /// let fdt = fdt.apply_overlay(overlay)?;
+ ///
+ /// Furthermore, `overlay` is _always_ corrupted by libfdt and will never refer to a valid
+ /// `Fdt` after this function returns and must therefore be discarded by the caller.
+ pub unsafe fn apply_overlay<'a>(&'a mut self, overlay: &mut Fdt) -> Result<&'a mut Self> {
+ // SAFETY: Our caller will properly discard overlay and/or self as needed.
+ unsafe { self.overlay_apply(overlay) }?;
+
Ok(self)
}
@@ -680,7 +669,7 @@
///
/// NOTE: This does not support individual "/memory@XXXX" banks.
pub fn memory(&self) -> Result<MemRegIterator> {
- let node = self.node(cstr!("/memory"))?.ok_or(FdtError::NotFound)?;
+ let node = self.root()?.subnode(cstr!("memory"))?.ok_or(FdtError::NotFound)?;
if node.device_type()? != Some(cstr!("memory")) {
return Err(FdtError::BadValue);
}
@@ -694,7 +683,7 @@
/// Returns the standard /chosen node.
pub fn chosen(&self) -> Result<Option<FdtNode>> {
- self.node(cstr!("/chosen"))
+ self.root()?.subnode(cstr!("chosen"))
}
/// Returns the standard /chosen node as mutable.
@@ -704,12 +693,12 @@
/// Returns the root node of the tree.
pub fn root(&self) -> Result<FdtNode> {
- self.node(cstr!("/"))?.ok_or(FdtError::Internal)
+ Ok(FdtNode { fdt: self, offset: NodeOffset::ROOT })
}
/// Returns the standard /__symbols__ node.
pub fn symbols(&self) -> Result<Option<FdtNode>> {
- self.node(cstr!("/__symbols__"))
+ self.root()?.subnode(cstr!("__symbols__"))
}
/// Returns the standard /__symbols__ node as mutable
@@ -750,7 +739,7 @@
/// Returns the mutable root node of the tree.
pub fn root_mut(&mut self) -> Result<FdtNodeMut> {
- self.node_mut(cstr!("/"))?.ok_or(FdtError::Internal)
+ Ok(FdtNodeMut { fdt: self, offset: NodeOffset::ROOT })
}
/// Returns a mutable tree node by its full path.
@@ -760,7 +749,11 @@
Ok(offset.map(|offset| FdtNodeMut { fdt: self, offset }))
}
- fn next_node_skip_subnodes(&self, node: c_int, depth: usize) -> Result<Option<(c_int, usize)>> {
+ fn next_node_skip_subnodes(
+ &self,
+ node: NodeOffset,
+ depth: usize,
+ ) -> Result<Option<(NodeOffset, usize)>> {
let mut iter = self.next_node(node, depth)?;
while let Some((offset, next_depth)) = iter {
if next_depth <= depth {
@@ -786,21 +779,14 @@
self.buffer.as_ptr().cast()
}
- fn as_mut_ptr(&mut self) -> *mut c_void {
- self.buffer.as_mut_ptr().cast::<_>()
- }
-
- fn capacity(&self) -> usize {
- self.buffer.len()
- }
-
- fn header(&self) -> &libfdt_bindgen::fdt_header {
- let p = self.as_ptr().cast();
+ fn header(&self) -> &FdtHeader {
+ let p = self.as_ptr().cast::<libfdt_bindgen::fdt_header>();
// SAFETY: A valid FDT (verified by constructor) must contain a valid fdt_header.
- unsafe { &*p }
+ let header = unsafe { &*p };
+ header.as_ref()
}
fn totalsize(&self) -> usize {
- u32::from_be(self.header().totalsize) as usize
+ self.header().totalsize.get().try_into().unwrap()
}
}
diff --git a/libs/libfdt/src/libfdt.rs b/libs/libfdt/src/libfdt.rs
index 7737718..1af9edf 100644
--- a/libs/libfdt/src/libfdt.rs
+++ b/libs/libfdt/src/libfdt.rs
@@ -18,11 +18,12 @@
//! user-friendly higher-level types, allowing the trait to be shared between different ones,
//! adapted to their use-cases (e.g. alloc-based userspace or statically allocated no_std).
-use core::ffi::{c_int, CStr};
+use core::ffi::CStr;
use core::mem;
use core::ptr;
-use crate::{fdt_err, fdt_err_expect_zero, fdt_err_or_option, FdtError, Phandle, Result};
+use crate::result::FdtRawResult;
+use crate::{FdtError, NodeOffset, Phandle, PropOffset, Result, StringOffset};
// Function names are the C function names without the `fdt_` prefix.
@@ -35,7 +36,7 @@
// There will be no memory write outside of the given fdt.
let ret = unsafe { libfdt_bindgen::fdt_create_empty_tree(fdt, len) };
- fdt_err_expect_zero(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_check_full()` (C function).
@@ -49,7 +50,7 @@
// calls as it expects the client code to keep track of the objects (DT, nodes, ...).
let ret = unsafe { libfdt_bindgen::fdt_check_full(fdt, len) };
- fdt_err_expect_zero(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Wrapper for the read-only libfdt.h functions.
@@ -67,7 +68,7 @@
fn as_fdt_slice(&self) -> &[u8];
/// Safe wrapper around `fdt_path_offset_namelen()` (C function).
- fn path_offset_namelen(&self, path: &[u8]) -> Result<Option<c_int>> {
+ fn path_offset_namelen(&self, path: &[u8]) -> Result<Option<NodeOffset>> {
let fdt = self.as_fdt_slice().as_ptr().cast();
// *_namelen functions don't include the trailing nul terminator in 'len'.
let len = path.len().try_into().map_err(|_| FdtError::BadPath)?;
@@ -76,37 +77,43 @@
// function respects the passed number of characters.
let ret = unsafe { libfdt_bindgen::fdt_path_offset_namelen(fdt, path, len) };
- fdt_err_or_option(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_node_offset_by_phandle()` (C function).
- fn node_offset_by_phandle(&self, phandle: Phandle) -> Result<Option<c_int>> {
+ fn node_offset_by_phandle(&self, phandle: Phandle) -> Result<Option<NodeOffset>> {
let fdt = self.as_fdt_slice().as_ptr().cast();
let phandle = phandle.into();
// SAFETY: Accesses are constrained to the DT totalsize.
let ret = unsafe { libfdt_bindgen::fdt_node_offset_by_phandle(fdt, phandle) };
- fdt_err_or_option(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_node_offset_by_compatible()` (C function).
- fn node_offset_by_compatible(&self, prev: c_int, compatible: &CStr) -> Result<Option<c_int>> {
+ fn node_offset_by_compatible(
+ &self,
+ prev: NodeOffset,
+ compatible: &CStr,
+ ) -> Result<Option<NodeOffset>> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let prev = prev.into();
let compatible = compatible.as_ptr();
// SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ret = unsafe { libfdt_bindgen::fdt_node_offset_by_compatible(fdt, prev, compatible) };
- fdt_err_or_option(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_next_node()` (C function).
- fn next_node(&self, node: c_int, depth: usize) -> Result<Option<(c_int, usize)>> {
+ fn next_node(&self, node: NodeOffset, depth: usize) -> Result<Option<(NodeOffset, usize)>> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let node = node.into();
let mut depth = depth.try_into().unwrap();
// SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ret = unsafe { libfdt_bindgen::fdt_next_node(fdt, node, &mut depth) };
- match fdt_err_or_option(ret)? {
+ match FdtRawResult::from(ret).try_into()? {
Some(offset) if depth >= 0 => {
let depth = depth.try_into().unwrap();
Ok(Some((offset, depth)))
@@ -118,90 +125,103 @@
/// Safe wrapper around `fdt_parent_offset()` (C function).
///
/// Note that this function returns a `Err` when called on a root.
- fn parent_offset(&self, node: c_int) -> Result<c_int> {
+ fn parent_offset(&self, node: NodeOffset) -> Result<NodeOffset> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let node = node.into();
// SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ret = unsafe { libfdt_bindgen::fdt_parent_offset(fdt, node) };
- fdt_err(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_supernode_atdepth_offset()` (C function).
///
/// Note that this function returns a `Err` when called on a node at a depth shallower than
/// the provided `depth`.
- fn supernode_atdepth_offset(&self, node: c_int, depth: usize) -> Result<c_int> {
+ fn supernode_atdepth_offset(&self, node: NodeOffset, depth: usize) -> Result<NodeOffset> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let node = node.into();
let depth = depth.try_into().unwrap();
let nodedepth = ptr::null_mut();
let ret =
// SAFETY: Accesses (read-only) are constrained to the DT totalsize.
unsafe { libfdt_bindgen::fdt_supernode_atdepth_offset(fdt, node, depth, nodedepth) };
- fdt_err(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_subnode_offset_namelen()` (C function).
- fn subnode_offset_namelen(&self, parent: c_int, name: &[u8]) -> Result<Option<c_int>> {
+ fn subnode_offset_namelen(
+ &self,
+ parent: NodeOffset,
+ name: &[u8],
+ ) -> Result<Option<NodeOffset>> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let parent = parent.into();
let namelen = name.len().try_into().unwrap();
let name = name.as_ptr().cast();
// SAFETY: Accesses are constrained to the DT totalsize (validated by ctor).
let ret = unsafe { libfdt_bindgen::fdt_subnode_offset_namelen(fdt, parent, name, namelen) };
- fdt_err_or_option(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_first_subnode()` (C function).
- fn first_subnode(&self, node: c_int) -> Result<Option<c_int>> {
+ fn first_subnode(&self, node: NodeOffset) -> Result<Option<NodeOffset>> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let node = node.into();
// SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ret = unsafe { libfdt_bindgen::fdt_first_subnode(fdt, node) };
- fdt_err_or_option(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_next_subnode()` (C function).
- fn next_subnode(&self, node: c_int) -> Result<Option<c_int>> {
+ fn next_subnode(&self, node: NodeOffset) -> Result<Option<NodeOffset>> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let node = node.into();
// SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ret = unsafe { libfdt_bindgen::fdt_next_subnode(fdt, node) };
- fdt_err_or_option(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_address_cells()` (C function).
- fn address_cells(&self, node: c_int) -> Result<usize> {
+ fn address_cells(&self, node: NodeOffset) -> Result<usize> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let node = node.into();
// SAFETY: Accesses are constrained to the DT totalsize (validated by ctor).
let ret = unsafe { libfdt_bindgen::fdt_address_cells(fdt, node) };
- Ok(fdt_err(ret)?.try_into().unwrap())
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_size_cells()` (C function).
- fn size_cells(&self, node: c_int) -> Result<usize> {
+ fn size_cells(&self, node: NodeOffset) -> Result<usize> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let node = node.into();
// SAFETY: Accesses are constrained to the DT totalsize (validated by ctor).
let ret = unsafe { libfdt_bindgen::fdt_size_cells(fdt, node) };
- Ok(fdt_err(ret)?.try_into().unwrap())
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_get_name()` (C function).
- fn get_name(&self, node: c_int) -> Result<&[u8]> {
+ fn get_name(&self, node: NodeOffset) -> Result<&[u8]> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let node = node.into();
let mut len = 0;
// SAFETY: Accesses are constrained to the DT totalsize (validated by ctor). On success, the
// function returns valid null terminating string and otherwise returned values are dropped.
let name = unsafe { libfdt_bindgen::fdt_get_name(fdt, node, &mut len) };
- let len = usize::try_from(fdt_err(len)?).unwrap().checked_add(1).unwrap();
+ let len = usize::try_from(FdtRawResult::from(len))?.checked_add(1).unwrap();
get_slice_at_ptr(self.as_fdt_slice(), name.cast(), len).ok_or(FdtError::Internal)
}
/// Safe wrapper around `fdt_getprop_namelen()` (C function).
- fn getprop_namelen(&self, node: c_int, name: &[u8]) -> Result<Option<&[u8]>> {
+ fn getprop_namelen(&self, node: NodeOffset, name: &[u8]) -> Result<Option<&[u8]>> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let node = node.into();
let namelen = name.len().try_into().map_err(|_| FdtError::BadPath)?;
let name = name.as_ptr().cast();
let mut len = 0;
@@ -210,8 +230,7 @@
// function respects the passed number of characters.
unsafe { libfdt_bindgen::fdt_getprop_namelen(fdt, node, name, namelen, &mut len) };
- if let Some(len) = fdt_err_or_option(len)? {
- let len = usize::try_from(len).unwrap();
+ if let Some(len) = FdtRawResult::from(len).try_into()? {
let bytes = get_slice_at_ptr(self.as_fdt_slice(), prop.cast(), len);
Ok(Some(bytes.ok_or(FdtError::Internal)?))
@@ -221,13 +240,14 @@
}
/// Safe wrapper around `fdt_get_property_by_offset()` (C function).
- fn get_property_by_offset(&self, offset: c_int) -> Result<&libfdt_bindgen::fdt_property> {
+ fn get_property_by_offset(&self, offset: PropOffset) -> Result<&libfdt_bindgen::fdt_property> {
let mut len = 0;
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let offset = offset.into();
// SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let prop = unsafe { libfdt_bindgen::fdt_get_property_by_offset(fdt, offset, &mut len) };
- let data_len = fdt_err(len)?.try_into().unwrap();
+ let data_len = FdtRawResult::from(len).try_into()?;
// TODO(stable_feature(offset_of)): mem::offset_of!(fdt_property, data).
let data_offset = memoffset::offset_of!(libfdt_bindgen::fdt_property, data);
let len = data_offset.checked_add(data_len).ok_or(FdtError::Internal)?;
@@ -247,21 +267,23 @@
}
/// Safe wrapper around `fdt_first_property_offset()` (C function).
- fn first_property_offset(&self, node: c_int) -> Result<Option<c_int>> {
+ fn first_property_offset(&self, node: NodeOffset) -> Result<Option<PropOffset>> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let node = node.into();
// SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ret = unsafe { libfdt_bindgen::fdt_first_property_offset(fdt, node) };
- fdt_err_or_option(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_next_property_offset()` (C function).
- fn next_property_offset(&self, prev: c_int) -> Result<Option<c_int>> {
+ fn next_property_offset(&self, prev: PropOffset) -> Result<Option<PropOffset>> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let prev = prev.into();
// SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ret = unsafe { libfdt_bindgen::fdt_next_property_offset(fdt, prev) };
- fdt_err_or_option(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_find_max_phandle()` (C function).
@@ -271,14 +293,15 @@
// SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ret = unsafe { libfdt_bindgen::fdt_find_max_phandle(fdt, &mut phandle) };
- fdt_err_expect_zero(ret)?;
+ FdtRawResult::from(ret).try_into()?;
phandle.try_into()
}
/// Safe wrapper around `fdt_string()` (C function).
- fn string(&self, offset: c_int) -> Result<&CStr> {
+ fn string(&self, offset: StringOffset) -> Result<&CStr> {
let fdt = self.as_fdt_slice().as_ptr().cast();
+ let offset = offset.into();
// SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ptr = unsafe { libfdt_bindgen::fdt_string(fdt, offset) };
let bytes =
@@ -286,6 +309,13 @@
CStr::from_bytes_until_nul(bytes).map_err(|_| FdtError::Internal)
}
+
+ /// Safe wrapper around `fdt_open_into()` (C function).
+ fn open_into(&self, dest: &mut [u8]) -> Result<()> {
+ let fdt = self.as_fdt_slice().as_ptr().cast();
+
+ open_into(fdt, dest)
+ }
}
/// Wrapper for the read-write libfdt.h functions.
@@ -309,28 +339,31 @@
fn as_fdt_slice_mut(&mut self) -> &mut [u8];
/// Safe wrapper around `fdt_nop_node()` (C function).
- fn nop_node(&mut self, node: c_int) -> Result<()> {
+ fn nop_node(&mut self, node: NodeOffset) -> Result<()> {
let fdt = self.as_fdt_slice_mut().as_mut_ptr().cast();
+ let node = node.into();
// SAFETY: Accesses are constrained to the DT totalsize (validated by ctor).
let ret = unsafe { libfdt_bindgen::fdt_nop_node(fdt, node) };
- fdt_err_expect_zero(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_add_subnode_namelen()` (C function).
- fn add_subnode_namelen(&mut self, node: c_int, name: &[u8]) -> Result<c_int> {
+ fn add_subnode_namelen(&mut self, node: NodeOffset, name: &[u8]) -> Result<NodeOffset> {
let fdt = self.as_fdt_slice_mut().as_mut_ptr().cast();
+ let node = node.into();
let namelen = name.len().try_into().unwrap();
let name = name.as_ptr().cast();
// SAFETY: Accesses are constrained to the DT totalsize (validated by ctor).
let ret = unsafe { libfdt_bindgen::fdt_add_subnode_namelen(fdt, node, name, namelen) };
- fdt_err(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_setprop()` (C function).
- fn setprop(&mut self, node: c_int, name: &CStr, value: &[u8]) -> Result<()> {
+ fn setprop(&mut self, node: NodeOffset, name: &CStr, value: &[u8]) -> Result<()> {
let fdt = self.as_fdt_slice_mut().as_mut_ptr().cast();
+ let node = node.into();
let name = name.as_ptr();
let len = value.len().try_into().map_err(|_| FdtError::BadValue)?;
let value = value.as_ptr().cast();
@@ -338,12 +371,18 @@
// (validated by underlying libfdt).
let ret = unsafe { libfdt_bindgen::fdt_setprop(fdt, node, name, value, len) };
- fdt_err_expect_zero(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_setprop_placeholder()` (C function).
- fn setprop_placeholder(&mut self, node: c_int, name: &CStr, size: usize) -> Result<&mut [u8]> {
+ fn setprop_placeholder(
+ &mut self,
+ node: NodeOffset,
+ name: &CStr,
+ size: usize,
+ ) -> Result<&mut [u8]> {
let fdt = self.as_fdt_slice_mut().as_mut_ptr().cast();
+ let node = node.into();
let name = name.as_ptr();
let len = size.try_into().unwrap();
let mut data = ptr::null_mut();
@@ -351,14 +390,15 @@
// SAFETY: Accesses are constrained to the DT totalsize (validated by ctor).
unsafe { libfdt_bindgen::fdt_setprop_placeholder(fdt, node, name, len, &mut data) };
- fdt_err_expect_zero(ret)?;
+ FdtRawResult::from(ret).try_into()?;
get_mut_slice_at_ptr(self.as_fdt_slice_mut(), data.cast(), size).ok_or(FdtError::Internal)
}
/// Safe wrapper around `fdt_setprop_inplace()` (C function).
- fn setprop_inplace(&mut self, node: c_int, name: &CStr, value: &[u8]) -> Result<()> {
+ fn setprop_inplace(&mut self, node: NodeOffset, name: &CStr, value: &[u8]) -> Result<()> {
let fdt = self.as_fdt_slice_mut().as_mut_ptr().cast();
+ let node = node.into();
let name = name.as_ptr();
let len = value.len().try_into().map_err(|_| FdtError::BadValue)?;
let value = value.as_ptr().cast();
@@ -366,43 +406,47 @@
// (validated by underlying libfdt).
let ret = unsafe { libfdt_bindgen::fdt_setprop_inplace(fdt, node, name, value, len) };
- fdt_err_expect_zero(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_appendprop()` (C function).
- fn appendprop(&mut self, node: c_int, name: &CStr, value: &[u8]) -> Result<()> {
+ fn appendprop(&mut self, node: NodeOffset, name: &CStr, value: &[u8]) -> Result<()> {
let fdt = self.as_fdt_slice_mut().as_mut_ptr().cast();
+ let node = node.into();
let name = name.as_ptr();
let len = value.len().try_into().map_err(|_| FdtError::BadValue)?;
let value = value.as_ptr().cast();
// SAFETY: Accesses are constrained to the DT totalsize (validated by ctor).
let ret = unsafe { libfdt_bindgen::fdt_appendprop(fdt, node, name, value, len) };
- fdt_err_expect_zero(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_appendprop_addrrange()` (C function).
fn appendprop_addrrange(
&mut self,
- parent: c_int,
- node: c_int,
+ parent: NodeOffset,
+ node: NodeOffset,
name: &CStr,
addr: u64,
size: u64,
) -> Result<()> {
let fdt = self.as_fdt_slice_mut().as_mut_ptr().cast();
+ let parent = parent.into();
+ let node = node.into();
let name = name.as_ptr();
// SAFETY: Accesses are constrained to the DT totalsize (validated by ctor).
let ret = unsafe {
libfdt_bindgen::fdt_appendprop_addrrange(fdt, parent, node, name, addr, size)
};
- fdt_err_expect_zero(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_delprop()` (C function).
- fn delprop(&mut self, node: c_int, name: &CStr) -> Result<()> {
+ fn delprop(&mut self, node: NodeOffset, name: &CStr) -> Result<()> {
let fdt = self.as_fdt_slice_mut().as_mut_ptr().cast();
+ let node = node.into();
let name = name.as_ptr();
// SAFETY: Accesses are constrained to the DT totalsize (validated by ctor) when the
// library locates the node's property. Removing the property may shift the offsets of
@@ -410,18 +454,61 @@
// being called when FdtNode instances are in use.
let ret = unsafe { libfdt_bindgen::fdt_delprop(fdt, node, name) };
- fdt_err_expect_zero(ret)
+ FdtRawResult::from(ret).try_into()
}
/// Safe wrapper around `fdt_nop_property()` (C function).
- fn nop_property(&mut self, node: c_int, name: &CStr) -> Result<()> {
+ fn nop_property(&mut self, node: NodeOffset, name: &CStr) -> Result<()> {
let fdt = self.as_fdt_slice_mut().as_mut_ptr().cast();
+ let node = node.into();
let name = name.as_ptr();
// SAFETY: Accesses are constrained to the DT totalsize (validated by ctor) when the
// library locates the node's property.
let ret = unsafe { libfdt_bindgen::fdt_nop_property(fdt, node, name) };
- fdt_err_expect_zero(ret)
+ FdtRawResult::from(ret).try_into()
+ }
+
+ /// Safe and aliasing-compatible wrapper around `fdt_open_into()` (C function).
+ ///
+ /// The C API allows both input (`const void*`) and output (`void *`) to point to the same
+ /// memory region but the borrow checker would reject an API such as
+ ///
+ /// self.open_into(&mut self.buffer)
+ ///
+ /// so this wrapper is provided to implement such a common aliasing case.
+ fn open_into_self(&mut self) -> Result<()> {
+ let fdt = self.as_fdt_slice_mut();
+
+ open_into(fdt.as_ptr().cast(), fdt)
+ }
+
+ /// Safe wrapper around `fdt_pack()` (C function).
+ fn pack(&mut self) -> Result<()> {
+ let fdt = self.as_fdt_slice_mut().as_mut_ptr().cast();
+ // SAFETY: Accesses (R/W) are constrained to the DT totalsize (validated by ctor).
+ let ret = unsafe { libfdt_bindgen::fdt_pack(fdt) };
+
+ FdtRawResult::from(ret).try_into()
+ }
+
+ /// Wrapper around `fdt_overlay_apply()` (C function).
+ ///
+ /// # Safety
+ ///
+ /// This function safely wraps the C function call but is unsafe because the caller must
+ ///
+ /// - discard `overlay` as a &LibfdtMut because libfdt corrupts its header before returning;
+ /// - on error, discard `self` as a &LibfdtMut for the same reason.
+ unsafe fn overlay_apply(&mut self, overlay: &mut Self) -> Result<()> {
+ let fdt = self.as_fdt_slice_mut().as_mut_ptr().cast();
+ let overlay = overlay.as_fdt_slice_mut().as_mut_ptr().cast();
+ // SAFETY: Both pointers are valid because they come from references, and fdt_overlay_apply
+ // doesn't keep them after it returns. It may corrupt their contents if there is an error,
+ // but that's our caller's responsibility.
+ let ret = unsafe { libfdt_bindgen::fdt_overlay_apply(fdt, overlay) };
+
+ FdtRawResult::from(ret).try_into()
}
}
@@ -449,6 +536,19 @@
})
}
+fn open_into(fdt: *const u8, dest: &mut [u8]) -> Result<()> {
+ let fdt = fdt.cast();
+ let len = dest.len().try_into().map_err(|_| FdtError::Internal)?;
+ let dest = dest.as_mut_ptr().cast();
+ // SAFETY: Reads the whole fdt slice (based on the validated totalsize) and, if it fits, copies
+ // it to the (properly mutable) dest buffer of size len. On success, the resulting dest
+ // contains a valid DT with the nodes and properties of the original one but of a different
+ // size, reflected in its fdt_header::totalsize.
+ let ret = unsafe { libfdt_bindgen::fdt_open_into(fdt, dest, len) };
+
+ FdtRawResult::from(ret).try_into()
+}
+
// TODO(stable_feature(pointer_is_aligned)): p.is_aligned()
fn is_aligned<T>(p: *const T) -> bool {
(p as usize) % mem::align_of::<T>() == 0
diff --git a/libs/libfdt/src/result.rs b/libs/libfdt/src/result.rs
index 9643e1e..52291ca 100644
--- a/libs/libfdt/src/result.rs
+++ b/libs/libfdt/src/result.rs
@@ -14,7 +14,7 @@
//! Rust types related to the libfdt C integer results.
-use core::ffi::c_int;
+use core::ffi::{c_int, c_uint};
use core::fmt;
use core::result;
@@ -94,46 +94,99 @@
/// Result type with FdtError enum.
pub type Result<T> = result::Result<T, FdtError>;
-pub(crate) fn fdt_err(val: c_int) -> Result<c_int> {
- if val >= 0 {
- Ok(val)
- } else {
- Err(match -val as _ {
- libfdt_bindgen::FDT_ERR_NOTFOUND => FdtError::NotFound,
- libfdt_bindgen::FDT_ERR_EXISTS => FdtError::Exists,
- libfdt_bindgen::FDT_ERR_NOSPACE => FdtError::NoSpace,
- libfdt_bindgen::FDT_ERR_BADOFFSET => FdtError::BadOffset,
- libfdt_bindgen::FDT_ERR_BADPATH => FdtError::BadPath,
- libfdt_bindgen::FDT_ERR_BADPHANDLE => FdtError::BadPhandle,
- libfdt_bindgen::FDT_ERR_BADSTATE => FdtError::BadState,
- libfdt_bindgen::FDT_ERR_TRUNCATED => FdtError::Truncated,
- libfdt_bindgen::FDT_ERR_BADMAGIC => FdtError::BadMagic,
- libfdt_bindgen::FDT_ERR_BADVERSION => FdtError::BadVersion,
- libfdt_bindgen::FDT_ERR_BADSTRUCTURE => FdtError::BadStructure,
- libfdt_bindgen::FDT_ERR_BADLAYOUT => FdtError::BadLayout,
- libfdt_bindgen::FDT_ERR_INTERNAL => FdtError::Internal,
- libfdt_bindgen::FDT_ERR_BADNCELLS => FdtError::BadNCells,
- libfdt_bindgen::FDT_ERR_BADVALUE => FdtError::BadValue,
- libfdt_bindgen::FDT_ERR_BADOVERLAY => FdtError::BadOverlay,
- libfdt_bindgen::FDT_ERR_NOPHANDLES => FdtError::NoPhandles,
- libfdt_bindgen::FDT_ERR_BADFLAGS => FdtError::BadFlags,
- libfdt_bindgen::FDT_ERR_ALIGNMENT => FdtError::Alignment,
- _ => FdtError::Unknown(val),
- })
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub(crate) struct FdtRawResult(c_int);
+
+impl From<c_int> for FdtRawResult {
+ fn from(value: c_int) -> Self {
+ Self(value)
}
}
-pub(crate) fn fdt_err_expect_zero(val: c_int) -> Result<()> {
- match fdt_err(val)? {
- 0 => Ok(()),
- _ => Err(FdtError::Unknown(val)),
+impl TryFrom<FdtRawResult> for c_int {
+ type Error = FdtError;
+
+ fn try_from(res: FdtRawResult) -> Result<Self> {
+ use libfdt_bindgen::{
+ FDT_ERR_ALIGNMENT, FDT_ERR_BADFLAGS, FDT_ERR_BADLAYOUT, FDT_ERR_BADMAGIC,
+ FDT_ERR_BADNCELLS, FDT_ERR_BADOFFSET, FDT_ERR_BADOVERLAY, FDT_ERR_BADPATH,
+ FDT_ERR_BADPHANDLE, FDT_ERR_BADSTATE, FDT_ERR_BADSTRUCTURE, FDT_ERR_BADVALUE,
+ FDT_ERR_BADVERSION, FDT_ERR_EXISTS, FDT_ERR_INTERNAL, FDT_ERR_NOPHANDLES,
+ FDT_ERR_NOSPACE, FDT_ERR_NOTFOUND, FDT_ERR_TRUNCATED,
+ };
+ match res.0 {
+ x if x >= 0 => Ok(x),
+ x if x == -(FDT_ERR_NOTFOUND as c_int) => Err(FdtError::NotFound),
+ x if x == -(FDT_ERR_EXISTS as c_int) => Err(FdtError::Exists),
+ x if x == -(FDT_ERR_NOSPACE as c_int) => Err(FdtError::NoSpace),
+ x if x == -(FDT_ERR_BADOFFSET as c_int) => Err(FdtError::BadOffset),
+ x if x == -(FDT_ERR_BADPATH as c_int) => Err(FdtError::BadPath),
+ x if x == -(FDT_ERR_BADPHANDLE as c_int) => Err(FdtError::BadPhandle),
+ x if x == -(FDT_ERR_BADSTATE as c_int) => Err(FdtError::BadState),
+ x if x == -(FDT_ERR_TRUNCATED as c_int) => Err(FdtError::Truncated),
+ x if x == -(FDT_ERR_BADMAGIC as c_int) => Err(FdtError::BadMagic),
+ x if x == -(FDT_ERR_BADVERSION as c_int) => Err(FdtError::BadVersion),
+ x if x == -(FDT_ERR_BADSTRUCTURE as c_int) => Err(FdtError::BadStructure),
+ x if x == -(FDT_ERR_BADLAYOUT as c_int) => Err(FdtError::BadLayout),
+ x if x == -(FDT_ERR_INTERNAL as c_int) => Err(FdtError::Internal),
+ x if x == -(FDT_ERR_BADNCELLS as c_int) => Err(FdtError::BadNCells),
+ x if x == -(FDT_ERR_BADVALUE as c_int) => Err(FdtError::BadValue),
+ x if x == -(FDT_ERR_BADOVERLAY as c_int) => Err(FdtError::BadOverlay),
+ x if x == -(FDT_ERR_NOPHANDLES as c_int) => Err(FdtError::NoPhandles),
+ x if x == -(FDT_ERR_BADFLAGS as c_int) => Err(FdtError::BadFlags),
+ x if x == -(FDT_ERR_ALIGNMENT as c_int) => Err(FdtError::Alignment),
+ x => Err(FdtError::Unknown(x)),
+ }
}
}
-pub(crate) fn fdt_err_or_option(val: c_int) -> Result<Option<c_int>> {
- match fdt_err(val) {
- Ok(val) => Ok(Some(val)),
- Err(FdtError::NotFound) => Ok(None),
- Err(e) => Err(e),
+impl TryFrom<FdtRawResult> for Option<c_int> {
+ type Error = FdtError;
+
+ fn try_from(res: FdtRawResult) -> Result<Self> {
+ match res.try_into() {
+ Ok(n) => Ok(Some(n)),
+ Err(FdtError::NotFound) => Ok(None),
+ Err(e) => Err(e),
+ }
+ }
+}
+
+impl TryFrom<FdtRawResult> for c_uint {
+ type Error = FdtError;
+
+ fn try_from(res: FdtRawResult) -> Result<Self> {
+ Ok(c_int::try_from(res)?.try_into().unwrap())
+ }
+}
+
+impl TryFrom<FdtRawResult> for usize {
+ type Error = FdtError;
+
+ fn try_from(res: FdtRawResult) -> Result<Self> {
+ Ok(c_int::try_from(res)?.try_into().unwrap())
+ }
+}
+
+impl TryFrom<FdtRawResult> for Option<usize> {
+ type Error = FdtError;
+
+ fn try_from(res: FdtRawResult) -> Result<Self> {
+ match res.try_into() {
+ Ok(n) => Ok(Some(n)),
+ Err(FdtError::NotFound) => Ok(None),
+ Err(e) => Err(e),
+ }
+ }
+}
+
+impl TryFrom<FdtRawResult> for () {
+ type Error = FdtError;
+
+ fn try_from(res: FdtRawResult) -> Result<Self> {
+ match res.try_into()? {
+ 0 => Ok(()),
+ n => Err(FdtError::Unknown(n)),
+ }
}
}
diff --git a/libs/libfdt/src/safe_types.rs b/libs/libfdt/src/safe_types.rs
new file mode 100644
index 0000000..3848542
--- /dev/null
+++ b/libs/libfdt/src/safe_types.rs
@@ -0,0 +1,228 @@
+// Copyright 2024, The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Safe zero-cost wrappers around integer values used by libfdt.
+
+use core::ffi::c_int;
+
+use crate::result::FdtRawResult;
+use crate::{FdtError, Result};
+
+use zerocopy::byteorder::big_endian;
+use zerocopy::{FromBytes, FromZeroes};
+
+macro_rules! assert_offset_eq {
+ // TODO(stable_feature(offset_of)): mem::offset_of
+ // TODO(const_feature(assert_eq)): assert_eq!()
+ ($t:ty, $u:ty, $id:ident) => {
+ static_assertions::const_assert_eq!(
+ memoffset::offset_of!($t, $id),
+ memoffset::offset_of!($u, $id),
+ );
+ };
+}
+
+/// Thin wrapper around `libfdt_bindgen::fdt_header` for transparent endianness handling.
+#[repr(C)]
+#[derive(Debug, FromZeroes, FromBytes)]
+pub struct FdtHeader {
+ /// magic word FDT_MAGIC
+ pub magic: big_endian::U32,
+ /// total size of DT block
+ pub totalsize: big_endian::U32,
+ /// offset to structure
+ pub off_dt_struct: big_endian::U32,
+ /// offset to strings
+ pub off_dt_strings: big_endian::U32,
+ /// offset to memory reserve map
+ pub off_mem_rsvmap: big_endian::U32,
+ /// format version
+ pub version: big_endian::U32,
+ /// last compatible version
+ pub last_comp_version: big_endian::U32,
+ /* version 2 fields below */
+ /// Which physical CPU id we're booting on
+ pub boot_cpuid_phys: big_endian::U32,
+ /* version 3 fields below */
+ /// size of the strings block
+ pub size_dt_strings: big_endian::U32,
+ /* version 17 fields below */
+ /// size of the structure block
+ pub size_dt_struct: big_endian::U32,
+}
+assert_offset_eq!(libfdt_bindgen::fdt_header, FdtHeader, magic);
+assert_offset_eq!(libfdt_bindgen::fdt_header, FdtHeader, totalsize);
+assert_offset_eq!(libfdt_bindgen::fdt_header, FdtHeader, off_dt_struct);
+assert_offset_eq!(libfdt_bindgen::fdt_header, FdtHeader, off_dt_strings);
+assert_offset_eq!(libfdt_bindgen::fdt_header, FdtHeader, off_mem_rsvmap);
+assert_offset_eq!(libfdt_bindgen::fdt_header, FdtHeader, version);
+assert_offset_eq!(libfdt_bindgen::fdt_header, FdtHeader, last_comp_version);
+assert_offset_eq!(libfdt_bindgen::fdt_header, FdtHeader, boot_cpuid_phys);
+assert_offset_eq!(libfdt_bindgen::fdt_header, FdtHeader, size_dt_strings);
+assert_offset_eq!(libfdt_bindgen::fdt_header, FdtHeader, size_dt_struct);
+
+impl AsRef<FdtHeader> for libfdt_bindgen::fdt_header {
+ fn as_ref(&self) -> &FdtHeader {
+ let ptr = self as *const _ as *const _;
+ // SAFETY: Types have the same layout (u32 and U32 have the same storage) and alignment.
+ unsafe { &*ptr }
+ }
+}
+
+/// Wrapper guaranteed to contain a valid phandle.
+#[repr(transparent)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
+pub struct Phandle(u32);
+
+impl Phandle {
+ /// Minimum valid value for device tree phandles.
+ pub const MIN: Self = Self(1);
+ /// Maximum valid value for device tree phandles.
+ pub const MAX: Self = Self(libfdt_bindgen::FDT_MAX_PHANDLE);
+
+ /// Creates a new Phandle
+ pub const fn new(value: u32) -> Option<Self> {
+ if Self::MIN.0 <= value && value <= Self::MAX.0 {
+ Some(Self(value))
+ } else {
+ None
+ }
+ }
+}
+
+impl From<Phandle> for u32 {
+ fn from(phandle: Phandle) -> u32 {
+ phandle.0
+ }
+}
+
+impl TryFrom<u32> for Phandle {
+ type Error = FdtError;
+
+ fn try_from(value: u32) -> Result<Self> {
+ Self::new(value).ok_or(FdtError::BadPhandle)
+ }
+}
+
+impl TryFrom<FdtRawResult> for Phandle {
+ type Error = FdtError;
+
+ fn try_from(res: FdtRawResult) -> Result<Self> {
+ Self::new(res.try_into()?).ok_or(FdtError::BadPhandle)
+ }
+}
+
+/// Safe zero-cost wrapper around libfdt device tree node offsets.
+///
+/// This type should only be obtained from properly wrapped successful libfdt calls.
+#[repr(transparent)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
+pub struct NodeOffset(c_int);
+
+impl NodeOffset {
+ /// Offset of the root node; 0, by definition.
+ pub const ROOT: Self = Self(0);
+}
+
+impl TryFrom<FdtRawResult> for NodeOffset {
+ type Error = FdtError;
+
+ fn try_from(res: FdtRawResult) -> Result<Self> {
+ Ok(Self(res.try_into()?))
+ }
+}
+
+impl TryFrom<FdtRawResult> for Option<NodeOffset> {
+ type Error = FdtError;
+
+ fn try_from(res: FdtRawResult) -> Result<Self> {
+ match res.try_into() {
+ Ok(n) => Ok(Some(n)),
+ Err(FdtError::NotFound) => Ok(None),
+ Err(e) => Err(e),
+ }
+ }
+}
+
+impl From<NodeOffset> for c_int {
+ fn from(offset: NodeOffset) -> Self {
+ offset.0
+ }
+}
+
+/// Safe zero-cost wrapper around libfdt device tree property offsets.
+///
+/// This type should only be obtained from properly wrapped successful libfdt calls.
+#[repr(transparent)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
+pub struct PropOffset(c_int);
+
+impl TryFrom<FdtRawResult> for PropOffset {
+ type Error = FdtError;
+
+ fn try_from(res: FdtRawResult) -> Result<Self> {
+ Ok(Self(res.try_into()?))
+ }
+}
+
+impl TryFrom<FdtRawResult> for Option<PropOffset> {
+ type Error = FdtError;
+
+ fn try_from(res: FdtRawResult) -> Result<Self> {
+ match res.try_into() {
+ Ok(n) => Ok(Some(n)),
+ Err(FdtError::NotFound) => Ok(None),
+ Err(e) => Err(e),
+ }
+ }
+}
+
+impl From<PropOffset> for c_int {
+ fn from(offset: PropOffset) -> Self {
+ offset.0
+ }
+}
+
+/// Safe zero-cost wrapper around libfdt device tree string offsets.
+///
+/// This type should only be obtained from properly wrapped successful libfdt calls.
+#[repr(transparent)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
+pub struct StringOffset(pub c_int); // TODO(ptosi): Move fdt_property wrapper here and remove pub.
+
+impl TryFrom<FdtRawResult> for StringOffset {
+ type Error = FdtError;
+
+ fn try_from(res: FdtRawResult) -> Result<Self> {
+ Ok(Self(res.try_into()?))
+ }
+}
+
+impl TryFrom<FdtRawResult> for Option<StringOffset> {
+ type Error = FdtError;
+
+ fn try_from(res: FdtRawResult) -> Result<Self> {
+ match res.try_into() {
+ Ok(n) => Ok(Some(n)),
+ Err(FdtError::NotFound) => Ok(None),
+ Err(e) => Err(e),
+ }
+ }
+}
+
+impl From<StringOffset> for c_int {
+ fn from(offset: StringOffset) -> Self {
+ offset.0
+ }
+}
diff --git a/libs/libfdt/tests/api_test.rs b/libs/libfdt/tests/api_test.rs
index ce85172..8f5b76d 100644
--- a/libs/libfdt/tests/api_test.rs
+++ b/libs/libfdt/tests/api_test.rs
@@ -267,8 +267,8 @@
let node = fdt.node(node_path).unwrap().unwrap();
assert_eq!(Ok(None), node.subnode_with_name_bytes(name));
- let mut node = fdt.node_mut(node_path).unwrap().unwrap();
- node.add_subnode_with_namelen(subnode_name, len).unwrap();
+ let node = fdt.node_mut(node_path).unwrap().unwrap();
+ let _ = node.add_subnode_with_namelen(subnode_name, len).unwrap();
let node = fdt.node(node_path).unwrap().unwrap();
assert_ne!(Ok(None), node.subnode_with_name_bytes(name));
@@ -472,7 +472,7 @@
let mut data = vec![0_u8; 1000];
let fdt = Fdt::create_empty_tree(&mut data).unwrap();
- let mut root = fdt.root_mut().unwrap();
+ let root = fdt.root_mut().unwrap();
let names = [cstr!("a"), cstr!("b")];
root.add_subnodes(&names).unwrap();
diff --git a/pvmfw/platform.dts b/pvmfw/platform.dts
index cea1c33..275a1c9 100644
--- a/pvmfw/platform.dts
+++ b/pvmfw/platform.dts
@@ -52,11 +52,40 @@
cpus {
#address-cells = <1>;
#size-cells = <0>;
- cpu@0 {
+
+ cpu-map {
+ cluster0 {
+ core0 { cpu = <PLACEHOLDER>; };
+ core1 { cpu = <PLACEHOLDER>; };
+ core2 { cpu = <PLACEHOLDER>; };
+ core3 { cpu = <PLACEHOLDER>; };
+ core4 { cpu = <PLACEHOLDER>; };
+ core5 { cpu = <PLACEHOLDER>; };
+ };
+ cluster1 {
+ core0 { cpu = <PLACEHOLDER>; };
+ core1 { cpu = <PLACEHOLDER>; };
+ core2 { cpu = <PLACEHOLDER>; };
+ core3 { cpu = <PLACEHOLDER>; };
+ core4 { cpu = <PLACEHOLDER>; };
+ core5 { cpu = <PLACEHOLDER>; };
+ };
+ cluster2 {
+ core0 { cpu = <PLACEHOLDER>; };
+ core1 { cpu = <PLACEHOLDER>; };
+ core2 { cpu = <PLACEHOLDER>; };
+ core3 { cpu = <PLACEHOLDER>; };
+ core4 { cpu = <PLACEHOLDER>; };
+ core5 { cpu = <PLACEHOLDER>; };
+ };
+ };
+
+ cpu0: cpu@0 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <0>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table0>;
opp_table0: opp-table-0 {
compatible = "operating-points-v2";
@@ -83,11 +112,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@1 {
+ cpu1: cpu@1 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <1>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table1>;
opp_table1: opp-table-1 {
compatible = "operating-points-v2";
@@ -114,11 +144,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@2 {
+ cpu2: cpu@2 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <2>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table2>;
opp_table2: opp-table-2 {
compatible = "operating-points-v2";
@@ -145,11 +176,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@3 {
+ cpu3: cpu@3 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <3>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table3>;
opp_table3: opp-table-3 {
compatible = "operating-points-v2";
@@ -176,11 +208,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@4 {
+ cpu4: cpu@4 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <4>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table4>;
opp_table4: opp-table-4 {
compatible = "operating-points-v2";
@@ -207,11 +240,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@5 {
+ cpu5: cpu@5 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <5>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table5>;
opp_table5: opp-table-5 {
compatible = "operating-points-v2";
@@ -238,11 +272,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@6 {
+ cpu6: cpu@6 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <6>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table6>;
opp_table6: opp-table-6 {
compatible = "operating-points-v2";
@@ -269,11 +304,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@7 {
+ cpu7: cpu@7 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <7>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table7>;
opp_table7: opp-table-7 {
compatible = "operating-points-v2";
@@ -300,12 +336,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@8 {
+ cpu8: cpu@8 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <8>;
-
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table8>;
opp_table8: opp-table-8 {
compatible = "operating-points-v2";
@@ -332,11 +368,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@9 {
+ cpu9: cpu@9 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <9>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table9>;
opp_table9: opp-table-9 {
compatible = "operating-points-v2";
@@ -363,11 +400,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@10 {
+ cpu10: cpu@10 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <10>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table10>;
opp_table10: opp-table-10 {
compatible = "operating-points-v2";
@@ -394,11 +432,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@11 {
+ cpu11: cpu@11 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <11>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table11>;
opp_table11: opp-table-11 {
compatible = "operating-points-v2";
@@ -425,11 +464,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@12 {
+ cpu12: cpu@12 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <12>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table12>;
opp_table12: opp-table-12 {
compatible = "operating-points-v2";
@@ -456,11 +496,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@13 {
+ cpu13: cpu@13 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <13>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table13>;
opp_table13: opp-table-13 {
compatible = "operating-points-v2";
@@ -487,11 +528,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@14 {
+ cpu14: cpu@14 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <14>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table14>;
opp_table14: opp-table-14 {
compatible = "operating-points-v2";
@@ -518,11 +560,12 @@
opp20 { opp-hz = <PLACEHOLDER2>; };
};
};
- cpu@15 {
+ cpu15: cpu@15 {
device_type = "cpu";
compatible = "arm,arm-v8";
enable-method = "psci";
reg = <15>;
+ capacity-dmips-mhz = <PLACEHOLDER>;
operating-points-v2 = <&opp_table15>;
opp_table15: opp-table-15 {
compatible = "operating-points-v2";
diff --git a/pvmfw/src/fdt.rs b/pvmfw/src/fdt.rs
index f20451a..146d012 100644
--- a/pvmfw/src/fdt.rs
+++ b/pvmfw/src/fdt.rs
@@ -21,6 +21,7 @@
use crate::RebootReason;
use alloc::collections::BTreeMap;
use alloc::ffi::CString;
+use alloc::format;
use alloc::vec::Vec;
use core::cmp::max;
use core::cmp::min;
@@ -37,6 +38,7 @@
use libfdt::FdtError;
use libfdt::FdtNode;
use libfdt::FdtNodeMut;
+use libfdt::Phandle;
use log::debug;
use log::error;
use log::info;
@@ -57,6 +59,8 @@
InvalidCpuCount(usize),
/// Invalid VCpufreq Range.
InvalidVcpufreq(u64, u64),
+ /// Forbidden /avf/untrusted property.
+ ForbiddenUntrustedProp(&'static CStr),
}
impl fmt::Display for FdtValidationError {
@@ -66,6 +70,9 @@
Self::InvalidVcpufreq(addr, size) => {
write!(f, "Invalid vcpufreq region: ({addr:#x}, {size:#x})")
}
+ Self::ForbiddenUntrustedProp(name) => {
+ write!(f, "Forbidden /avf/untrusted property '{name:?}'")
+ }
}
}
}
@@ -178,10 +185,10 @@
.setprop_inplace(cstr!("reg"), [addr.to_be(), size.to_be()].as_bytes())
}
-//TODO: Need to add info for cpu capacity
#[derive(Debug, Default)]
struct CpuInfo {
opptable_info: Option<ArrayVec<[u64; CpuInfo::MAX_OPPTABLES]>>,
+ cpu_capacity: Option<u32>,
}
impl CpuInfo {
@@ -200,10 +207,64 @@
Ok(table)
}
-fn read_cpu_info_from(fdt: &Fdt) -> libfdt::Result<ArrayVec<[CpuInfo; DeviceTreeInfo::MAX_CPUS]>> {
+#[derive(Debug, Default)]
+struct ClusterTopology {
+ // TODO: Support multi-level clusters & threads.
+ cores: [Option<usize>; ClusterTopology::MAX_CORES_PER_CLUSTER],
+}
+
+impl ClusterTopology {
+ const MAX_CORES_PER_CLUSTER: usize = 6;
+}
+
+#[derive(Debug, Default)]
+struct CpuTopology {
+ // TODO: Support sockets.
+ clusters: [Option<ClusterTopology>; CpuTopology::MAX_CLUSTERS],
+}
+
+impl CpuTopology {
+ const MAX_CLUSTERS: usize = 3;
+}
+
+fn read_cpu_map_from(fdt: &Fdt) -> libfdt::Result<Option<BTreeMap<Phandle, (usize, usize)>>> {
+ let Some(cpu_map) = fdt.node(cstr!("/cpus/cpu-map"))? else {
+ return Ok(None);
+ };
+
+ let mut topology = BTreeMap::new();
+ for n in 0..CpuTopology::MAX_CLUSTERS {
+ let name = CString::new(format!("cluster{n}")).unwrap();
+ let Some(cluster) = cpu_map.subnode(&name)? else {
+ break;
+ };
+ for m in 0..ClusterTopology::MAX_CORES_PER_CLUSTER {
+ let name = CString::new(format!("core{m}")).unwrap();
+ let Some(core) = cluster.subnode(&name)? else {
+ break;
+ };
+ let cpu = core.getprop_u32(cstr!("cpu"))?.ok_or(FdtError::NotFound)?;
+ let prev = topology.insert(cpu.try_into()?, (n, m));
+ if prev.is_some() {
+ return Err(FdtError::BadValue);
+ }
+ }
+ }
+
+ Ok(Some(topology))
+}
+
+fn read_cpu_info_from(
+ fdt: &Fdt,
+) -> libfdt::Result<(ArrayVec<[CpuInfo; DeviceTreeInfo::MAX_CPUS]>, Option<CpuTopology>)> {
let mut cpus = ArrayVec::new();
+
+ let cpu_map = read_cpu_map_from(fdt)?;
+ let mut topology: CpuTopology = Default::default();
+
let mut cpu_nodes = fdt.compatible_nodes(cstr!("arm,arm-v8"))?;
- for cpu in cpu_nodes.by_ref().take(cpus.capacity()) {
+ for (idx, cpu) in cpu_nodes.by_ref().take(cpus.capacity()).enumerate() {
+ let cpu_capacity = cpu.getprop_u32(cstr!("capacity-dmips-mhz"))?;
let opp_phandle = cpu.getprop_u32(cstr!("operating-points-v2"))?;
let opptable_info = if let Some(phandle) = opp_phandle {
let phandle = phandle.try_into()?;
@@ -212,21 +273,31 @@
} else {
None
};
- let info = CpuInfo { opptable_info };
+ let info = CpuInfo { opptable_info, cpu_capacity };
cpus.push(info);
+
+ if let Some(ref cpu_map) = cpu_map {
+ let phandle = cpu.get_phandle()?.ok_or(FdtError::NotFound)?;
+ let (cluster, core_idx) = cpu_map.get(&phandle).ok_or(FdtError::BadValue)?;
+ let cluster = topology.clusters[*cluster].get_or_insert(Default::default());
+ if cluster.cores[*core_idx].is_some() {
+ return Err(FdtError::BadValue);
+ }
+ cluster.cores[*core_idx] = Some(idx);
+ }
}
+
if cpu_nodes.next().is_some() {
warn!("DT has more than {} CPU nodes: discarding extra nodes.", cpus.capacity());
}
- Ok(cpus)
+ Ok((cpus, cpu_map.map(|_| topology)))
}
fn validate_cpu_info(cpus: &[CpuInfo]) -> Result<(), FdtValidationError> {
if cpus.is_empty() {
return Err(FdtValidationError::InvalidCpuCount(0));
}
-
Ok(())
}
@@ -304,19 +375,74 @@
Ok(node)
}
-fn patch_cpus(fdt: &mut Fdt, cpus: &[CpuInfo]) -> libfdt::Result<()> {
+fn patch_cpus(
+ fdt: &mut Fdt,
+ cpus: &[CpuInfo],
+ topology: &Option<CpuTopology>,
+) -> libfdt::Result<()> {
const COMPAT: &CStr = cstr!("arm,arm-v8");
+ let mut cpu_phandles = Vec::new();
for (idx, cpu) in cpus.iter().enumerate() {
- let cur = get_nth_compatible(fdt, idx, COMPAT)?.ok_or(FdtError::NoSpace)?;
+ let mut cur = get_nth_compatible(fdt, idx, COMPAT)?.ok_or(FdtError::NoSpace)?;
+ let phandle = cur.as_node().get_phandle()?.unwrap();
+ cpu_phandles.push(phandle);
+ if let Some(cpu_capacity) = cpu.cpu_capacity {
+ cur.setprop_inplace(cstr!("capacity-dmips-mhz"), &cpu_capacity.to_be_bytes())?;
+ }
patch_opptable(cur, cpu.opptable_info)?;
}
let mut next = get_nth_compatible(fdt, cpus.len(), COMPAT)?;
while let Some(current) = next {
next = current.delete_and_next_compatible(COMPAT)?;
}
+
+ if let Some(topology) = topology {
+ for (n, cluster) in topology.clusters.iter().enumerate() {
+ let path = CString::new(format!("/cpus/cpu-map/cluster{n}")).unwrap();
+ let cluster_node = fdt.node_mut(&path)?.unwrap();
+ if let Some(cluster) = cluster {
+ let mut iter = cluster_node.first_subnode()?;
+ for core in cluster.cores {
+ let mut core_node = iter.unwrap();
+ iter = if let Some(core_idx) = core {
+ let phandle = *cpu_phandles.get(core_idx).unwrap();
+ let value = u32::from(phandle).to_be_bytes();
+ core_node.setprop_inplace(cstr!("cpu"), &value)?;
+ core_node.next_subnode()?
+ } else {
+ core_node.delete_and_next_subnode()?
+ };
+ }
+ assert!(iter.is_none());
+ } else {
+ cluster_node.nop()?;
+ }
+ }
+ } else {
+ fdt.node_mut(cstr!("/cpus/cpu-map"))?.unwrap().nop()?;
+ }
+
Ok(())
}
+/// Reads the /avf/untrusted DT node, which the host can use to pass properties (no subnodes) to
+/// the guest that don't require being validated by pvmfw.
+fn parse_untrusted_props(fdt: &Fdt) -> libfdt::Result<BTreeMap<CString, Vec<u8>>> {
+ let mut props = BTreeMap::new();
+ if let Some(node) = fdt.node(cstr!("/avf/untrusted"))? {
+ for property in node.properties()? {
+ let name = property.name()?;
+ let value = property.value()?;
+ props.insert(CString::from(name), value.to_vec());
+ }
+ if node.subnodes()?.next().is_some() {
+ warn!("Discarding unexpected /avf/untrusted subnodes.");
+ }
+ }
+
+ Ok(props)
+}
+
/// Read candidate properties' names from DT which could be overlaid
fn parse_vm_ref_dt(fdt: &Fdt) -> libfdt::Result<BTreeMap<CString, Vec<u8>>> {
let mut property_map = BTreeMap::new();
@@ -333,6 +459,19 @@
Ok(property_map)
}
+fn validate_untrusted_props(props: &BTreeMap<CString, Vec<u8>>) -> Result<(), FdtValidationError> {
+ const FORBIDDEN_PROPS: &[&CStr] =
+ &[cstr!("compatible"), cstr!("linux,phandle"), cstr!("phandle")];
+
+ for name in FORBIDDEN_PROPS {
+ if props.contains_key(*name) {
+ return Err(FdtValidationError::ForbiddenUntrustedProp(name));
+ }
+ }
+
+ Ok(())
+}
+
/// Overlay VM reference DT into VM DT based on the props_info. Property is overlaid in vm_dt only
/// when it exists both in vm_ref_dt and props_info. If the values mismatch, it returns error.
fn validate_vm_ref_dt(
@@ -340,7 +479,7 @@
vm_ref_dt: &Fdt,
props_info: &BTreeMap<CString, Vec<u8>>,
) -> libfdt::Result<()> {
- let mut root_vm_dt = vm_dt.root_mut()?;
+ let root_vm_dt = vm_dt.root_mut()?;
let mut avf_vm_dt = root_vm_dt.add_subnode(cstr!("avf"))?;
// TODO(b/318431677): Validate nodes beyond /avf.
let avf_node = vm_ref_dt.node(cstr!("/avf"))?.ok_or(FdtError::NotFound)?;
@@ -734,6 +873,23 @@
node.setprop_inplace(cstr!("interrupts"), value.as_bytes())
}
+fn patch_untrusted_props(fdt: &mut Fdt, props: &BTreeMap<CString, Vec<u8>>) -> libfdt::Result<()> {
+ let avf_node = if let Some(node) = fdt.node_mut(cstr!("/avf"))? {
+ node
+ } else {
+ fdt.root_mut()?.add_subnode(cstr!("avf"))?
+ };
+
+ // The node shouldn't already be present; if it is, return the error.
+ let mut node = avf_node.add_subnode(cstr!("untrusted"))?;
+
+ for (name, value) in props {
+ node.setprop(name, value)?;
+ }
+
+ Ok(())
+}
+
#[derive(Debug)]
struct VcpufreqInfo {
addr: u64,
@@ -756,10 +912,12 @@
pub memory_range: Range<usize>,
bootargs: Option<CString>,
cpus: ArrayVec<[CpuInfo; DeviceTreeInfo::MAX_CPUS]>,
+ cpu_topology: Option<CpuTopology>,
pci_info: PciInfo,
serial_info: SerialInfo,
pub swiotlb_info: SwiotlbInfo,
device_assignment: Option<DeviceAssignmentInfo>,
+ untrusted_props: BTreeMap<CString, Vec<u8>>,
vm_ref_dt_props_info: BTreeMap<CString, Vec<u8>>,
vcpufreq_info: Option<VcpufreqInfo>,
}
@@ -865,7 +1023,7 @@
RebootReason::InvalidFdt
})?;
- let cpus = read_cpu_info_from(fdt).map_err(|e| {
+ let (cpus, cpu_topology) = read_cpu_info_from(fdt).map_err(|e| {
error!("Failed to read CPU info from DT: {e}");
RebootReason::InvalidFdt
})?;
@@ -919,6 +1077,15 @@
None => None,
};
+ let untrusted_props = parse_untrusted_props(fdt).map_err(|e| {
+ error!("Failed to read untrusted properties: {e}");
+ RebootReason::InvalidFdt
+ })?;
+ validate_untrusted_props(&untrusted_props).map_err(|e| {
+ error!("Failed to validate untrusted properties: {e}");
+ RebootReason::InvalidFdt
+ })?;
+
let vm_ref_dt_props_info = parse_vm_ref_dt(fdt).map_err(|e| {
error!("Failed to read names of properties under /avf from DT: {e}");
RebootReason::InvalidFdt
@@ -930,10 +1097,12 @@
memory_range,
bootargs,
cpus,
+ cpu_topology,
pci_info,
serial_info,
swiotlb_info,
device_assignment,
+ untrusted_props,
vm_ref_dt_props_info,
vcpufreq_info,
})
@@ -956,7 +1125,7 @@
RebootReason::InvalidFdt
})?;
}
- patch_cpus(fdt, &info.cpus).map_err(|e| {
+ patch_cpus(fdt, &info.cpus, &info.cpu_topology).map_err(|e| {
error!("Failed to patch cpus to DT: {e}");
RebootReason::InvalidFdt
})?;
@@ -992,6 +1161,10 @@
RebootReason::InvalidFdt
})?;
}
+ patch_untrusted_props(fdt, &info.untrusted_props).map_err(|e| {
+ error!("Failed to patch untrusted properties: {e}");
+ RebootReason::InvalidFdt
+ })?;
Ok(())
}
diff --git a/rialto/src/main.rs b/rialto/src/main.rs
index ad9b776..48b69b3 100644
--- a/rialto/src/main.rs
+++ b/rialto/src/main.rs
@@ -177,7 +177,9 @@
let mut vsock_stream = VsockStream::new(socket_device, host_addr())?;
while let ServiceVmRequest::Process(req) = vsock_stream.read_request()? {
+ info!("Received request: {}", req.name());
let response = process_request(req, bcc_handover.as_ref());
+ info!("Sending response: {}", response.name());
vsock_stream.write_response(&response)?;
vsock_stream.flush()?;
}
diff --git a/service_vm/comm/src/message.rs b/service_vm/comm/src/message.rs
index 80a9608..9f83b78 100644
--- a/service_vm/comm/src/message.rs
+++ b/service_vm/comm/src/message.rs
@@ -56,6 +56,18 @@
RequestClientVmAttestation(ClientVmAttestationParams),
}
+impl Request {
+ /// Returns the name of the request.
+ pub fn name(&self) -> &'static str {
+ match self {
+ Self::Reverse(_) => "Reverse",
+ Self::GenerateEcdsaP256KeyPair => "GenerateEcdsaP256KeyPair",
+ Self::GenerateCertificateRequest(_) => "GenerateCertificateRequest",
+ Self::RequestClientVmAttestation(_) => "RequestClientVmAttestation",
+ }
+ }
+}
+
/// Represents the params passed to `Request::RequestClientVmAttestation`.
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct ClientVmAttestationParams {
@@ -98,6 +110,19 @@
Err(RequestProcessingError),
}
+impl Response {
+ /// Returns the name of the response.
+ pub fn name(&self) -> &'static str {
+ match self {
+ Self::Reverse(_) => "Reverse",
+ Self::GenerateEcdsaP256KeyPair(_) => "GenerateEcdsaP256KeyPair",
+ Self::GenerateCertificateRequest(_) => "GenerateCertificateRequest",
+ Self::RequestClientVmAttestation(_) => "RequestClientVmAttestation",
+ Self::Err(_) => "Err",
+ }
+ }
+}
+
/// Errors related to request processing.
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub enum RequestProcessingError {
diff --git a/tests/benchmark/src/java/com/android/microdroid/benchmark/MicrodroidBenchmarks.java b/tests/benchmark/src/java/com/android/microdroid/benchmark/MicrodroidBenchmarks.java
index 2d52732..77cae32 100644
--- a/tests/benchmark/src/java/com/android/microdroid/benchmark/MicrodroidBenchmarks.java
+++ b/tests/benchmark/src/java/com/android/microdroid/benchmark/MicrodroidBenchmarks.java
@@ -415,8 +415,9 @@
long guestRss = 0;
long guestPss = 0;
boolean hasGuestMaps = false;
- for (ProcessUtil.SMapEntry entry :
- ProcessUtil.getProcessSmaps(vmPid, shellExecutor)) {
+ List<ProcessUtil.SMapEntry> smaps =
+ ProcessUtil.getProcessSmaps(vmPid, shellExecutor);
+ for (ProcessUtil.SMapEntry entry : smaps) {
long rss = entry.metrics.get("Rss");
long pss = entry.metrics.get("Pss");
if (entry.name.contains("crosvm_guest")) {
@@ -429,8 +430,12 @@
}
}
if (!hasGuestMaps) {
+ StringBuilder sb = new StringBuilder();
+ for (ProcessUtil.SMapEntry entry : smaps) {
+ sb.append(entry.toString());
+ }
throw new IllegalStateException(
- "found no crosvm_guest smap entry in crosvm process");
+ "found no crosvm_guest smap entry in crosvm process: " + sb);
}
mHostRss = hostRss;
mHostPss = hostPss;
diff --git a/tests/helper/src/java/com/android/microdroid/test/common/ProcessUtil.java b/tests/helper/src/java/com/android/microdroid/test/common/ProcessUtil.java
index c72d91e..e058674 100644
--- a/tests/helper/src/java/com/android/microdroid/test/common/ProcessUtil.java
+++ b/tests/helper/src/java/com/android/microdroid/test/common/ProcessUtil.java
@@ -33,13 +33,24 @@
public static class SMapEntry {
public String name;
public Map<String, Long> metrics;
+
+ @Override
+ public String toString() {
+ StringBuilder sb = new StringBuilder();
+ sb.append("name: " + name + "\n");
+ metrics.forEach(
+ (k, v) -> {
+ sb.append(" " + k + ": " + v + "\n");
+ });
+ return sb.toString();
+ }
}
/** Gets metrics key and values mapping of specified process id */
public static List<SMapEntry> getProcessSmaps(int pid, Function<String, String> shellExecutor)
throws IOException {
String path = "/proc/" + pid + "/smaps";
- return parseMemoryInfo(shellExecutor.apply("cat " + path + " || true"));
+ return parseMemoryInfo(shellExecutor.apply("cat " + path));
}
/** Gets metrics key and values mapping of specified process id */
diff --git a/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java b/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
index 8e11218..2c72561 100644
--- a/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
+++ b/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
@@ -62,6 +62,7 @@
private static final String TAG = "MicrodroidDeviceTestBase";
private final String MAX_PERFORMANCE_TASK_PROFILE = "CPUSET_SP_TOP_APP";
+ protected static final String KERNEL_VERSION = SystemProperties.get("ro.kernel.version");
protected static final Set<String> SUPPORTED_GKI_VERSIONS =
Collections.unmodifiableSet(new HashSet(Arrays.asList("android14-6.1")));
@@ -110,7 +111,7 @@
}
}
- private Context mCtx;
+ private final Context mCtx = ApplicationProvider.getApplicationContext();
private boolean mProtectedVm;
private String mGki;
@@ -165,10 +166,7 @@
}
public void prepareTestSetup(boolean protectedVm, String gki) {
- mCtx = ApplicationProvider.getApplicationContext();
- assume().withMessage("Device doesn't support AVF")
- .that(mCtx.getPackageManager().hasSystemFeature(FEATURE_VIRTUALIZATION_FRAMEWORK))
- .isTrue();
+ assumeFeatureVirtualizationFramework();
mProtectedVm = protectedVm;
mGki = gki;
@@ -194,6 +192,18 @@
}
}
+ protected void assumeFeatureVirtualizationFramework() {
+ assume().withMessage("Device doesn't support AVF")
+ .that(mCtx.getPackageManager().hasSystemFeature(FEATURE_VIRTUALIZATION_FRAMEWORK))
+ .isTrue();
+ }
+
+ protected void assumeSupportedDevice() {
+ assume().withMessage("Skip on 5.4 kernel. b/218303240")
+ .that(KERNEL_VERSION)
+ .isNotEqualTo("5.4");
+ }
+
public abstract static class VmEventListener implements VirtualMachineCallback {
private ExecutorService mExecutorService = Executors.newSingleThreadExecutor();
private OptionalLong mVcpuStartedNanoTime = OptionalLong.empty();
diff --git a/tests/hostside/Android.bp b/tests/hostside/Android.bp
index e3d9cbe..13a9925 100644
--- a/tests/hostside/Android.bp
+++ b/tests/hostside/Android.bp
@@ -37,6 +37,8 @@
"lpunpack",
"sign_virt_apex",
"simg2img",
+ "dtdiff",
+ "dtc", // for dtdiff
],
// java_test_host doesn't have data_native_libs but jni_libs can be used to put
// native modules under ./lib directory.
@@ -51,5 +53,6 @@
"liblp",
"libsparse",
"libz",
+ "libfdt", // for dtc
],
}
diff --git a/tests/hostside/java/com/android/microdroid/test/MicrodroidHostTests.java b/tests/hostside/java/com/android/microdroid/test/MicrodroidHostTests.java
index 2cd4577..20b5a50 100644
--- a/tests/hostside/java/com/android/microdroid/test/MicrodroidHostTests.java
+++ b/tests/hostside/java/com/android/microdroid/test/MicrodroidHostTests.java
@@ -47,6 +47,7 @@
import com.android.tradefed.device.TestDevice;
import com.android.tradefed.testtype.DeviceJUnit4ClassRunner.TestMetrics;
import com.android.tradefed.util.CommandResult;
+import com.android.tradefed.util.CommandStatus;
import com.android.tradefed.util.FileUtil;
import com.android.tradefed.util.RunUtil;
import com.android.tradefed.util.xml.AbstractXmlParser;
@@ -985,23 +986,71 @@
@Test
public void testDeviceAssignment() throws Exception {
- assumeProtectedVm();
+ // Check for preconditions
assumeVfioPlatformSupported();
List<String> devices = getAssignableDevices();
assumeFalse("no assignable devices", devices.isEmpty());
+ String vmFdtPath = "/sys/firmware/fdt";
+ File testDir = FileUtil.createTempDir("device_assignment_test");
+ File baseFdtFile = new File(testDir, "base_fdt.dtb");
+ File fdtFile = new File(testDir, "fdt.dtb");
+
+ // Generates baseline DT
+ launchWithDeviceAssignment(/* device= */ null);
+ assertThat(mMicrodroidDevice.pullFile(vmFdtPath, baseFdtFile)).isTrue();
+ getAndroidDevice().shutdownMicrodroid(mMicrodroidDevice);
+
+ // Prepares to run dtdiff. It requires dtc.
+ File dtdiff = findTestFile("dtdiff");
+ RunUtil runner = new RunUtil();
+ String separator = System.getProperty("path.separator");
+ String path = dtdiff.getParent() + separator + System.getenv("PATH");
+ runner.setEnvVariable("PATH", path);
+
+ // Try assign devices one by one
+ for (String device : devices) {
+ assertThat(device).isNotNull();
+ launchWithDeviceAssignment(device);
+ assertThat(mMicrodroidDevice.pullFile(vmFdtPath, fdtFile)).isTrue();
+ getAndroidDevice().shutdownMicrodroid(mMicrodroidDevice);
+
+ CommandResult result =
+ runner.runTimedCmd(
+ 500,
+ dtdiff.getAbsolutePath(),
+ baseFdtFile.getPath(),
+ fdtFile.getPath());
+
+ assertWithMessage(
+ "VM's device tree hasn't changed when assigning "
+ + device
+ + ", result="
+ + result)
+ .that(result.getStatus())
+ .isNotEqualTo(CommandStatus.SUCCESS);
+ }
+
+ mMicrodroidDevice = null;
+ }
+
+ private void launchWithDeviceAssignment(String device) throws Exception {
final String configPath = "assets/" + mOs + "/vm_config.json";
- mMicrodroidDevice =
+
+ MicrodroidBuilder builder =
MicrodroidBuilder.fromDevicePath(getPathForPackage(PACKAGE_NAME), configPath)
.debugLevel("full")
.memoryMib(minMemorySize())
.cpuTopology("match_host")
- .protectedVm(true)
- .addAssignableDevice(devices.get(0))
- .build(getAndroidDevice());
+ .protectedVm(mProtectedVm);
+ if (device != null) {
+ builder.addAssignableDevice(device);
+ }
+ mMicrodroidDevice = builder.build(getAndroidDevice());
- mMicrodroidDevice.waitForBootComplete(BOOT_COMPLETE_TIMEOUT);
+ assertThat(mMicrodroidDevice.waitForBootComplete(BOOT_COMPLETE_TIMEOUT)).isTrue();
+ assertThat(mMicrodroidDevice.enableAdbRoot()).isTrue();
}
@Test
diff --git a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidCapabilitiesTest.java b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidCapabilitiesTest.java
new file mode 100644
index 0000000..a0826c9
--- /dev/null
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidCapabilitiesTest.java
@@ -0,0 +1,58 @@
+/*
+ * Copyright 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.microdroid.test;
+
+import static com.google.common.truth.Truth.assertWithMessage;
+
+import android.system.virtualmachine.VirtualMachineManager;
+
+import com.android.compatibility.common.util.CddTest;
+import com.android.microdroid.test.device.MicrodroidDeviceTestBase;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Test the advertised AVF capabilities include the ability to start some type of VM.
+ *
+ * <p>Tests in MicrodroidTests run on either protected or non-protected VMs, provided they are
+ * supported. If neither is they are all skipped. So we need a separate test (that doesn't call
+ * {@link #prepareTestSetup}) to make sure that at least one of these is available.
+ */
+@RunWith(JUnit4.class)
+public class MicrodroidCapabilitiesTest extends MicrodroidDeviceTestBase {
+ @Test
+ @CddTest(requirements = {"9.17/C-1-1", "9.17/C-2-1"})
+ public void supportForProtectedOrNonProtectedVms() {
+ assumeSupportedDevice();
+
+ // (There's a test for devices that don't expose the system feature over in
+ // NoMicrodroidTest.)
+ assumeFeatureVirtualizationFramework();
+
+ int capabilities = getVirtualMachineManager().getCapabilities();
+ int vmCapabilities =
+ capabilities
+ & (VirtualMachineManager.CAPABILITY_PROTECTED_VM
+ | VirtualMachineManager.CAPABILITY_NON_PROTECTED_VM);
+ assertWithMessage(
+ "A device that has FEATURE_VIRTUALIZATION_FRAMEWORK must support at least"
+ + " one of protected or non-protected VMs")
+ .that(vmCapabilities)
+ .isNotEqualTo(0);
+ }
+}
diff --git a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
index 0687a7b..070478e 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -118,8 +118,6 @@
@Rule public Timeout globalTimeout = Timeout.seconds(300);
- private static final String KERNEL_VERSION = SystemProperties.get("ro.kernel.version");
-
@Parameterized.Parameters(name = "protectedVm={0},gki={1}")
public static Collection<Object[]> params() {
List<Object[]> ret = new ArrayList<>();
@@ -2312,10 +2310,4 @@
return 0;
}
- private void assumeSupportedDevice() {
- assume()
- .withMessage("Skip on 5.4 kernel. b/218303240")
- .that(KERNEL_VERSION)
- .isNotEqualTo("5.4");
- }
}
diff --git a/virtualizationmanager/Android.bp b/virtualizationmanager/Android.bp
index 48b5cd1..c46385c 100644
--- a/virtualizationmanager/Android.bp
+++ b/virtualizationmanager/Android.bp
@@ -35,6 +35,7 @@
"libavflog",
"libbase_rust",
"libbinder_rs",
+ "libcfg_if",
"libclap",
"libcstr",
"libcommand_fds",
diff --git a/virtualizationmanager/src/crosvm.rs b/virtualizationmanager/src/crosvm.rs
index 2c23441..3380df3 100644
--- a/virtualizationmanager/src/crosvm.rs
+++ b/virtualizationmanager/src/crosvm.rs
@@ -450,20 +450,20 @@
let mut vm_metric = self.vm_metric.lock().unwrap();
// Get CPU Information
- if let Ok(guest_time) = get_guest_time(pid) {
- vm_metric.cpu_guest_time = Some(guest_time);
- } else {
- error!("Failed to parse /proc/[pid]/stat");
+ match get_guest_time(pid) {
+ Ok(guest_time) => vm_metric.cpu_guest_time = Some(guest_time),
+ Err(e) => error!("Failed to get guest CPU time: {e:?}"),
}
// Get Memory Information
- if let Ok(rss) = get_rss(pid) {
- vm_metric.rss = match &vm_metric.rss {
- Some(x) => Some(Rss::extract_max(x, &rss)),
- None => Some(rss),
+ match get_rss(pid) {
+ Ok(rss) => {
+ vm_metric.rss = match &vm_metric.rss {
+ Some(x) => Some(Rss::extract_max(x, &rss)),
+ None => Some(rss),
+ }
}
- } else {
- error!("Failed to parse /proc/[pid]/smaps");
+ Err(e) => error!("Failed to get guest RSS: {}", e),
}
}
@@ -812,7 +812,11 @@
if config.host_cpu_topology {
if cfg!(virt_cpufreq) {
command.arg("--host-cpu-topology");
- command.arg("--virt-cpufreq");
+ cfg_if::cfg_if! {
+ if #[cfg(any(target_arch = "aarch64"))] {
+ command.arg("--virt-cpufreq");
+ }
+ }
} else if let Some(cpus) = get_num_cpus() {
command.arg("--cpus").arg(cpus.to_string());
} else {
diff --git a/virtualizationmanager/src/dt_overlay.rs b/virtualizationmanager/src/dt_overlay.rs
index 83f7734..71d3a26 100644
--- a/virtualizationmanager/src/dt_overlay.rs
+++ b/virtualizationmanager/src/dt_overlay.rs
@@ -57,20 +57,19 @@
let fdt =
Fdt::create_empty_tree(buffer).map_err(|e| anyhow!("Failed to create empty Fdt: {e:?}"))?;
- let mut root = fdt.root_mut().map_err(|e| anyhow!("Failed to get root: {e:?}"))?;
+ let root = fdt.root_mut().map_err(|e| anyhow!("Failed to get root: {e:?}"))?;
let mut node =
root.add_subnode(cstr!("fragment@0")).map_err(|e| anyhow!("Failed to fragment: {e:?}"))?;
node.setprop(cstr!("target-path"), b"/\0")
.map_err(|e| anyhow!("Failed to set target-path: {e:?}"))?;
- let mut node = node
+ let node = node
.add_subnode(cstr!("__overlay__"))
.map_err(|e| anyhow!("Failed to __overlay__ node: {e:?}"))?;
if !untrusted_props.is_empty() {
let mut node = node
.add_subnode(AVF_NODE_NAME)
- .map_err(|e| anyhow!("Failed to add avf node: {e:?}"))?;
- let mut node = node
+ .map_err(|e| anyhow!("Failed to add avf node: {e:?}"))?
.add_subnode(UNTRUSTED_NODE_NAME)
.map_err(|e| anyhow!("Failed to add /avf/untrusted node: {e:?}"))?;
for (name, value) in untrusted_props {
diff --git a/vmbase/example/src/main.rs b/vmbase/example/src/main.rs
index 6f513ee..48b24be 100644
--- a/vmbase/example/src/main.rs
+++ b/vmbase/example/src/main.rs
@@ -181,7 +181,7 @@
info!("FDT successfully unpacked.");
let path = cstr!("/memory");
- let mut node = writer.node_mut(path).unwrap().unwrap();
+ let node = writer.node_mut(path).unwrap().unwrap();
let name = cstr!("child");
let mut child = node.add_subnode(name).unwrap();
info!("Created subnode '{}/{}'.", path.to_str().unwrap(), name.to_str().unwrap());