Skip to content

rust/kernel/of: construct of_match_table structure at build time #380

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions drivers/char/hw_random/bcm2835_rng_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use kernel::{
file_operations::{FileOpener, FileOperations},
io_buffer::IoBufferWriter,
miscdev,
of::OfMatchTable,
of::ConstOfMatchTable,
platdev::PlatformDriver,
prelude::*,
{c_str, platdev},
Expand Down Expand Up @@ -72,11 +72,12 @@ struct RngModule {

impl KernelModule for RngModule {
fn init() -> Result<Self> {
let of_match_tbl = OfMatchTable::new(&c_str!("brcm,bcm2835-rng"))?;
const OF_MATCH_TBL: ConstOfMatchTable<1> =
ConstOfMatchTable::new_const([&c_str!("brcm,bcm2835-rng")]);

let pdev = platdev::Registration::new_pinned::<RngDriver>(
c_str!("bcm2835-rng-rust"),
Some(of_match_tbl),
Some(&OF_MATCH_TBL),
&THIS_MODULE,
)?;

Expand Down
133 changes: 75 additions & 58 deletions rust/kernel/of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,80 +4,97 @@
//!
//! C header: [`include/linux/of_*.h`](../../../../include/linux/of_*.h)

use alloc::boxed::Box;
use crate::{bindings, c_types, str::CStr};

use crate::{
bindings, c_types,
error::{Error, Result},
str::CStr,
types::PointerWrapper,
};
use core::ops::Deref;
use core::ptr;

use core::mem::transmute;

type InnerTable = Box<[bindings::of_device_id; 2]>;

/// Wraps a kernel Open Firmware / devicetree match table.
/// A kernel Open Firmware / devicetree match table.
///
/// Rust drivers may create this structure to match against devices
/// described in the devicetree.
///
/// The ['PointerWrapper'] trait provides conversion to/from a raw pointer,
/// suitable to be assigned to a `bindings::device_driver::of_match_table`.
/// Can only exist as an `&OfMatchTable` reference (akin to `&str` or
/// `&Path` in Rust std).
///
/// # Invariants
///
/// The final array element is always filled with zeros (the default).
pub struct OfMatchTable(InnerTable);
/// The inner reference points to a sentinel-terminated C array.
#[repr(transparent)]
pub struct OfMatchTable(bindings::of_device_id);

impl OfMatchTable {
/// Creates a [`OfMatchTable`] from a single `compatible` string.
pub fn new(compatible: &'static CStr) -> Result<Self> {
let tbl = Box::try_new([
Self::new_of_device_id(compatible)?,
bindings::of_device_id::default(),
])?;
// INVARIANTS: we allocated an array with `default()` as its final
// element, therefore that final element will be filled with zeros,
// and the invariant above will hold.
Ok(Self(tbl))
/// Returns the table as a reference to a static lifetime, sentinel-terminated C array.
///
/// This is suitable to be coerced into the kernel's `of_match_table` field.
pub fn as_ptr(&'static self) -> &'static bindings::of_device_id {
// The inner reference points to a sentinel-terminated C array, as per
// the type invariant.
&self.0
}
}

fn new_of_device_id(compatible: &'static CStr) -> Result<bindings::of_device_id> {
let mut buf = [0_u8; 128];
if compatible.len() > buf.len() {
return Err(Error::EINVAL);
}
buf.get_mut(..compatible.len())
.ok_or(Error::EINVAL)?
.copy_from_slice(compatible.as_bytes());
Ok(bindings::of_device_id {
// SAFETY: re-interpretation from [u8] to [c_types::c_char] of same length is always safe.
compatible: unsafe { transmute::<[u8; 128], [c_types::c_char; 128]>(buf) },
..Default::default()
})
}
/// An Open Firmware Match Table that can be constructed at build time.
///
/// # Invariants
///
/// `sentinel` always contains zeroes.
#[repr(C)]
pub struct ConstOfMatchTable<const N: usize> {
table: [bindings::of_device_id; N],
sentinel: bindings::of_device_id,
}

impl PointerWrapper for OfMatchTable {
type Borrowed = <InnerTable as PointerWrapper>::Borrowed;
impl<const N: usize> ConstOfMatchTable<N> {
/// Creates a new Open Firmware Match Table from a list of compatible strings.
pub const fn new_const(compatibles: [&'static CStr; N]) -> Self {
let mut table = [Self::zeroed_of_device_id(); N];
let mut i = 0;
while i < N {
table[i] = Self::new_of_device_id(compatibles[i]);
i += 1;
}
Self {
table,
// INVARIANTS: we zero the sentinel here, and never change it
// anywhere. Therefore it always contains zeroes.
sentinel: Self::zeroed_of_device_id(),
}
}

fn into_pointer(self) -> *const c_types::c_void {
// Per the invariant above, the generated pointer points to an
// array of `bindings::of_device_id`, where the final element is
// filled with zeros (the sentinel). Therefore, it's suitable to
// be assigned to `bindings::device_driver::of_match_table`.
self.0.into_pointer()
const fn zeroed_of_device_id() -> bindings::of_device_id {
bindings::of_device_id {
name: [0; 32],
type_: [0; 32],
compatible: [0; 128],
data: ptr::null(),
}
}

unsafe fn borrow(ptr: *const c_types::c_void) -> Self::Borrowed {
// SAFETY: The safety requirements for this function are the same as the ones for
// `InnerTable::borrow`.
unsafe { InnerTable::borrow(ptr) }
const fn new_of_device_id(compatible: &'static CStr) -> bindings::of_device_id {
let mut id = Self::zeroed_of_device_id();
let compatible = compatible.as_bytes_with_nul();
let mut i = 0;
while i < compatible.len() {
// if `compatible` does not fit in `id.compatible`, an
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// if `compatible` does not fit in `id.compatible`, an
// If `compatible` does not fit in `id.compatible`, an

// "index out of bounds" build time exception will be triggered.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// "index out of bounds" build time exception will be triggered.
// "index out of bounds" build time error will be triggered.

id.compatible[i] = compatible[i] as c_types::c_char;
i += 1;
}
id
}
}

impl<const N: usize> Deref for ConstOfMatchTable<N> {
type Target = OfMatchTable;

unsafe fn from_pointer(p: *const c_types::c_void) -> Self {
// SAFETY: The passed pointer comes from a previous call to [`InnerTable::into_pointer()`].
Self(unsafe { InnerTable::from_pointer(p) })
fn deref(&self) -> &OfMatchTable {
// INVARIANTS: `head` points to a sentinel-terminated C array,
// as per the `ConstOfMatchTable` type invariant, therefore
// `&OfMatchTable`'s inner reference will point to a sentinel-terminated C array.
let head = &self.table[0] as *const bindings::of_device_id as *const OfMatchTable;
// SAFETY: The returned reference must remain valid for the lifetime of `self`.
Comment on lines +92 to +93
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let head = &self.table[0] as *const bindings::of_device_id as *const OfMatchTable;
// SAFETY: The returned reference must remain valid for the lifetime of `self`.
let head = &self.table[0] as *const bindings::of_device_id as *const OfMatchTable;
// SAFETY: The returned reference must remain valid for the lifetime of `self`.

// The raw pointer `head` points to memory inside `self`. So the reference created
// from this raw pointer has the same lifetime as `self`.
// Therefore this reference remains valid for the lifetime of `self`, and
// is safe to return.
unsafe { &*head }
}
}
21 changes: 6 additions & 15 deletions rust/kernel/platdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use core::{marker::PhantomPinned, pin::Pin};
#[derive(Default)]
pub struct Registration {
registered: bool,
of_table: Option<*const c_types::c_void>,
pdrv: bindings::platform_driver,
_pin: PhantomPinned,
}
Expand Down Expand Up @@ -83,7 +82,7 @@ impl Registration {
fn register<P: PlatformDriver>(
self: Pin<&mut Self>,
name: &'static CStr,
of_match_table: Option<OfMatchTable>,
of_match_table: Option<&'static OfMatchTable>,
module: &'static crate::ThisModule,
) -> Result {
// SAFETY: We must ensure that we never move out of `this`.
Expand All @@ -94,9 +93,7 @@ impl Registration {
}
this.pdrv.driver.name = name.as_char_ptr();
if let Some(tbl) = of_match_table {
let ptr = tbl.into_pointer();
this.of_table = Some(ptr);
this.pdrv.driver.of_match_table = ptr.cast();
this.pdrv.driver.of_match_table = tbl.as_ptr();
}
this.pdrv.probe = Some(probe_callback::<P>);
this.pdrv.remove = Some(remove_callback::<P>);
Expand All @@ -105,10 +102,9 @@ impl Registration {
// - `name` pointer has static lifetime.
// - `module.0` lives at least as long as the module.
// - `probe()` and `remove()` are static functions.
// - `of_match_table` is either:
// - a raw pointer which lives until after the call to
// `bindings::platform_driver_unregister()`, or
// - null.
// - `of_match_table` is either a raw pointer with static lifetime,
// as guaranteed by the [`of::OfMatchTable::as_ptr()`] invariant,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// as guaranteed by the [`of::OfMatchTable::as_ptr()`] invariant,
// as guaranteed by the [`of::OfMatchTable::as_ptr()`] return type,

?

// or null.
let ret = unsafe { bindings::__platform_driver_register(&mut this.pdrv, module.0) };
if ret < 0 {
return Err(Error::from_kernel_errno(ret));
Expand All @@ -122,7 +118,7 @@ impl Registration {
/// Returns a pinned heap-allocated representation of the registration.
pub fn new_pinned<P: PlatformDriver>(
name: &'static CStr,
of_match_tbl: Option<OfMatchTable>,
of_match_tbl: Option<&'static OfMatchTable>,
module: &'static crate::ThisModule,
) -> Result<Pin<Box<Self>>> {
let mut r = Pin::from(Box::try_new(Self::default())?);
Expand All @@ -139,11 +135,6 @@ impl Drop for Registration {
// safe to call.
unsafe { bindings::platform_driver_unregister(&mut self.pdrv) }
}
if let Some(ptr) = self.of_table {
// SAFETY: `ptr` came from an `OfMatchTable`.
let tbl = unsafe { OfMatchTable::from_pointer(ptr) };
drop(tbl);
}
}
}

Expand Down