rust: dma: drop dma_read! and dma_write! API

The primitive read/write use case is covered by the `io_read!` and
`io_write!` macro. The non-primitive use case was finicky; they should
either be achieved using `CoherentBox` or `as_ref()/as_mut()` to assert the
lack of concurrent access, or should be using memcpy-like APIs to express
the non-atomic and tearable nature.

Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Link: https://patch.msgid.link/20260706-io_projection-v6-18-72cd5d055d54@garyguo.net
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
This commit is contained in:
Gary Guo 2026-07-06 13:44:31 +01:00 committed by Danilo Krummrich
parent 0722567f50
commit 6ff7d69b7e
2 changed files with 8 additions and 131 deletions

View File

@ -661,52 +661,6 @@ pub unsafe fn as_mut(&self) -> &mut T {
// SAFETY: per safety requirement.
unsafe { &mut *self.as_mut_ptr() }
}
/// 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.
// - Using read_volatile() here is not sound as per the usual rules, the usage here is
// a special exception with the following notes in place. When dealing with a potential
// race from a hardware or code outside kernel (e.g. user-space program), we need that
// read on a valid memory is not UB. Currently read_volatile() is used for this, and the
// rationale behind is that it should generate the same code as READ_ONCE() which the
// kernel already relies on to avoid UB on data races. Note that the usage of
// read_volatile() is limited to this particular case, it cannot be used to prevent
// the UB caused by racing between two kernel functions nor do they provide atomicity.
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.
// - Using write_volatile() here is not sound as per the usual rules, the usage here is
// a special exception with the following notes in place. When dealing with a potential
// race from a hardware or code outside kernel (e.g. user-space program), we need that
// write on a valid memory is not UB. Currently write_volatile() is used for this, and the
// rationale behind is that it should generate the same code as WRITE_ONCE() which the
// kernel already relies on to avoid UB on data races. Note that the usage of
// write_volatile() is limited to this particular case, it cannot be used to prevent
// the UB caused by racing between two kernel functions nor do they provide atomicity.
unsafe { field.write_volatile(val) }
}
}
impl<T: AsBytes + FromBytes> Coherent<T> {
@ -1266,85 +1220,3 @@ fn as_view(self) -> CoherentView<'a, Self::Target> {
}
}
}
/// Reads a field of an item from an allocated region of structs.
///
/// The syntax is of the form `kernel::dma_read!(dma, proj)` where `dma` is an expression evaluating
/// to a [`Coherent`] and `proj` is a [projection specification](kernel::ptr::project!).
///
/// # Examples
///
/// ```
/// use kernel::device::Device;
/// use kernel::dma::{attrs::*, Coherent};
///
/// 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(alloc: &kernel::dma::Coherent<[MyStruct]>) -> Result {
/// let whole = kernel::dma_read!(alloc, [try: 2]);
/// let field = kernel::dma_read!(alloc, [panic: 1].field);
/// # Ok::<(), Error>(()) }
/// ```
#[macro_export]
macro_rules! dma_read {
($dma:expr, $($proj:tt)*) => {{
let dma = &$dma;
let ptr = $crate::ptr::project!(
$crate::dma::Coherent::as_ptr(dma), $($proj)*
);
// SAFETY: The pointer created by the projection is within the DMA region.
unsafe { $crate::dma::Coherent::field_read(dma, ptr) }
}};
}
/// Writes to a field of an item from an allocated region of structs.
///
/// The syntax is of the form `kernel::dma_write!(dma, proj, val)` where `dma` is an expression
/// evaluating to a [`Coherent`], `proj` is a
/// [projection specification](kernel::ptr::project!), and `val` is the value to be written to the
/// projected location.
///
/// # Examples
///
/// ```
/// use kernel::device::Device;
/// use kernel::dma::{attrs::*, Coherent};
///
/// 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(alloc: &kernel::dma::Coherent<[MyStruct]>) -> Result {
/// kernel::dma_write!(alloc, [try: 2].member, 0xf);
/// kernel::dma_write!(alloc, [panic: 1], MyStruct { member: 0xf });
/// # Ok::<(), Error>(()) }
/// ```
#[macro_export]
macro_rules! dma_write {
(@parse [$dma:expr] [$($proj:tt)*] [, $val:expr]) => {{
let dma = &$dma;
let ptr = $crate::ptr::project!(
mut $crate::dma::Coherent::as_mut_ptr(dma), $($proj)*
);
let val = $val;
// SAFETY: The pointer created by the projection is within the DMA region.
unsafe { $crate::dma::Coherent::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)*] [[$flavor:ident: $index:expr] $($rest:tt)*]) => {
$crate::dma_write!(@parse [$dma] [$($proj)* [$flavor: $index]] [$($rest)*])
};
($dma:expr, $($rest:tt)*) => {
$crate::dma_write!(@parse [$dma] [] [$($rest)*])
};
}

View File

@ -12,6 +12,10 @@
Device,
DmaMask, //
},
io::{
io_project,
io_read, //
},
page, pci,
prelude::*,
scatterlist::{Owned, SGTable},
@ -77,7 +81,8 @@ fn probe<'bound>(
Coherent::zeroed_slice(pdev.as_ref(), TEST_VALUES.len(), GFP_KERNEL)?;
for (i, value) in TEST_VALUES.into_iter().enumerate() {
kernel::dma_write!(ca, [try: i], MyStruct::new(value.0, value.1));
// SAFETY: `ca` is not yet shared with device or other threads.
unsafe { *io_project!(ca, [panic: i]).as_mut() = MyStruct::new(value.0, value.1) };
}
let size = 4 * page::PAGE_SIZE;
@ -97,8 +102,8 @@ fn probe<'bound>(
impl DmaSampleDriver {
fn check_dma(&self) {
for (i, value) in TEST_VALUES.into_iter().enumerate() {
let val0 = kernel::dma_read!(self.ca, [panic: i].h);
let val1 = kernel::dma_read!(self.ca, [panic: i].b);
let val0 = io_read!(self.ca, [panic: i].h);
let val1 = io_read!(self.ca, [panic: i].b);
assert_eq!(val0, value.0);
assert_eq!(val1, value.1);