* Re: Patch "rust: dma: use pointer projection infra for `dma_{read,write}` macro" has been added to the 6.19-stable tree
[not found] <20260315144041.25312-1-sashal@kernel.org>
@ 2026-03-15 14:48 ` Gary Guo
2026-03-16 14:00 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: Gary Guo @ 2026-03-15 14:48 UTC (permalink / raw)
To: stable, stable-commits, gary
Cc: Danilo Krummrich, Alice Ryhl, Alexandre Courbot, David Airlie,
Simona Vetter, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
Benno Lossin, Trevor Gross
On Sun Mar 15, 2026 at 2:40 PM GMT, Sasha Levin wrote:
> This is a note to let you know that I've just added the patch titled
>
> rust: dma: use pointer projection infra for `dma_{read,write}` macro
>
> to the 6.19-stable tree which can be found at:
> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
> rust-dma-use-pointer-projection-infra-for-dma_-read-.patch
> and it can be found in the queue-6.19 subdirectory.
Hi Sasha,
commit 08da98f18f4f ("rust: ptr: add `KnownSize` trait to support DST size info
extraction") and commit f41941aab3ac ("rust: ptr: add projection
infrastructure") are dependencies of this fix.
It doesn't look like these commits are currently being picked. They're needed
for building.
They're part of the same series: https://lore.kernel.org/rust-for-linux/20260302164239.284084-1-gary@kernel.org/
Best,
Gary
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.
>
>
>
> commit 26dd613c37a79884da7fd824263d4c93123688c3
> Author: Gary Guo <gary@garyguo.net>
> Date: Mon Mar 2 16:42:36 2026 +0000
>
> rust: dma: use pointer projection infra for `dma_{read,write}` macro
>
> [ Upstream commit 4da879a0d3fd170a70994b73baa554c6913918b5 ]
>
> 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.
> It also has a soundness issue where it does not protect against unaligned
> fields (when `#[repr(packed)]` is used) so it can generate misaligned
> accesses.
>
> This commit migrates them to use the general pointer projection
> infrastructure, which handles these cases correctly.
>
> As part of migration, the macro is updated to have an improved surface
> syntax. 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.
>
> 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.
>
> Fixes: ad2907b4e308 ("rust: add dma coherent allocator abstraction")
> Reviewed-by: Benno Lossin <lossin@kernel.org>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> Link: https://patch.msgid.link/20260302164239.284084-4-gary@kernel.org
> [ Capitalize safety comments; slightly improve wording in doc-comments.
> - Danilo ]
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
>
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index 174feaca0a6b9..25cd48514c777 100644
> --- a/drivers/gpu/nova-core/gsp.rs
> +++ b/drivers/gpu/nova-core/gsp.rs
> @@ -143,14 +143,14 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
> // _kgspInitLibosLoggingStructures (allocates memory for buffers)
> // kgspSetupLibosInitArgs_IMPL (creates pLibosInitArgs[] array)
> dma_write!(
> - libos[0] = LibosMemoryRegionInitArgument::new("LOGINIT", &loginit.0)
> - )?;
> + libos, [0]?, LibosMemoryRegionInitArgument::new("LOGINIT", &loginit.0)
> + );
> dma_write!(
> - libos[1] = LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0)
> - )?;
> - dma_write!(libos[2] = LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0))?;
> - dma_write!(rmargs[0].inner = fw::GspArgumentsCached::new(cmdq))?;
> - dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", rmargs))?;
> + libos, [1]?, LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0)
> + );
> + dma_write!(libos, [2]?, LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0));
> + dma_write!(rmargs, [0]?.inner, fw::GspArgumentsCached::new(cmdq));
> + dma_write!(libos, [3]?, LibosMemoryRegionInitArgument::new("RMARGS", rmargs));
> },
> }))
> })
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> index 54937606b5b0a..38b4682ff01be 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -160,7 +160,7 @@ pub(crate) fn boot(
>
> let wpr_meta =
> CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
> - dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
> + dma_write!(wpr_meta, [0]?, GspFwWprMeta::new(&gsp_fw, &fb_layout));
>
> self.cmdq
> .send_command(bar, commands::SetSystemInfo::new(pdev))?;
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 3991ccc0c10f1..1cdd1ccfe5702 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -201,9 +201,13 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
>
> let gsp_mem =
> CoherentAllocation::<GspMem>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
> - dma_write!(gsp_mem[0].ptes = PteArray::new(gsp_mem.dma_handle())?)?;
> - dma_write!(gsp_mem[0].cpuq.tx = MsgqTxHeader::new(MSGQ_SIZE, RX_HDR_OFF, MSGQ_NUM_PAGES))?;
> - dma_write!(gsp_mem[0].cpuq.rx = MsgqRxHeader::new())?;
> + dma_write!(gsp_mem, [0]?.ptes, PteArray::new(gsp_mem.dma_handle())?);
> + dma_write!(
> + gsp_mem,
> + [0]?.cpuq.tx,
> + MsgqTxHeader::new(MSGQ_SIZE, RX_HDR_OFF, MSGQ_NUM_PAGES)
> + );
> + dma_write!(gsp_mem, [0]?.cpuq.rx, MsgqRxHeader::new());
>
> Ok(Self(gsp_mem))
> }
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index acc65b1e0f245..37e125bb423ad 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -444,6 +444,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()
> @@ -564,23 +577,6 @@ pub unsafe fn write(&mut self, src: &[T], offset: usize) -> Result {
> Ok(())
> }
>
> - /// 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.as_ptr().add(offset) })
> - }
> -
> /// Reads the value of `field` and ensures that its type is [`FromBytes`].
> ///
> /// # Safety
> @@ -653,6 +649,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 the form `kernel::dma_read!(dma, proj)` where `dma` is an expression evaluating
> +/// to a [`CoherentAllocation`] and `proj` is a [projection specification](kernel::ptr::project!).
> +///
> /// # Examples
> ///
> /// ```
> @@ -667,36 +666,29 @@ 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)*) => {{
> - (|| -> ::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:expr, $($proj:tt)*) => {{
> + let dma = &$dma;
> + let ptr = $crate::ptr::project!(
> + $crate::dma::CoherentAllocation::as_ptr(dma), $($proj)*
> + );
> + // SAFETY: The pointer created by the projection is within the DMA region.
> + unsafe { $crate::dma::CoherentAllocation::field_read(dma, ptr) }
> }};
> - ($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.
> ///
> +/// The syntax is of the form `kernel::dma_write!(dma, proj, val)` where `dma` is an expression
> +/// evaluating to a [`CoherentAllocation`], `proj` is a
> +/// [projection specification](kernel::ptr::project!), and `val` is the value to be written to the
> +/// projected location.
> +///
> /// # Examples
> ///
> /// ```
> @@ -711,37 +703,31 @@ 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)*)
> + (@parse [$dma:expr] [$($proj:tt)*] [, $val:expr]) => {{
> + let dma = &$dma;
> + let ptr = $crate::ptr::project!(
> + mut $crate::dma::CoherentAllocation::as_mut_ptr(dma), $($proj)*
> + );
> + let val = $val;
> + // SAFETY: The pointer created by the projection is within the DMA region.
> + unsafe { $crate::dma::CoherentAllocation::field_write(dma, ptr, val) }
> }};
> - ($dma:expr, $idx: expr, = $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) }
> - ::core::result::Result::Ok(())
> - })()
> + (@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, $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(())
> - })()
> + ($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 f53bce2a73e3b..bcba1a6e6aaf4 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;
> @@ -85,6 +85,20 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
> }
> }
>
> +impl DmaSampleDriver {
> + fn check_dma(&self) -> Result {
> + 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_eq!(val0, value.0);
> + assert_eq!(val1, value.1);
> + }
> +
> + Ok(())
> + }
> +}
> +
> #[pinned_drop]
> impl PinnedDrop for DmaSampleDriver {
> fn drop(self: Pin<&mut Self>) {
> @@ -92,19 +106,7 @@ fn drop(self: Pin<&mut Self>) {
>
> dev_info!(dev, "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());
> -
> - if let Ok(val0) = val0 {
> - assert_eq!(val0, value.0);
> - }
> - if let Ok(val1) = val1 {
> - assert_eq!(val1, value.1);
> - }
> - }
> + assert!(self.check_dma().is_ok());
>
> for (i, entry) in self.sgt.iter().enumerate() {
> dev_info!(dev, "Entry[{}]: DMA address: {:#x}", i, entry.dma_address());
^ permalink raw reply [flat|nested] 4+ messages in thread