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

Use const generics and const fns to construct our `of_match_table`
structure at build time. A few runtime errors now become build time
errors, which is a good thing.

If the platdev's `Registration` structure takes a const generics
table object directly, it acquires/inherits the table's const
generics, which looks very complex. Prevent this from happening
by passing the table to `Registration` as a non-generic "reference
object" in the style of `&str` and `&Path`.

Suggested-by: Gary Guo <[email protected]>
Signed-off-by: Sven Van Asbroeck <[email protected]>

Author: Sven Van Asbroeck

drivers/char/hw_random/bcm2835_rng_rust.rs

@@ -12,7 +12,7 @@ use kernel::{
     file_operations::{FileOpener, FileOperations},
     io_buffer::IoBufferWriter,
     miscdev,
-    of::OfMatchTable,
+    of::ConstOfMatchTable,
     platdev::PlatformDriver,
     prelude::*,
     {c_str, platdev},
@@ -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,
         )?;
 

rust/kernel/of.rs

@@ -4,72 +4,93 @@
 //!
 //! 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;
+/// A kernel Open Firmware / devicetree match table.
+///
+/// Can only exist as an `&OfMatchTable` reference (akin to `&str` or
+/// `&Path` in Rust std).
+#[repr(transparent)]
+pub struct OfMatchTable(bindings::of_device_id);
 
-type InnerTable = Box<[bindings::of_device_id; 2]>;
+impl<const N: usize> Deref for ConstOfMatchTable<N> {
+    type Target = OfMatchTable;
 
-/// Wraps 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`.
-///
-/// # Invariants
-///
-/// The final array element is always filled with zeros (the default).
-pub struct OfMatchTable(InnerTable);
+    fn deref(&self) -> &OfMatchTable {
+        let head = &self.table[0] as *const bindings::of_device_id as *const OfMatchTable;
+        // 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 is safe to return.
+        unsafe { &*head }
+    }
+}
 
 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))
+    /// Return the table as a static lifetime, sentinel-terminated C array.
+    ///
+    /// This is suitable to be assigned to the kernel's `of_match_table` field.
+    pub fn as_ptr(&'static self) -> *const bindings::of_device_id {
+        &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);
+/// 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<const N: usize> ConstOfMatchTable<N> {
+    /// Create 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;
+        loop {
+            if i == N {
+                break;
+            }
+            table[i] = Self::new_of_device_id(compatibles[i]);
+            i += 1;
+        }
+        Self {
+            table,
+            // INVARIANT: we zero the sentinel here, and never change it
+            // anwhere. Therefore it always contains zeroes.
+            sentinel: Self::zeroed_of_device_id(),
         }
-        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()
-        })
     }
-}
 
-impl PointerWrapper for OfMatchTable {
-    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; 32usize],
+            type_: [0; 32usize],
+            compatible: [0; 128usize],
+            data: ptr::null(),
+        }
     }
 
-    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) })
+    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;
+        loop {
+            if i == compatible.len() {
+                break;
+            }
+            // if `compatible` does not fit in `id.compatible`, an
+            // "index out of bounds" build time exception will be triggered.
+            id.compatible[i] = compatible[i] as c_types::c_char;
+            i += 1;
+        }
+        id
     }
 }

rust/kernel/platdev.rs

@@ -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,
 }
@@ -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`.
@@ -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>);
@@ -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,
+        //      or null.
         let ret = unsafe { bindings::__platform_driver_register(&mut this.pdrv, module.0) };
         if ret < 0 {
             return Err(Error::from_kernel_errno(ret));
@@ -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())?);
@@ -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);
-        }
     }
 }