* [PATCH v13 1/7] rust: error: Add EOVERFLOW
2025-03-07 11:06 [PATCH v13 0/7] rust: add dma coherent allocator abstraction Abdiel Janulgue
@ 2025-03-07 11:06 ` Abdiel Janulgue
2025-03-07 11:06 ` [PATCH v13 2/7] rust: add dma coherent allocator abstraction Abdiel Janulgue
` (6 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2025-03-07 11:06 UTC (permalink / raw)
To: rust-for-linux, daniel.almeida, dakr, robin.murphy, aliceryhl
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS,
Abdiel Janulgue
Trivial addition for missing EOVERFLOW error. This is used by a
subsequent patch that might require returning EOVERFLOW as a result
of `checked_mul`.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@gmail.com>
---
rust/kernel/error.rs | 1 +
1 file changed, 1 insertion(+)
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index f6ecf09cb65f..1e510181432c 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -64,6 +64,7 @@ macro_rules! declare_err {
declare_err!(EPIPE, "Broken pipe.");
declare_err!(EDOM, "Math argument out of domain of func.");
declare_err!(ERANGE, "Math result not representable.");
+ declare_err!(EOVERFLOW, "Value too large for defined data type.");
declare_err!(ERESTARTSYS, "Restart the system call.");
declare_err!(ERESTARTNOINTR, "System call was interrupted by a signal and will be restarted.");
declare_err!(ERESTARTNOHAND, "Restart if no handler.");
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v13 2/7] rust: add dma coherent allocator abstraction.
2025-03-07 11:06 [PATCH v13 0/7] rust: add dma coherent allocator abstraction Abdiel Janulgue
2025-03-07 11:06 ` [PATCH v13 1/7] rust: error: Add EOVERFLOW Abdiel Janulgue
@ 2025-03-07 11:06 ` Abdiel Janulgue
2025-03-07 11:17 ` Alice Ryhl
` (2 more replies)
2025-03-07 11:06 ` [PATCH v13 3/7] rust: pci: impl AsMut<Device> for pci::Device Abdiel Janulgue
` (5 subsequent siblings)
7 siblings, 3 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2025-03-07 11:06 UTC (permalink / raw)
To: rust-for-linux, daniel.almeida, dakr, robin.murphy, aliceryhl
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS,
Abdiel Janulgue
Add a simple dma coherent allocator rust abstraction. Based on
Andreas Hindborg's dma abstractions from the rnvme driver, which
was also based on earlier work by Wedson Almeida Filho.
A CoherentAllocation is wrapped in Devres which basically guarantees
that a driver can't make a CoherentAllocation out-live driver unbind.
This is needed, since DMA allocations potentially also result in
programming of the IOMMU. IOMMU mappings are device resources and
hence the device / driver lifecycle needs to be enforced.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@gmail.com>
---
rust/bindings/bindings_helper.h | 1 +
rust/kernel/dma.rs | 378 ++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 1 +
3 files changed, 380 insertions(+)
create mode 100644 rust/kernel/dma.rs
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index f46cf3bb7069..bf1110590c19 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -12,6 +12,7 @@
#include <linux/blkdev.h>
#include <linux/cred.h>
#include <linux/device/faux.h>
+#include <linux/dma-mapping.h>
#include <linux/errname.h>
#include <linux/ethtool.h>
#include <linux/file.h>
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
new file mode 100644
index 000000000000..8a250242641c
--- /dev/null
+++ b/rust/kernel/dma.rs
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Direct memory access (DMA).
+//!
+//! C header: [`include/linux/dma-mapping.h`](srctree/include/linux/dma-mapping.h)
+
+use crate::{
+ bindings, build_assert,
+ device::Device,
+ devres::Devres,
+ error::code::*,
+ error::Result,
+ transmute::{AsBytes, FromBytes},
+ types::ARef,
+};
+use kernel::prelude::*;
+
+/// Possible attributes associated with a DMA mapping.
+///
+/// They can be combined with the operators `|`, `&`, and `!`.
+///
+/// Values can be used from the [`attrs`] module.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::{device::Device, devres::Devres,};
+/// use kernel::dma::{attrs::*, CoherentAllocation};
+///
+/// # fn test(dev: &Device) -> Result {
+/// let attribs = DMA_ATTR_FORCE_CONTIGUOUS | DMA_ATTR_NO_WARN;
+/// let c: Devres<CoherentAllocation<u64>> =
+/// CoherentAllocation::alloc_attrs(dev, 4, GFP_KERNEL, attribs)?;
+/// # Ok::<(), Error>(()) }
+/// ```
+#[derive(Clone, Copy, PartialEq)]
+#[repr(transparent)]
+pub struct Attrs(u32);
+
+impl Attrs {
+ /// Get the raw representation of this attribute.
+ pub(crate) fn as_raw(self) -> crate::ffi::c_ulong {
+ self.0 as _
+ }
+
+ /// Check whether `flags` is contained in `self`.
+ pub fn contains(self, flags: Attrs) -> bool {
+ (self & flags) == flags
+ }
+}
+
+impl core::ops::BitOr for Attrs {
+ type Output = Self;
+ fn bitor(self, rhs: Self) -> Self::Output {
+ Self(self.0 | rhs.0)
+ }
+}
+
+impl core::ops::BitAnd for Attrs {
+ type Output = Self;
+ fn bitand(self, rhs: Self) -> Self::Output {
+ Self(self.0 & rhs.0)
+ }
+}
+
+impl core::ops::Not for Attrs {
+ type Output = Self;
+ fn not(self) -> Self::Output {
+ Self(!self.0)
+ }
+}
+
+/// DMA mapping attributes.
+pub mod attrs {
+ use super::Attrs;
+
+ /// Specifies that reads and writes to the mapping may be weakly ordered, that is that reads
+ /// and writes may pass each other.
+ pub const DMA_ATTR_WEAK_ORDERING: Attrs = Attrs(bindings::DMA_ATTR_WEAK_ORDERING);
+
+ /// Specifies that writes to the mapping may be buffered to improve performance.
+ pub const DMA_ATTR_WRITE_COMBINE: Attrs = Attrs(bindings::DMA_ATTR_WRITE_COMBINE);
+
+ /// Lets the platform to avoid creating a kernel virtual mapping for the allocated buffer.
+ pub const DMA_ATTR_NO_KERNEL_MAPPING: Attrs = Attrs(bindings::DMA_ATTR_NO_KERNEL_MAPPING);
+
+ /// Allows platform code to skip synchronization of the CPU cache for the given buffer assuming
+ /// that it has been already transferred to 'device' domain.
+ pub const DMA_ATTR_SKIP_CPU_SYNC: Attrs = Attrs(bindings::DMA_ATTR_SKIP_CPU_SYNC);
+
+ /// Forces contiguous allocation of the buffer in physical memory.
+ pub const DMA_ATTR_FORCE_CONTIGUOUS: Attrs = Attrs(bindings::DMA_ATTR_FORCE_CONTIGUOUS);
+
+ /// This is a hint to the DMA-mapping subsystem that it's probably not worth the time to try
+ /// to allocate memory to in a way that gives better TLB efficiency.
+ pub const DMA_ATTR_ALLOC_SINGLE_PAGES: Attrs = Attrs(bindings::DMA_ATTR_ALLOC_SINGLE_PAGES);
+
+ /// This tells the DMA-mapping subsystem to suppress allocation failure reports (similarly to
+ /// __GFP_NOWARN).
+ pub const DMA_ATTR_NO_WARN: Attrs = Attrs(bindings::DMA_ATTR_NO_WARN);
+
+ /// Used to indicate that the buffer is fully accessible at an elevated privilege level (and
+ /// ideally inaccessible or at least read-only at lesser-privileged levels).
+ pub const DMA_ATTR_PRIVILEGED: Attrs = Attrs(bindings::DMA_ATTR_PRIVILEGED);
+}
+
+/// An abstraction of the `dma_alloc_coherent` API.
+///
+/// This is an abstraction around the `dma_alloc_coherent` API which is used to allocate and map
+/// large consistent DMA regions.
+///
+/// A [`CoherentAllocation`] instance contains a pointer to the allocated region (in the
+/// processor's virtual address space) and the device address which can be given to the device
+/// as the DMA address base of the region. The region is released once [`CoherentAllocation`]
+/// is dropped.
+///
+/// # Invariants
+///
+/// For the lifetime of an instance of [`CoherentAllocation`], the `cpu_addr` is a valid pointer
+/// to an allocated region of consistent memory and `dma_handle` is the DMA address base of
+/// the region.
+pub struct CoherentAllocation<T: AsBytes + FromBytes> {
+ dev: ARef<Device>,
+ dma_handle: bindings::dma_addr_t,
+ count: usize,
+ cpu_addr: *mut T,
+ dma_attrs: Attrs,
+}
+
+impl<T: AsBytes + FromBytes> CoherentAllocation<T> {
+ /// Allocates a region of `size_of::<T> * count` of consistent memory.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::{device::Device, devres::Devres,};
+ /// use kernel::dma::{attrs::*, CoherentAllocation};
+ ///
+ /// # fn test(dev: &Device) -> Result {
+ /// let c: Devres<CoherentAllocation<u64>> =
+ /// CoherentAllocation::alloc_attrs(dev, 4, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
+ /// # Ok::<(), Error>(()) }
+ /// ```
+ pub fn alloc_attrs(
+ dev: &Device,
+ count: usize,
+ gfp_flags: kernel::alloc::Flags,
+ dma_attrs: Attrs,
+ ) -> Result<Devres<CoherentAllocation<T>>> {
+ build_assert!(
+ core::mem::size_of::<T>() > 0,
+ "It doesn't make sense for the allocated type to be a ZST"
+ );
+
+ let size = count
+ .checked_mul(core::mem::size_of::<T>())
+ .ok_or(EOVERFLOW)?;
+ let mut dma_handle = 0;
+ // SAFETY: Device pointer is guaranteed as valid by the type invariant on `Device`.
+ let ret = unsafe {
+ bindings::dma_alloc_attrs(
+ dev.as_raw(),
+ size,
+ &mut dma_handle,
+ gfp_flags.as_raw(),
+ dma_attrs.as_raw(),
+ )
+ };
+ if ret.is_null() {
+ return Err(ENOMEM);
+ }
+ // INVARIANT: We just successfully allocated a coherent region which is accessible for
+ // `count` elements, hence the cpu address is valid. We also hold a refcounted reference
+ // to the device.
+ let devres = Devres::new(
+ dev,
+ Self {
+ dev: dev.into(),
+ dma_handle,
+ count,
+ cpu_addr: ret as *mut T,
+ dma_attrs,
+ },
+ GFP_KERNEL,
+ )?;
+
+ Ok(devres)
+ }
+
+ /// Performs the same functionality as [`alloc_attrs`], except the `dma_attrs` is 0 by default.
+ pub fn alloc_coherent(
+ dev: &Device,
+ count: usize,
+ gfp_flags: kernel::alloc::Flags,
+ ) -> Result<Devres<CoherentAllocation<T>>> {
+ CoherentAllocation::alloc_attrs(dev, count, gfp_flags, Attrs(0))
+ }
+
+ /// 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
+ }
+
+ /// Returns the base address to the allocated region in the CPU's virtual address space as
+ /// a mutable pointer.
+ pub fn start_ptr_mut(&mut self) -> *mut T {
+ self.cpu_addr
+ }
+
+ /// Returns a DMA handle which may given to the device as the DMA address base of
+ /// the region.
+ pub fn dma_handle(&self) -> bindings::dma_addr_t {
+ self.dma_handle
+ }
+
+ /// Returns a pointer to an element from the region with bounds checking. `offset` is in
+ /// units of `T`, not the number of bytes.
+ ///
+ /// Public but hidden since it should only be used from [`dma_read`] and [`dma_write`] macros.
+ #[doc(hidden)]
+ pub fn item_from_index(&self, offset: usize) -> Result<*mut T> {
+ if offset >= self.count {
+ return Err(EINVAL);
+ }
+ // SAFETY:
+ // - The pointer is valid due to type invariant on `CoherentAllocation`
+ // and we've just checked that the range and index is within bounds.
+ // - `offset` can't overflow since it is smaller than `self.count` and we've checked
+ // that `self.count` won't overflow early in the constructor.
+ Ok(unsafe { self.cpu_addr.add(offset) })
+ }
+
+ /// Reads the value of `field` and ensures that its type is [`FromBytes`].
+ ///
+ /// # Safety
+ ///
+ /// This must be called from the [`dma_read`] macro which ensures that the `field` pointer is
+ /// validated beforehand.
+ ///
+ /// Public but hidden since it should only be used from [`dma_read`] macro.
+ #[doc(hidden)]
+ pub unsafe fn field_read<F: FromBytes>(&self, field: *const F) -> F {
+ // SAFETY: By the safety requirements field is valid.
+ unsafe { field.read_volatile() }
+ }
+
+ /// Writes a value to `field` and ensures that its type is [`AsBytes`].
+ ///
+ /// # Safety
+ ///
+ /// This must be called from the [`dma_write`] macro which ensures that the `field` pointer is
+ /// validated beforehand.
+ ///
+ /// Public but hidden since it should only be used from [`dma_write`] macro.
+ #[doc(hidden)]
+ pub unsafe fn field_write<F: AsBytes>(&self, field: *mut F, val: F) {
+ // SAFETY: By the safety requirements field is valid.
+ unsafe { field.write_volatile(val) }
+ }
+}
+
+/// Note that the device configured to do DMA must be halted before this object is dropped.
+impl<T: AsBytes + FromBytes> Drop for CoherentAllocation<T> {
+ fn drop(&mut self) {
+ let size = self.count * core::mem::size_of::<T>();
+ // SAFETY: Device pointer is guaranteed as valid by the type invariant on `Device`.
+ // The cpu address, and the dma handle are valid due to the type invariants on
+ // `CoherentAllocation`.
+ unsafe {
+ bindings::dma_free_attrs(
+ self.dev.as_raw(),
+ size,
+ self.cpu_addr as _,
+ self.dma_handle,
+ self.dma_attrs.as_raw(),
+ )
+ }
+ }
+}
+
+/// Reads a field of an item from an allocated region of structs.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::{device::Device, devres::Devres,};
+/// use kernel::dma::{attrs::*, CoherentAllocation};
+///
+/// struct MyStruct { field: u32, }
+///
+/// // SAFETY: All bit patterns are acceptable values for `MyStruct`.
+/// unsafe impl kernel::transmute::FromBytes for MyStruct{};
+/// // SAFETY: Instances of `MyStruct` have no uninitialized portions.
+/// unsafe impl kernel::transmute::AsBytes for MyStruct{};
+///
+/// # fn test(reg: &Devres<CoherentAllocation<MyStruct>>) -> Result {
+/// let alloc = reg.try_access().ok_or(ENXIO)?;
+/// 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)*) => {{
+ 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)*);
+ $crate::dma::CoherentAllocation::field_read(&$dma, ptr_field)
+ }
+ }};
+ ($dma:ident [ $idx:expr ] $($field:tt)* ) => {
+ $crate::dma_read!($dma, $idx, $($field)*);
+ };
+ ($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => {
+ $crate::dma_read!($($dma).*, $idx, $($field)*);
+ };
+}
+
+/// Writes to a field of an item from an allocated region of structs.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::{device::Device, devres::Devres,};
+/// use kernel::dma::{attrs::*, CoherentAllocation};
+///
+/// struct MyStruct { member: u32, }
+///
+/// // SAFETY: All bit patterns are acceptable values for `MyStruct`.
+/// unsafe impl kernel::transmute::FromBytes for MyStruct{};
+/// // SAFETY: Instances of `MyStruct` have no uninitialized portions.
+/// unsafe impl kernel::transmute::AsBytes for MyStruct{};
+///
+/// # fn test(reg: &Devres<CoherentAllocation<MyStruct>>) -> Result {
+/// let alloc = reg.try_access().ok_or(ENXIO)?;
+/// 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) => {
+ 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) }
+ };
+ ($dma:expr, $idx: expr, $(.$field:ident)* = $val:expr) => {
+ 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)
+ }
+ };
+}
+
+/// Helper function to set the bit mask for DMA addressing.
+pub const fn dma_bit_mask(n: usize) -> u64 {
+ if n > 64 {
+ return 0;
+ }
+ if n == 64 {
+ !0
+ } else {
+ (1 << (n)) - 1
+ }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 398242f92a96..8e76ef9b4346 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -44,6 +44,7 @@
pub mod device;
pub mod device_id;
pub mod devres;
+pub mod dma;
pub mod driver;
pub mod error;
pub mod faux;
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v13 2/7] rust: add dma coherent allocator abstraction.
2025-03-07 11:06 ` [PATCH v13 2/7] rust: add dma coherent allocator abstraction Abdiel Janulgue
@ 2025-03-07 11:17 ` Alice Ryhl
2025-03-07 20:40 ` Andreas Hindborg
2025-03-21 17:23 ` Jason Gunthorpe
2 siblings, 0 replies; 23+ messages in thread
From: Alice Ryhl @ 2025-03-07 11:17 UTC (permalink / raw)
To: Abdiel Janulgue
Cc: rust-for-linux, daniel.almeida, dakr, robin.murphy, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Valentin Obst,
open list, Christoph Hellwig, Marek Szyprowski, airlied,
open list:DMA MAPPING HELPERS
On Fri, Mar 07, 2025 at 01:06:19PM +0200, Abdiel Janulgue wrote:
> Add a simple dma coherent allocator rust abstraction. Based on
> Andreas Hindborg's dma abstractions from the rnvme driver, which
> was also based on earlier work by Wedson Almeida Filho.
>
> A CoherentAllocation is wrapped in Devres which basically guarantees
> that a driver can't make a CoherentAllocation out-live driver unbind.
> This is needed, since DMA allocations potentially also result in
> programming of the IOMMU. IOMMU mappings are device resources and
> hence the device / driver lifecycle needs to be enforced.
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@gmail.com>
You might want to add #[inline] annotations to the various methods, but
otherwise LGTM.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> +/// Helper function to set the bit mask for DMA addressing.
> +pub const fn dma_bit_mask(n: usize) -> u64 {
> + if n > 64 {
> + return 0;
> + }
> + if n == 64 {
> + !0
> + } else {
> + (1 << (n)) - 1
> + }
> +}
You don't use this method?
Alice
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v13 2/7] rust: add dma coherent allocator abstraction.
2025-03-07 11:06 ` [PATCH v13 2/7] rust: add dma coherent allocator abstraction Abdiel Janulgue
2025-03-07 11:17 ` Alice Ryhl
@ 2025-03-07 20:40 ` Andreas Hindborg
2025-03-21 17:23 ` Jason Gunthorpe
2 siblings, 0 replies; 23+ messages in thread
From: Andreas Hindborg @ 2025-03-07 20:40 UTC (permalink / raw)
To: Abdiel Janulgue
Cc: rust-for-linux, daniel.almeida, dakr, robin.murphy, aliceryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross, Valentin Obst,
linux-kernel, Christoph Hellwig, Marek Szyprowski, airlied, iommu
"Abdiel Janulgue" <abdiel.janulgue@gmail.com> writes:
> Add a simple dma coherent allocator rust abstraction. Based on
> Andreas Hindborg's dma abstractions from the rnvme driver, which
> was also based on earlier work by Wedson Almeida Filho.
>
> A CoherentAllocation is wrapped in Devres which basically guarantees
> that a driver can't make a CoherentAllocation out-live driver unbind.
> This is needed, since DMA allocations potentially also result in
> programming of the IOMMU. IOMMU mappings are device resources and
> hence the device / driver lifecycle needs to be enforced.
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@gmail.com>
> ---
> +
> + /// Reads the value of `field` and ensures that its type is [`FromBytes`].
> + ///
> + /// # Safety
> + ///
> + /// This must be called from the [`dma_read`] macro which ensures that the `field` pointer is
> + /// validated beforehand.
> + ///
> + /// Public but hidden since it should only be used from [`dma_read`] macro.
> + #[doc(hidden)]
> + pub unsafe fn field_read<F: FromBytes>(&self, field: *const F) -> F {
> + // SAFETY: By the safety requirements field is valid.
> + unsafe { field.read_volatile() }
From the docs of `read_volatile`:
Just like in C, whether an operation is volatile has no bearing
whatsoever on questions involving concurrent access from multiple
threads. Volatile accesses behave exactly like non-atomic accesses in
that regard. In particular, a race between a read_volatile and any
write operation to the same location is undefined behavior.
So the safety requirement is not sufficient. Alice responded to my
question on v12:
> Volatile reads/writes that race are OK?
I will not give a blanket yes to that. If you read their docs, you
will find that they claim to not allow it. But they are the correct
choice for DMA memory, and there's no way in practice to get
miscompilations on memory locations that are only accessed with
volatile operations, and never have references to them created.
In general, this will fall into the exception that we've been given
from the Rust people. In cases such as this where the Rust language
does not give us the operation we want, do it like you do in C. Since
Rust uses LLVM which does not miscompile the C part of the kernel, it
should not miscompile the Rust part either.
Since we have an exception for the safety requirements in the
documentation, we should make that clear in the safety comment at the
call site.
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v13 2/7] rust: add dma coherent allocator abstraction.
2025-03-07 11:06 ` [PATCH v13 2/7] rust: add dma coherent allocator abstraction Abdiel Janulgue
2025-03-07 11:17 ` Alice Ryhl
2025-03-07 20:40 ` Andreas Hindborg
@ 2025-03-21 17:23 ` Jason Gunthorpe
2025-03-21 17:34 ` Danilo Krummrich
2 siblings, 1 reply; 23+ messages in thread
From: Jason Gunthorpe @ 2025-03-21 17:23 UTC (permalink / raw)
To: Abdiel Janulgue
Cc: rust-for-linux, daniel.almeida, dakr, robin.murphy, aliceryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS
On Fri, Mar 07, 2025 at 01:06:19PM +0200, Abdiel Janulgue wrote:
> + // SAFETY: Device pointer is guaranteed as valid by the type invariant on `Device`.
> + let ret = unsafe {
> + bindings::dma_alloc_attrs(
> + dev.as_raw(),
> + size,
> + &mut dma_handle,
> + gfp_flags.as_raw(),
> + dma_attrs.as_raw(),
> + )
This is not the correct safety statement, the device must have a driver
bound to call this function, a struct device reference is not
sufficient.
I belive Danilo was suggesting to ignore this unsafety for now, but if
so it should be documented correctly.
Also think the use of devres here is going to be very problematic for
drivers to use as I said in other emails. :(
Jason
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v13 2/7] rust: add dma coherent allocator abstraction.
2025-03-21 17:23 ` Jason Gunthorpe
@ 2025-03-21 17:34 ` Danilo Krummrich
2025-03-21 18:29 ` Jason Gunthorpe
0 siblings, 1 reply; 23+ messages in thread
From: Danilo Krummrich @ 2025-03-21 17:34 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Abdiel Janulgue, rust-for-linux, daniel.almeida, robin.murphy,
aliceryhl, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS
On Fri, Mar 21, 2025 at 02:23:53PM -0300, Jason Gunthorpe wrote:
> On Fri, Mar 07, 2025 at 01:06:19PM +0200, Abdiel Janulgue wrote:
>
> > + // SAFETY: Device pointer is guaranteed as valid by the type invariant on `Device`.
> > + let ret = unsafe {
> > + bindings::dma_alloc_attrs(
> > + dev.as_raw(),
> > + size,
> > + &mut dma_handle,
> > + gfp_flags.as_raw(),
> > + dma_attrs.as_raw(),
> > + )
>
> This is not the correct safety statement, the device must have a driver
> bound to call this function, a struct device reference is not
> sufficient.
>
> I belive Danilo was suggesting to ignore this unsafety for now, but if
> so it should be documented correctly.
If just landed patches [1], which are the foundation of addressing this issue.
With the next cycle, this will be ensured by the type system.
>
> Also think the use of devres here is going to be very problematic for
> drivers to use as I said in other emails. :(
In an earlier reply today in a different thread already gave you the link [2] of
what we landed, which, besides explaining the situation, also makes clear that
there is *no* Devres wrapper around a CoherentAllocation and why.
[1] https://lore.kernel.org/lkml/20250314160932.100165-1-dakr@kernel.org/
[2] https://github.com/Rust-for-Linux/linux/blob/rust-next/rust/kernel/dma.rs#L120
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v13 2/7] rust: add dma coherent allocator abstraction.
2025-03-21 17:34 ` Danilo Krummrich
@ 2025-03-21 18:29 ` Jason Gunthorpe
2025-03-21 19:16 ` Danilo Krummrich
0 siblings, 1 reply; 23+ messages in thread
From: Jason Gunthorpe @ 2025-03-21 18:29 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Abdiel Janulgue, rust-for-linux, daniel.almeida, robin.murphy,
aliceryhl, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS
On Fri, Mar 21, 2025 at 06:34:53PM +0100, Danilo Krummrich wrote:
> On Fri, Mar 21, 2025 at 02:23:53PM -0300, Jason Gunthorpe wrote:
> > On Fri, Mar 07, 2025 at 01:06:19PM +0200, Abdiel Janulgue wrote:
> >
> > > + // SAFETY: Device pointer is guaranteed as valid by the type invariant on `Device`.
> > > + let ret = unsafe {
> > > + bindings::dma_alloc_attrs(
> > > + dev.as_raw(),
> > > + size,
> > > + &mut dma_handle,
> > > + gfp_flags.as_raw(),
> > > + dma_attrs.as_raw(),
> > > + )
> >
> > This is not the correct safety statement, the device must have a driver
> > bound to call this function, a struct device reference is not
> > sufficient.
> >
> > I belive Danilo was suggesting to ignore this unsafety for now, but if
> > so it should be documented correctly.
>
> If just landed patches [1], which are the foundation of addressing this issue.
Those patches say:
The context types can be extended as required, e.g. to limit availability of
certain (bus) device functions to probe().
Which is not an appropriate limitation for dma_alloc_coherent, we
expect it to be called outside probe in real drivers. Is there more to
that story?
Regardless, the safety comment should not be merged with incorrect
information. :\
> > Also think the use of devres here is going to be very problematic for
> > drivers to use as I said in other emails. :(
>
> In an earlier reply today in a different thread already gave you the link [2] of
> what we landed, which, besides explaining the situation, also makes clear that
> there is *no* Devres wrapper around a CoherentAllocation and why.
Yes, I see, I have so much email right now it is hard find all the
different versions of everything.
Jason
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v13 2/7] rust: add dma coherent allocator abstraction.
2025-03-21 18:29 ` Jason Gunthorpe
@ 2025-03-21 19:16 ` Danilo Krummrich
0 siblings, 0 replies; 23+ messages in thread
From: Danilo Krummrich @ 2025-03-21 19:16 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Abdiel Janulgue, rust-for-linux, daniel.almeida, robin.murphy,
aliceryhl, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS
On Fri, Mar 21, 2025 at 03:29:01PM -0300, Jason Gunthorpe wrote:
> On Fri, Mar 21, 2025 at 06:34:53PM +0100, Danilo Krummrich wrote:
> > On Fri, Mar 21, 2025 at 02:23:53PM -0300, Jason Gunthorpe wrote:
> > > On Fri, Mar 07, 2025 at 01:06:19PM +0200, Abdiel Janulgue wrote:
> > >
> > > > + // SAFETY: Device pointer is guaranteed as valid by the type invariant on `Device`.
> > > > + let ret = unsafe {
> > > > + bindings::dma_alloc_attrs(
> > > > + dev.as_raw(),
> > > > + size,
> > > > + &mut dma_handle,
> > > > + gfp_flags.as_raw(),
> > > > + dma_attrs.as_raw(),
> > > > + )
> > >
> > > This is not the correct safety statement, the device must have a driver
> > > bound to call this function, a struct device reference is not
> > > sufficient.
> > >
> > > I belive Danilo was suggesting to ignore this unsafety for now, but if
> > > so it should be documented correctly.
> >
> > If just landed patches [1], which are the foundation of addressing this issue.
>
> Those patches say:
>
> The context types can be extended as required, e.g. to limit availability of
> certain (bus) device functions to probe().
>
> Which is not an appropriate limitation for dma_alloc_coherent, we
> expect it to be called outside probe in real drivers. Is there more to
> that story?
Yeah, we can also use them to derive specifically typed Device instances from
other entry points of the driver where we know for sure that at this point the
device must (still) be bound to the driver.
For instance, bus callbacks, subsystem callbacks, certain (but not all) IOCTLs,
IRQ handlers, etc.
All those cases can be covered by only the type system, without additional
locks. We could even use this as an optimization to bypass Devres'
try_access() calls when holding a corresponding device instance in those places.
>
> Regardless, the safety comment should not be merged with incorrect
> information. :\
v15 did land in rust-next, so unfortunately this was overlooked. Since you
caught, mind sending a patch improving the comment?
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v13 3/7] rust: pci: impl AsMut<Device> for pci::Device
2025-03-07 11:06 [PATCH v13 0/7] rust: add dma coherent allocator abstraction Abdiel Janulgue
2025-03-07 11:06 ` [PATCH v13 1/7] rust: error: Add EOVERFLOW Abdiel Janulgue
2025-03-07 11:06 ` [PATCH v13 2/7] rust: add dma coherent allocator abstraction Abdiel Janulgue
@ 2025-03-07 11:06 ` Abdiel Janulgue
2025-03-07 11:18 ` Alice Ryhl
2025-03-07 14:18 ` Greg KH
2025-03-07 11:06 ` [PATCH v13 4/7] rust: device: add dma addressing capabilities Abdiel Janulgue
` (4 subsequent siblings)
7 siblings, 2 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2025-03-07 11:06 UTC (permalink / raw)
To: rust-for-linux, daniel.almeida, dakr, robin.murphy, aliceryhl
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS
From: Danilo Krummrich <dakr@kernel.org>
Some device methods require mutable references, since they change the
underlying struct device without lock protection.
Hence, make it possible to retrieve a mutable reference to a Device from
a mutable pci::Device.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/pci.rs | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 4c98b5b9aa1e..141430dac2d5 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -432,3 +432,14 @@ fn as_ref(&self) -> &device::Device {
&self.0
}
}
+
+impl AsMut<device::Device> for Device {
+ fn as_mut(&mut self) -> &mut device::Device {
+ // SAFETY:
+ // - `self.0.as_raw()` is valid by the type invariant of `device::Device`,
+ // - `struct device` is embedded in `struct pci_dev`, hence it is safe to give out a
+ // mutable reference for `device::Device` if we have a mutable reference to the
+ // corresponding `pci::Device`.
+ unsafe { &mut *self.0.as_raw().cast() }
+ }
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v13 3/7] rust: pci: impl AsMut<Device> for pci::Device
2025-03-07 11:06 ` [PATCH v13 3/7] rust: pci: impl AsMut<Device> for pci::Device Abdiel Janulgue
@ 2025-03-07 11:18 ` Alice Ryhl
2025-03-07 11:45 ` Danilo Krummrich
2025-03-07 14:18 ` Greg KH
1 sibling, 1 reply; 23+ messages in thread
From: Alice Ryhl @ 2025-03-07 11:18 UTC (permalink / raw)
To: Abdiel Janulgue
Cc: rust-for-linux, daniel.almeida, dakr, robin.murphy, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Valentin Obst,
open list, Christoph Hellwig, Marek Szyprowski, airlied,
open list:DMA MAPPING HELPERS
On Fri, Mar 07, 2025 at 01:06:20PM +0200, Abdiel Janulgue wrote:
> From: Danilo Krummrich <dakr@kernel.org>
>
> Some device methods require mutable references, since they change the
> underlying struct device without lock protection.
>
> Hence, make it possible to retrieve a mutable reference to a Device from
> a mutable pci::Device.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> rust/kernel/pci.rs | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 4c98b5b9aa1e..141430dac2d5 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -432,3 +432,14 @@ fn as_ref(&self) -> &device::Device {
> &self.0
> }
> }
> +
> +impl AsMut<device::Device> for Device {
> + fn as_mut(&mut self) -> &mut device::Device {
> + // SAFETY:
> + // - `self.0.as_raw()` is valid by the type invariant of `device::Device`,
> + // - `struct device` is embedded in `struct pci_dev`, hence it is safe to give out a
> + // mutable reference for `device::Device` if we have a mutable reference to the
> + // corresponding `pci::Device`.
> + unsafe { &mut *self.0.as_raw().cast() }
> + }
> +}
This makes it possible to call `mem::swap` on two devices, which I doubt
is allowed.
Alice
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v13 3/7] rust: pci: impl AsMut<Device> for pci::Device
2025-03-07 11:18 ` Alice Ryhl
@ 2025-03-07 11:45 ` Danilo Krummrich
0 siblings, 0 replies; 23+ messages in thread
From: Danilo Krummrich @ 2025-03-07 11:45 UTC (permalink / raw)
To: Alice Ryhl
Cc: Abdiel Janulgue, rust-for-linux, daniel.almeida, robin.murphy,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS
On Fri, Mar 07, 2025 at 11:18:03AM +0000, Alice Ryhl wrote:
> On Fri, Mar 07, 2025 at 01:06:20PM +0200, Abdiel Janulgue wrote:
> > From: Danilo Krummrich <dakr@kernel.org>
> >
> > Some device methods require mutable references, since they change the
> > underlying struct device without lock protection.
> >
> > Hence, make it possible to retrieve a mutable reference to a Device from
> > a mutable pci::Device.
> >
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > ---
> > rust/kernel/pci.rs | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> > index 4c98b5b9aa1e..141430dac2d5 100644
> > --- a/rust/kernel/pci.rs
> > +++ b/rust/kernel/pci.rs
> > @@ -432,3 +432,14 @@ fn as_ref(&self) -> &device::Device {
> > &self.0
> > }
> > }
> > +
> > +impl AsMut<device::Device> for Device {
> > + fn as_mut(&mut self) -> &mut device::Device {
> > + // SAFETY:
> > + // - `self.0.as_raw()` is valid by the type invariant of `device::Device`,
> > + // - `struct device` is embedded in `struct pci_dev`, hence it is safe to give out a
> > + // mutable reference for `device::Device` if we have a mutable reference to the
> > + // corresponding `pci::Device`.
> > + unsafe { &mut *self.0.as_raw().cast() }
> > + }
> > +}
>
> This makes it possible to call `mem::swap` on two devices, which I doubt
> is allowed.
Good catch, that's of course not allowed.
Obviously, this is for Device::dma_set_mask(), introduced in a subsequent patch.
Thinking about it, maybe it should be implemented by bus devices instead (e.g.
pci::Device) -- dma_set_mask() doesn't make sense for every device.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 3/7] rust: pci: impl AsMut<Device> for pci::Device
2025-03-07 11:06 ` [PATCH v13 3/7] rust: pci: impl AsMut<Device> for pci::Device Abdiel Janulgue
2025-03-07 11:18 ` Alice Ryhl
@ 2025-03-07 14:18 ` Greg KH
2025-03-07 17:53 ` Abdiel Janulgue
1 sibling, 1 reply; 23+ messages in thread
From: Greg KH @ 2025-03-07 14:18 UTC (permalink / raw)
To: Abdiel Janulgue
Cc: rust-for-linux, daniel.almeida, dakr, robin.murphy, aliceryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS
On Fri, Mar 07, 2025 at 01:06:20PM +0200, Abdiel Janulgue wrote:
> From: Danilo Krummrich <dakr@kernel.org>
>
> Some device methods require mutable references, since they change the
> underlying struct device without lock protection.
>
> Hence, make it possible to retrieve a mutable reference to a Device from
> a mutable pci::Device.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
Note, you can't forward on a patch from someone else without also
signing off on it. Please read the DCO for what this means and why.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 3/7] rust: pci: impl AsMut<Device> for pci::Device
2025-03-07 14:18 ` Greg KH
@ 2025-03-07 17:53 ` Abdiel Janulgue
0 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2025-03-07 17:53 UTC (permalink / raw)
To: Greg KH
Cc: rust-for-linux, daniel.almeida, dakr, robin.murphy, aliceryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS
On 07/03/2025 16:18, Greg KH wrote:
> On Fri, Mar 07, 2025 at 01:06:20PM +0200, Abdiel Janulgue wrote:
>> From: Danilo Krummrich <dakr@kernel.org>
>>
>> Some device methods require mutable references, since they change the
>> underlying struct device without lock protection.
>>
>> Hence, make it possible to retrieve a mutable reference to a Device from
>> a mutable pci::Device.
>>
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> ---
>
> Note, you can't forward on a patch from someone else without also
> signing off on it. Please read the DCO for what this means and why.
>
Thanks for the reminder! Will do so in next revision.
Regards,
Abdiel
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v13 4/7] rust: device: add dma addressing capabilities
2025-03-07 11:06 [PATCH v13 0/7] rust: add dma coherent allocator abstraction Abdiel Janulgue
` (2 preceding siblings ...)
2025-03-07 11:06 ` [PATCH v13 3/7] rust: pci: impl AsMut<Device> for pci::Device Abdiel Janulgue
@ 2025-03-07 11:06 ` Abdiel Janulgue
2025-03-07 20:12 ` Andreas Hindborg
2025-03-07 11:06 ` [PATCH v13 5/7] samples: rust: add Rust dma test sample driver Abdiel Janulgue
` (3 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Abdiel Janulgue @ 2025-03-07 11:06 UTC (permalink / raw)
To: rust-for-linux, daniel.almeida, dakr, robin.murphy, aliceryhl
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS,
Abdiel Janulgue
Add functions to set the DMA mask to inform the kernel about the
device's DMA addressing capabilities.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@gmail.com>
---
rust/helpers/dma.c | 8 ++++++++
rust/helpers/helpers.c | 1 +
rust/kernel/device.rs | 29 +++++++++++++++++++++++++++++
3 files changed, 38 insertions(+)
create mode 100644 rust/helpers/dma.c
diff --git a/rust/helpers/dma.c b/rust/helpers/dma.c
new file mode 100644
index 000000000000..8eb482386f93
--- /dev/null
+++ b/rust/helpers/dma.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/dma-mapping.h>
+
+int rust_helper_dma_set_mask_and_coherent(struct device *dev, u64 mask)
+{
+ return dma_set_mask_and_coherent(dev, mask);
+}
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 0640b7e115be..8f3808c8b7fe 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -13,6 +13,7 @@
#include "build_bug.c"
#include "cred.c"
#include "device.c"
+#include "dma.c"
#include "err.c"
#include "fs.c"
#include "io.c"
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index db2d9658ba47..f9d3d4f60ddb 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -6,10 +6,12 @@
use crate::{
bindings,
+ error::Result,
str::CStr,
types::{ARef, Opaque},
};
use core::{fmt, ptr};
+use kernel::prelude::*;
#[cfg(CONFIG_PRINTK)]
use crate::c_str;
@@ -187,6 +189,33 @@ pub fn property_present(&self, name: &CStr) -> bool {
// SAFETY: By the invariant of `CStr`, `name` is null-terminated.
unsafe { bindings::device_property_present(self.as_raw().cast_const(), name.as_char_ptr()) }
}
+
+ /// Inform the kernel about the device's DMA addressing capabilities.
+ ///
+ /// Set both the DMA mask and the coherent DMA mask to the same thing.
+ /// Note that we don't check the return value from the C `dma_set_coherent_mask`
+ /// as the DMA API guarantees that the coherent DMA mask can be set to
+ /// the same or smaller than the streaming DMA mask.
+ pub fn dma_set_mask_and_coherent(&mut self, mask: u64) -> Result {
+ // SAFETY: device pointer is guaranteed as valid by invariant on `Device`.
+ let ret = unsafe { bindings::dma_set_mask_and_coherent(self.as_raw(), mask) };
+ if ret != 0 {
+ Err(Error::from_errno(ret))
+ } else {
+ Ok(())
+ }
+ }
+
+ /// Same as [`dma_set_mask_and_coherent`], but set the mask only for streaming mappings.
+ pub fn dma_set_mask(&mut self, mask: u64) -> Result {
+ // SAFETY: device pointer is guaranteed as valid by invariant on `Device`.
+ let ret = unsafe { bindings::dma_set_mask(self.as_raw(), mask) };
+ if ret != 0 {
+ Err(Error::from_errno(ret))
+ } else {
+ Ok(())
+ }
+ }
}
// SAFETY: Instances of `Device` are always reference-counted.
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v13 4/7] rust: device: add dma addressing capabilities
2025-03-07 11:06 ` [PATCH v13 4/7] rust: device: add dma addressing capabilities Abdiel Janulgue
@ 2025-03-07 20:12 ` Andreas Hindborg
2025-03-11 17:45 ` Abdiel Janulgue
0 siblings, 1 reply; 23+ messages in thread
From: Andreas Hindborg @ 2025-03-07 20:12 UTC (permalink / raw)
To: Abdiel Janulgue
Cc: rust-for-linux, daniel.almeida, dakr, robin.murphy, aliceryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross, Valentin Obst,
linux-kernel, Christoph Hellwig, Marek Szyprowski, airlied, iommu
"Abdiel Janulgue" <abdiel.janulgue@gmail.com> writes:
> Add functions to set the DMA mask to inform the kernel about the
> device's DMA addressing capabilities.
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@gmail.com>
> ---
> rust/helpers/dma.c | 8 ++++++++
> rust/helpers/helpers.c | 1 +
> rust/kernel/device.rs | 29 +++++++++++++++++++++++++++++
> 3 files changed, 38 insertions(+)
> create mode 100644 rust/helpers/dma.c
>
> diff --git a/rust/helpers/dma.c b/rust/helpers/dma.c
> new file mode 100644
> index 000000000000..8eb482386f93
> --- /dev/null
> +++ b/rust/helpers/dma.c
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/dma-mapping.h>
> +
> +int rust_helper_dma_set_mask_and_coherent(struct device *dev, u64 mask)
> +{
> + return dma_set_mask_and_coherent(dev, mask);
> +}
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 0640b7e115be..8f3808c8b7fe 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -13,6 +13,7 @@
> #include "build_bug.c"
> #include "cred.c"
> #include "device.c"
> +#include "dma.c"
> #include "err.c"
> #include "fs.c"
> #include "io.c"
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index db2d9658ba47..f9d3d4f60ddb 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -6,10 +6,12 @@
>
> use crate::{
> bindings,
> + error::Result,
> str::CStr,
> types::{ARef, Opaque},
> };
> use core::{fmt, ptr};
> +use kernel::prelude::*;
>
> #[cfg(CONFIG_PRINTK)]
> use crate::c_str;
> @@ -187,6 +189,33 @@ pub fn property_present(&self, name: &CStr) -> bool {
> // SAFETY: By the invariant of `CStr`, `name` is null-terminated.
> unsafe { bindings::device_property_present(self.as_raw().cast_const(), name.as_char_ptr()) }
> }
> +
> + /// Inform the kernel about the device's DMA addressing capabilities.
> + ///
> + /// Set both the DMA mask and the coherent DMA mask to the same thing.
> + /// Note that we don't check the return value from the C `dma_set_coherent_mask`
> + /// as the DMA API guarantees that the coherent DMA mask can be set to
> + /// the same or smaller than the streaming DMA mask.
> + pub fn dma_set_mask_and_coherent(&mut self, mask: u64) -> Result {
> + // SAFETY: device pointer is guaranteed as valid by invariant on `Device`.
> + let ret = unsafe { bindings::dma_set_mask_and_coherent(self.as_raw(), mask) };
> + if ret != 0 {
> + Err(Error::from_errno(ret))
> + } else {
> + Ok(())
> + }
> + }
I think we can use `Error::from_errno` here (and below). As far as I can
tell, these C functions return negative on error.
> +
> + /// Same as [`dma_set_mask_and_coherent`], but set the mask only for streaming mappings.
> + pub fn dma_set_mask(&mut self, mask: u64) -> Result {
> + // SAFETY: device pointer is guaranteed as valid by invariant on `Device`.
> + let ret = unsafe { bindings::dma_set_mask(self.as_raw(), mask) };
> + if ret != 0 {
> + Err(Error::from_errno(ret))
> + } else {
> + Ok(())
> + }
> + }
> }
>
> // SAFETY: Instances of `Device` are always reference-counted.
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v13 4/7] rust: device: add dma addressing capabilities
2025-03-07 20:12 ` Andreas Hindborg
@ 2025-03-11 17:45 ` Abdiel Janulgue
2025-03-11 18:35 ` Miguel Ojeda
0 siblings, 1 reply; 23+ messages in thread
From: Abdiel Janulgue @ 2025-03-11 17:45 UTC (permalink / raw)
To: Andreas Hindborg
Cc: rust-for-linux, daniel.almeida, dakr, robin.murphy, aliceryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross, Valentin Obst,
linux-kernel, Christoph Hellwig, Marek Szyprowski, airlied, iommu
Hi,
On 07/03/2025 22:12, Andreas Hindborg wrote:
> "Abdiel Janulgue" <abdiel.janulgue@gmail.com> writes:
>
>> Add functions to set the DMA mask to inform the kernel about the
>> device's DMA addressing capabilities.
>>
>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@gmail.com>
>> ---
>> rust/helpers/dma.c | 8 ++++++++
>> rust/helpers/helpers.c | 1 +
>> rust/kernel/device.rs | 29 +++++++++++++++++++++++++++++
>> 3 files changed, 38 insertions(+)
>> create mode 100644 rust/helpers/dma.c
>>
>> diff --git a/rust/helpers/dma.c b/rust/helpers/dma.c
>> new file mode 100644
>> index 000000000000..8eb482386f93
>> --- /dev/null
>> +++ b/rust/helpers/dma.c
>> @@ -0,0 +1,8 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/dma-mapping.h>
>> +
>> +int rust_helper_dma_set_mask_and_coherent(struct device *dev, u64 mask)
>> +{
>> + return dma_set_mask_and_coherent(dev, mask);
>> +}
>> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
>> index 0640b7e115be..8f3808c8b7fe 100644
>> --- a/rust/helpers/helpers.c
>> +++ b/rust/helpers/helpers.c
>> @@ -13,6 +13,7 @@
>> #include "build_bug.c"
>> #include "cred.c"
>> #include "device.c"
>> +#include "dma.c"
>> #include "err.c"
>> #include "fs.c"
>> #include "io.c"
>> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
>> index db2d9658ba47..f9d3d4f60ddb 100644
>> --- a/rust/kernel/device.rs
>> +++ b/rust/kernel/device.rs
>> @@ -6,10 +6,12 @@
>>
>> use crate::{
>> bindings,
>> + error::Result,
>> str::CStr,
>> types::{ARef, Opaque},
>> };
>> use core::{fmt, ptr};
>> +use kernel::prelude::*;
>>
>> #[cfg(CONFIG_PRINTK)]
>> use crate::c_str;
>> @@ -187,6 +189,33 @@ pub fn property_present(&self, name: &CStr) -> bool {
>> // SAFETY: By the invariant of `CStr`, `name` is null-terminated.
>> unsafe { bindings::device_property_present(self.as_raw().cast_const(), name.as_char_ptr()) }
>> }
>> +
>> + /// Inform the kernel about the device's DMA addressing capabilities.
>> + ///
>> + /// Set both the DMA mask and the coherent DMA mask to the same thing.
>> + /// Note that we don't check the return value from the C `dma_set_coherent_mask`
>> + /// as the DMA API guarantees that the coherent DMA mask can be set to
>> + /// the same or smaller than the streaming DMA mask.
>> + pub fn dma_set_mask_and_coherent(&mut self, mask: u64) -> Result {
>> + // SAFETY: device pointer is guaranteed as valid by invariant on `Device`.
>> + let ret = unsafe { bindings::dma_set_mask_and_coherent(self.as_raw(), mask) };
>> + if ret != 0 {
>> + Err(Error::from_errno(ret))
>> + } else {
>> + Ok(())
>> + }
>> + }
>
> I think we can use `Error::from_errno` here (and below). As far as I can
> tell, these C functions return negative on error.
This already uses `Error::from_errno`?
Thanks!
>
>> +
>> + /// Same as [`dma_set_mask_and_coherent`], but set the mask only for streaming mappings.
>> + pub fn dma_set_mask(&mut self, mask: u64) -> Result {
>> + // SAFETY: device pointer is guaranteed as valid by invariant on `Device`.
>> + let ret = unsafe { bindings::dma_set_mask(self.as_raw(), mask) };
>> + if ret != 0 {
>> + Err(Error::from_errno(ret))
>> + } else {
>> + Ok(())
>> + }
>> + }
>> }
>>
>> // SAFETY: Instances of `Device` are always reference-counted.
>
>
> Best regards,
> Andreas Hindborg
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v13 4/7] rust: device: add dma addressing capabilities
2025-03-11 17:45 ` Abdiel Janulgue
@ 2025-03-11 18:35 ` Miguel Ojeda
2025-03-11 20:16 ` Andreas Hindborg
0 siblings, 1 reply; 23+ messages in thread
From: Miguel Ojeda @ 2025-03-11 18:35 UTC (permalink / raw)
To: Abdiel Janulgue
Cc: Andreas Hindborg, rust-for-linux, daniel.almeida, dakr,
robin.murphy, aliceryhl, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Trevor Gross,
Valentin Obst, linux-kernel, Christoph Hellwig, Marek Szyprowski,
airlied, iommu
On Tue, Mar 11, 2025 at 6:45 PM Abdiel Janulgue
<abdiel.janulgue@gmail.com> wrote:
>
> This already uses `Error::from_errno`?
Maybe Andreas meant `to_result`?
Cheers,
Miguel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 4/7] rust: device: add dma addressing capabilities
2025-03-11 18:35 ` Miguel Ojeda
@ 2025-03-11 20:16 ` Andreas Hindborg
0 siblings, 0 replies; 23+ messages in thread
From: Andreas Hindborg @ 2025-03-11 20:16 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Abdiel Janulgue, rust-for-linux, daniel.almeida, dakr,
robin.murphy, aliceryhl, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Trevor Gross,
Valentin Obst, linux-kernel, Christoph Hellwig, Marek Szyprowski,
airlied, iommu
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> writes:
> On Tue, Mar 11, 2025 at 6:45 PM Abdiel Janulgue
> <abdiel.janulgue@gmail.com> wrote:
>>
>> This already uses `Error::from_errno`?
>
> Maybe Andreas meant `to_result`?
Yes, sorry. It includes the if/else, but it tests for ret < 0. But I
think that is fine here.
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v13 5/7] samples: rust: add Rust dma test sample driver
2025-03-07 11:06 [PATCH v13 0/7] rust: add dma coherent allocator abstraction Abdiel Janulgue
` (3 preceding siblings ...)
2025-03-07 11:06 ` [PATCH v13 4/7] rust: device: add dma addressing capabilities Abdiel Janulgue
@ 2025-03-07 11:06 ` Abdiel Janulgue
2025-03-07 11:06 ` [PATCH v13 6/7] MAINTAINERS: add entry for Rust dma mapping helpers device driver API Abdiel Janulgue
` (2 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2025-03-07 11:06 UTC (permalink / raw)
To: rust-for-linux, daniel.almeida, dakr, robin.murphy, aliceryhl
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS,
Abdiel Janulgue
Add a simple driver to excercise the basics of the Rust DMA
coherent allocator bindings.
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@gmail.com>
---
samples/rust/Kconfig | 11 +++++
samples/rust/Makefile | 1 +
samples/rust/rust_dma.rs | 104 +++++++++++++++++++++++++++++++++++++++
3 files changed, 116 insertions(+)
create mode 100644 samples/rust/rust_dma.rs
diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index 3b6eae84b297..04bbba870bd1 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -78,4 +78,15 @@ config SAMPLE_RUST_HOSTPROGS
If unsure, say N.
+config SAMPLE_RUST_DRIVER_DMA
+ tristate "DMA test driver"
+ depends on PCI
+ help
+ This option builds the Rust dma test driver sample.
+
+ To compile this as a module, choose M here:
+ the module will be called dma.
+
+ If unsure, say N.
+
endif # SAMPLES_RUST
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 0dbc6d90f1ef..1a9aff6e8d6a 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_SAMPLE_RUST_PRINT) += rust_print.o
obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI) += rust_driver_pci.o
obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM) += rust_driver_platform.o
obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX) += rust_driver_faux.o
+obj-$(CONFIG_SAMPLE_RUST_DRIVER_DMA) += rust_dma.o
rust_print-y := rust_print_main.o rust_print_events.o
diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
new file mode 100644
index 000000000000..aed55ae4dcfe
--- /dev/null
+++ b/samples/rust/rust_dma.rs
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust DMA api test (based on QEMU's `pci-testdev`).
+//!
+//! To make this driver probe, QEMU must be run with `-device pci-testdev`.
+
+use kernel::{bindings, dma::CoherentAllocation, pci, prelude::*};
+
+struct DmaSampleDriver {
+ pdev: pci::Device,
+ ca: kernel::devres::Devres<CoherentAllocation<MyStruct>>,
+}
+
+const TEST_VALUES: [(u32, u32); 5] = [
+ (0xa, 0xb),
+ (0xc, 0xd),
+ (0xe, 0xf),
+ (0xab, 0xba),
+ (0xcd, 0xef),
+];
+
+struct MyStruct {
+ h: u32,
+ b: u32,
+}
+
+impl MyStruct {
+ fn new(h: u32, b: u32) -> Self {
+ Self { h, b }
+ }
+}
+// SAFETY: All bit patterns are acceptable values for `MyStruct`.
+unsafe impl kernel::transmute::AsBytes for MyStruct {}
+// SAFETY: Instances of `MyStruct` have no uninitialized portions.
+unsafe impl kernel::transmute::FromBytes for MyStruct {}
+
+kernel::pci_device_table!(
+ PCI_TABLE,
+ MODULE_PCI_TABLE,
+ <DmaSampleDriver as pci::Driver>::IdInfo,
+ [(
+ pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_REDHAT, 0x5),
+ ()
+ )]
+);
+
+impl pci::Driver for DmaSampleDriver {
+ type IdInfo = ();
+ const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
+
+ fn probe(pdev: &mut pci::Device, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
+ let dev = pdev.as_mut();
+
+ dev.dma_set_mask_and_coherent(kernel::dma::dma_bit_mask(64))?;
+
+ dev_info!(dev, "Probe DMA test driver.\n");
+
+ let ca: kernel::devres::Devres<CoherentAllocation<MyStruct>> =
+ CoherentAllocation::alloc_coherent(dev, TEST_VALUES.len(), GFP_KERNEL)?;
+
+ || -> Result {
+ let reg = ca.try_access().ok_or(ENXIO)?;
+
+ for (i, value) in TEST_VALUES.into_iter().enumerate() {
+ kernel::dma_write!(reg[i] = MyStruct::new(value.0, value.1));
+ }
+
+ Ok(())
+ }()?;
+
+ let drvdata = KBox::new(
+ Self {
+ pdev: pdev.clone(),
+ ca,
+ },
+ GFP_KERNEL,
+ )?;
+
+ Ok(drvdata.into())
+ }
+}
+
+impl Drop for DmaSampleDriver {
+ fn drop(&mut self) {
+ dev_info!(self.pdev.as_ref(), "Unload DMA test driver.\n");
+
+ let _ = || -> Result {
+ let reg = self.ca.try_access().ok_or(ENXIO)?;
+ for (i, value) in TEST_VALUES.into_iter().enumerate() {
+ assert_eq!(kernel::dma_read!(reg[i].h), value.0);
+ assert_eq!(kernel::dma_read!(reg[i].b), value.1);
+ }
+ Ok(())
+ }();
+ }
+}
+
+kernel::module_pci_driver! {
+ type: DmaSampleDriver,
+ name: "rust_dma",
+ author: "Abdiel Janulgue",
+ description: "Rust DMA test",
+ license: "GPL v2",
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v13 6/7] MAINTAINERS: add entry for Rust dma mapping helpers device driver API
2025-03-07 11:06 [PATCH v13 0/7] rust: add dma coherent allocator abstraction Abdiel Janulgue
` (4 preceding siblings ...)
2025-03-07 11:06 ` [PATCH v13 5/7] samples: rust: add Rust dma test sample driver Abdiel Janulgue
@ 2025-03-07 11:06 ` Abdiel Janulgue
2025-03-07 11:06 ` [PATCH v13 7/7] rust: dma: add as_slice/write functions for CoherentAllocation Abdiel Janulgue
2025-03-07 11:12 ` [PATCH v13 0/7] rust: add dma coherent allocator abstraction Danilo Krummrich
7 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2025-03-07 11:06 UTC (permalink / raw)
To: rust-for-linux, daniel.almeida, dakr, robin.murphy, aliceryhl
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS,
Abdiel Janulgue
Add an entry for the Rust dma mapping helpers abstractions.
Nacked-by: Christoph Hellwig <hch@lst.de>
Acked-by: Danilo Krummrich <dakr@kernel.org>
Acked-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@gmail.com>
---
MAINTAINERS | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 12f2d79ce174..72daf5db0242 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6902,6 +6902,18 @@ F: include/linux/dma-mapping.h
F: include/linux/swiotlb.h
F: kernel/dma/
+DMA MAPPING HELPERS DEVICE DRIVER API [RUST]
+M: Abdiel Janulgue <abdiel.janulgue@gmail.com>
+M: Danilo Krummrich <dakr@kernel.org>
+R: Daniel Almeida <daniel.almeida@collabora.com>
+R: Robin Murphy <robin.murphy@arm.com>
+R: Andreas Hindborg <a.hindborg@kernel.org>
+L: rust-for-linux@vger.kernel.org
+S: Supported
+W: https://rust-for-linux.com
+T: git https://github.com/Rust-for-Linux/linux.git rust-next
+F: rust/kernel/dma.rs
+
DMA-BUF HEAPS FRAMEWORK
M: Sumit Semwal <sumit.semwal@linaro.org>
R: Benjamin Gaignard <benjamin.gaignard@collabora.com>
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v13 7/7] rust: dma: add as_slice/write functions for CoherentAllocation
2025-03-07 11:06 [PATCH v13 0/7] rust: add dma coherent allocator abstraction Abdiel Janulgue
` (5 preceding siblings ...)
2025-03-07 11:06 ` [PATCH v13 6/7] MAINTAINERS: add entry for Rust dma mapping helpers device driver API Abdiel Janulgue
@ 2025-03-07 11:06 ` Abdiel Janulgue
2025-03-07 11:12 ` [PATCH v13 0/7] rust: add dma coherent allocator abstraction Danilo Krummrich
7 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2025-03-07 11:06 UTC (permalink / raw)
To: rust-for-linux, daniel.almeida, dakr, robin.murphy, aliceryhl
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS,
Abdiel Janulgue
Add unsafe accessors for the region for reading or writing large
blocks of data.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@gmail.com>
---
rust/kernel/dma.rs | 86 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 8a250242641c..cee7adc2fc22 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -213,6 +213,92 @@ pub fn dma_handle(&self) -> bindings::dma_addr_t {
self.dma_handle
}
+ /// Returns the data from the region starting from `offset` as a slice.
+ /// `offset` and `count` are in units of `T`, not the number of bytes.
+ ///
+ /// Due to the safety requirements of slice, the caller should consider that the region could
+ /// be modified by the device at anytime. For ringbuffer type of r/w access or use-cases where
+ /// the pointer to the live data is needed, `start_ptr()` or `start_ptr_mut()` could be
+ /// used instead.
+ ///
+ /// # Safety
+ ///
+ /// * Callers must ensure that no hardware operations that involve the buffer are currently
+ /// taking place while the returned slice is live.
+ /// * Callers must ensure that this call does not race with a write to the same region while
+ /// while the returned slice is live.
+ pub unsafe fn as_slice(&self, offset: usize, count: usize) -> Result<&[T]> {
+ let end = offset.checked_add(count).ok_or(EOVERFLOW)?;
+ if end >= self.count {
+ return Err(EINVAL);
+ }
+ // SAFETY:
+ // - The pointer is valid due to type invariant on `CoherentAllocation`,
+ // we've just checked that the range and index is within bounds. The immutability of the
+ // of data is also guaranteed by the safety requirements of the function.
+ // - `offset` can't overflow since it is smaller than `self.count` and we've checked
+ // that `self.count` won't overflow early in the constructor.
+ Ok(unsafe { core::slice::from_raw_parts(self.cpu_addr.add(offset), count) })
+ }
+
+ /// Performs the same functionality as [`as_slice`], except that a mutable slice is returned.
+ ///
+ /// # Safety
+ ///
+ /// * Callers must ensure that no hardware operations that involve the buffer are currently
+ /// taking place while the returned slice is live.
+ /// * Callers must ensure that this call does not race with a read or write to the same region
+ /// while the returned slice is live.
+ pub unsafe fn as_slice_mut(&self, offset: usize, count: usize) -> Result<&mut [T]> {
+ let end = offset.checked_add(count).ok_or(EOVERFLOW)?;
+ if end >= self.count {
+ return Err(EINVAL);
+ }
+ // SAFETY:
+ // - The pointer is valid due to type invariant on `CoherentAllocation`,
+ // we've just checked that the range and index is within bounds. The immutability of the
+ // of data is also guaranteed by the safety requirements of the function.
+ // - `offset` can't overflow since it is smaller than `self.count` and we've checked
+ // that `self.count` won't overflow early in the constructor.
+ Ok(unsafe { core::slice::from_raw_parts_mut(self.cpu_addr.add(offset), count) })
+ }
+
+ /// Writes data to the region starting from `offset`. `offset` is in units of `T`, not the
+ /// number of bytes.
+ ///
+ /// # Safety
+ ///
+ /// * Callers must ensure that no hardware operations that involve the buffer overlaps with
+ /// this write.
+ /// * Callers must ensure that this call does not race with a read or write to the same region
+ /// that overlaps with this write.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// # fn test(alloc: &mut kernel::dma::CoherentAllocation<u8>) -> Result {
+ /// let somedata: [u8; 4] = [0xf; 4];
+ /// let buf: &[u8] = &somedata;
+ /// // SAFETY: No hw operation on the device and no other r/w access to the region at this point.
+ /// unsafe { alloc.write(buf, 0)?; }
+ /// # Ok::<(), Error>(()) }
+ /// ```
+ pub unsafe fn write(&self, src: &[T], offset: usize) -> Result {
+ let end = offset.checked_add(src.len()).ok_or(EOVERFLOW)?;
+ if end >= self.count {
+ return Err(EINVAL);
+ }
+ // SAFETY:
+ // - The pointer is valid due to type invariant on `CoherentAllocation`
+ // and we've just checked that the range and index is within bounds.
+ // - `offset` can't overflow since it is smaller than `self.count` and we've checked
+ // that `self.count` won't overflow early in the constructor.
+ unsafe {
+ core::ptr::copy_nonoverlapping(src.as_ptr(), self.cpu_addr.add(offset), src.len())
+ };
+ Ok(())
+ }
+
/// Returns a pointer to an element from the region with bounds checking. `offset` is in
/// units of `T`, not the number of bytes.
///
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v13 0/7] rust: add dma coherent allocator abstraction
2025-03-07 11:06 [PATCH v13 0/7] rust: add dma coherent allocator abstraction Abdiel Janulgue
` (6 preceding siblings ...)
2025-03-07 11:06 ` [PATCH v13 7/7] rust: dma: add as_slice/write functions for CoherentAllocation Abdiel Janulgue
@ 2025-03-07 11:12 ` Danilo Krummrich
7 siblings, 0 replies; 23+ messages in thread
From: Danilo Krummrich @ 2025-03-07 11:12 UTC (permalink / raw)
To: Abdiel Janulgue
Cc: rust-for-linux, daniel.almeida, robin.murphy, aliceryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Valentin Obst, open list, Christoph Hellwig,
Marek Szyprowski, airlied, open list:DMA MAPPING HELPERS
On Fri, Mar 07, 2025 at 01:06:17PM +0200, Abdiel Janulgue wrote:
> This series adds the rust bindings for the dma coherent allocator which
> is needed for nova-core[0]. For v13, the biggest change is to move out
> the as_slice and write helpers for later discussion. The object is also
> now wrapped in Devres to explicitly tie it to the lifetime of the
> device[1].
Just to clarify:
What is meant here is that a CoherentAllocation can't out-live driver unbind.
This is not equivalent to the lifetime of the corresponding struct device.
^ permalink raw reply [flat|nested] 23+ messages in thread