* [PATCH 2/4] rust: dma: generalize `dma_{read,write}` macro
[not found] <20260214053344.1994776-1-gary@garyguo.net>
@ 2026-02-14 5:33 ` Gary Guo
2026-02-14 10:04 ` Benno Lossin
2026-02-14 5:33 ` [PATCH 4/4] rust: dma: remove old dma_{read,write} macro compatibility syntax Gary Guo
1 sibling, 1 reply; 6+ messages in thread
From: Gary Guo @ 2026-02-14 5:33 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy
Cc: Alexandre Courbot, rust-for-linux, driver-core, linux-kernel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] rust: dma: remove old dma_{read,write} macro compatibility syntax
[not found] <20260214053344.1994776-1-gary@garyguo.net>
2026-02-14 5:33 ` [PATCH 2/4] rust: dma: generalize `dma_{read,write}` macro Gary Guo
@ 2026-02-14 5:33 ` Gary Guo
2026-02-14 10:05 ` Benno Lossin
1 sibling, 1 reply; 6+ messages in thread
From: Gary Guo @ 2026-02-14 5:33 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy
Cc: Alexandre Courbot, rust-for-linux, driver-core, linux-kernel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/4] rust: dma: generalize `dma_{read,write}` macro
2026-02-14 5:33 ` [PATCH 2/4] rust: dma: generalize `dma_{read,write}` macro Gary Guo
@ 2026-02-14 10:04 ` Benno Lossin
2026-02-14 10:46 ` Gary Guo
0 siblings, 1 reply; 6+ messages in thread
From: Benno Lossin @ 2026-02-14 10:04 UTC (permalink / raw)
To: Gary Guo, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Abdiel Janulgue, Daniel Almeida, Robin Murphy
Cc: Alexandre Courbot, rust-for-linux, driver-core, linux-kernel
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() {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 4/4] rust: dma: remove old dma_{read,write} macro compatibility syntax
2026-02-14 5:33 ` [PATCH 4/4] rust: dma: remove old dma_{read,write} macro compatibility syntax Gary Guo
@ 2026-02-14 10:05 ` Benno Lossin
0 siblings, 0 replies; 6+ messages in thread
From: Benno Lossin @ 2026-02-14 10:05 UTC (permalink / raw)
To: Gary Guo, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Abdiel Janulgue, Daniel Almeida, Robin Murphy
Cc: Alexandre Courbot, rust-for-linux, driver-core, linux-kernel
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(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/4] rust: dma: generalize `dma_{read,write}` macro
2026-02-14 10:04 ` Benno Lossin
@ 2026-02-14 10:46 ` Gary Guo
2026-02-14 14:53 ` Benno Lossin
0 siblings, 1 reply; 6+ messages in thread
From: Gary Guo @ 2026-02-14 10:46 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Boqun Feng, Björn Roy Baron, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Abdiel Janulgue,
Daniel Almeida, Robin Murphy, Alexandre Courbot, rust-for-linux,
driver-core, linux-kernel
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() {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/4] rust: dma: generalize `dma_{read,write}` macro
2026-02-14 10:46 ` Gary Guo
@ 2026-02-14 14:53 ` Benno Lossin
0 siblings, 0 replies; 6+ messages in thread
From: Benno Lossin @ 2026-02-14 14:53 UTC (permalink / raw)
To: Gary Guo
Cc: Miguel Ojeda, Boqun Feng, Björn Roy Baron, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Abdiel Janulgue,
Daniel Almeida, Robin Murphy, Alexandre Courbot, rust-for-linux,
driver-core, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-02-14 14:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260214053344.1994776-1-gary@garyguo.net>
2026-02-14 5:33 ` [PATCH 2/4] rust: dma: generalize `dma_{read,write}` macro Gary Guo
2026-02-14 10:04 ` Benno Lossin
2026-02-14 10:46 ` Gary Guo
2026-02-14 14:53 ` Benno Lossin
2026-02-14 5:33 ` [PATCH 4/4] rust: dma: remove old dma_{read,write} macro compatibility syntax Gary Guo
2026-02-14 10:05 ` Benno Lossin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox