* [PATCH 1/7] rust: dma: add from-slice constructors for Coherent and CoherentBox
2026-03-21 13:36 [PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core Alexandre Courbot
@ 2026-03-21 13:36 ` Alexandre Courbot
2026-03-23 16:55 ` Gary Guo
2026-03-24 14:29 ` Andreas Hindborg
2026-03-21 13:36 ` [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent Alexandre Courbot
` (6 subsequent siblings)
7 siblings, 2 replies; 22+ messages in thread
From: Alexandre Courbot @ 2026-03-21 13:36 UTC (permalink / raw)
To: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel, Alexandre Courbot
A very common pattern is to create a block of coherent memory with the
content of an already-existing slice of bytes (e.g. a loaded firmware
blob).
`CoherentBox` makes this easier, but still implies a potentially
panicking operation with `copy_from_slice` that requires a `PANIC`
comment.
Add `from_slice_with_attrs` and `from_slice` methods to both `Coherent`
and `CoherentBox` to turn this into a trivial one-step operation.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/dma.rs | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 102 insertions(+)
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 6d2bec52806b..a5cc993c919e 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -453,6 +453,62 @@ pub fn init_at<E>(&mut self, i: usize, init: impl Init<T, E>) -> Result
Ok(())
}
+
+ /// Allocates a region of coherent memory of the same size as `data` and initializes it with a
+ /// copy of its contents.
+ ///
+ /// This is the [`CoherentBox`] variant of [`Coherent::from_slice_with_attrs`].
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use core::ops::Deref;
+ ///
+ /// # use kernel::device::{Bound, Device};
+ /// use kernel::dma::{
+ /// attrs::*,
+ /// CoherentBox
+ /// };
+ ///
+ /// # fn test(dev: &Device<Bound>) -> Result {
+ /// let data = [0u8, 1u8, 2u8, 3u8];
+ /// let c: CoherentBox<[u8]> =
+ /// CoherentBox::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
+ ///
+ /// assert_eq!(c.deref(), &data);
+ /// # Ok::<(), Error>(()) }
+ /// ```
+ pub fn from_slice_with_attrs(
+ dev: &device::Device<Bound>,
+ data: &[T],
+ gfp_flags: kernel::alloc::Flags,
+ dma_attrs: Attrs,
+ ) -> Result<Self>
+ where
+ T: Copy,
+ {
+ Coherent::<T>::alloc_slice_with_attrs(dev, data.len(), gfp_flags, dma_attrs)
+ .map(Self)
+ .map(|mut slice| {
+ // PANIC: `slice` was created with length `data.len()`.
+ slice.copy_from_slice(data);
+ slice
+ })
+ }
+
+ /// Performs the same functionality as [`CoherentBox::from_slice_with_attrs`], except the
+ /// `dma_attrs` is 0 by default.
+ #[inline]
+ pub fn from_slice(
+ dev: &device::Device<Bound>,
+ data: &[T],
+ gfp_flags: kernel::alloc::Flags,
+ ) -> Result<Self>
+ where
+ T: Copy,
+ {
+ Self::from_slice_with_attrs(dev, data, gfp_flags, Attrs(0))
+ }
}
impl<T: AsBytes + FromBytes> CoherentBox<T> {
@@ -827,6 +883,52 @@ pub fn zeroed_slice(
) -> Result<Coherent<[T]>> {
Self::zeroed_slice_with_attrs(dev, len, gfp_flags, Attrs(0))
}
+
+ /// Allocates a region of coherent memory of the same size as `data` and initializes it with a
+ /// copy of its contents.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// # use kernel::device::{Bound, Device};
+ /// use kernel::dma::{
+ /// attrs::*,
+ /// Coherent
+ /// };
+ ///
+ /// # fn test(dev: &Device<Bound>) -> Result {
+ /// let data = [0u8, 1u8, 2u8, 3u8];
+ /// // `c` has the same content as `data`.
+ /// let c: Coherent<[u8]> =
+ /// Coherent::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
+ ///
+ /// # Ok::<(), Error>(()) }
+ /// ```
+ pub fn from_slice_with_attrs(
+ dev: &device::Device<Bound>,
+ data: &[T],
+ gfp_flags: kernel::alloc::Flags,
+ dma_attrs: Attrs,
+ ) -> Result<Coherent<[T]>>
+ where
+ T: Copy,
+ {
+ CoherentBox::from_slice_with_attrs(dev, data, gfp_flags, dma_attrs).map(Into::into)
+ }
+
+ /// Performs the same functionality as [`Coherent::from_slice_with_attrs`], except the
+ /// `dma_attrs` is 0 by default.
+ #[inline]
+ pub fn from_slice(
+ dev: &device::Device<Bound>,
+ data: &[T],
+ gfp_flags: kernel::alloc::Flags,
+ ) -> Result<Coherent<[T]>>
+ where
+ T: Copy,
+ {
+ Self::from_slice_with_attrs(dev, data, gfp_flags, Attrs(0))
+ }
}
impl<T> Coherent<[T]> {
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 1/7] rust: dma: add from-slice constructors for Coherent and CoherentBox
2026-03-21 13:36 ` [PATCH 1/7] rust: dma: add from-slice constructors for Coherent and CoherentBox Alexandre Courbot
@ 2026-03-23 16:55 ` Gary Guo
2026-03-26 14:59 ` Alexandre Courbot
2026-03-24 14:29 ` Andreas Hindborg
1 sibling, 1 reply; 22+ messages in thread
From: Gary Guo @ 2026-03-23 16:55 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Abdiel Janulgue,
Daniel Almeida, Robin Murphy, Andreas Hindborg, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel
On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
> A very common pattern is to create a block of coherent memory with the
> content of an already-existing slice of bytes (e.g. a loaded firmware
> blob).
>
> `CoherentBox` makes this easier, but still implies a potentially
> panicking operation with `copy_from_slice` that requires a `PANIC`
> comment.
>
> Add `from_slice_with_attrs` and `from_slice` methods to both `Coherent`
> and `CoherentBox` to turn this into a trivial one-step operation.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> rust/kernel/dma.rs | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 102 insertions(+)
>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 6d2bec52806b..a5cc993c919e 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -453,6 +453,62 @@ pub fn init_at<E>(&mut self, i: usize, init: impl Init<T, E>) -> Result
>
> Ok(())
> }
> +
> + /// Allocates a region of coherent memory of the same size as `data` and initializes it with a
> + /// copy of its contents.
> + ///
> + /// This is the [`CoherentBox`] variant of [`Coherent::from_slice_with_attrs`].
> + ///
> + /// # Examples
> + ///
> + /// ```
> + /// use core::ops::Deref;
> + ///
> + /// # use kernel::device::{Bound, Device};
> + /// use kernel::dma::{
> + /// attrs::*,
> + /// CoherentBox
> + /// };
> + ///
> + /// # fn test(dev: &Device<Bound>) -> Result {
> + /// let data = [0u8, 1u8, 2u8, 3u8];
> + /// let c: CoherentBox<[u8]> =
> + /// CoherentBox::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
> + ///
> + /// assert_eq!(c.deref(), &data);
> + /// # Ok::<(), Error>(()) }
> + /// ```
> + pub fn from_slice_with_attrs(
> + dev: &device::Device<Bound>,
> + data: &[T],
> + gfp_flags: kernel::alloc::Flags,
> + dma_attrs: Attrs,
> + ) -> Result<Self>
> + where
> + T: Copy,
> + {
> + Coherent::<T>::alloc_slice_with_attrs(dev, data.len(), gfp_flags, dma_attrs)
> + .map(Self)
I'd rather just use `?` and not use map.
> + .map(|mut slice| {
> + // PANIC: `slice` was created with length `data.len()`.
> + slice.copy_from_slice(data);
> + slice
> + })
> + }
> +
> + /// Performs the same functionality as [`CoherentBox::from_slice_with_attrs`], except the
> + /// `dma_attrs` is 0 by default.
> + #[inline]
> + pub fn from_slice(
> + dev: &device::Device<Bound>,
> + data: &[T],
> + gfp_flags: kernel::alloc::Flags,
> + ) -> Result<Self>
> + where
> + T: Copy,
> + {
> + Self::from_slice_with_attrs(dev, data, gfp_flags, Attrs(0))
> + }
> }
>
> impl<T: AsBytes + FromBytes> CoherentBox<T> {
> @@ -827,6 +883,52 @@ pub fn zeroed_slice(
> ) -> Result<Coherent<[T]>> {
> Self::zeroed_slice_with_attrs(dev, len, gfp_flags, Attrs(0))
> }
> +
> + /// Allocates a region of coherent memory of the same size as `data` and initializes it with a
> + /// copy of its contents.
> + ///
> + /// # Examples
> + ///
> + /// ```
> + /// # use kernel::device::{Bound, Device};
> + /// use kernel::dma::{
> + /// attrs::*,
> + /// Coherent
> + /// };
> + ///
> + /// # fn test(dev: &Device<Bound>) -> Result {
> + /// let data = [0u8, 1u8, 2u8, 3u8];
> + /// // `c` has the same content as `data`.
> + /// let c: Coherent<[u8]> =
> + /// Coherent::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
> + ///
> + /// # Ok::<(), Error>(()) }
> + /// ```
> + pub fn from_slice_with_attrs(
> + dev: &device::Device<Bound>,
> + data: &[T],
> + gfp_flags: kernel::alloc::Flags,
> + dma_attrs: Attrs,
> + ) -> Result<Coherent<[T]>>
> + where
> + T: Copy,
> + {
> + CoherentBox::from_slice_with_attrs(dev, data, gfp_flags, dma_attrs).map(Into::into)
This function can be inline as it's just wrapping another.
Best,
Gary
> + }
> +
> + /// Performs the same functionality as [`Coherent::from_slice_with_attrs`], except the
> + /// `dma_attrs` is 0 by default.
> + #[inline]
> + pub fn from_slice(
> + dev: &device::Device<Bound>,
> + data: &[T],
> + gfp_flags: kernel::alloc::Flags,
> + ) -> Result<Coherent<[T]>>
> + where
> + T: Copy,
> + {
> + Self::from_slice_with_attrs(dev, data, gfp_flags, Attrs(0))
> + }
> }
>
> impl<T> Coherent<[T]> {
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/7] rust: dma: add from-slice constructors for Coherent and CoherentBox
2026-03-23 16:55 ` Gary Guo
@ 2026-03-26 14:59 ` Alexandre Courbot
2026-03-26 15:02 ` Danilo Krummrich
2026-03-27 10:39 ` Miguel Ojeda
0 siblings, 2 replies; 22+ messages in thread
From: Alexandre Courbot @ 2026-03-26 14:59 UTC (permalink / raw)
To: Gary Guo
Cc: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, David Airlie,
Simona Vetter, John Hubbard, Alistair Popple, Joel Fernandes,
Timur Tabi, Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel
On Tue Mar 24, 2026 at 1:55 AM JST, Gary Guo wrote:
> On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
>> A very common pattern is to create a block of coherent memory with the
>> content of an already-existing slice of bytes (e.g. a loaded firmware
>> blob).
>>
>> `CoherentBox` makes this easier, but still implies a potentially
>> panicking operation with `copy_from_slice` that requires a `PANIC`
>> comment.
>>
>> Add `from_slice_with_attrs` and `from_slice` methods to both `Coherent`
>> and `CoherentBox` to turn this into a trivial one-step operation.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>> rust/kernel/dma.rs | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 102 insertions(+)
>>
>> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
>> index 6d2bec52806b..a5cc993c919e 100644
>> --- a/rust/kernel/dma.rs
>> +++ b/rust/kernel/dma.rs
>> @@ -453,6 +453,62 @@ pub fn init_at<E>(&mut self, i: usize, init: impl Init<T, E>) -> Result
>>
>> Ok(())
>> }
>> +
>> + /// Allocates a region of coherent memory of the same size as `data` and initializes it with a
>> + /// copy of its contents.
>> + ///
>> + /// This is the [`CoherentBox`] variant of [`Coherent::from_slice_with_attrs`].
>> + ///
>> + /// # Examples
>> + ///
>> + /// ```
>> + /// use core::ops::Deref;
>> + ///
>> + /// # use kernel::device::{Bound, Device};
>> + /// use kernel::dma::{
>> + /// attrs::*,
>> + /// CoherentBox
>> + /// };
>> + ///
>> + /// # fn test(dev: &Device<Bound>) -> Result {
>> + /// let data = [0u8, 1u8, 2u8, 3u8];
>> + /// let c: CoherentBox<[u8]> =
>> + /// CoherentBox::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
>> + ///
>> + /// assert_eq!(c.deref(), &data);
>> + /// # Ok::<(), Error>(()) }
>> + /// ```
>> + pub fn from_slice_with_attrs(
>> + dev: &device::Device<Bound>,
>> + data: &[T],
>> + gfp_flags: kernel::alloc::Flags,
>> + dma_attrs: Attrs,
>> + ) -> Result<Self>
>> + where
>> + T: Copy,
>> + {
>> + Coherent::<T>::alloc_slice_with_attrs(dev, data.len(), gfp_flags, dma_attrs)
>> + .map(Self)
>
> I'd rather just use `?` and not use map.
Then it looks like this:
let mut slice = Self(Coherent::<T>::alloc_slice_with_attrs(
dev,
data.len(),
gfp_flags,
dma_attrs,
)?);
// PANIC: `slice` was created with length `data.len()`.
slice.copy_from_slice(data);
Ok(slice)
FWIW I find using `map` more elegant, but I've made the change for v2
nonetheless.
>
>> + .map(|mut slice| {
>> + // PANIC: `slice` was created with length `data.len()`.
>> + slice.copy_from_slice(data);
>> + slice
>> + })
>> + }
>> +
>> + /// Performs the same functionality as [`CoherentBox::from_slice_with_attrs`], except the
>> + /// `dma_attrs` is 0 by default.
>> + #[inline]
>> + pub fn from_slice(
>> + dev: &device::Device<Bound>,
>> + data: &[T],
>> + gfp_flags: kernel::alloc::Flags,
>> + ) -> Result<Self>
>> + where
>> + T: Copy,
>> + {
>> + Self::from_slice_with_attrs(dev, data, gfp_flags, Attrs(0))
>> + }
>> }
>>
>> impl<T: AsBytes + FromBytes> CoherentBox<T> {
>> @@ -827,6 +883,52 @@ pub fn zeroed_slice(
>> ) -> Result<Coherent<[T]>> {
>> Self::zeroed_slice_with_attrs(dev, len, gfp_flags, Attrs(0))
>> }
>> +
>> + /// Allocates a region of coherent memory of the same size as `data` and initializes it with a
>> + /// copy of its contents.
>> + ///
>> + /// # Examples
>> + ///
>> + /// ```
>> + /// # use kernel::device::{Bound, Device};
>> + /// use kernel::dma::{
>> + /// attrs::*,
>> + /// Coherent
>> + /// };
>> + ///
>> + /// # fn test(dev: &Device<Bound>) -> Result {
>> + /// let data = [0u8, 1u8, 2u8, 3u8];
>> + /// // `c` has the same content as `data`.
>> + /// let c: Coherent<[u8]> =
>> + /// Coherent::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
>> + ///
>> + /// # Ok::<(), Error>(()) }
>> + /// ```
>> + pub fn from_slice_with_attrs(
>> + dev: &device::Device<Bound>,
>> + data: &[T],
>> + gfp_flags: kernel::alloc::Flags,
>> + dma_attrs: Attrs,
>> + ) -> Result<Coherent<[T]>>
>> + where
>> + T: Copy,
>> + {
>> + CoherentBox::from_slice_with_attrs(dev, data, gfp_flags, dma_attrs).map(Into::into)
>
> This function can be inline as it's just wrapping another.
Indeed, thanks!
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/7] rust: dma: add from-slice constructors for Coherent and CoherentBox
2026-03-26 14:59 ` Alexandre Courbot
@ 2026-03-26 15:02 ` Danilo Krummrich
2026-03-27 10:39 ` Miguel Ojeda
1 sibling, 0 replies; 22+ messages in thread
From: Danilo Krummrich @ 2026-03-26 15:02 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Gary Guo, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, David Airlie,
Simona Vetter, John Hubbard, Alistair Popple, Joel Fernandes,
Timur Tabi, Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel
On 3/26/26 3:59 PM, Alexandre Courbot wrote:
> Then it looks like this:
>
> let mut slice = Self(Coherent::<T>::alloc_slice_with_attrs(
> dev,
> data.len(),
> gfp_flags,
> dma_attrs,
> )?);
>
> // PANIC: `slice` was created with length `data.len()`.
> slice.copy_from_slice(data);
>
> Ok(slice)
>
> FWIW I find using `map` more elegant, but I've made the change for v2
> nonetheless.
I also prefer map() in this case.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/7] rust: dma: add from-slice constructors for Coherent and CoherentBox
2026-03-26 14:59 ` Alexandre Courbot
2026-03-26 15:02 ` Danilo Krummrich
@ 2026-03-27 10:39 ` Miguel Ojeda
1 sibling, 0 replies; 22+ messages in thread
From: Miguel Ojeda @ 2026-03-27 10:39 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Gary Guo, Danilo Krummrich, Abdiel Janulgue, Daniel Almeida,
Robin Murphy, Andreas Hindborg, Miguel Ojeda, Boqun Feng,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, John Hubbard, Alistair Popple,
Joel Fernandes, Timur Tabi, Zhi Wang, Eliot Courtney, driver-core,
rust-for-linux, linux-kernel
On Thu, Mar 26, 2026 at 3:59 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> Then it looks like this:
>
> let mut slice = Self(Coherent::<T>::alloc_slice_with_attrs(
> dev,
> data.len(),
> gfp_flags,
> dma_attrs,
> )?);
>
> // PANIC: `slice` was created with length `data.len()`.
> slice.copy_from_slice(data);
>
> Ok(slice)
Hmm... I think I prefer this version because (conceptually) it has
less calls, closures and conditionals (i.e. 2 `map`s vs. 1 `?`). Of
course, it should compile to the same thing.
In general, I think removing wrapper types early is better than
carrying them around. It also seems closer spiritually to the early
return style of the kernel in the C side.
For this case, I think both are understandable anyway.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/7] rust: dma: add from-slice constructors for Coherent and CoherentBox
2026-03-21 13:36 ` [PATCH 1/7] rust: dma: add from-slice constructors for Coherent and CoherentBox Alexandre Courbot
2026-03-23 16:55 ` Gary Guo
@ 2026-03-24 14:29 ` Andreas Hindborg
1 sibling, 0 replies; 22+ messages in thread
From: Andreas Hindborg @ 2026-03-24 14:29 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Abdiel Janulgue,
Daniel Almeida, Robin Murphy, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel, Alexandre Courbot
"Alexandre Courbot" <acourbot@nvidia.com> writes:
> A very common pattern is to create a block of coherent memory with the
> content of an already-existing slice of bytes (e.g. a loaded firmware
> blob).
>
> `CoherentBox` makes this easier, but still implies a potentially
> panicking operation with `copy_from_slice` that requires a `PANIC`
> comment.
>
> Add `from_slice_with_attrs` and `from_slice` methods to both `Coherent`
> and `CoherentBox` to turn this into a trivial one-step operation.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
I like `?` more than `.map()` as well, but it's fine to me either way.
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent
2026-03-21 13:36 [PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core Alexandre Courbot
2026-03-21 13:36 ` [PATCH 1/7] rust: dma: add from-slice constructors for Coherent and CoherentBox Alexandre Courbot
@ 2026-03-21 13:36 ` Alexandre Courbot
2026-03-21 14:58 ` Gary Guo
2026-03-21 13:36 ` [PATCH 3/7] gpu: nova-core: firmware: fwsec: " Alexandre Courbot
` (5 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Alexandre Courbot @ 2026-03-21 13:36 UTC (permalink / raw)
To: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel, Alexandre Courbot
Replace the nova-core local `DmaObject` with a `Coherent` that can
fulfill the same role.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/firmware/riscv.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
index 14aad2f0ee8a..2afa7f36404e 100644
--- a/drivers/gpu/nova-core/firmware/riscv.rs
+++ b/drivers/gpu/nova-core/firmware/riscv.rs
@@ -5,13 +5,13 @@
use kernel::{
device,
+ dma::Coherent,
firmware::Firmware,
prelude::*,
transmute::FromBytes, //
};
use crate::{
- dma::DmaObject,
firmware::BinFirmware,
num::FromSafeCast, //
};
@@ -66,7 +66,7 @@ pub(crate) struct RiscvFirmware {
/// Application version.
pub(crate) app_version: u32,
/// Device-mapped firmware image.
- pub(crate) ucode: DmaObject,
+ pub(crate) ucode: Coherent<[u8]>,
}
impl RiscvFirmware {
@@ -81,7 +81,7 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<
let len = usize::from_safe_cast(bin_fw.hdr.data_size);
let end = start.checked_add(len).ok_or(EINVAL)?;
- DmaObject::from_data(dev, fw.data().get(start..end).ok_or(EINVAL)?)?
+ Coherent::from_slice(dev, fw.data().get(start..end).ok_or(EINVAL)?, GFP_KERNEL)?
};
Ok(Self {
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent
2026-03-21 13:36 ` [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent Alexandre Courbot
@ 2026-03-21 14:58 ` Gary Guo
2026-03-23 6:15 ` Alexandre Courbot
0 siblings, 1 reply; 22+ messages in thread
From: Gary Guo @ 2026-03-21 14:58 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Abdiel Janulgue,
Daniel Almeida, Robin Murphy, Andreas Hindborg, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel
On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
> Replace the nova-core local `DmaObject` with a `Coherent` that can
> fulfill the same role.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> drivers/gpu/nova-core/firmware/riscv.rs | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
> index 14aad2f0ee8a..2afa7f36404e 100644
> --- a/drivers/gpu/nova-core/firmware/riscv.rs
> +++ b/drivers/gpu/nova-core/firmware/riscv.rs
> @@ -5,13 +5,13 @@
>
> use kernel::{
> device,
> + dma::Coherent,
> firmware::Firmware,
> prelude::*,
> transmute::FromBytes, //
> };
>
> use crate::{
> - dma::DmaObject,
> firmware::BinFirmware,
> num::FromSafeCast, //
> };
> @@ -66,7 +66,7 @@ pub(crate) struct RiscvFirmware {
> /// Application version.
> pub(crate) app_version: u32,
> /// Device-mapped firmware image.
> - pub(crate) ucode: DmaObject,
> + pub(crate) ucode: Coherent<[u8]>,
> }
>
> impl RiscvFirmware {
> @@ -81,7 +81,7 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<
> let len = usize::from_safe_cast(bin_fw.hdr.data_size);
> let end = start.checked_add(len).ok_or(EINVAL)?;
>
> - DmaObject::from_data(dev, fw.data().get(start..end).ok_or(EINVAL)?)?
> + Coherent::from_slice(dev, fw.data().get(start..end).ok_or(EINVAL)?, GFP_KERNEL)?
`DmaObject` rounds the data up to be page-sized, while this new API doesn't.
It has impact on alignment, as the allocator aligns things to the largest
power-of-two exponent of the allocated size.
Best,
Gary
> };
>
> Ok(Self {
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent
2026-03-21 14:58 ` Gary Guo
@ 2026-03-23 6:15 ` Alexandre Courbot
2026-03-23 13:05 ` Gary Guo
0 siblings, 1 reply; 22+ messages in thread
From: Alexandre Courbot @ 2026-03-23 6:15 UTC (permalink / raw)
To: Gary Guo
Cc: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, David Airlie,
Simona Vetter, John Hubbard, Alistair Popple, Joel Fernandes,
Timur Tabi, Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel
On Sat Mar 21, 2026 at 11:58 PM JST, Gary Guo wrote:
> On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
>> Replace the nova-core local `DmaObject` with a `Coherent` that can
>> fulfill the same role.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>> drivers/gpu/nova-core/firmware/riscv.rs | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
>> index 14aad2f0ee8a..2afa7f36404e 100644
>> --- a/drivers/gpu/nova-core/firmware/riscv.rs
>> +++ b/drivers/gpu/nova-core/firmware/riscv.rs
>> @@ -5,13 +5,13 @@
>>
>> use kernel::{
>> device,
>> + dma::Coherent,
>> firmware::Firmware,
>> prelude::*,
>> transmute::FromBytes, //
>> };
>>
>> use crate::{
>> - dma::DmaObject,
>> firmware::BinFirmware,
>> num::FromSafeCast, //
>> };
>> @@ -66,7 +66,7 @@ pub(crate) struct RiscvFirmware {
>> /// Application version.
>> pub(crate) app_version: u32,
>> /// Device-mapped firmware image.
>> - pub(crate) ucode: DmaObject,
>> + pub(crate) ucode: Coherent<[u8]>,
>> }
>>
>> impl RiscvFirmware {
>> @@ -81,7 +81,7 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<
>> let len = usize::from_safe_cast(bin_fw.hdr.data_size);
>> let end = start.checked_add(len).ok_or(EINVAL)?;
>>
>> - DmaObject::from_data(dev, fw.data().get(start..end).ok_or(EINVAL)?)?
>> + Coherent::from_slice(dev, fw.data().get(start..end).ok_or(EINVAL)?, GFP_KERNEL)?
>
> `DmaObject` rounds the data up to be page-sized, while this new API doesn't.
>
> It has impact on alignment, as the allocator aligns things to the largest
> power-of-two exponent of the allocated size.
Doesn't `dma_alloc_coherent` always allocate from the page pool and thus
returns page-aligned memory though? I'm not sure why `DmaObject` was
doing that but it was redundant I think.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent
2026-03-23 6:15 ` Alexandre Courbot
@ 2026-03-23 13:05 ` Gary Guo
2026-03-23 14:33 ` Alexandre Courbot
0 siblings, 1 reply; 22+ messages in thread
From: Gary Guo @ 2026-03-23 13:05 UTC (permalink / raw)
To: Alexandre Courbot, Gary Guo
Cc: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, David Airlie,
Simona Vetter, John Hubbard, Alistair Popple, Joel Fernandes,
Timur Tabi, Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel
On Mon Mar 23, 2026 at 6:15 AM GMT, Alexandre Courbot wrote:
> On Sat Mar 21, 2026 at 11:58 PM JST, Gary Guo wrote:
>> On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
>>> Replace the nova-core local `DmaObject` with a `Coherent` that can
>>> fulfill the same role.
>>>
>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>> ---
>>> drivers/gpu/nova-core/firmware/riscv.rs | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
>>> index 14aad2f0ee8a..2afa7f36404e 100644
>>> --- a/drivers/gpu/nova-core/firmware/riscv.rs
>>> +++ b/drivers/gpu/nova-core/firmware/riscv.rs
>>> @@ -5,13 +5,13 @@
>>>
>>> use kernel::{
>>> device,
>>> + dma::Coherent,
>>> firmware::Firmware,
>>> prelude::*,
>>> transmute::FromBytes, //
>>> };
>>>
>>> use crate::{
>>> - dma::DmaObject,
>>> firmware::BinFirmware,
>>> num::FromSafeCast, //
>>> };
>>> @@ -66,7 +66,7 @@ pub(crate) struct RiscvFirmware {
>>> /// Application version.
>>> pub(crate) app_version: u32,
>>> /// Device-mapped firmware image.
>>> - pub(crate) ucode: DmaObject,
>>> + pub(crate) ucode: Coherent<[u8]>,
>>> }
>>>
>>> impl RiscvFirmware {
>>> @@ -81,7 +81,7 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<
>>> let len = usize::from_safe_cast(bin_fw.hdr.data_size);
>>> let end = start.checked_add(len).ok_or(EINVAL)?;
>>>
>>> - DmaObject::from_data(dev, fw.data().get(start..end).ok_or(EINVAL)?)?
>>> + Coherent::from_slice(dev, fw.data().get(start..end).ok_or(EINVAL)?, GFP_KERNEL)?
>>
>> `DmaObject` rounds the data up to be page-sized, while this new API doesn't.
>>
>> It has impact on alignment, as the allocator aligns things to the largest
>> power-of-two exponent of the allocated size.
>
> Doesn't `dma_alloc_coherent` always allocate from the page pool and thus
> returns page-aligned memory though?
Oh you're right, this is not from DMA pool, so allocations are page-aligned
indeed.
I brought this up because I got bite by this size adjustment behaviour
because in the `from_data` I initially put
unsafe { dma_obj.as_mut().copy_from_slice(data) };
but that doesn't work due to the size being aligned up so dma_obj.len() !=
data.len().
But if this behaviour is not needed it does simplify things quite a bit.
> I'm not sure why `DmaObject` was doing that but it was redundant I think.
A question for yourself I guess? :-)
https://lore.kernel.org/all/20250619-nova-frts-v6-13-ecf41ef99252@nvidia.com/
Best,
Gary
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent
2026-03-23 13:05 ` Gary Guo
@ 2026-03-23 14:33 ` Alexandre Courbot
0 siblings, 0 replies; 22+ messages in thread
From: Alexandre Courbot @ 2026-03-23 14:33 UTC (permalink / raw)
To: Gary Guo
Cc: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, David Airlie,
Simona Vetter, John Hubbard, Alistair Popple, Joel Fernandes,
Timur Tabi, Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel
On Mon Mar 23, 2026 at 10:05 PM JST, Gary Guo wrote:
> On Mon Mar 23, 2026 at 6:15 AM GMT, Alexandre Courbot wrote:
>> On Sat Mar 21, 2026 at 11:58 PM JST, Gary Guo wrote:
>>> On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
>>>> Replace the nova-core local `DmaObject` with a `Coherent` that can
>>>> fulfill the same role.
>>>>
>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>> ---
>>>> drivers/gpu/nova-core/firmware/riscv.rs | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
>>>> index 14aad2f0ee8a..2afa7f36404e 100644
>>>> --- a/drivers/gpu/nova-core/firmware/riscv.rs
>>>> +++ b/drivers/gpu/nova-core/firmware/riscv.rs
>>>> @@ -5,13 +5,13 @@
>>>>
>>>> use kernel::{
>>>> device,
>>>> + dma::Coherent,
>>>> firmware::Firmware,
>>>> prelude::*,
>>>> transmute::FromBytes, //
>>>> };
>>>>
>>>> use crate::{
>>>> - dma::DmaObject,
>>>> firmware::BinFirmware,
>>>> num::FromSafeCast, //
>>>> };
>>>> @@ -66,7 +66,7 @@ pub(crate) struct RiscvFirmware {
>>>> /// Application version.
>>>> pub(crate) app_version: u32,
>>>> /// Device-mapped firmware image.
>>>> - pub(crate) ucode: DmaObject,
>>>> + pub(crate) ucode: Coherent<[u8]>,
>>>> }
>>>>
>>>> impl RiscvFirmware {
>>>> @@ -81,7 +81,7 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<
>>>> let len = usize::from_safe_cast(bin_fw.hdr.data_size);
>>>> let end = start.checked_add(len).ok_or(EINVAL)?;
>>>>
>>>> - DmaObject::from_data(dev, fw.data().get(start..end).ok_or(EINVAL)?)?
>>>> + Coherent::from_slice(dev, fw.data().get(start..end).ok_or(EINVAL)?, GFP_KERNEL)?
>>>
>>> `DmaObject` rounds the data up to be page-sized, while this new API doesn't.
>>>
>>> It has impact on alignment, as the allocator aligns things to the largest
>>> power-of-two exponent of the allocated size.
>>
>> Doesn't `dma_alloc_coherent` always allocate from the page pool and thus
>> returns page-aligned memory though?
>
> Oh you're right, this is not from DMA pool, so allocations are page-aligned
> indeed.
>
> I brought this up because I got bite by this size adjustment behaviour
> because in the `from_data` I initially put
>
> unsafe { dma_obj.as_mut().copy_from_slice(data) };
>
> but that doesn't work due to the size being aligned up so dma_obj.len() !=
> data.len().
>
> But if this behaviour is not needed it does simplify things quite a bit.
I was contemplating hardening the `Coherent` allocations by padding the
structs when there are specific alignment needs (in nova-core it's only
a couple of types affected), but then thought this was quite restrictive
and should be handled by an alignment parameter on `Coherent` directly -
and `Coherent` doesn't have such a parameter because the underlying C
API doesn't either, and the page alignment seems to be a property of the
API itself.
Maybe that point is worth mentioning in the documentation of `Coherent`?
>
>> I'm not sure why `DmaObject` was doing that but it was redundant I think.
>
> A question for yourself I guess? :-)
>
> https://lore.kernel.org/all/20250619-nova-frts-v6-13-ecf41ef99252@nvidia.com/
Ha, I was precisely wondering who was the lesser engineer who wrote that. :)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/7] gpu: nova-core: firmware: fwsec: use dma::Coherent
2026-03-21 13:36 [PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core Alexandre Courbot
2026-03-21 13:36 ` [PATCH 1/7] rust: dma: add from-slice constructors for Coherent and CoherentBox Alexandre Courbot
2026-03-21 13:36 ` [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent Alexandre Courbot
@ 2026-03-21 13:36 ` Alexandre Courbot
2026-03-21 13:36 ` [PATCH 4/7] gpu: nova-core: falcon: " Alexandre Courbot
` (4 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Alexandre Courbot @ 2026-03-21 13:36 UTC (permalink / raw)
To: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel, Alexandre Courbot
Replace the nova-core local `DmaObject` with a `Coherent` that can
fulfill the same role.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/firmware/fwsec/bootloader.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs b/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
index 342dba59b2f9..7c6bb0a871c1 100644
--- a/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
@@ -12,6 +12,7 @@
self,
Device, //
},
+ dma::Coherent,
prelude::*,
ptr::{
Alignable,
@@ -25,7 +26,6 @@
};
use crate::{
- dma::DmaObject,
driver::Bar0,
falcon::{
self,
@@ -126,7 +126,7 @@ unsafe impl AsBytes for BootloaderDmemDescV2 {}
/// operation.
pub(crate) struct FwsecFirmwareWithBl {
/// DMA object the bootloader will copy the firmware from.
- _firmware_dma: DmaObject,
+ _firmware_dma: Coherent<[u8]>,
/// Code of the bootloader to be loaded into non-secure IMEM.
ucode: KVec<u8>,
/// Descriptor to be loaded into DMEM for the bootloader to read.
@@ -208,7 +208,7 @@ pub(crate) fn new(
(
align_padding,
- DmaObject::from_data(dev, firmware_obj.as_slice())?,
+ Coherent::from_slice(dev, firmware_obj.as_slice(), GFP_KERNEL)?,
)
};
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 4/7] gpu: nova-core: falcon: use dma::Coherent
2026-03-21 13:36 [PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core Alexandre Courbot
` (2 preceding siblings ...)
2026-03-21 13:36 ` [PATCH 3/7] gpu: nova-core: firmware: fwsec: " Alexandre Courbot
@ 2026-03-21 13:36 ` Alexandre Courbot
2026-03-25 2:14 ` Eliot Courtney
2026-03-21 13:36 ` [PATCH 5/7] gpu: nova-core: fb: " Alexandre Courbot
` (3 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Alexandre Courbot @ 2026-03-21 13:36 UTC (permalink / raw)
To: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel, Alexandre Courbot
Replace the nova-core local `DmaObject` with a `Coherent` that can
fulfill the same role.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/falcon.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 5bf8da8760bf..f6239c44dd80 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -10,6 +10,7 @@
Device, //
},
dma::{
+ Coherent,
DmaAddress,
DmaMask, //
},
@@ -20,7 +21,6 @@
};
use crate::{
- dma::DmaObject,
driver::Bar0,
falcon::hal::LoadMethod,
gpu::Chipset,
@@ -636,7 +636,7 @@ pub(crate) fn pio_load<F: FalconFirmware<Target = E> + FalconPioLoadable>(
fn dma_wr(
&self,
bar: &Bar0,
- dma_obj: &DmaObject,
+ dma_obj: &Coherent<[u8]>,
target_mem: FalconMem,
load_offsets: FalconDmaLoadTarget,
) -> Result {
@@ -740,7 +740,7 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(
fw: &F,
) -> Result {
// Create DMA object with firmware content as the source of the DMA engine.
- let dma_obj = DmaObject::from_data(dev, fw.as_slice())?;
+ let dma_obj = Coherent::from_slice(dev, fw.as_slice(), GFP_KERNEL)?;
self.dma_reset(bar);
regs::NV_PFALCON_FBIF_TRANSCFG::update(bar, &E::ID, 0, |v| {
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 4/7] gpu: nova-core: falcon: use dma::Coherent
2026-03-21 13:36 ` [PATCH 4/7] gpu: nova-core: falcon: " Alexandre Courbot
@ 2026-03-25 2:14 ` Eliot Courtney
2026-03-26 15:04 ` Alexandre Courbot
0 siblings, 1 reply; 22+ messages in thread
From: Eliot Courtney @ 2026-03-25 2:14 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Abdiel Janulgue,
Daniel Almeida, Robin Murphy, Andreas Hindborg, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel
On Sat Mar 21, 2026 at 10:36 PM JST, Alexandre Courbot wrote:
> Replace the nova-core local `DmaObject` with a `Coherent` that can
> fulfill the same role.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> drivers/gpu/nova-core/falcon.rs | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> index 5bf8da8760bf..f6239c44dd80 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -10,6 +10,7 @@
> Device, //
> },
> dma::{
> + Coherent,
> DmaAddress,
> DmaMask, //
> },
> @@ -20,7 +21,6 @@
> };
>
> use crate::{
> - dma::DmaObject,
> driver::Bar0,
> falcon::hal::LoadMethod,
> gpu::Chipset,
> @@ -636,7 +636,7 @@ pub(crate) fn pio_load<F: FalconFirmware<Target = E> + FalconPioLoadable>(
> fn dma_wr(
> &self,
> bar: &Bar0,
> - dma_obj: &DmaObject,
> + dma_obj: &Coherent<[u8]>,
> target_mem: FalconMem,
> load_offsets: FalconDmaLoadTarget,
> ) -> Result {
> @@ -740,7 +740,7 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(
> fw: &F,
> ) -> Result {
> // Create DMA object with firmware content as the source of the DMA engine.
> - let dma_obj = DmaObject::from_data(dev, fw.as_slice())?;
> + let dma_obj = Coherent::from_slice(dev, fw.as_slice(), GFP_KERNEL)?;
Is it guaranteed that fw.as_slice() is a multiple of 256 in size?
In `dma_wr` it breaks this up into 256 byte transfers. Since this
no longer pads out to a page boundary, it means that it could now error
(around "DMA transfer goes beyond range of DMA object") if the Dmem
section's size is not divisible by 256. But tbh, I find it odd that
`dma_wr` doesn't check that FalconDmaLoadTarget's length is a
multiple of 256 anyway, because it looks like it'll write a bunch of
unrelated bytes (since it rounds up to the nearest 256 to copy).
Maybe we should enforce that `FalconDmaLoadTarget` length is divisible
by 256?
For this series if for all firmwares it's divisible by 256 then I think
it's fine to leave this as is for now, but I do find the lack of
checking in `dma_wr` (or anywhere else for FalconDmaLoadTarget) a bit
odd.
>
> self.dma_reset(bar);
> regs::NV_PFALCON_FBIF_TRANSCFG::update(bar, &E::ID, 0, |v| {
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 4/7] gpu: nova-core: falcon: use dma::Coherent
2026-03-25 2:14 ` Eliot Courtney
@ 2026-03-26 15:04 ` Alexandre Courbot
2026-03-26 15:35 ` Gary Guo
0 siblings, 1 reply; 22+ messages in thread
From: Alexandre Courbot @ 2026-03-26 15:04 UTC (permalink / raw)
To: Eliot Courtney
Cc: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, John Hubbard, Alistair Popple,
Joel Fernandes, Timur Tabi, Zhi Wang, driver-core, rust-for-linux,
linux-kernel
On Wed Mar 25, 2026 at 11:14 AM JST, Eliot Courtney wrote:
> On Sat Mar 21, 2026 at 10:36 PM JST, Alexandre Courbot wrote:
>> Replace the nova-core local `DmaObject` with a `Coherent` that can
>> fulfill the same role.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>> drivers/gpu/nova-core/falcon.rs | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
>> index 5bf8da8760bf..f6239c44dd80 100644
>> --- a/drivers/gpu/nova-core/falcon.rs
>> +++ b/drivers/gpu/nova-core/falcon.rs
>> @@ -10,6 +10,7 @@
>> Device, //
>> },
>> dma::{
>> + Coherent,
>> DmaAddress,
>> DmaMask, //
>> },
>> @@ -20,7 +21,6 @@
>> };
>>
>> use crate::{
>> - dma::DmaObject,
>> driver::Bar0,
>> falcon::hal::LoadMethod,
>> gpu::Chipset,
>> @@ -636,7 +636,7 @@ pub(crate) fn pio_load<F: FalconFirmware<Target = E> + FalconPioLoadable>(
>> fn dma_wr(
>> &self,
>> bar: &Bar0,
>> - dma_obj: &DmaObject,
>> + dma_obj: &Coherent<[u8]>,
>> target_mem: FalconMem,
>> load_offsets: FalconDmaLoadTarget,
>> ) -> Result {
>> @@ -740,7 +740,7 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(
>> fw: &F,
>> ) -> Result {
>> // Create DMA object with firmware content as the source of the DMA engine.
>> - let dma_obj = DmaObject::from_data(dev, fw.as_slice())?;
>> + let dma_obj = Coherent::from_slice(dev, fw.as_slice(), GFP_KERNEL)?;
>
> Is it guaranteed that fw.as_slice() is a multiple of 256 in size?
> In `dma_wr` it breaks this up into 256 byte transfers. Since this
> no longer pads out to a page boundary, it means that it could now error
> (around "DMA transfer goes beyond range of DMA object") if the Dmem
> section's size is not divisible by 256. But tbh, I find it odd that
> `dma_wr` doesn't check that FalconDmaLoadTarget's length is a
> multiple of 256 anyway, because it looks like it'll write a bunch of
> unrelated bytes (since it rounds up to the nearest 256 to copy).
>
> Maybe we should enforce that `FalconDmaLoadTarget` length is divisible
> by 256?
>
> For this series if for all firmwares it's divisible by 256 then I think
> it's fine to leave this as is for now, but I do find the lack of
> checking in `dma_wr` (or anywhere else for FalconDmaLoadTarget) a bit
> odd.
All coherent allocations are page-aligned (and use full pages), so we
are safe in terms of overflows.
Also `dma_wr` uses `div_ceil(256)` which will skip the last data block
entirely if it is not a multiple of 256. It might be a bit more robust
to explicitly check that the size is a multiple of 256 and return an
error if that is not the case indeed.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 4/7] gpu: nova-core: falcon: use dma::Coherent
2026-03-26 15:04 ` Alexandre Courbot
@ 2026-03-26 15:35 ` Gary Guo
2026-03-28 13:03 ` Alexandre Courbot
0 siblings, 1 reply; 22+ messages in thread
From: Gary Guo @ 2026-03-26 15:35 UTC (permalink / raw)
To: Alexandre Courbot, Eliot Courtney
Cc: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, John Hubbard, Alistair Popple,
Joel Fernandes, Timur Tabi, Zhi Wang, driver-core, rust-for-linux,
linux-kernel
On Thu Mar 26, 2026 at 3:04 PM GMT, Alexandre Courbot wrote:
> On Wed Mar 25, 2026 at 11:14 AM JST, Eliot Courtney wrote:
>> On Sat Mar 21, 2026 at 10:36 PM JST, Alexandre Courbot wrote:
>>> Replace the nova-core local `DmaObject` with a `Coherent` that can
>>> fulfill the same role.
>>>
>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>> ---
>>> drivers/gpu/nova-core/falcon.rs | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
>>> index 5bf8da8760bf..f6239c44dd80 100644
>>> --- a/drivers/gpu/nova-core/falcon.rs
>>> +++ b/drivers/gpu/nova-core/falcon.rs
>>> @@ -10,6 +10,7 @@
>>> Device, //
>>> },
>>> dma::{
>>> + Coherent,
>>> DmaAddress,
>>> DmaMask, //
>>> },
>>> @@ -20,7 +21,6 @@
>>> };
>>>
>>> use crate::{
>>> - dma::DmaObject,
>>> driver::Bar0,
>>> falcon::hal::LoadMethod,
>>> gpu::Chipset,
>>> @@ -636,7 +636,7 @@ pub(crate) fn pio_load<F: FalconFirmware<Target = E> + FalconPioLoadable>(
>>> fn dma_wr(
>>> &self,
>>> bar: &Bar0,
>>> - dma_obj: &DmaObject,
>>> + dma_obj: &Coherent<[u8]>,
>>> target_mem: FalconMem,
>>> load_offsets: FalconDmaLoadTarget,
>>> ) -> Result {
>>> @@ -740,7 +740,7 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(
>>> fw: &F,
>>> ) -> Result {
>>> // Create DMA object with firmware content as the source of the DMA engine.
>>> - let dma_obj = DmaObject::from_data(dev, fw.as_slice())?;
>>> + let dma_obj = Coherent::from_slice(dev, fw.as_slice(), GFP_KERNEL)?;
>>
>> Is it guaranteed that fw.as_slice() is a multiple of 256 in size?
>> In `dma_wr` it breaks this up into 256 byte transfers. Since this
>> no longer pads out to a page boundary, it means that it could now error
>> (around "DMA transfer goes beyond range of DMA object") if the Dmem
>> section's size is not divisible by 256. But tbh, I find it odd that
>> `dma_wr` doesn't check that FalconDmaLoadTarget's length is a
>> multiple of 256 anyway, because it looks like it'll write a bunch of
>> unrelated bytes (since it rounds up to the nearest 256 to copy).
>>
>> Maybe we should enforce that `FalconDmaLoadTarget` length is divisible
>> by 256?
>>
>> For this series if for all firmwares it's divisible by 256 then I think
>> it's fine to leave this as is for now, but I do find the lack of
>> checking in `dma_wr` (or anywhere else for FalconDmaLoadTarget) a bit
>> odd.
>
> All coherent allocations are page-aligned (and use full pages), so we
> are safe in terms of overflows.
Let's not rely on this behaviour. There is no guarantee on what's at the end
of allocation whatsoever. There's no guarantee that it will be initialized.
Even with __GFP_ZERO only the size provided will be zeroed.
If the GPU is going to read beyond ranges covered by `Coherent` (not just rely
on the alignment), let's align up the allocation.
>
> Also `dma_wr` uses `div_ceil(256)` which will skip the last data block
> entirely if it is not a multiple of 256. It might be a bit more robust
> to explicitly check that the size is a multiple of 256 and return an
> error if that is not the case indeed.
div_ceil will not skip the last block, it will over-read beyond the end.
div_floor would have skipped the block.
Best,
Gary
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 4/7] gpu: nova-core: falcon: use dma::Coherent
2026-03-26 15:35 ` Gary Guo
@ 2026-03-28 13:03 ` Alexandre Courbot
0 siblings, 0 replies; 22+ messages in thread
From: Alexandre Courbot @ 2026-03-28 13:03 UTC (permalink / raw)
To: Gary Guo
Cc: Eliot Courtney, Danilo Krummrich, Abdiel Janulgue, Daniel Almeida,
Robin Murphy, Andreas Hindborg, Miguel Ojeda, Boqun Feng,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, John Hubbard, Alistair Popple,
Joel Fernandes, Timur Tabi, Zhi Wang, driver-core, rust-for-linux,
linux-kernel
On Fri Mar 27, 2026 at 12:35 AM JST, Gary Guo wrote:
> On Thu Mar 26, 2026 at 3:04 PM GMT, Alexandre Courbot wrote:
>> On Wed Mar 25, 2026 at 11:14 AM JST, Eliot Courtney wrote:
>>> On Sat Mar 21, 2026 at 10:36 PM JST, Alexandre Courbot wrote:
>>>> Replace the nova-core local `DmaObject` with a `Coherent` that can
>>>> fulfill the same role.
>>>>
>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>> ---
>>>> drivers/gpu/nova-core/falcon.rs | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
>>>> index 5bf8da8760bf..f6239c44dd80 100644
>>>> --- a/drivers/gpu/nova-core/falcon.rs
>>>> +++ b/drivers/gpu/nova-core/falcon.rs
>>>> @@ -10,6 +10,7 @@
>>>> Device, //
>>>> },
>>>> dma::{
>>>> + Coherent,
>>>> DmaAddress,
>>>> DmaMask, //
>>>> },
>>>> @@ -20,7 +21,6 @@
>>>> };
>>>>
>>>> use crate::{
>>>> - dma::DmaObject,
>>>> driver::Bar0,
>>>> falcon::hal::LoadMethod,
>>>> gpu::Chipset,
>>>> @@ -636,7 +636,7 @@ pub(crate) fn pio_load<F: FalconFirmware<Target = E> + FalconPioLoadable>(
>>>> fn dma_wr(
>>>> &self,
>>>> bar: &Bar0,
>>>> - dma_obj: &DmaObject,
>>>> + dma_obj: &Coherent<[u8]>,
>>>> target_mem: FalconMem,
>>>> load_offsets: FalconDmaLoadTarget,
>>>> ) -> Result {
>>>> @@ -740,7 +740,7 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(
>>>> fw: &F,
>>>> ) -> Result {
>>>> // Create DMA object with firmware content as the source of the DMA engine.
>>>> - let dma_obj = DmaObject::from_data(dev, fw.as_slice())?;
>>>> + let dma_obj = Coherent::from_slice(dev, fw.as_slice(), GFP_KERNEL)?;
>>>
>>> Is it guaranteed that fw.as_slice() is a multiple of 256 in size?
>>> In `dma_wr` it breaks this up into 256 byte transfers. Since this
>>> no longer pads out to a page boundary, it means that it could now error
>>> (around "DMA transfer goes beyond range of DMA object") if the Dmem
>>> section's size is not divisible by 256. But tbh, I find it odd that
>>> `dma_wr` doesn't check that FalconDmaLoadTarget's length is a
>>> multiple of 256 anyway, because it looks like it'll write a bunch of
>>> unrelated bytes (since it rounds up to the nearest 256 to copy).
>>>
>>> Maybe we should enforce that `FalconDmaLoadTarget` length is divisible
>>> by 256?
>>>
>>> For this series if for all firmwares it's divisible by 256 then I think
>>> it's fine to leave this as is for now, but I do find the lack of
>>> checking in `dma_wr` (or anywhere else for FalconDmaLoadTarget) a bit
>>> odd.
>>
>> All coherent allocations are page-aligned (and use full pages), so we
>> are safe in terms of overflows.
>
> Let's not rely on this behaviour. There is no guarantee on what's at the end
> of allocation whatsoever. There's no guarantee that it will be initialized.
> Even with __GFP_ZERO only the size provided will be zeroed.
>
> If the GPU is going to read beyond ranges covered by `Coherent` (not just rely
> on the alignment), let's align up the allocation.
>
>>
>> Also `dma_wr` uses `div_ceil(256)` which will skip the last data block
>> entirely if it is not a multiple of 256. It might be a bit more robust
>> to explicitly check that the size is a multiple of 256 and return an
>> error if that is not the case indeed.
>
> div_ceil will not skip the last block, it will over-read beyond the end.
> div_floor would have skipped the block.
Ooopsie yes, of course. Making `dma_wr` check that the data is a
multiple of 256 is the simplest, I'll send a patch for that (with maybe
some padding code as I think I remember Turing at least did not always
follow the 256-alignment requirement).
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 5/7] gpu: nova-core: fb: use dma::Coherent
2026-03-21 13:36 [PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core Alexandre Courbot
` (3 preceding siblings ...)
2026-03-21 13:36 ` [PATCH 4/7] gpu: nova-core: falcon: " Alexandre Courbot
@ 2026-03-21 13:36 ` Alexandre Courbot
2026-03-21 13:36 ` [PATCH 6/7] gpu: nova-core: firmware: gsp: use dma::Coherent for signatures Alexandre Courbot
` (2 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Alexandre Courbot @ 2026-03-21 13:36 UTC (permalink / raw)
To: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel, Alexandre Courbot
Replace the nova-core local `DmaObject` with a `Coherent` that can
fulfill the same role.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/fb.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index 6536d0035cb1..ba971a114a06 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -7,6 +7,7 @@
use kernel::{
device,
+ dma::Coherent,
fmt,
prelude::*,
ptr::{
@@ -18,7 +19,6 @@
};
use crate::{
- dma::DmaObject,
driver::Bar0,
firmware::gsp::GspFirmware,
gpu::Chipset,
@@ -52,7 +52,7 @@ pub(crate) struct SysmemFlush {
chipset: Chipset,
device: ARef<device::Device>,
/// Keep the page alive as long as we need it.
- page: DmaObject,
+ page: Coherent<[u8]>,
}
impl SysmemFlush {
@@ -62,7 +62,7 @@ pub(crate) fn register(
bar: &Bar0,
chipset: Chipset,
) -> Result<Self> {
- let page = DmaObject::new(dev, kernel::page::PAGE_SIZE)?;
+ let page = Coherent::zeroed_slice(dev, kernel::page::PAGE_SIZE, GFP_KERNEL)?;
hal::fb_hal(chipset).write_sysmem_flush_page(bar, page.dma_handle())?;
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 6/7] gpu: nova-core: firmware: gsp: use dma::Coherent for signatures
2026-03-21 13:36 [PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core Alexandre Courbot
` (4 preceding siblings ...)
2026-03-21 13:36 ` [PATCH 5/7] gpu: nova-core: fb: " Alexandre Courbot
@ 2026-03-21 13:36 ` Alexandre Courbot
2026-03-21 13:36 ` [PATCH 7/7] gpu: nova-core: firmware: gsp: use dma::Coherent for level0 table Alexandre Courbot
2026-03-23 17:01 ` [PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core Gary Guo
7 siblings, 0 replies; 22+ messages in thread
From: Alexandre Courbot @ 2026-03-21 13:36 UTC (permalink / raw)
To: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel, Alexandre Courbot
Replace the nova-core local `DmaObject` with a `Coherent` that can
fulfill the same role.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/firmware/gsp.rs | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
index 9488a626352f..1e0d545a74fe 100644
--- a/drivers/gpu/nova-core/firmware/gsp.rs
+++ b/drivers/gpu/nova-core/firmware/gsp.rs
@@ -3,6 +3,7 @@
use kernel::{
device,
dma::{
+ Coherent,
DataDirection,
DmaAddress, //
},
@@ -140,7 +141,7 @@ pub(crate) struct GspFirmware {
/// Size in bytes of the firmware contained in [`Self::fw`].
pub(crate) size: usize,
/// Device-mapped GSP signatures matching the GPU's [`Chipset`].
- pub(crate) signatures: DmaObject,
+ pub(crate) signatures: Coherent<[u8]>,
/// GSP bootloader, verifies the GSP firmware before loading and running it.
pub(crate) bootloader: RiscvFirmware,
}
@@ -226,7 +227,7 @@ pub(crate) fn new<'a>(
elf::elf64_section(firmware.data(), sigs_section)
.ok_or(EINVAL)
- .and_then(|data| DmaObject::from_data(dev, data))?
+ .and_then(|data| Coherent::from_slice(dev, data, GFP_KERNEL))?
},
bootloader: {
let bl = super::request_firmware(dev, chipset, "bootloader", ver)?;
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 7/7] gpu: nova-core: firmware: gsp: use dma::Coherent for level0 table
2026-03-21 13:36 [PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core Alexandre Courbot
` (5 preceding siblings ...)
2026-03-21 13:36 ` [PATCH 6/7] gpu: nova-core: firmware: gsp: use dma::Coherent for signatures Alexandre Courbot
@ 2026-03-21 13:36 ` Alexandre Courbot
2026-03-23 17:01 ` [PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core Gary Guo
7 siblings, 0 replies; 22+ messages in thread
From: Alexandre Courbot @ 2026-03-21 13:36 UTC (permalink / raw)
To: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel, Alexandre Courbot
Replace the nova-core local `DmaObject` with a `CoherentBox` that can
fulfill the same role.
Since `CoherentBox` is more flexible than `DmaObject`, we can use the
native `u64` type for page table entries instead of messing with bytes.
The `dma` module becomes unused with that change, so remove it as well.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/dma.rs | 53 -----------------------------------
drivers/gpu/nova-core/firmware/gsp.rs | 22 ++++++++-------
drivers/gpu/nova-core/nova_core.rs | 1 -
3 files changed, 12 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/nova-core/dma.rs b/drivers/gpu/nova-core/dma.rs
deleted file mode 100644
index 3c19d5ffcfe8..000000000000
--- a/drivers/gpu/nova-core/dma.rs
+++ /dev/null
@@ -1,53 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-//! Simple DMA object wrapper.
-
-use core::ops::{
- Deref,
- DerefMut, //
-};
-
-use kernel::{
- device,
- dma::Coherent,
- page::PAGE_SIZE,
- prelude::*, //
-};
-
-pub(crate) struct DmaObject {
- dma: Coherent<[u8]>,
-}
-
-impl DmaObject {
- pub(crate) fn new(dev: &device::Device<device::Bound>, len: usize) -> Result<Self> {
- let len = core::alloc::Layout::from_size_align(len, PAGE_SIZE)
- .map_err(|_| EINVAL)?
- .pad_to_align()
- .size();
- let dma = Coherent::zeroed_slice(dev, len, GFP_KERNEL)?;
-
- Ok(Self { dma })
- }
-
- pub(crate) fn from_data(dev: &device::Device<device::Bound>, data: &[u8]) -> Result<Self> {
- let dma_obj = Self::new(dev, data.len())?;
- // SAFETY: We have just allocated the DMA memory, we are the only users and
- // we haven't made the device aware of the handle yet.
- unsafe { dma_obj.as_mut()[..data.len()].copy_from_slice(data) };
- Ok(dma_obj)
- }
-}
-
-impl Deref for DmaObject {
- type Target = Coherent<[u8]>;
-
- fn deref(&self) -> &Self::Target {
- &self.dma
- }
-}
-
-impl DerefMut for DmaObject {
- fn deref_mut(&mut self) -> &mut Self::Target {
- &mut self.dma
- }
-}
diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
index 1e0d545a74fe..86fb3f074195 100644
--- a/drivers/gpu/nova-core/firmware/gsp.rs
+++ b/drivers/gpu/nova-core/firmware/gsp.rs
@@ -4,10 +4,10 @@
device,
dma::{
Coherent,
+ CoherentBox,
DataDirection,
DmaAddress, //
},
- kvec,
prelude::*,
scatterlist::{
Owned,
@@ -16,7 +16,6 @@
};
use crate::{
- dma::DmaObject,
firmware::riscv::RiscvFirmware,
gpu::{
Architecture,
@@ -137,7 +136,7 @@ pub(crate) struct GspFirmware {
#[pin]
level1: SGTable<Owned<VVec<u8>>>,
/// Level 0 page table (single 4KB page) with one entry: DMA address of first level 1 page.
- level0: DmaObject,
+ level0: Coherent<[u64]>,
/// Size in bytes of the firmware contained in [`Self::fw`].
pub(crate) size: usize,
/// Device-mapped GSP signatures matching the GPU's [`Chipset`].
@@ -198,17 +197,20 @@ pub(crate) fn new<'a>(
// Allocate the level 0 page table as a device-visible DMA object, and map the
// level 1 page table onto it.
- // Level 0 page table data.
- let mut level0_data = kvec![0u8; GSP_PAGE_SIZE]?;
-
// Fill level 1 page entry.
let level1_entry = level1.iter().next().ok_or(EINVAL)?;
let level1_entry_addr = level1_entry.dma_address();
- let dst = &mut level0_data[..size_of_val(&level1_entry_addr)];
- dst.copy_from_slice(&level1_entry_addr.to_le_bytes());
- // Turn the level0 page table into a [`DmaObject`].
- DmaObject::from_data(dev, &level0_data)?
+ // Create level 0 page table data and fill its first entry with the level 1
+ // table.
+ let mut level0 = CoherentBox::<[u64]>::zeroed_slice(
+ dev,
+ GSP_PAGE_SIZE / size_of::<u64>(),
+ GFP_KERNEL
+ )?;
+ level0[0] = level1_entry_addr.to_le();
+
+ level0.into()
},
size,
signatures: {
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index b5caf1044697..e0271e0c6335 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -5,7 +5,6 @@
#[macro_use]
mod bitfield;
-mod dma;
mod driver;
mod falcon;
mod fb;
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core
2026-03-21 13:36 [PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core Alexandre Courbot
` (6 preceding siblings ...)
2026-03-21 13:36 ` [PATCH 7/7] gpu: nova-core: firmware: gsp: use dma::Coherent for level0 table Alexandre Courbot
@ 2026-03-23 17:01 ` Gary Guo
7 siblings, 0 replies; 22+ messages in thread
From: Gary Guo @ 2026-03-23 17:01 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Abdiel Janulgue,
Daniel Almeida, Robin Murphy, Andreas Hindborg, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, driver-core, rust-for-linux,
linux-kernel
On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
> nova-core's `DmaObject` type has been created to serve the same purpose
> as `dma::Coherent`, with the addition of a handy constructor to create
> an object from a slice of bytes, but without the flexibility of
> `dma::Coherent` since `DmaObject` is limited to slices of bytes.
>
> This series adds new constructors to `Coherent` and `CoherentBox` to
> cover this (arguably common) use-case, and updates the nova-core code to
> use them. This results in more consistent code overall, and allows us to
> retire `DmaObject` and nova-core's `dma` module.
>
> Depends on drm-rust-next as of 2026-03-21 + [1].
>
> [1] https://lore.kernel.org/all/20260320194626.36263-1-dakr@kernel.org/
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Looks like a good cleanup. For the series (with nits fixed in first patch):
Reviewed-by: Gary Guo <gary@garyguo.net>
Best,
Gary
> ---
> Alexandre Courbot (7):
> rust: dma: add from-slice constructors for Coherent and CoherentBox
> gpu: nova-core: firmware: riscv: use dma::Coherent
> gpu: nova-core: firmware: fwsec: use dma::Coherent
> gpu: nova-core: falcon: use dma::Coherent
> gpu: nova-core: fb: use dma::Coherent
> gpu: nova-core: firmware: gsp: use dma::Coherent for signatures
> gpu: nova-core: firmware: gsp: use dma::Coherent for level0 table
>
> drivers/gpu/nova-core/dma.rs | 53 -----------
> drivers/gpu/nova-core/falcon.rs | 6 +-
> drivers/gpu/nova-core/fb.rs | 6 +-
> drivers/gpu/nova-core/firmware/fwsec/bootloader.rs | 6 +-
> drivers/gpu/nova-core/firmware/gsp.rs | 27 +++---
> drivers/gpu/nova-core/firmware/riscv.rs | 6 +-
> drivers/gpu/nova-core/nova_core.rs | 1 -
> rust/kernel/dma.rs | 102 +++++++++++++++++++++
> 8 files changed, 129 insertions(+), 78 deletions(-)
> ---
> base-commit: 19761c783a46bc4fb2ba0ef22111f7dd27b441e6
> change-id: 20260321-b4-nova-dma-removal-a7e88d4a6790
>
> Best regards,
^ permalink raw reply [flat|nested] 22+ messages in thread