[PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[Gary Guo]

This adds a general pointer projection infrastructure which is intended to
serve as the basis for generic I/O projections. The idea is that we will
have a unified way of reading/writing MMIO or DMA allocation, creating
typed subviews into them, and able to read/write subfields safely.

This is the first step towards that direction, where an infra for
projecting arbitrary pointers are added. This takes care to not assume
pointers are part of a valid allocation, so it can be used to project I/O
pointers, too. `dma_read!` and `dma_write!` is converted to use this infra,
as they currently have soundness issues [1] and users have already
encountered limitation of it [2].

To aid the logistics, patch 2 retains a compatibility layer where the
existing syntax and behaviour is still supported. Patch 3 converts Nova
which is currently the only in-tree user of the API, and patch 4 removes
this compatibility layer.

I have more patches in works which will further simplify Nova side, however
the overall dma_{read,write} syntax would remain the same.

Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/Soundness.20of.20.60dma_read!.28.29.60/with/573645610 [1]
Link: https://lore.kernel.org/rust-for-linux/DGE9DBH7X8CO.2EES6EX8UA5ZF@kernel.org/T/#mc17b9c0309ac10346b3d6c6bd8461488e7c73161 [2]

Gary Guo (4):
  rust: add projection infrastructure
  rust: dma: generalize `dma_{read,write}` macro
  gpu: nova-core: convert to use new `dma_write!` syntax
  rust: dma: remove old dma_{read,write} macro compatibility syntax

 drivers/gpu/nova-core/gsp.rs      |  14 +-
 drivers/gpu/nova-core/gsp/boot.rs |   2 +-
 drivers/gpu/nova-core/gsp/cmdq.rs |  10 +-
 rust/kernel/dma.rs                |  97 +++++------
 rust/kernel/lib.rs                |   5 +
 rust/kernel/projection.rs         | 269 ++++++++++++++++++++++++++++++
 samples/rust/rust_dma.rs          |  18 +-
 scripts/Makefile.build            |   4 +-
 8 files changed, 350 insertions(+), 69 deletions(-)
 create mode 100644 rust/kernel/projection.rs


base-commit: 9152bc8cebcb14dc16b03ec81f2377ee8ce12268
-- 
2.51.2

[PATCH 1/4] rust: add projection infrastructure

[Gary Guo]

Add a generic infrastructure for performing field and index projections on
raw pointers. This will form the basis of performing I/O projections.

Pointers manipulations are intentionally using the safe wrapping variants
instead of the unsafe variants, as the latter requires pointers to be
inside an allocation which is not necessarily true for I/O pointers.

This projection macro protects against rogue `Deref` implementation, which
can causes the projected pointer to be outside the bounds of starting
pointer. This is extremely unlikely and Rust has a lint to catch this, but
is unsoundness regardless. The protection works by inducing type inference
ambiguity when `Deref` is implemented.

The projection macro supports both fallible and infallible index
projections. These are described in detail inside the documentation.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/lib.rs        |   5 +
 rust/kernel/projection.rs | 269 ++++++++++++++++++++++++++++++++++++++
 scripts/Makefile.build    |   4 +-
 3 files changed, 277 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/projection.rs

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 3da92f18f4ee..50866b481bdb 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -20,6 +20,7 @@
 #![feature(generic_nonzero)]
 #![feature(inline_const)]
 #![feature(pointer_is_aligned)]
+#![feature(slice_ptr_len)]
 //
 // Stable since Rust 1.80.0.
 #![feature(slice_flatten)]
@@ -37,6 +38,9 @@
 #![feature(const_ptr_write)]
 #![feature(const_refs_to_cell)]
 //
+// Stable since Rust 1.84.0.
+#![feature(strict_provenance)]
+//
 // Expected to become stable.
 #![feature(arbitrary_self_types)]
 //
@@ -130,6 +134,7 @@
 pub mod prelude;
 pub mod print;
 pub mod processor;
+pub mod projection;
 pub mod ptr;
 #[cfg(CONFIG_RUST_PWM_ABSTRACTIONS)]
 pub mod pwm;
diff --git a/rust/kernel/projection.rs b/rust/kernel/projection.rs
new file mode 100644
index 000000000000..200d116c39e2
--- /dev/null
+++ b/rust/kernel/projection.rs
@@ -0,0 +1,269 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Infrastructure for handling projections.
+
+use core::{
+    mem::MaybeUninit,
+    ops::Deref, //
+};
+
+use crate::{
+    build_error,
+    error::{
+        code::ERANGE,
+        Error, //
+    }, //
+};
+
+/// Error raised when a projection is attempted on array or slices out of bounds.
+pub struct OutOfBound;
+
+impl From<OutOfBound> for Error {
+    #[inline(always)]
+    fn from(_: OutOfBound) -> Self {
+        ERANGE
+    }
+}
+
+/// A helper trait to perform index projection.
+///
+/// This is similar to `core::slice::SliceIndex`, but operate on raw pointers safely and fallibly.
+///
+/// # Safety
+///
+/// `get` must return a pointer in bounds of the provided pointer.
+#[doc(hidden)]
+pub unsafe trait ProjectIndex<T: ?Sized>: Sized {
+    type Output: ?Sized;
+
+    /// Returns an index-projected pointer, if in bounds.
+    fn get(self, slice: *mut T) -> Option<*mut Self::Output>;
+
+    /// Returns an index-projected pointer; fail the build if it cannot be proved to be in bounds.
+    #[inline(always)]
+    fn index(self, slice: *mut T) -> *mut Self::Output {
+        Self::get(self, slice).unwrap_or_else(|| build_error!())
+    }
+}
+
+// Forward array impl to slice impl.
+// SAFETY: `get` returned pointers are in bounds.
+unsafe impl<T, I, const N: usize> ProjectIndex<[T; N]> for I
+where
+    I: ProjectIndex<[T]>,
+{
+    type Output = <I as ProjectIndex<[T]>>::Output;
+
+    #[inline(always)]
+    fn get(self, slice: *mut [T; N]) -> Option<*mut Self::Output> {
+        <I as ProjectIndex<[T]>>::get(self, slice)
+    }
+
+    #[inline(always)]
+    fn index(self, slice: *mut [T; N]) -> *mut Self::Output {
+        <I as ProjectIndex<[T]>>::index(self, slice)
+    }
+}
+
+// SAFETY: `get` returned pointers are in bounds.
+unsafe impl<T> ProjectIndex<[T]> for usize {
+    type Output = T;
+
+    #[inline(always)]
+    fn get(self, slice: *mut [T]) -> Option<*mut T> {
+        if self > slice.len() {
+            None
+        } else {
+            Some(slice.cast::<T>().wrapping_add(self))
+        }
+    }
+}
+
+// SAFETY: `get` returned pointers are in bounds.
+unsafe impl<T> ProjectIndex<[T]> for core::ops::Range<usize> {
+    type Output = [T];
+
+    #[inline(always)]
+    fn get(self, slice: *mut [T]) -> Option<*mut [T]> {
+        let new_len = self.end.checked_sub(self.start)?;
+        if self.end > slice.len() {
+            return None;
+        }
+        Some(core::ptr::slice_from_raw_parts_mut(
+            slice.cast::<T>().wrapping_add(self.start),
+            new_len,
+        ))
+    }
+}
+
+// SAFETY: `get` returned pointers are in bounds.
+unsafe impl<T> ProjectIndex<[T]> for core::ops::RangeTo<usize> {
+    type Output = [T];
+
+    #[inline(always)]
+    fn get(self, slice: *mut [T]) -> Option<*mut [T]> {
+        (0..self.end).get(slice)
+    }
+}
+
+// SAFETY: `get` returned pointers are in bounds.
+unsafe impl<T> ProjectIndex<[T]> for core::ops::RangeFrom<usize> {
+    type Output = [T];
+
+    #[inline(always)]
+    fn get(self, slice: *mut [T]) -> Option<*mut [T]> {
+        (self.start..slice.len()).get(slice)
+    }
+}
+
+// SAFETY: `get` returned pointers are in bounds.
+unsafe impl<T> ProjectIndex<[T]> for core::ops::RangeFull {
+    type Output = [T];
+
+    #[inline(always)]
+    fn get(self, slice: *mut [T]) -> Option<*mut [T]> {
+        Some(slice)
+    }
+}
+
+/// A helper trait to perform field projection.
+///
+/// This trait has a `DEREF` generic parameter so it can be implemented twice for types that
+/// implement `Deref`. This will cause an ambiguity error and thus block `Deref` types being used
+/// as base of projection, as they can inject unsoundness.
+///
+/// # Safety
+///
+/// `proj` should invoke `f` with valid allocation, as documentation described.
+#[doc(hidden)]
+pub unsafe trait ProjectField<const DEREF: bool> {
+    /// Project a pointer to a type to a pointer of a field.
+    ///
+    /// `f` is always invoked with valid allocation so it can safely obtain raw pointers to fields
+    /// using `&raw mut`.
+    ///
+    /// This is needed because `base` might not point to a valid allocation, while `&raw mut`
+    /// requires pointers to be in bounds of a valid allocation.
+    ///
+    /// # Safety
+    ///
+    /// `f` must returns a pointer in bounds of the provided pointer.
+    unsafe fn proj<F>(base: *mut Self, f: impl FnOnce(*mut Self) -> *mut F) -> *mut F;
+}
+
+// SAFETY: `proj` invokes `f` with valid allocation.
+unsafe impl<T> ProjectField<false> for T {
+    #[inline(always)]
+    unsafe fn proj<F>(base: *mut Self, f: impl FnOnce(*mut Self) -> *mut F) -> *mut F {
+        // Create a valid allocation to start projection, as `base` is not necessarily so.
+        let mut place = MaybeUninit::uninit();
+        let place_base = place.as_mut_ptr();
+        let field = f(place_base);
+        // SAFETY: `field` is in bounds from `base` per safety requirement.
+        let offset = unsafe { field.byte_offset_from(place_base) };
+        base.wrapping_byte_offset(offset).cast()
+    }
+}
+
+// SAFETY: vacuously satisfied.
+unsafe impl<T: Deref> ProjectField<true> for T {
+    #[inline(always)]
+    unsafe fn proj<F>(_: *mut Self, _: impl FnOnce(*mut Self) -> *mut F) -> *mut F {
+        build_error!("this function is a guard against `Deref` impl and is never invoked");
+    }
+}
+
+/// Create a projection from a raw pointer.
+///
+/// Supported projections include field projections and index projections.
+/// It is not allowed to project into types that implement custom `Deref` or `Index`.
+///
+/// The macro has basic syntax of `kernel::project_pointer!(ptr, projection)`, where `ptr` is an
+/// expression that evaluates to a raw pointer which serves as the base of projection. `projection`
+/// can be a projection expression of form `.field` (normally identifer, or numeral in case of
+/// tuple structs) or of form `[index]`.
+///
+/// If mutable pointer is needed, the macro input can be prefixed with `mut` keyword, i.e.
+/// `kernel::project_pointer!(mut ptr, projection)`. By default, a const pointer is created.
+///
+/// `project_pointer!` macro can perform both fallible indexing and build-time checked indexing.
+/// `[index]` form performs build-time bounds checking; if compiler fails to prove `[index]` is in
+/// bounds, compilation will fail. `[index]?` can be used to perform runtime bounds checking;
+/// `OutOfBound` error is raised via `?` if the index is out of bounds.
+///
+/// # Examples
+///
+/// Field projections are performed with `.field_name`:
+/// ```
+/// struct MyStruct { field: u32, }
+/// let ptr: *const MyStruct = core::ptr::dangling();
+/// let field_ptr: *const u32 = kernel::project_pointer!(ptr, .field);
+///
+/// struct MyTupleStruct(u32, u32);
+/// let ptr: *const MyTupleStruct = core::ptr::dangling();
+/// let field_ptr: *const u32 = kernel::project_pointer!(ptr, .1);
+/// ```
+///
+/// Index projections are performed with `[index]`:
+/// ```
+/// let ptr: *const [u8; 32] = core::ptr::dangling();
+/// let field_ptr: *const u8 = kernel::project_pointer!(ptr, [1]);
+/// // This will fail the build.
+/// // kernel::project_pointer!(ptr, [128]);
+/// // This will raise an `OutOfBound` error (which is convertable to `ERANGE`).
+/// // kernel::project_pointer!(ptr, [128]?);
+/// ```
+///
+/// If you need to match on the error instead of propagate, put the invocation inside a closure:
+/// ```
+/// let ptr: *const [u8; 32] = core::ptr::dangling();
+/// let field_ptr: Result<*const u8> = (|| -> Result<_> {
+///     Ok(kernel::project_pointer!(ptr, [128]?))
+/// })();
+/// assert!(field_ptr.is_err());
+/// ```
+///
+/// For mutable pointers, put `mut` as the first token in macro invocation.
+/// ```
+/// let ptr: *mut [(u8, u16); 32] = core::ptr::dangling_mut();
+/// let field_ptr: *mut u16 = kernel::project_pointer!(mut ptr, [1].1);
+/// ```
+#[macro_export]
+macro_rules! project_pointer {
+    (@gen $ptr:ident, ) => {};
+    // Field projection. `$field` needs to be `tt` to support tuple index like `.0`.
+    (@gen $ptr:ident, .$field:tt $($rest:tt)*) => {
+        // SAFETY: the provided closure always return in bounds pointer.
+        let $ptr = unsafe {
+            $crate::projection::ProjectField::proj($ptr, #[inline(always)] |ptr| {
+                // SAFETY: `$field` is in bounds, and no implicit `Deref` is possible (if the
+                // type implements `Deref`, Rust cannot infer the generic parameter `DEREF`).
+                &raw mut (*ptr).$field
+            })
+        };
+        $crate::project_pointer!(@gen $ptr, $($rest)*)
+    };
+    // Fallible index projection.
+    (@gen $ptr:ident, [$index:expr]? $($rest:tt)*) => {
+        let $ptr = $crate::projection::ProjectIndex::get($index, $ptr)
+            .ok_or($crate::projection::OutOfBound)?;
+        $crate::project_pointer!(@gen $ptr, $($rest)*)
+    };
+    // Build-time checked index projection.
+    (@gen $ptr:ident, [$index:expr] $($rest:tt)*) => {
+        let $ptr = $crate::projection::ProjectIndex::index($index, $ptr);
+        $crate::project_pointer!(@gen $ptr, $($rest)*)
+    };
+    (mut $ptr:expr, $($proj:tt)*) => {{
+        let ptr = $ptr;
+        $crate::project_pointer!(@gen ptr, $($proj)*);
+        ptr
+    }};
+    ($ptr:expr, $($proj:tt)*) => {{
+        let ptr = $ptr.cast_mut();
+        // We currently always project using mutable pointer, as it is not decided whether `&raw
+        // const` allows the resulting pointer to be mutated (see documentation of `addr_of!`).
+        $crate::project_pointer!(@gen ptr, $($proj)*);
+        ptr.cast_const()
+    }};
+}
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 32e209bc7985..3652b85be545 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -310,16 +310,18 @@ $(obj)/%.lst: $(obj)/%.c FORCE
 
 # The features in this list are the ones allowed for non-`rust/` code.
 #
+#   - Stable since Rust 1.79.0: `feature(slice_ptr_len)`.
 #   - Stable since Rust 1.81.0: `feature(lint_reasons)`.
 #   - Stable since Rust 1.82.0: `feature(asm_const)`,
 #     `feature(offset_of_nested)`, `feature(raw_ref_op)`.
+#   - Stable since Rust 1.84.0: `feature(strict_provenance)`.
 #   - Stable since Rust 1.87.0: `feature(asm_goto)`.
 #   - Expected to become stable: `feature(arbitrary_self_types)`.
 #   - To be determined: `feature(used_with_arg)`.
 #
 # Please see https://github.com/Rust-for-Linux/linux/issues/2 for details on
 # the unstable features in use.
-rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons,offset_of_nested,raw_ref_op,used_with_arg
+rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons,offset_of_nested,raw_ref_op,slice_ptr_len,strict_provenance,used_with_arg
 
 # `--out-dir` is required to avoid temporaries being created by `rustc` in the
 # current working directory, which may be not accessible in the out-of-tree
-- 
2.51.2

[PATCH 2/4] rust: dma: generalize `dma_{read,write}` macro

[Gary Guo]

The current macro have

    dma_read!(a.b.c[d].e.f)

to mean `a.b.c` is a DMA coherent allocation and it should project into it
with `[d].e.f` and do a read, which is confusing as it makes the indexing
operator integral to the macro (so it will break if you have an array of
`CoherentAllocation`, for example).

This also is problematic as we would like to generalize
`CoherentAllocation` from just slices to arbitrary types.

Make the macro expects `dma_read!(path.to.dma, .path.inside.dma)` as the
canonical syntax. The index operator is no longer special and is just one
type of projection (in additional to field projection). Similarly, make
`dma_write!(path.to.dma, .path.inside.dma, value)` become the canonical
syntax for writing.

Current `dma_read!`, `dma_write!` macros also use a custom
`addr_of!()`-based implementation for projecting pointers, which has
soundness issue as it relies on absence of `Deref` implementation on types.
This commit migrates them to use the general pointer projection
infrastructure, which handles these cases correctly.

Another issue of the current macro is that it is always fallible. This
makes sense with existing design of `CoherentAllocation`, but once we
support fixed size arrays with `CoherentAllocation`, it is desirable to
have the ability to perform infallible indexing as well, e.g. doing a `[0]`
index of `[Foo; 2]` is okay and can be checked at build-time, so forcing
falliblity is non-ideal. To capture this, the macro is changed to use
`[idx]` as infallible projection and `[idx]?` as fallible index projection
(those syntax are part of the general projection infra). A benefit of this
is that while individual indexing operation may fail, the overall
read/write operation is not fallible.

For migration, the old syntax is still kept for now.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/dma.rs       | 107 +++++++++++++++++++++++----------------
 samples/rust/rust_dma.rs |  18 +++----
 2 files changed, 73 insertions(+), 52 deletions(-)

diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 909d56fd5118..2338dc6b9374 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -461,6 +461,19 @@ pub fn size(&self) -> usize {
         self.count * core::mem::size_of::<T>()
     }
 
+    /// Returns the raw pointer to the allocated region in the CPU's virtual address space.
+    #[inline]
+    pub fn as_ptr(&self) -> *const [T] {
+        core::ptr::slice_from_raw_parts(self.cpu_addr.as_ptr(), self.count)
+    }
+
+    /// Returns the raw pointer to the allocated region in the CPU's virtual address space as
+    /// a mutable pointer.
+    #[inline]
+    pub fn as_mut_ptr(&self) -> *mut [T] {
+        core::ptr::slice_from_raw_parts_mut(self.cpu_addr.as_ptr(), self.count)
+    }
+
     /// Returns the base address to the allocated region in the CPU's virtual address space.
     pub fn start_ptr(&self) -> *const T {
         self.cpu_addr.as_ptr()
@@ -670,6 +683,9 @@ unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {}
 
 /// Reads a field of an item from an allocated region of structs.
 ///
+/// The syntax is of form `kernel::dma_read!(dma, proj)` where `dma` is an expression to an
+/// [`CoherentAllocation`] and `proj` is a [projection specification](kernel::project_pointer!).
+///
 /// # Examples
 ///
 /// ```
@@ -684,36 +700,40 @@ unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {}
 /// unsafe impl kernel::transmute::AsBytes for MyStruct{};
 ///
 /// # fn test(alloc: &kernel::dma::CoherentAllocation<MyStruct>) -> Result {
-/// let whole = kernel::dma_read!(alloc[2]);
-/// let field = kernel::dma_read!(alloc[1].field);
+/// let whole = kernel::dma_read!(alloc, [2]?);
+/// let field = kernel::dma_read!(alloc, [1]?.field);
 /// # Ok::<(), Error>(()) }
 /// ```
 #[macro_export]
 macro_rules! dma_read {
-    ($dma:expr, $idx: expr, $($field:tt)*) => {{
+    // Compatibility for old syntax.
+    ($dma:ident [ $idx:expr ] $($proj:tt)* ) => {
         (|| -> ::core::result::Result<_, $crate::error::Error> {
-            let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
-            // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be
-            // dereferenced. The compiler also further validates the expression on whether `field`
-            // is a member of `item` when expanded by the macro.
-            unsafe {
-                let ptr_field = ::core::ptr::addr_of!((*item) $($field)*);
-                ::core::result::Result::Ok(
-                    $crate::dma::CoherentAllocation::field_read(&$dma, ptr_field)
-                )
-            }
-        })()
-    }};
-    ($dma:ident [ $idx:expr ] $($field:tt)* ) => {
-        $crate::dma_read!($dma, $idx, $($field)*)
+            ::core::result::Result::Ok($crate::dma_read!($dma, [$idx]? $($proj)*))
+        })
     };
-    ($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => {
-        $crate::dma_read!($($dma).*, $idx, $($field)*)
+    ($($dma:ident).* [ $idx:expr ] $($proj:tt)* ) => {
+        (|| -> ::core::result::Result<_, $crate::error::Error> {
+            ::core::result::Result::Ok($crate::dma_write!($($dma).*, [$idx]? $($proj)*))
+        })
     };
+
+    ($dma:expr, $($proj:tt)*) => {{
+        let ptr = $crate::project_pointer!(
+            $crate::dma::CoherentAllocation::as_ptr(&$dma), $($proj)*
+        );
+        // SAFETY: pointer created by projection is within DMA region.
+        unsafe { $crate::dma::CoherentAllocation::field_read(&$dma, ptr) }
+    }};
 }
 
 /// Writes to a field of an item from an allocated region of structs.
 ///
+/// The syntax is of form `kernel::dma_write!(dma, proj, val)` where `dma` is an expression to an
+/// [`CoherentAllocation`] and `proj` is a [projection specification](kernel::project_pointer!),
+/// and `val` is the value to be written to the projected location.
+///
+///
 /// # Examples
 ///
 /// ```
@@ -728,37 +748,38 @@ macro_rules! dma_read {
 /// unsafe impl kernel::transmute::AsBytes for MyStruct{};
 ///
 /// # fn test(alloc: &kernel::dma::CoherentAllocation<MyStruct>) -> Result {
-/// kernel::dma_write!(alloc[2].member = 0xf);
-/// kernel::dma_write!(alloc[1] = MyStruct { member: 0xf });
+/// kernel::dma_write!(alloc, [2]?.member, 0xf);
+/// kernel::dma_write!(alloc, [1]?, MyStruct { member: 0xf });
 /// # Ok::<(), Error>(()) }
 /// ```
 #[macro_export]
 macro_rules! dma_write {
-    ($dma:ident [ $idx:expr ] $($field:tt)*) => {{
-        $crate::dma_write!($dma, $idx, $($field)*)
-    }};
-    ($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => {{
-        $crate::dma_write!($($dma).*, $idx, $($field)*)
-    }};
-    ($dma:expr, $idx: expr, = $val:expr) => {
+    // Compatibility for old syntax.
+    ($dma:ident [ $idx:expr ] $(.$field:ident)* = $val:expr) => {
         (|| -> ::core::result::Result<_, $crate::error::Error> {
-            let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
-            // SAFETY: `item_from_index` ensures that `item` is always a valid item.
-            unsafe { $crate::dma::CoherentAllocation::field_write(&$dma, item, $val) }
+            $crate::dma_write!($dma, [$idx]? $(.$field)*, $val);
             ::core::result::Result::Ok(())
         })()
     };
-    ($dma:expr, $idx: expr, $(.$field:ident)* = $val:expr) => {
-        (|| -> ::core::result::Result<_, $crate::error::Error> {
-            let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
-            // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be
-            // dereferenced. The compiler also further validates the expression on whether `field`
-            // is a member of `item` when expanded by the macro.
-            unsafe {
-                let ptr_field = ::core::ptr::addr_of_mut!((*item) $(.$field)*);
-                $crate::dma::CoherentAllocation::field_write(&$dma, ptr_field, $val)
-            }
-            ::core::result::Result::Ok(())
-        })()
+
+    (@parse [$dma:expr] [$($proj:tt)*] [, $val:expr]) => {
+        let ptr = $crate::project_pointer!(
+            mut $crate::dma::CoherentAllocation::as_mut_ptr(&$dma), $($proj)*
+        );
+        let val = $val;
+        // SAFETY: pointer created by projection is within DMA region.
+        unsafe { $crate::dma::CoherentAllocation::field_write(&$dma, ptr, val) }
+    };
+    (@parse [$dma:expr] [$($proj:tt)*] [.$field:tt $($rest:tt)*]) => {
+        $crate::dma_write!(@parse [$dma] [$($proj)* .$field] [$($rest)*])
+    };
+    (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr]? $($rest:tt)*]) => {
+        $crate::dma_write!(@parse [$dma] [$($proj)* [$index]?] [$($rest)*])
+    };
+    (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr] $($rest:tt)*]) => {
+        $crate::dma_write!(@parse [$dma] [$($proj)* [$index]] [$($rest)*])
+    };
+    ($dma:expr, $($rest:tt)*) => {
+        $crate::dma_write!(@parse [$dma] [] [$($rest)*])
     };
 }
diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
index 9c45851c876e..b772ada2c65c 100644
--- a/samples/rust/rust_dma.rs
+++ b/samples/rust/rust_dma.rs
@@ -68,7 +68,7 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
                 CoherentAllocation::alloc_coherent(pdev.as_ref(), TEST_VALUES.len(), GFP_KERNEL)?;
 
             for (i, value) in TEST_VALUES.into_iter().enumerate() {
-                kernel::dma_write!(ca[i] = MyStruct::new(value.0, value.1))?;
+                kernel::dma_write!(ca, [i]?, MyStruct::new(value.0, value.1));
             }
 
             let size = 4 * page::PAGE_SIZE;
@@ -91,17 +91,17 @@ fn drop(self: Pin<&mut Self>) {
         dev_info!(self.pdev, "Unload DMA test driver.\n");
 
         for (i, value) in TEST_VALUES.into_iter().enumerate() {
-            let val0 = kernel::dma_read!(self.ca[i].h);
-            let val1 = kernel::dma_read!(self.ca[i].b);
-            assert!(val0.is_ok());
-            assert!(val1.is_ok());
+            let result = (|| -> Result<_> {
+                let val0 = kernel::dma_read!(self.ca, [i]?.h);
+                let val1 = kernel::dma_read!(self.ca, [i]?.b);
 
-            if let Ok(val0) = val0 {
                 assert_eq!(val0, value.0);
-            }
-            if let Ok(val1) = val1 {
                 assert_eq!(val1, value.1);
-            }
+
+                Ok(())
+            })();
+
+            assert!(result.is_ok());
         }
 
         for (i, entry) in self.sgt.iter().enumerate() {
-- 
2.51.2

[PATCH 3/4] gpu: nova-core: convert to use new `dma_write!` syntax

[Gary Guo]

`dma_write!(dma, projection, value)` is the new syntax to be used.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 drivers/gpu/nova-core/gsp.rs      | 14 +++++++-------
 drivers/gpu/nova-core/gsp/boot.rs |  2 +-
 drivers/gpu/nova-core/gsp/cmdq.rs | 10 +++++++---
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 174feaca0a6b..25cd48514c77 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -143,14 +143,14 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
                     // _kgspInitLibosLoggingStructures (allocates memory for buffers)
                     // kgspSetupLibosInitArgs_IMPL (creates pLibosInitArgs[] array)
                     dma_write!(
-                        libos[0] = LibosMemoryRegionInitArgument::new("LOGINIT", &loginit.0)
-                    )?;
+                        libos, [0]?, LibosMemoryRegionInitArgument::new("LOGINIT", &loginit.0)
+                    );
                     dma_write!(
-                        libos[1] = LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0)
-                    )?;
-                    dma_write!(libos[2] = LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0))?;
-                    dma_write!(rmargs[0].inner = fw::GspArgumentsCached::new(cmdq))?;
-                    dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", rmargs))?;
+                        libos, [1]?, LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0)
+                    );
+                    dma_write!(libos, [2]?, LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0));
+                    dma_write!(rmargs, [0]?.inner, fw::GspArgumentsCached::new(cmdq));
+                    dma_write!(libos, [3]?, LibosMemoryRegionInitArgument::new("RMARGS", rmargs));
                 },
             }))
         })
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index be427fe26a58..94833f7996e8 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -157,7 +157,7 @@ pub(crate) fn boot(
 
         let wpr_meta =
             CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
-        dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
+        dma_write!(wpr_meta, [0]?, GspFwWprMeta::new(&gsp_fw, &fb_layout));
 
         self.cmdq
             .send_command(bar, commands::SetSystemInfo::new(pdev))?;
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 46819a82a51a..ae54708c38eb 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -201,9 +201,13 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
 
         let gsp_mem =
             CoherentAllocation::<GspMem>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
-        dma_write!(gsp_mem[0].ptes = PteArray::new(gsp_mem.dma_handle())?)?;
-        dma_write!(gsp_mem[0].cpuq.tx = MsgqTxHeader::new(MSGQ_SIZE, RX_HDR_OFF, MSGQ_NUM_PAGES))?;
-        dma_write!(gsp_mem[0].cpuq.rx = MsgqRxHeader::new())?;
+        dma_write!(gsp_mem, [0]?.ptes, PteArray::new(gsp_mem.dma_handle())?);
+        dma_write!(
+            gsp_mem,
+            [0]?.cpuq.tx,
+            MsgqTxHeader::new(MSGQ_SIZE, RX_HDR_OFF, MSGQ_NUM_PAGES)
+        );
+        dma_write!(gsp_mem, [0]?.cpuq.rx, MsgqRxHeader::new());
 
         Ok(Self(gsp_mem))
     }
-- 
2.51.2

[PATCH 4/4] rust: dma: remove old dma_{read,write} macro compatibility syntax

[Gary Guo]

With users converted, the old compatibility syntax can be removed.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/dma.rs | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 2338dc6b9374..f55033264849 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -706,18 +706,6 @@ unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {}
 /// ```
 #[macro_export]
 macro_rules! dma_read {
-    // Compatibility for old syntax.
-    ($dma:ident [ $idx:expr ] $($proj:tt)* ) => {
-        (|| -> ::core::result::Result<_, $crate::error::Error> {
-            ::core::result::Result::Ok($crate::dma_read!($dma, [$idx]? $($proj)*))
-        })
-    };
-    ($($dma:ident).* [ $idx:expr ] $($proj:tt)* ) => {
-        (|| -> ::core::result::Result<_, $crate::error::Error> {
-            ::core::result::Result::Ok($crate::dma_write!($($dma).*, [$idx]? $($proj)*))
-        })
-    };
-
     ($dma:expr, $($proj:tt)*) => {{
         let ptr = $crate::project_pointer!(
             $crate::dma::CoherentAllocation::as_ptr(&$dma), $($proj)*
@@ -754,14 +742,6 @@ macro_rules! dma_read {
 /// ```
 #[macro_export]
 macro_rules! dma_write {
-    // Compatibility for old syntax.
-    ($dma:ident [ $idx:expr ] $(.$field:ident)* = $val:expr) => {
-        (|| -> ::core::result::Result<_, $crate::error::Error> {
-            $crate::dma_write!($dma, [$idx]? $(.$field)*, $val);
-            ::core::result::Result::Ok(())
-        })()
-    };
-
     (@parse [$dma:expr] [$($proj:tt)*] [, $val:expr]) => {
         let ptr = $crate::project_pointer!(
             mut $crate::dma::CoherentAllocation::as_mut_ptr(&$dma), $($proj)*
-- 
2.51.2

Re: [PATCH 1/4] rust: add projection infrastructure

[Benno Lossin]

On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
> Add a generic infrastructure for performing field and index projections on
> raw pointers. This will form the basis of performing I/O projections.
>
> Pointers manipulations are intentionally using the safe wrapping variants
> instead of the unsafe variants, as the latter requires pointers to be
> inside an allocation which is not necessarily true for I/O pointers.
>
> This projection macro protects against rogue `Deref` implementation, which
> can causes the projected pointer to be outside the bounds of starting
> pointer. This is extremely unlikely and Rust has a lint to catch this, but
> is unsoundness regardless. The protection works by inducing type inference
> ambiguity when `Deref` is implemented.
>
> The projection macro supports both fallible and infallible index
> projections. These are described in detail inside the documentation.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Cool work!

I was wondering how you'd make this safe and general, but just having a
primitive pointer projection macro makes a lot of sense. We'll have lots
of projection macros that use this under the hood instead of a single
one. I like this as a stop-gap solution until we have projections in the
language.

I have a few comments, with those addressed:

Reviewed-by: Benno Lossin <lossin@kernel.org>

> ---
>  rust/kernel/lib.rs        |   5 +
>  rust/kernel/projection.rs | 269 ++++++++++++++++++++++++++++++++++++++
>  scripts/Makefile.build    |   4 +-
>  3 files changed, 277 insertions(+), 1 deletion(-)
>  create mode 100644 rust/kernel/projection.rs

> +// SAFETY: `proj` invokes `f` with valid allocation.
> +unsafe impl<T> ProjectField<false> for T {
> +    #[inline(always)]
> +    unsafe fn proj<F>(base: *mut Self, f: impl FnOnce(*mut Self) -> *mut F) -> *mut F {
> +        // Create a valid allocation to start projection, as `base` is not necessarily so.
> +        let mut place = MaybeUninit::uninit();
> +        let place_base = place.as_mut_ptr();
> +        let field = f(place_base);
> +        // SAFETY: `field` is in bounds from `base` per safety requirement.
> +        let offset = unsafe { field.byte_offset_from(place_base) };
> +        base.wrapping_byte_offset(offset).cast()
> +    }

There are several limitations with this impl. I don't think we can do
anything about them, but it's probably good to list them somewhere:
1. We do not support projecting fields of unsized types, so `MyStruct<dyn Trait>`.
   (note that slices are supported with `ProjectIndex`)
2. Since this creates a `MaybeUninit<T>` on the stack, only small `T`
   are supported. I'm not sure how much of this will be optimized away,
   but it might be the case that it is not. Projecting in the same
   function call stack multiple times might result in overrunning the
   stack pretty quickly.
3. The `wrapping_byte_offset` function generates potentially worse
   codegen when `base` points into a real allocation.

> +}
> +
> +// SAFETY: vacuously satisfied.
> +unsafe impl<T: Deref> ProjectField<true> for T {
> +    #[inline(always)]
> +    unsafe fn proj<F>(_: *mut Self, _: impl FnOnce(*mut Self) -> *mut F) -> *mut F {
> +        build_error!("this function is a guard against `Deref` impl and is never invoked");
> +    }
> +}
> +
> +/// Create a projection from a raw pointer.
> +///

I'd add a paragraph that explains that the pointer does not need to be
valid in any way. It should also explain that the returned pointer is
only valid when the original pointer was valid.

> +/// Supported projections include field projections and index projections.
> +/// It is not allowed to project into types that implement custom `Deref` or `Index`.
> +///
> +/// The macro has basic syntax of `kernel::project_pointer!(ptr, projection)`, where `ptr` is an
> +/// expression that evaluates to a raw pointer which serves as the base of projection. `projection`
> +/// can be a projection expression of form `.field` (normally identifer, or numeral in case of
> +/// tuple structs) or of form `[index]`.
> +///
> +/// If mutable pointer is needed, the macro input can be prefixed with `mut` keyword, i.e.
> +/// `kernel::project_pointer!(mut ptr, projection)`. By default, a const pointer is created.
> +///
> +/// `project_pointer!` macro can perform both fallible indexing and build-time checked indexing.
> +/// `[index]` form performs build-time bounds checking; if compiler fails to prove `[index]` is in
> +/// bounds, compilation will fail. `[index]?` can be used to perform runtime bounds checking;
> +/// `OutOfBound` error is raised via `?` if the index is out of bounds.
> +///
> +/// # Examples
> +///
> +/// Field projections are performed with `.field_name`:
> +/// ```
> +/// struct MyStruct { field: u32, }
> +/// let ptr: *const MyStruct = core::ptr::dangling();

I would only include one example that uses `dangling` and for the rest
just define a function that projects a raw pointer.

> +/// let field_ptr: *const u32 = kernel::project_pointer!(ptr, .field);
> +///
> +/// struct MyTupleStruct(u32, u32);
> +/// let ptr: *const MyTupleStruct = core::ptr::dangling();
> +/// let field_ptr: *const u32 = kernel::project_pointer!(ptr, .1);
> +/// ```
> +///
> +/// Index projections are performed with `[index]`:
> +/// ```
> +/// let ptr: *const [u8; 32] = core::ptr::dangling();
> +/// let field_ptr: *const u8 = kernel::project_pointer!(ptr, [1]);
> +/// // This will fail the build.
> +/// // kernel::project_pointer!(ptr, [128]);
> +/// // This will raise an `OutOfBound` error (which is convertable to `ERANGE`).
> +/// // kernel::project_pointer!(ptr, [128]?);
> +/// ```
> +///
> +/// If you need to match on the error instead of propagate, put the invocation inside a closure:
> +/// ```
> +/// let ptr: *const [u8; 32] = core::ptr::dangling();
> +/// let field_ptr: Result<*const u8> = (|| -> Result<_> {
> +///     Ok(kernel::project_pointer!(ptr, [128]?))
> +/// })();
> +/// assert!(field_ptr.is_err());
> +/// ```
> +///
> +/// For mutable pointers, put `mut` as the first token in macro invocation.
> +/// ```
> +/// let ptr: *mut [(u8, u16); 32] = core::ptr::dangling_mut();
> +/// let field_ptr: *mut u16 = kernel::project_pointer!(mut ptr, [1].1);
> +/// ```
> +#[macro_export]
> +macro_rules! project_pointer {
> +    (@gen $ptr:ident, ) => {};
> +    // Field projection. `$field` needs to be `tt` to support tuple index like `.0`.
> +    (@gen $ptr:ident, .$field:tt $($rest:tt)*) => {
> +        // SAFETY: the provided closure always return in bounds pointer.
> +        let $ptr = unsafe {
> +            $crate::projection::ProjectField::proj($ptr, #[inline(always)] |ptr| {
> +                // SAFETY: `$field` is in bounds, and no implicit `Deref` is possible (if the
> +                // type implements `Deref`, Rust cannot infer the generic parameter `DEREF`).
> +                &raw mut (*ptr).$field
> +            })
> +        };
> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
> +    };
> +    // Fallible index projection.
> +    (@gen $ptr:ident, [$index:expr]? $($rest:tt)*) => {
> +        let $ptr = $crate::projection::ProjectIndex::get($index, $ptr)
> +            .ok_or($crate::projection::OutOfBound)?;
> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
> +    };
> +    // Build-time checked index projection.
> +    (@gen $ptr:ident, [$index:expr] $($rest:tt)*) => {
> +        let $ptr = $crate::projection::ProjectIndex::index($index, $ptr);
> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
> +    };
> +    (mut $ptr:expr, $($proj:tt)*) => {{
> +        let ptr = $ptr;

I'd add a type ascription `let ptr: *mut _ = $ptr;`

> +        $crate::project_pointer!(@gen ptr, $($proj)*);
> +        ptr
> +    }};
> +    ($ptr:expr, $($proj:tt)*) => {{
> +        let ptr = $ptr.cast_mut();

This allows `$ptr` to be a random type with a `cast_mut` function. How
about:

    let ptr: *const _ = $ptr;
    let ptr: *mut _ = ::core::ptr::cast_mut(ptr);

Cheers,
Benno

> +        // We currently always project using mutable pointer, as it is not decided whether `&raw
> +        // const` allows the resulting pointer to be mutated (see documentation of `addr_of!`).
> +        $crate::project_pointer!(@gen ptr, $($proj)*);
> +        ptr.cast_const()
> +    }};
> +}

Re: [PATCH 2/4] rust: dma: generalize `dma_{read,write}` macro

[Benno Lossin]

On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
> +    (@parse [$dma:expr] [$($proj:tt)*] [, $val:expr]) => {
> +        let ptr = $crate::project_pointer!(
> +            mut $crate::dma::CoherentAllocation::as_mut_ptr(&$dma), $($proj)*
> +        );
> +        let val = $val;
> +        // SAFETY: pointer created by projection is within DMA region.
> +        unsafe { $crate::dma::CoherentAllocation::field_write(&$dma, ptr, val) }

This evaluates `$dma` for a second time (and also places it inside an
`unsafe` block).

> +    };

Missing surrounding `{}` to allow this in expression position?

> +    (@parse [$dma:expr] [$($proj:tt)*] [.$field:tt $($rest:tt)*]) => {
> +        $crate::dma_write!(@parse [$dma] [$($proj)* .$field] [$($rest)*])
> +    };
> +    (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr]? $($rest:tt)*]) => {
> +        $crate::dma_write!(@parse [$dma] [$($proj)* [$index]?] [$($rest)*])
> +    };
> +    (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr] $($rest:tt)*]) => {
> +        $crate::dma_write!(@parse [$dma] [$($proj)* [$index]] [$($rest)*])
> +    };
> +    ($dma:expr, $($rest:tt)*) => {
> +        $crate::dma_write!(@parse [$dma] [] [$($rest)*])
>      };

I'm wondering if this also works:
    
    ($dma:expr, $($(.$field:ident)? $([$index:expr])?)*, $val:expr) => {{
        let dma = &$dma;
        let ptr = $crate::project_pointer!(
            mut $crate::dma::CoherentAllocation::as_mut_ptr(dma),
            $($(.$field)? $([$index])?)*,
        );
        let val = $val;
        // SAFETY: pointer created by projection is within DMA region.
        unsafe { $crate::dma::CoherentAllocation::field_write(dma, ptr, val) }
    }}

>  }
> diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
> index 9c45851c876e..b772ada2c65c 100644
> --- a/samples/rust/rust_dma.rs
> +++ b/samples/rust/rust_dma.rs
> @@ -68,7 +68,7 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
>                  CoherentAllocation::alloc_coherent(pdev.as_ref(), TEST_VALUES.len(), GFP_KERNEL)?;
>  
>              for (i, value) in TEST_VALUES.into_iter().enumerate() {
> -                kernel::dma_write!(ca[i] = MyStruct::new(value.0, value.1))?;
> +                kernel::dma_write!(ca, [i]?, MyStruct::new(value.0, value.1));
>              }
>  
>              let size = 4 * page::PAGE_SIZE;
> @@ -91,17 +91,17 @@ fn drop(self: Pin<&mut Self>) {
>          dev_info!(self.pdev, "Unload DMA test driver.\n");
>  
>          for (i, value) in TEST_VALUES.into_iter().enumerate() {
> -            let val0 = kernel::dma_read!(self.ca[i].h);
> -            let val1 = kernel::dma_read!(self.ca[i].b);
> -            assert!(val0.is_ok());
> -            assert!(val1.is_ok());
> +            let result = (|| -> Result<_> {
> +                let val0 = kernel::dma_read!(self.ca, [i]?.h);
> +                let val1 = kernel::dma_read!(self.ca, [i]?.b);
>  
> -            if let Ok(val0) = val0 {
>                  assert_eq!(val0, value.0);
> -            }
> -            if let Ok(val1) = val1 {
>                  assert_eq!(val1, value.1);
> -            }
> +
> +                Ok(())
> +            })();

I dislike that we have to reintroduce the budget-try block here. Ideally
we could add something like `try` at the beginning of the macro and then
automatically add the try block. Feel free to make that a future series.

Cheers,
Benno

> +
> +            assert!(result.is_ok());
>          }
>  
>          for (i, entry) in self.sgt.iter().enumerate() {

Re: [PATCH 4/4] rust: dma: remove old dma_{read,write} macro compatibility syntax

[Benno Lossin]

On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
> With users converted, the old compatibility syntax can be removed.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Reviewed-by: Benno Lossin <lossin@kernel.org>

Cheers,
Benno

> ---
>  rust/kernel/dma.rs | 20 --------------------
>  1 file changed, 20 deletions(-)

Re: [PATCH 3/4] gpu: nova-core: convert to use new `dma_write!` syntax

[Benno Lossin]

On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
> `dma_write!(dma, projection, value)` is the new syntax to be used.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Reviewed-by: Benno Lossin <lossin@kernel.org>

Cheers,
Benno

> ---
>  drivers/gpu/nova-core/gsp.rs      | 14 +++++++-------
>  drivers/gpu/nova-core/gsp/boot.rs |  2 +-
>  drivers/gpu/nova-core/gsp/cmdq.rs | 10 +++++++---
>  3 files changed, 15 insertions(+), 11 deletions(-)

Re: [PATCH 1/4] rust: add projection infrastructure

[Danilo Krummrich]

On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
> diff --git a/rust/kernel/projection.rs b/rust/kernel/projection.rs
> new file mode 100644
> index 000000000000..200d116c39e2
> --- /dev/null
> +++ b/rust/kernel/projection.rs
> @@ -0,0 +1,269 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Infrastructure for handling projections.
> +
> +use core::{
> +    mem::MaybeUninit,
> +    ops::Deref, //
> +};
> +
> +use crate::{
> +    build_error,
> +    error::{
> +        code::ERANGE,
> +        Error, //

NIT: Why not use prelude instead?

> +    }, //
> +};

Re: [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[Danilo Krummrich]

On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
>   rust: dma: generalize `dma_{read,write}` macro
>   gpu: nova-core: convert to use new `dma_write!` syntax
>   rust: dma: remove old dma_{read,write} macro compatibility syntax

As we treat soundness holes as bugs, I think it is easier to just squash those
into a single commit and stick a Fixes: tag on it.

>   rust: add projection infrastructure

Miguel: If you don't mind I would like to take this one through the driver-core
tree as well, as it is not only the base for this fix, but also the base for the
upcoming I/O infrastructure work.

Thanks,
Danilo

Re: [PATCH 1/4] rust: add projection infrastructure

[Gary Guo]

On 2026-02-14 09:53, Benno Lossin wrote:
> On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
>> Add a generic infrastructure for performing field and index projections on
>> raw pointers. This will form the basis of performing I/O projections.
>>
>> Pointers manipulations are intentionally using the safe wrapping variants
>> instead of the unsafe variants, as the latter requires pointers to be
>> inside an allocation which is not necessarily true for I/O pointers.
>>
>> This projection macro protects against rogue `Deref` implementation, which
>> can causes the projected pointer to be outside the bounds of starting
>> pointer. This is extremely unlikely and Rust has a lint to catch this, but
>> is unsoundness regardless. The protection works by inducing type inference
>> ambiguity when `Deref` is implemented.
>>
>> The projection macro supports both fallible and infallible index
>> projections. These are described in detail inside the documentation.
>>
>> Signed-off-by: Gary Guo <gary@garyguo.net>
> 
> Cool work!
> 
> I was wondering how you'd make this safe and general, but just having a
> primitive pointer projection macro makes a lot of sense. We'll have lots
> of projection macros that use this under the hood instead of a single
> one. I like this as a stop-gap solution until we have projections in the
> language.
> 
> I have a few comments, with those addressed:
> 
> Reviewed-by: Benno Lossin <lossin@kernel.org>
> 
>> ---
>>  rust/kernel/lib.rs        |   5 +
>>  rust/kernel/projection.rs | 269 ++++++++++++++++++++++++++++++++++++++
>>  scripts/Makefile.build    |   4 +-
>>  3 files changed, 277 insertions(+), 1 deletion(-)
>>  create mode 100644 rust/kernel/projection.rs
> 
>> +// SAFETY: `proj` invokes `f` with valid allocation.
>> +unsafe impl<T> ProjectField<false> for T {
>> +    #[inline(always)]
>> +    unsafe fn proj<F>(base: *mut Self, f: impl FnOnce(*mut Self) -> *mut F) -> *mut F {
>> +        // Create a valid allocation to start projection, as `base` is not necessarily so.
>> +        let mut place = MaybeUninit::uninit();
>> +        let place_base = place.as_mut_ptr();
>> +        let field = f(place_base);
>> +        // SAFETY: `field` is in bounds from `base` per safety requirement.
>> +        let offset = unsafe { field.byte_offset_from(place_base) };
>> +        base.wrapping_byte_offset(offset).cast()
>> +    }
> 
> There are several limitations with this impl. I don't think we can do
> anything about them, but it's probably good to list them somewhere:
> 1. We do not support projecting fields of unsized types, so `MyStruct<dyn Trait>`.
>    (note that slices are supported with `ProjectIndex`)
> 2. Since this creates a `MaybeUninit<T>` on the stack, only small `T`
>    are supported. I'm not sure how much of this will be optimized away,
>    but it might be the case that it is not. Projecting in the same
>    function call stack multiple times might result in overrunning the
>    stack pretty quickly.

I've verified codegen and haven't managed to get this to actually generate `T` on the stack.
LLVM always figures out that the offset is the only thing that matters and optimize away
everything. `memoffset` crate also creates a temporary `MaybeUninit`, and given that it was
very widely used before `offset_of!` is stable, I think we should be able to rely on this being
okay even for large types.

Note that I've taken care to mark everything `#[inline(always)]` when possible, even
closures passed to `proj`.

> 3. The `wrapping_byte_offset` function generates potentially worse
>    codegen when `base` points into a real allocation.

I'm highly skeptical that we'll lose any optimization, but this is indeed
a possibility in theory.

> 
>> +}
>> +
>> +// SAFETY: vacuously satisfied.
>> +unsafe impl<T: Deref> ProjectField<true> for T {
>> +    #[inline(always)]
>> +    unsafe fn proj<F>(_: *mut Self, _: impl FnOnce(*mut Self) -> *mut F) -> *mut F {
>> +        build_error!("this function is a guard against `Deref` impl and is never invoked");
>> +    }
>> +}
>> +
>> +/// Create a projection from a raw pointer.
>> +///
> 
> I'd add a paragraph that explains that the pointer does not need to be
> valid in any way. It should also explain that the returned pointer is
> only valid when the original pointer was valid.
> 
>> +/// Supported projections include field projections and index projections.
>> +/// It is not allowed to project into types that implement custom `Deref` or `Index`.
>> +///
>> +/// The macro has basic syntax of `kernel::project_pointer!(ptr, projection)`, where `ptr` is an
>> +/// expression that evaluates to a raw pointer which serves as the base of projection. `projection`
>> +/// can be a projection expression of form `.field` (normally identifer, or numeral in case of
>> +/// tuple structs) or of form `[index]`.
>> +///
>> +/// If mutable pointer is needed, the macro input can be prefixed with `mut` keyword, i.e.
>> +/// `kernel::project_pointer!(mut ptr, projection)`. By default, a const pointer is created.
>> +///
>> +/// `project_pointer!` macro can perform both fallible indexing and build-time checked indexing.
>> +/// `[index]` form performs build-time bounds checking; if compiler fails to prove `[index]` is in
>> +/// bounds, compilation will fail. `[index]?` can be used to perform runtime bounds checking;
>> +/// `OutOfBound` error is raised via `?` if the index is out of bounds.
>> +///
>> +/// # Examples
>> +///
>> +/// Field projections are performed with `.field_name`:
>> +/// ```
>> +/// struct MyStruct { field: u32, }
>> +/// let ptr: *const MyStruct = core::ptr::dangling();
> 
> I would only include one example that uses `dangling` and for the rest
> just define a function that projects a raw pointer.
> 
>> +/// let field_ptr: *const u32 = kernel::project_pointer!(ptr, .field);
>> +///
>> +/// struct MyTupleStruct(u32, u32);
>> +/// let ptr: *const MyTupleStruct = core::ptr::dangling();
>> +/// let field_ptr: *const u32 = kernel::project_pointer!(ptr, .1);
>> +/// ```
>> +///
>> +/// Index projections are performed with `[index]`:
>> +/// ```
>> +/// let ptr: *const [u8; 32] = core::ptr::dangling();
>> +/// let field_ptr: *const u8 = kernel::project_pointer!(ptr, [1]);
>> +/// // This will fail the build.
>> +/// // kernel::project_pointer!(ptr, [128]);
>> +/// // This will raise an `OutOfBound` error (which is convertable to `ERANGE`).
>> +/// // kernel::project_pointer!(ptr, [128]?);
>> +/// ```
>> +///
>> +/// If you need to match on the error instead of propagate, put the invocation inside a closure:
>> +/// ```
>> +/// let ptr: *const [u8; 32] = core::ptr::dangling();
>> +/// let field_ptr: Result<*const u8> = (|| -> Result<_> {
>> +///     Ok(kernel::project_pointer!(ptr, [128]?))
>> +/// })();
>> +/// assert!(field_ptr.is_err());
>> +/// ```
>> +///
>> +/// For mutable pointers, put `mut` as the first token in macro invocation.
>> +/// ```
>> +/// let ptr: *mut [(u8, u16); 32] = core::ptr::dangling_mut();
>> +/// let field_ptr: *mut u16 = kernel::project_pointer!(mut ptr, [1].1);
>> +/// ```
>> +#[macro_export]
>> +macro_rules! project_pointer {
>> +    (@gen $ptr:ident, ) => {};
>> +    // Field projection. `$field` needs to be `tt` to support tuple index like `.0`.
>> +    (@gen $ptr:ident, .$field:tt $($rest:tt)*) => {
>> +        // SAFETY: the provided closure always return in bounds pointer.
>> +        let $ptr = unsafe {
>> +            $crate::projection::ProjectField::proj($ptr, #[inline(always)] |ptr| {
>> +                // SAFETY: `$field` is in bounds, and no implicit `Deref` is possible (if the
>> +                // type implements `Deref`, Rust cannot infer the generic parameter `DEREF`).
>> +                &raw mut (*ptr).$field
>> +            })
>> +        };
>> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
>> +    };
>> +    // Fallible index projection.
>> +    (@gen $ptr:ident, [$index:expr]? $($rest:tt)*) => {
>> +        let $ptr = $crate::projection::ProjectIndex::get($index, $ptr)
>> +            .ok_or($crate::projection::OutOfBound)?;
>> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
>> +    };
>> +    // Build-time checked index projection.
>> +    (@gen $ptr:ident, [$index:expr] $($rest:tt)*) => {
>> +        let $ptr = $crate::projection::ProjectIndex::index($index, $ptr);
>> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
>> +    };
>> +    (mut $ptr:expr, $($proj:tt)*) => {{
>> +        let ptr = $ptr;
> 
> I'd add a type ascription `let ptr: *mut _ = $ptr;`
> 
>> +        $crate::project_pointer!(@gen ptr, $($proj)*);
>> +        ptr
>> +    }};
>> +    ($ptr:expr, $($proj:tt)*) => {{
>> +        let ptr = $ptr.cast_mut();
> 
> This allows `$ptr` to be a random type with a `cast_mut` function. How
> about:
> 
>     let ptr: *const _ = $ptr;
>     let ptr: *mut _ = ::core::ptr::cast_mut(ptr);

I think `<*const _>::cast_mut($ptr)` probably would also do.

Thanks a lot for the review.

Best,
Gary

> 
> Cheers,
> Benno
> 
>> +        // We currently always project using mutable pointer, as it is not decided whether `&raw
>> +        // const` allows the resulting pointer to be mutated (see documentation of `addr_of!`).
>> +        $crate::project_pointer!(@gen ptr, $($proj)*);
>> +        ptr.cast_const()
>> +    }};
>> +}

Re: [PATCH 2/4] rust: dma: generalize `dma_{read,write}` macro

[Gary Guo]

On 2026-02-14 10:04, Benno Lossin wrote:
> On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
>> +    (@parse [$dma:expr] [$($proj:tt)*] [, $val:expr]) => {
>> +        let ptr = $crate::project_pointer!(
>> +            mut $crate::dma::CoherentAllocation::as_mut_ptr(&$dma), $($proj)*
>> +        );
>> +        let val = $val;
>> +        // SAFETY: pointer created by projection is within DMA region.
>> +        unsafe { $crate::dma::CoherentAllocation::field_write(&$dma, ptr, val) }
> 
> This evaluates `$dma` for a second time (and also places it inside an
> `unsafe` block).

Ah good point. The macro that we have today put `$val` inside unsafe and I've spotted and
lifted it out, but I didn't spot the `$dma` part.

> 
>> +    };
> 
> Missing surrounding `{}` to allow this in expression position?

Yeah, I also spotted this myself after sending the series out.

> 
>> +    (@parse [$dma:expr] [$($proj:tt)*] [.$field:tt $($rest:tt)*]) => {
>> +        $crate::dma_write!(@parse [$dma] [$($proj)* .$field] [$($rest)*])
>> +    };
>> +    (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr]? $($rest:tt)*]) => {
>> +        $crate::dma_write!(@parse [$dma] [$($proj)* [$index]?] [$($rest)*])
>> +    };
>> +    (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr] $($rest:tt)*]) => {
>> +        $crate::dma_write!(@parse [$dma] [$($proj)* [$index]] [$($rest)*])
>> +    };
>> +    ($dma:expr, $($rest:tt)*) => {
>> +        $crate::dma_write!(@parse [$dma] [] [$($rest)*])
>>      };
> 
> I'm wondering if this also works:
>     
>     ($dma:expr, $($(.$field:ident)? $([$index:expr])?)*, $val:expr) => {{
>         let dma = &$dma;
>         let ptr = $crate::project_pointer!(
>             mut $crate::dma::CoherentAllocation::as_mut_ptr(dma),
>             $($(.$field)? $([$index])?)*,
>         );
>         let val = $val;
>         // SAFETY: pointer created by projection is within DMA region.
>         unsafe { $crate::dma::CoherentAllocation::field_write(dma, ptr, val) }
>     }}

Rust would complain that the outer repetition can match empty token tree.

> 
>>  }
>> diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
>> index 9c45851c876e..b772ada2c65c 100644
>> --- a/samples/rust/rust_dma.rs
>> +++ b/samples/rust/rust_dma.rs
>> @@ -68,7 +68,7 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
>>                  CoherentAllocation::alloc_coherent(pdev.as_ref(), TEST_VALUES.len(), GFP_KERNEL)?;
>>  
>>              for (i, value) in TEST_VALUES.into_iter().enumerate() {
>> -                kernel::dma_write!(ca[i] = MyStruct::new(value.0, value.1))?;
>> +                kernel::dma_write!(ca, [i]?, MyStruct::new(value.0, value.1));
>>              }
>>  
>>              let size = 4 * page::PAGE_SIZE;
>> @@ -91,17 +91,17 @@ fn drop(self: Pin<&mut Self>) {
>>          dev_info!(self.pdev, "Unload DMA test driver.\n");
>>  
>>          for (i, value) in TEST_VALUES.into_iter().enumerate() {
>> -            let val0 = kernel::dma_read!(self.ca[i].h);
>> -            let val1 = kernel::dma_read!(self.ca[i].b);
>> -            assert!(val0.is_ok());
>> -            assert!(val1.is_ok());
>> +            let result = (|| -> Result<_> {
>> +                let val0 = kernel::dma_read!(self.ca, [i]?.h);
>> +                let val1 = kernel::dma_read!(self.ca, [i]?.b);
>>  
>> -            if let Ok(val0) = val0 {
>>                  assert_eq!(val0, value.0);
>> -            }
>> -            if let Ok(val1) = val1 {
>>                  assert_eq!(val1, value.1);
>> -            }
>> +
>> +                Ok(())
>> +            })();
> 
> I dislike that we have to reintroduce the budget-try block here. Ideally
> we could add something like `try` at the beginning of the macro and then
> automatically add the try block. Feel free to make that a future series.

I don't think this is an issue. It's visible inside the samples because
we are testing the values, but in practice most users would propagate the
errors out.

I also dislike that the budget-try block that we have inside `dma_read!`
currently hard-codes the error type.

Best,
Gary

> 
> Cheers,
> Benno
> 
>> +
>> +            assert!(result.is_ok());
>>          }
>>  
>>          for (i, entry) in self.sgt.iter().enumerate() {

Re: [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[Gary Guo]

On 2026-02-14 10:33, Danilo Krummrich wrote:
> On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
>>   rust: dma: generalize `dma_{read,write}` macro
>>   gpu: nova-core: convert to use new `dma_write!` syntax
>>   rust: dma: remove old dma_{read,write} macro compatibility syntax
> 
> As we treat soundness holes as bugs, I think it is easier to just 
> squash those
> into a single commit and stick a Fixes: tag on it.

I put these as separate commits so it is possible for you to take first 
2 patches
if taking the 3rd and 4th patch would cause merge conflicts. My upcoming 
changes
to `dma::Coherent` would also further update the Nova-side callers (I 
guess if you're
also taking these from driver-core tree then it's not an issue).

If you think it's fine to take them via the driver-core tree without 
causing
trouble for drm-rust tree then I can squash them in next version.

Best,
Gary

> 
>>   rust: add projection infrastructure
> 
> Miguel: If you don't mind I would like to take this one through the 
> driver-core
> tree as well, as it is not only the base for this fix, but also the 
> base for the
> upcoming I/O infrastructure work.
> 
> Thanks,
> Danilo

Re: [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[Danilo Krummrich]

On Sat Feb 14, 2026 at 11:50 AM CET, Gary Guo wrote:
> If you think it's fine to take them via the driver-core tree without causing
> trouble for drm-rust tree then I can squash them in next version.

I'd take them through -fixes (as the soundness hole is technically a bug) and
just backmerge the corresponding -rc into drm-rust-next. So, squashing them
should be fine.

Re: [PATCH 1/4] rust: add projection infrastructure

[Benno Lossin]

On Sat Feb 14, 2026 at 11:36 AM CET, Gary Guo wrote:
> On 2026-02-14 09:53, Benno Lossin wrote:
>> On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
>>> +// SAFETY: `proj` invokes `f` with valid allocation.
>>> +unsafe impl<T> ProjectField<false> for T {
>>> +    #[inline(always)]
>>> +    unsafe fn proj<F>(base: *mut Self, f: impl FnOnce(*mut Self) -> *mut F) -> *mut F {
>>> +        // Create a valid allocation to start projection, as `base` is not necessarily so.
>>> +        let mut place = MaybeUninit::uninit();
>>> +        let place_base = place.as_mut_ptr();
>>> +        let field = f(place_base);
>>> +        // SAFETY: `field` is in bounds from `base` per safety requirement.
>>> +        let offset = unsafe { field.byte_offset_from(place_base) };
>>> +        base.wrapping_byte_offset(offset).cast()
>>> +    }
>> 
>> There are several limitations with this impl. I don't think we can do
>> anything about them, but it's probably good to list them somewhere:
>> 1. We do not support projecting fields of unsized types, so `MyStruct<dyn Trait>`.
>>    (note that slices are supported with `ProjectIndex`)
>> 2. Since this creates a `MaybeUninit<T>` on the stack, only small `T`
>>    are supported. I'm not sure how much of this will be optimized away,
>>    but it might be the case that it is not. Projecting in the same
>>    function call stack multiple times might result in overrunning the
>>    stack pretty quickly.
>
> I've verified codegen and haven't managed to get this to actually generate `T` on the stack.
> LLVM always figures out that the offset is the only thing that matters and optimize away
> everything. `memoffset` crate also creates a temporary `MaybeUninit`, and given that it was
> very widely used before `offset_of!` is stable, I think we should be able to rely on this being
> okay even for large types.

Oh that's neat.

> Note that I've taken care to mark everything `#[inline(always)]` when possible, even
> closures passed to `proj`.

Yeah I saw that.

People might still encounter this issue in some fringe situation. I'm
not too worried, since klint can warn about the stack frame being too
large.

Speaking of klint, could it be possible to have a
`#[klint::optimized_away]` attribute that we can put on the `let place`,
klint would then error (or warn) when it's not optimized away (the name
isn't great :)

>
>> 3. The `wrapping_byte_offset` function generates potentially worse
>>    codegen when `base` points into a real allocation.
>
> I'm highly skeptical that we'll lose any optimization, but this is indeed
> a possibility in theory.

I remember some Rust codegen expert wanting to use `offset` instead of
`wrapping_offset` in the projection operator of `NonNull` and raw
pointers (the original RFC I think).

>>> +    ($ptr:expr, $($proj:tt)*) => {{
>>> +        let ptr = $ptr.cast_mut();
>> 
>> This allows `$ptr` to be a random type with a `cast_mut` function. How
>> about:
>> 
>>     let ptr: *const _ = $ptr;
>>     let ptr: *mut _ = ::core::ptr::cast_mut(ptr);
>
> I think `<*const _>::cast_mut($ptr)` probably would also do.

That also works.

Cheers,
Benno

Re: [PATCH 2/4] rust: dma: generalize `dma_{read,write}` macro

[Benno Lossin]

On Sat Feb 14, 2026 at 11:46 AM CET, Gary Guo wrote:
> On 2026-02-14 10:04, Benno Lossin wrote:
>> On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
>>> +    (@parse [$dma:expr] [$($proj:tt)*] [.$field:tt $($rest:tt)*]) => {
>>> +        $crate::dma_write!(@parse [$dma] [$($proj)* .$field] [$($rest)*])
>>> +    };
>>> +    (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr]? $($rest:tt)*]) => {
>>> +        $crate::dma_write!(@parse [$dma] [$($proj)* [$index]?] [$($rest)*])
>>> +    };
>>> +    (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr] $($rest:tt)*]) => {
>>> +        $crate::dma_write!(@parse [$dma] [$($proj)* [$index]] [$($rest)*])
>>> +    };
>>> +    ($dma:expr, $($rest:tt)*) => {
>>> +        $crate::dma_write!(@parse [$dma] [] [$($rest)*])
>>>      };
>> 
>> I'm wondering if this also works:
>>     
>>     ($dma:expr, $($(.$field:ident)? $([$index:expr])?)*, $val:expr) => {{
>>         let dma = &$dma;
>>         let ptr = $crate::project_pointer!(
>>             mut $crate::dma::CoherentAllocation::as_mut_ptr(dma),
>>             $($(.$field)? $([$index])?)*,
>>         );
>>         let val = $val;
>>         // SAFETY: pointer created by projection is within DMA region.
>>         unsafe { $crate::dma::CoherentAllocation::field_write(dma, ptr, val) }
>>     }}
>
> Rust would complain that the outer repetition can match empty token tree.

Ah right.

>>> @@ -91,17 +91,17 @@ fn drop(self: Pin<&mut Self>) {
>>>          dev_info!(self.pdev, "Unload DMA test driver.\n");
>>>  
>>>          for (i, value) in TEST_VALUES.into_iter().enumerate() {
>>> -            let val0 = kernel::dma_read!(self.ca[i].h);
>>> -            let val1 = kernel::dma_read!(self.ca[i].b);
>>> -            assert!(val0.is_ok());
>>> -            assert!(val1.is_ok());
>>> +            let result = (|| -> Result<_> {
>>> +                let val0 = kernel::dma_read!(self.ca, [i]?.h);
>>> +                let val1 = kernel::dma_read!(self.ca, [i]?.b);
>>>  
>>> -            if let Ok(val0) = val0 {
>>>                  assert_eq!(val0, value.0);
>>> -            }
>>> -            if let Ok(val1) = val1 {
>>>                  assert_eq!(val1, value.1);
>>> -            }
>>> +
>>> +                Ok(())
>>> +            })();
>> 
>> I dislike that we have to reintroduce the budget-try block here. Ideally
>> we could add something like `try` at the beginning of the macro and then
>> automatically add the try block. Feel free to make that a future series.
>
> I don't think this is an issue. It's visible inside the samples because
> we are testing the values, but in practice most users would propagate the
> errors out.

Maybe we should just have a function that returns a Result in this test
that's called from drop.

> I also dislike that the budget-try block that we have inside `dma_read!`
> currently hard-codes the error type.

Yeah me too.

Cheers,
Benno

Re: [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[Miguel Ojeda]

On Sat, Feb 14, 2026 at 11:34 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> Miguel: If you don't mind I would like to take this one through the driver-core
> tree as well, as it is not only the base for this fix, but also the base for the
> upcoming I/O infrastructure work.

Is there some code that is currently "actually" buggy, i.e. not just a
soundness bug, but actually triggering UB or similar?

It is a bit of an edge case, this one... Hmm... I don't want to delay
things, but if there is no caller exploiting the unsoundness now, then
I would prefer we at least have a reasonable review time for this.
After all, it is a new feature with a new module and docs and all.

(We treat the unsoundness bugs as bugs to be backported etc. as a
policy, but on the C side we don't really have/do that, and I suspect
this would be over the "threshold"...)

Cheers,
Miguel

Re: [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[Danilo Krummrich]

On Sun Feb 15, 2026 at 1:47 AM CET, Miguel Ojeda wrote:
> I would prefer we at least have a reasonable review time for this.

What gave you the impression that this should not be properly reviewed?

> (We treat the unsoundness bugs as bugs to be backported etc. as a
> policy, but on the C side we don't really have/do that, and I suspect
> this would be over the "threshold"...)

I do not see any reason for this to be backported, but it is reasonable to be
included in a -fixes PR.

Re: [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[Miguel Ojeda]

On Sun, Feb 15, 2026 at 12:06 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> What gave you the impression that this should not be properly reviewed?

I didn't say this would not be properly reviewed (I think you meant "would"?)

I just want to have a reasonable review time for the first patch since
it is new code.

Depending on the subsystem, fixes go in sometimes way, way faster than
features (days vs. weeks).

That is why I asked whether there is UB "actually" happening today,
because I wanted to understand how urgent we were talking about.

> I do not see any reason for this to be backported, but it is reasonable to be
> included in a -fixes PR.

The backporting part was to mention that you were right that we treat
soundness issues as bugs -- even to the point of backporting. I
introduced that policy to try to raise the bar compared to C, since we
want that "extra layer" of protection and to keep it up even in stable
kernels.

But it is all a balance, i.e. in the C side, it wouldn't be even
considered a bug to begin with, unless there was an "actual issue",
and thus unlikely to be justified for a fixes PR. So I want to make
sure we don't overdo it on the Rust side.

To me, as long as there is a reasonable review time, on the order that
a new feature would get via the normal channels, it is fine.

Cheers,
Miguel

Re: [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[Danilo Krummrich]

On Sun Feb 15, 2026 at 1:06 PM CET, Miguel Ojeda wrote:
> On Sun, Feb 15, 2026 at 12:06 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> I just want to have a reasonable review time for the first patch since
> it is new code.

That's a bit vague as this may mean something else to different people and may
even depend on the actual patch and the people involed. I.e. we may have a
different understandng of what is reasonable in this case. :)

For instance, for the scope of this specific patch, given that it was sent by
Gary and already had a review pass from Benno I would not have any concerns to
see it land within three weeks or so (assuming that it will also receive
positive feedback from Alice).

But of course it depends and as a maintainer I also constantly re-evaluate when
a patch is considered ready.

To me time is a less important measure; a more important measure for me is
whether the key stakeholders are in agreement that things go in the right
direction.

Having that said, it is perfectly fine not to rely on the judgement of another
maintainer is this case. It is of course your decision if and *when* you provide
the ACK for this to go through another tree. :)

> That is why I asked whether there is UB "actually" happening today,
> because I wanted to understand how urgent we were talking about.

Ah, I thought you already know that there is no rush, as this topic was also
raised by Gary in the last R4L call.

>> I do not see any reason for this to be backported, but it is reasonable to be
>> included in a -fixes PR.
>
> The backporting part was to mention that you were right that we treat
> soundness issues as bugs -- even to the point of backporting. I
> introduced that policy to try to raise the bar compared to C, since we
> want that "extra layer" of protection and to keep it up even in stable
> kernels.
>
> But it is all a balance, i.e. in the C side, it wouldn't be even
> considered a bug to begin with, unless there was an "actual issue",
> and thus unlikely to be justified for a fixes PR. So I want to make
> sure we don't overdo it on the Rust side.

Independent of the above, this is how I see it (and treat it so far) for both C
and Rust code:

If there is a potential bug (i.e. code that is broken, but no user experiences
it so far) then I usually take the fix through a -fixes PR but don't flag it for
backporting.

The reason I say usually is because it is a judgement call in the end, as it
also highly depends on where we are in the cycle.

For instance, if it is early in the cycle and the fix is considered low risk, I
rather take it (e.g. before another subsequent fix regresses due to the
potential bug). But if it is late in the cycle, even for a low risk fix, I
rather hold it back.

To me a soundness bug (that is not yet "abused") falls into this category and
receives the same handling.

Re: [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[Benno Lossin]

On Sun Feb 15, 2026 at 1:47 AM CET, Miguel Ojeda wrote:
> On Sat, Feb 14, 2026 at 11:34 AM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> Miguel: If you don't mind I would like to take this one through the driver-core
>> tree as well, as it is not only the base for this fix, but also the base for the
>> upcoming I/O infrastructure work.
>
> Is there some code that is currently "actually" buggy, i.e. not just a
> soundness bug, but actually triggering UB or similar?

I investigated this a bit in the beginning and I don't think this can be
triggered by us at the moment.

Here is a comprehensive explanation (maybe Gary can pick this up for the
commit message?):

The unsoundness comes from the fact that in the current version of the
`dma_read!` macro does not prevent the field access from "going through
a deref". Here is an example of an expression experiencing UB:

    dma_read!(dma[0].derefable.field)

Given `dma: &CoherentAllocation<Derefable>` and

    struct Derefable {
        inner: KBox<Struct>,
    }

    impl Deref for Derefable {
        type Target = Struct;

        fn deref(&self) -> &Struct {
            self.inner.deref()
        }
    }

    struct Struct {
        field: Field,
    }

The `dma_read!` creates a raw pointer to the dma allocation at the
provided offset. It then offsets that pointer according to the macro
input:  `&raw const (*raw_ptr).derefable.field`. This is the step that
produces UB:
- the Rust compiler finds no field named `field` on the `Derefable`
  type. It thus checks if it implements `Deref`, which it does, so it
  proceeds to try the target type.
- The target type is `Struct`, which does have a field named `field`, so
  it desugars the raw pointer offset to this:

    &raw const Deref::deref(&(*raw_ptr).derefable).field

This thus creates a *reference* into the dma-coherent memory, which
AFAIK is UB. It then uses that to call the `Deref::deref` function,
which reads from dma without volatile and then dereferences the read
value again.

Now there is a lint called `dangerous_implicit_autorefs` [1], which
exists to warn specifically on these implicit reference conversions. It
was introduced as error-by-default in Rust 1.89, so as long as we test
with latest stable, we should catch these.

An additional "layer of defense" is that `CoherentAllocation` requires
the generic to implement the `FromBytes` trait, which is incompatible
with "being a pointer" (this impl is missing from the example code
above).

I discussed a related question on the rust-lang zulip in which we also
talked about this unsoundness. The recommendation from the Rust
developers was to **not** rely on a lint for soundness [2]. There are
several ways to disable lints and it's not guaranteed that the lint will
catch everything.

[1]: https://doc.rust-lang.org/rustc/lints/listing/deny-by-default.html#dangerous-implicit-autorefs
[2]: https://rust-lang.zulipchat.com/#narrow/channel/213817-t-lang/topic/dangerous_implicit_autorefs.20and.20Box/near/572196666

Cheers,
Benno

Re: [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[Miguel Ojeda]

On Sun, Feb 15, 2026 at 1:56 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> see it land within three weeks or so

That sounds fine -- as I mentioned, what I don't want is that we land
patches like this one (i.e. soundness fixes that require a new
feature) with the urgency of usual fixes, but rather more on the side
of that of a new feature, since they contain one.

(It is not so much about this patch in particular, it is more about
trying to establish a pattern.)

> Ah, I thought you already know that there is no rush, as this topic was also
> raised by Gary in the last R4L call.

I know, but Gary finished the latest approach a couple days ago, and
it is now when we have it in the list. If we could have used the lint
approach, for instance, or if the new feature was smaller, then the
review time for the new feature patch would not really matter.

So I wanted to make sure it was clear here whether this was urgent and
how much, and whether we are all in agreement there is no known UB
somewhere or not.

I see Benno has just written a very useful response to my question --
thanks Benno -- and I agree with him we should add the summary/context
to the commit message.

> If there is a potential bug (i.e. code that is broken, but no user experiences
> it so far) then I usually take the fix through a -fixes PR but don't flag it for
> backporting.

That sounds fine -- my only note is that we do our best to backport
Rust soundness fixes (i.e. unless there is a reason not to, e.g. it
could be that it is just way too much effort and the risk of getting
actually abused is low).

I have just added this section -- we may also want to add it
elsewhere, e.g. the guidelines, for more visibility:

  https://rust-for-linux.com/contributing#soundness-issues-and-backporting

In general, if code is actually broken, even if a user didn't report
an issue yet, the stable team also takes them (going by the things
they usually pick). But I assume you meant it more in an abstract
sense of broken ("conceptually broken", "theoretical issues"), like
Rust soundness bugs, yeah. So Rust soundness bugs are exceptional in
that sense, i.e. we nevertheless backport them even if "theoretical".

> To me a soundness bug (that is not yet "abused") falls into this category and
> receives the same handling.

This sounds also fine to me, i.e. we add soundness issues to fixes PRs
in general. Here we ended up needing a new feature, so that is why I
mentioned the review time.

It also means that for something like this, that contains new code and
is low risk, it may be a good idea to mark the backport as "Cc: stable
# After a couple weeks in mainline" or similar to get extra testing
before it gets backported.

Cheers,
Miguel

Re: [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[Danilo Krummrich]

On Sun Feb 15, 2026 at 4:16 PM CET, Miguel Ojeda wrote:
> On Sun, Feb 15, 2026 at 1:56 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> see it land within three weeks or so
>
> That sounds fine -- as I mentioned, what I don't want is that we land
> patches like this one (i.e. soundness fixes that require a new
> feature) with the urgency of usual fixes, but rather more on the side
> of that of a new feature, since they contain one.

The fact that you keep repeating this and that you said in your fist reply "I
would prefer we at least have a reasonable review time for this." confuses me a
bit, since repeating it and saying "at least" reads to me as if I would have
ever said something different, when I noted:

	If you don't mind I would like to take this one through the driver-core
	tree as well, as it is not only the base for this fix, but also the base
	for the upcoming I/O infrastructure work.

I did not mean to create any impression of urgency, but only wanted to know
whether that's fine with you in general, e.g. "Yep, will ACK it once I feel it's
ready.".

So, again to avoid any possible misunderstanding, we never were in disagreement.

> (It is not so much about this patch in particular, it is more about
> trying to establish a pattern.)

Not sure there is a new pattern to be established, it's always a judgement call
how to handle those things. As I already said, I don't think that soundness
holes that do not lead to an actual bug yet are any different to other cases of
potential bugs, regardless of C or Rust.

For instance, if you have an API on the C side that is supposed to be safe
against concurrency, which is buggy, but the only user does not use it in a
concurrent way, that's the exact same situation I think.

Hence, I do apply the same general handling and judgement.

>> If there is a potential bug (i.e. code that is broken, but no user experiences
>> it so far) then I usually take the fix through a -fixes PR but don't flag it for
>> backporting.
>
> That sounds fine -- my only note is that we do our best to backport
> Rust soundness fixes (i.e. unless there is a reason not to, e.g. it
> could be that it is just way too much effort and the risk of getting
> actually abused is low).
>
> I have just added this section -- we may also want to add it
> elsewhere, e.g. the guidelines, for more visibility:
>
>   https://rust-for-linux.com/contributing#soundness-issues-and-backporting
>
> In general, if code is actually broken, even if a user didn't report
> an issue yet, the stable team also takes them (going by the things
> they usually pick). But I assume you meant it more in an abstract
> sense of broken ("conceptually broken", "theoretical issues"), like
> Rust soundness bugs, yeah.

(Yes, this is what I meant.)

> So Rust soundness bugs are exceptional in that sense, i.e. we nevertheless
> backport them even if "theoretical".

I'm a bit confused by this paragraph. In your last message you raised the
following concern.

	"But it is all a balance, i.e. in the C side, it wouldn't be even
	considered a bug to begin with, unless there was an "actual issue", and
	thus unlikely to be justified for a fixes PR. So I want to make sure we
	don't overdo it on the Rust side."

But now, additional to my usual handling I described, you seem to ask for a
backport in such cases, as your link mentions "In general, fixes for Rust
soundness issues should be marked for backport, unless there is a reason not
to.".

Can you please clarify?

Besides that, as I mentioned above, I don't really see a reason to handle this
different than any other "potential bug", such as the C example I gave above.

Re: [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[Miguel Ojeda]

On Sun, Feb 15, 2026 at 6:02 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> The fact that you keep repeating this and that you said in your fist reply "I
> would prefer we at least have a reasonable review time for this." confuses me a
> bit, since repeating it and saying "at least" reads to me as if I would have
> ever said something different, when I noted:
>
>         If you don't mind I would like to take this one through the driver-core
>         tree as well, as it is not only the base for this fix, but also the base
>         for the upcoming I/O infrastructure work.
>
> I did not mean to create any impression of urgency, but only wanted to know
> whether that's fine with you in general, e.g. "Yep, will ACK it once I feel it's
> ready.".

I was thinking about how to best handle the patch. Fixes are more
urgent than features, some fixes are more urgent than others,
sometimes maintainers have other constraints, and so on. When you said
in a message:

    I'd take them through -fixes (as the soundness hole is technically a bug)

I didn't know how quickly that was meant to be or when you needed to
send a PR etc., and the parentheses part in your message made me think
about the "soundness issues are bugs" policy we have, and that I don't
want that the policy I introduced creates undue pressure on
maintainers to land things more quickly than otherwise they would
have.

I didn't mean that you were going to do anything wrong or X or Y in
particular -- I didn't know what your plan was -- or that the patch
wouldn't be reviewed properly.

> I'm a bit confused by this paragraph. In your last message you raised the
> following concern.
>
>         "But it is all a balance, i.e. in the C side, it wouldn't be even
>         considered a bug to begin with, unless there was an "actual issue", and
>         thus unlikely to be justified for a fixes PR. So I want to make sure we
>         don't overdo it on the Rust side."
>
> But now, additional to my usual handling I described, you seem to ask for a
> backport in such cases, as your link mentions "In general, fixes for Rust
> soundness issues should be marked for backport, unless there is a reason not
> to.".
>
> Can you please clarify?

Not sure what you mean, but that paragraph was about being flexible
with soundness issues -- it relates to what I said above about the
policy not creating undue pressure, i.e. that the intention is not to
add pressure even if they are considered bugs. It wasn't saying we
don't backport them (or that we don't take them as fixes etc.).

Let me try to clarify by summarizing in other words:

  - We consider soundness issues bugs, so we try to fix them, add
Fixes: tags for them, etc.

  - However, even if they are considered bugs, we should remain
flexible about them. For instance, here, we have a patch that requires
a new feature, and there doesn't seem to be UB, so we can take our
time with it.

  - We backport them when possible/reasonable, even if one could
consider it out of the scope of the usual stable rules. The stable
kernel was OK with us sending those when we discussed it years ago. Of
course, some things may not be worth backporting, and some patches may
be rejected, but we generally try.

(And this is not meant to say you said something different, or that
you didn't know about it etc.)

I hope that helps...

Cheers,
Miguel

Re: [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA

[Danilo Krummrich]

On Wed Feb 18, 2026 at 11:49 AM CET, Miguel Ojeda wrote:
> I didn't know what your plan was

I don't generally have a fixed plan for individual patches. Instead, I
constantly evaluate the situation, considering dependencies, synergies, the
release cycle, general feedback and stakeholder input -- and make a judgement
call as the context evolves.

For instance, in this case, there is a potential synergy with another fix, which
I did not mention so far as it does not define the timeline for this series as
it can also be addressed otherwise.

Re: [PATCH 1/4] rust: add projection infrastructure

[Benno Lossin]

On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
> +#[macro_export]
> +macro_rules! project_pointer {
> +    (@gen $ptr:ident, ) => {};
> +    // Field projection. `$field` needs to be `tt` to support tuple index like `.0`.
> +    (@gen $ptr:ident, .$field:tt $($rest:tt)*) => {
> +        // SAFETY: the provided closure always return in bounds pointer.
> +        let $ptr = unsafe {
> +            $crate::projection::ProjectField::proj($ptr, #[inline(always)] |ptr| {

By the way, how does this avoid `#![feature(stmt_expr_attributes)]`?

Cheers,
Benno

> +                // SAFETY: `$field` is in bounds, and no implicit `Deref` is possible (if the
> +                // type implements `Deref`, Rust cannot infer the generic parameter `DEREF`).
> +                &raw mut (*ptr).$field
> +            })
> +        };
> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
> +    };

Re: [PATCH 1/4] rust: add projection infrastructure

[Gary Guo]

On 2026-02-22 00:57, Benno Lossin wrote:
> On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
>> +#[macro_export]
>> +macro_rules! project_pointer {
>> +    (@gen $ptr:ident, ) => {};
>> +    // Field projection. `$field` needs to be `tt` to support tuple 
>> index like `.0`.
>> +    (@gen $ptr:ident, .$field:tt $($rest:tt)*) => {
>> +        // SAFETY: the provided closure always return in bounds 
>> pointer.
>> +        let $ptr = unsafe {
>> +            $crate::projection::ProjectField::proj($ptr, 
>> #[inline(always)] |ptr| {
> 
> By the way, how does this avoid `#![feature(stmt_expr_attributes)]`?

I don't know how, but attributes on closures passed direclty to 
functions has been stable
for basically ~forever. A quick check says that this is available since 
Rust 1.11.

Best,
Gary

> 
> Cheers,
> Benno
> 
>> +                // SAFETY: `$field` is in bounds, and no implicit 
>> `Deref` is possible (if the
>> +                // type implements `Deref`, Rust cannot infer the 
>> generic parameter `DEREF`).
>> +                &raw mut (*ptr).$field
>> +            })
>> +        };
>> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
>> +    };