Re: [PATCH 2/8] rust: dma: add generalized container for types other than slices

Re: [PATCH 2/8] rust: dma: add generalized container for types other than slices

[Alice Ryhl]

On Tue, Mar 03, 2026 at 05:22:53PM +0100, Danilo Krummrich wrote:
> From: Gary Guo <gary@garyguo.net>
> 
> Currently, `CoherentAllocation` is concecptually a DMA coherent container
> of a slice of `[T]` of runtime-checked length. Generalize it by creating
> `dma::Coherent<T>` which can hold any value of `T`.
> `Coherent::alloc_with_attrs` is implemented but not yet exposed, as I
> believe we should not expose the way to obtain an uninitialized coherent
> region.
> 
> `Coherent<[T]>` provides a `len` method instead of the previous `count()`
> method to be consistent with methods on slices.
> 
> The existing type is re-defined as a type alias of `Coherent<[T]>` to ease
> transition. Methods in use are not yet removed.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

I see you have both `size()` and `len()`. I think it would be clearer to
rename `size()` to `len_bytes()`.

Otherwise LGTM.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Alice

Re: [PATCH 2/8] rust: dma: add generalized container for types other than slices

[Gary Guo]

On Tue Mar 17, 2026 at 6:50 AM GMT, Alice Ryhl wrote:
> On Tue, Mar 03, 2026 at 05:22:53PM +0100, Danilo Krummrich wrote:
>> From: Gary Guo <gary@garyguo.net>
>> 
>> Currently, `CoherentAllocation` is concecptually a DMA coherent container
>> of a slice of `[T]` of runtime-checked length. Generalize it by creating
>> `dma::Coherent<T>` which can hold any value of `T`.
>> `Coherent::alloc_with_attrs` is implemented but not yet exposed, as I
>> believe we should not expose the way to obtain an uninitialized coherent
>> region.
>> 
>> `Coherent<[T]>` provides a `len` method instead of the previous `count()`
>> method to be consistent with methods on slices.
>> 
>> The existing type is re-defined as a type alias of `Coherent<[T]>` to ease
>> transition. Methods in use are not yet removed.
>> 
>> Signed-off-by: Gary Guo <gary@garyguo.net>
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>
> I see you have both `size()` and `len()`. I think it would be clearer to
> rename `size()` to `len_bytes()`.

This is an API that already exists and I simply moved it around so it is
implemented for all types, not just `[T]`.

I also think that `size()` quite unambiguously means the size in bytes (like, in
all allocation and I/O APIs, `size` means size of object in bytes); I don't see
much benefit in renaming it `len_bytes`.

Best,
Gary

>
> Otherwise LGTM.
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
> Alice

Re: [PATCH 2/8] rust: dma: add generalized container for types other than slices

[Alexandre Courbot]

On Tue Mar 17, 2026 at 9:56 PM JST, Gary Guo wrote:
> On Tue Mar 17, 2026 at 6:50 AM GMT, Alice Ryhl wrote:
>> On Tue, Mar 03, 2026 at 05:22:53PM +0100, Danilo Krummrich wrote:
>>> From: Gary Guo <gary@garyguo.net>
>>> 
>>> Currently, `CoherentAllocation` is concecptually a DMA coherent container
>>> of a slice of `[T]` of runtime-checked length. Generalize it by creating
>>> `dma::Coherent<T>` which can hold any value of `T`.
>>> `Coherent::alloc_with_attrs` is implemented but not yet exposed, as I
>>> believe we should not expose the way to obtain an uninitialized coherent
>>> region.
>>> 
>>> `Coherent<[T]>` provides a `len` method instead of the previous `count()`
>>> method to be consistent with methods on slices.
>>> 
>>> The existing type is re-defined as a type alias of `Coherent<[T]>` to ease
>>> transition. Methods in use are not yet removed.
>>> 
>>> Signed-off-by: Gary Guo <gary@garyguo.net>
>>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>>
>> I see you have both `size()` and `len()`. I think it would be clearer to
>> rename `size()` to `len_bytes()`.
>
> This is an API that already exists and I simply moved it around so it is
> implemented for all types, not just `[T]`.
>
> I also think that `size()` quite unambiguously means the size in bytes (like, in
> all allocation and I/O APIs, `size` means size of object in bytes); I don't see
> much benefit in renaming it `len_bytes`.

Agreed, furthermore `len` is used in the standard library so the pattern
of it returning a number of elements is pretty established.

Generally speaking I tend to prefer having the units mentioned in the
doccomment rather than the method name.

Re: [PATCH 4/8] rust: dma: introduce dma::CoherentInit for memory initialization

[Alice Ryhl]

On Tue, Mar 03, 2026 at 05:22:55PM +0100, Danilo Krummrich wrote:
> Currently, dma::Coherent cannot safely provide (mutable) access to its
> underlying memory because the memory might be concurrently accessed by a
> DMA device. This makes it difficult to safely initialize the memory
> before handing it over to the hardware.
> 
> Introduce dma::CoherentInit, a type that encapsulates a dma::Coherent
> before its DMA address is exposed to the device. dma::CoherentInit can
> guarantee exclusive access to the inner dma::Coherent and implement
> Deref and DerefMut.
> 
> Once the memory is properly initialized, dma::CoherentInit can be
> converted into a regular dma::Coherent.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

overall LGTM

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

> +        let ptr = core::ptr::from_mut(&mut self[i]);

&raw mut self[i].

Alice

Re: [PATCH 4/8] rust: dma: introduce dma::CoherentInit for memory initialization

[Danilo Krummrich]

On Tue Mar 17, 2026 at 7:52 AM CET, Alice Ryhl wrote:
> On Tue, Mar 03, 2026 at 05:22:55PM +0100, Danilo Krummrich wrote:
>> Currently, dma::Coherent cannot safely provide (mutable) access to its
>> underlying memory because the memory might be concurrently accessed by a
>> DMA device. This makes it difficult to safely initialize the memory
>> before handing it over to the hardware.
>> 
>> Introduce dma::CoherentInit, a type that encapsulates a dma::Coherent
>> before its DMA address is exposed to the device. dma::CoherentInit can
>> guarantee exclusive access to the inner dma::Coherent and implement
>> Deref and DerefMut.
>> 
>> Once the memory is properly initialized, dma::CoherentInit can be
>> converted into a regular dma::Coherent.
>> 
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>
> overall LGTM
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Gary and me concluded that dma::CoherentBox would probably be a better name for
what this structure represents. I.e. you can carry it around and use it similar
to a "real" Box until it is converted to dma::Coherent in order to make it
available to the device.

Is your RB still valid with this rename?

>> +        let ptr = core::ptr::from_mut(&mut self[i]);
>
> &raw mut self[i].
>
> Alice

Re: [PATCH 4/8] rust: dma: introduce dma::CoherentInit for memory initialization

[Alice Ryhl]

On Tue, Mar 17, 2026 at 3:40 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Tue Mar 17, 2026 at 7:52 AM CET, Alice Ryhl wrote:
> > On Tue, Mar 03, 2026 at 05:22:55PM +0100, Danilo Krummrich wrote:
> >> Currently, dma::Coherent cannot safely provide (mutable) access to its
> >> underlying memory because the memory might be concurrently accessed by a
> >> DMA device. This makes it difficult to safely initialize the memory
> >> before handing it over to the hardware.
> >>
> >> Introduce dma::CoherentInit, a type that encapsulates a dma::Coherent
> >> before its DMA address is exposed to the device. dma::CoherentInit can
> >> guarantee exclusive access to the inner dma::Coherent and implement
> >> Deref and DerefMut.
> >>
> >> Once the memory is properly initialized, dma::CoherentInit can be
> >> converted into a regular dma::Coherent.
> >>
> >> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> >
> > overall LGTM
> >
> > Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
> Gary and me concluded that dma::CoherentBox would probably be a better name for
> what this structure represents. I.e. you can carry it around and use it similar
> to a "real" Box until it is converted to dma::Coherent in order to make it
> available to the device.
>
> Is your RB still valid with this rename?

Yes.

Re: [PATCH 4/8] rust: dma: introduce dma::CoherentInit for memory initialization

[Alexandre Courbot]

On Wed Mar 4, 2026 at 1:22 AM JST, Danilo Krummrich wrote:
<snip>
> @@ -352,6 +358,151 @@ fn from(direction: DataDirection) -> Self {
>      }
>  }
>  
> +/// Initializer type for [`Coherent`].
> +///
> +/// A [`Coherent`] object can't provide access to its memory as (mutable) slice safely, since it
> +/// can't fulfill the requirements for creating a slice. For instance, it is not valid to have a
> +/// (mutable) slice to of the memory of a [`Coherent`] while the memory might be accessed by a
> +/// device.
> +///
> +/// In contrast, this initializer type is able to fulfill the requirements to safely obtain a
> +/// (mutable) slice, as it neither provides access to the DMA address of the embedded [`Coherent`],
> +/// nor can it be used with the DMA projection accessors.
> +///
> +/// Once initialized, this type can be converted to a regular [`Coherent`] object.
> +///
> +/// # Examples
> +///
> +/// `CoherentInit<T>`:
> +///
> +/// ```
> +/// # use kernel::device::{
> +/// #     Bound,
> +/// #     Device,
> +/// # };
> +/// use kernel::dma::{attrs::*,
> +///     Coherent,
> +///     CoherentInit,
> +/// };
> +///
> +/// # fn test(dev: &Device<Bound>) -> Result {
> +/// let mut dmem: CoherentInit<u64> =
> +///     CoherentInit::zeroed_with_attrs(dev, GFP_KERNEL, DMA_ATTR_NO_WARN)?;

Since this is an example, shall we use the simpler
`CoherentInit::zeroed`?

> +/// *dmem = 42;
> +/// let dmem: Coherent<u64> = dmem.into();
> +/// # Ok::<(), Error>(()) }
> +/// ```
> +///
> +/// `CoherentInit<[T]>`:
> +///
> +///
> +/// ```
> +/// # use kernel::device::{
> +/// #     Bound,
> +/// #     Device,
> +/// # };
> +/// use kernel::dma::{attrs::*,
> +///     Coherent,
> +///     CoherentInit,
> +/// };
> +///
> +/// # fn test(dev: &Device<Bound>) -> Result {
> +/// let mut dmem: CoherentInit<[u64]> =
> +///     CoherentInit::zeroed_slice_with_attrs(dev, 4, GFP_KERNEL, DMA_ATTR_NO_WARN)?;

Same here.

> +/// dmem.fill(42);
> +/// let dmem: Coherent<[u64]> = dmem.into();
> +/// # Ok::<(), Error>(()) }
> +/// ```
> +pub struct CoherentInit<T: AsBytes + FromBytes + KnownSize + ?Sized>(Coherent<T>);
> +
> +impl<T: AsBytes + FromBytes> CoherentInit<[T]> {
> +    /// Initializer variant of [`Coherent::zeroed_slice_with_attrs`].
> +    pub fn zeroed_slice_with_attrs(
> +        dev: &device::Device<Bound>,
> +        count: usize,
> +        gfp_flags: kernel::alloc::Flags,
> +        dma_attrs: Attrs,
> +    ) -> Result<Self> {
> +        Coherent::zeroed_slice_with_attrs(dev, count, gfp_flags, dma_attrs).map(Self)
> +    }
> +
> +    /// Same as [CoherentInit::zeroed_slice_with_attrs], but with `dma::Attrs(0)`.
> +    pub fn zeroed_slice(
> +        dev: &device::Device<Bound>,
> +        count: usize,
> +        gfp_flags: kernel::alloc::Flags,
> +    ) -> Result<Self> {
> +        Self::zeroed_slice_with_attrs(dev, count, gfp_flags, Attrs(0))
> +    }
> +
> +    /// Initializes the element at `i` using the given initializer.
> +    ///
> +    /// Returns `EINVAL` if `i` is out of bounds.
> +    pub fn init_at<E>(&mut self, i: usize, init: impl Init<T, E>) -> Result

Should this method be introduced in the next patch, or even in its own
patch? It feels a bit out of place at this stage since the non-array
`CoherentInit` doesn't have an equivalent.

I was also wondering whether we could have an `init` method that
initializes all the elements without having to zero the whole array
first, but I guess it might be a bit difficult to implement in a
flexible enough way.

Re: [PATCH 4/8] rust: dma: introduce dma::CoherentInit for memory initialization

[Danilo Krummrich]

On 3/20/2026 3:35 PM, Alexandre Courbot wrote:
> On Wed Mar 4, 2026 at 1:22 AM JST, Danilo Krummrich wrote:
>> +/// # fn test(dev: &Device<Bound>) -> Result {
>> +/// let mut dmem: CoherentInit<u64> =
>> +///     CoherentInit::zeroed_with_attrs(dev, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
> 
> Since this is an example, shall we use the simpler
> `CoherentInit::zeroed`?

Sure.

>> +    /// Initializes the element at `i` using the given initializer.
>> +    ///
>> +    /// Returns `EINVAL` if `i` is out of bounds.
>> +    pub fn init_at<E>(&mut self, i: usize, init: impl Init<T, E>) -> Result
> 
> Should this method be introduced in the next patch, or even in its own
> patch? It feels a bit out of place at this stage since the non-array
> `CoherentInit` doesn't have an equivalent.

The non-slice variant doesn't need it.

I don't see an advantage creating a separate patch for this.

> I was also wondering whether we could have an `init` method that
> initializes all the elements without having to zero the whole array
> first, but I guess it might be a bit difficult to implement in a
> flexible enough way.

You have this in the next patch, Coherent::init() works with arrays.

Re: [PATCH 3/8] rust: dma: add zeroed constructor to `Coherent`

[Alice Ryhl]

On Tue, Mar 03, 2026 at 05:22:54PM +0100, Danilo Krummrich wrote:
> From: Gary Guo <gary@garyguo.net>
> 
> These constructors create a coherent container of a single object
> instead of slice. They are named `zeroed` and `zeroed_with_attrs` to
> emphasis that they are created initialized zeroed. It is intended that
> there'll be new constructors that take `PinInit` instead of zeroing.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 3/8] rust: dma: add zeroed constructor to `Coherent`

[Alexandre Courbot]

On Wed Mar 4, 2026 at 1:22 AM JST, Danilo Krummrich wrote:
<snip>
> @@ -572,6 +598,41 @@ fn alloc_slice_with_attrs(
>              dma_attrs,
>          })
>      }
> +
> +    /// Allocates a zeroed region of type `T` of coherent memory.
> +    ///
> +    /// Unlike `Coherent::<[T; N]>::zeroed_with_attrs`, `Coherent::<T>::zeored_slices` support

s/zeored_slices/zeroed_slices

Otherwise,

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

Re: [PATCH 6/8] gpu: nova-core: use Coherent::init to initialize GspFwWprMeta

[Alexandre Courbot]

On Wed Mar 4, 2026 at 1:22 AM JST, Danilo Krummrich wrote:
> Convert wpr_meta to use Coherent::init() and simplify the
> initialization.  It also avoids a separate initialization of
> GspFwWprMeta on the stack.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

Re: [PATCH 8/8] gpu: nova-core: convert to new dma::Coherent API

[Alexandre Courbot]

On Wed Mar 4, 2026 at 1:22 AM JST, Danilo Krummrich wrote:
> From: Gary Guo <gary@garyguo.net>
>
> Remove all usages of dma::CoherentAllocation and use the new
> dma::Coherent type instead.
>
> Note that there are still remainders of the old dma::CoherentAllocation
> API, such as as_slice() and as_slice_mut().
>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  drivers/gpu/nova-core/dma.rs      | 19 +++++------
>  drivers/gpu/nova-core/falcon.rs   |  7 ++--
>  drivers/gpu/nova-core/firmware.rs | 10 ++----
>  drivers/gpu/nova-core/gsp.rs      | 18 ++++++----
>  drivers/gpu/nova-core/gsp/cmdq.rs | 55 ++++++++++++-------------------
>  5 files changed, 46 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/dma.rs b/drivers/gpu/nova-core/dma.rs
> index 7215398969da..3c19d5ffcfe8 100644
> --- a/drivers/gpu/nova-core/dma.rs
> +++ b/drivers/gpu/nova-core/dma.rs
> @@ -9,13 +9,13 @@
>  
>  use kernel::{
>      device,
> -    dma::CoherentAllocation,
> +    dma::Coherent,
>      page::PAGE_SIZE,
>      prelude::*, //
>  };
>  
>  pub(crate) struct DmaObject {
> -    dma: CoherentAllocation<u8>,
> +    dma: Coherent<[u8]>,
>  }

Actually, do we even still have a need for `DmaObject` now that
`Coherent` is available? It would be nice to check (as a follow-up
patch, I can look into it) whether we can remove that whole nova-local
`dma` module.

>  
>  impl DmaObject {
> @@ -24,23 +24,22 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, len: usize) -> Result<Sel
>              .map_err(|_| EINVAL)?
>              .pad_to_align()
>              .size();
> -        let dma = CoherentAllocation::alloc_coherent(dev, len, GFP_KERNEL | __GFP_ZERO)?;
> +        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> {
> -        Self::new(dev, data.len()).and_then(|mut dma_obj| {
> -            // 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.write(data, 0)? }
> -            Ok(dma_obj)
> -        })
> +        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 = CoherentAllocation<u8>;
> +    type Target = Coherent<[u8]>;
>  
>      fn deref(&self) -> &Self::Target {
>          &self.dma
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> index 37bfee1d0949..39f5df568ddb 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -25,10 +25,7 @@
>      driver::Bar0,
>      falcon::hal::LoadMethod,
>      gpu::Chipset,
> -    num::{
> -        FromSafeCast,
> -        IntoSafeCast, //
> -    },
> +    num::FromSafeCast,
>      regs,
>      regs::macros::RegisterBase, //
>  };
> @@ -434,7 +431,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
>              }
>              FalconMem::Dmem => (
>                  0,
> -                fw.dma_handle_with_offset(load_offsets.src_start.into_safe_cast())?,
> +                fw.dma_handle() + DmaAddress::from(load_offsets.src_start),

Aren't we losing the bounds checking done by `dma_handle_with_offset`?

Mmm, but looking below we are checking that the end of the transfer
doesn't go beyond the object bounds, so that was probably redundant
anyway. Maybe we should do a `checked_add` still for safety.

Re: [PATCH 8/8] gpu: nova-core: convert to new dma::Coherent API

[Gary Guo]

On Fri Mar 20, 2026 at 2:36 PM GMT, Alexandre Courbot wrote:
> On Wed Mar 4, 2026 at 1:22 AM JST, Danilo Krummrich wrote:
>> From: Gary Guo <gary@garyguo.net>
>>
>> Remove all usages of dma::CoherentAllocation and use the new
>> dma::Coherent type instead.
>>
>> Note that there are still remainders of the old dma::CoherentAllocation
>> API, such as as_slice() and as_slice_mut().
>>
>> Signed-off-by: Gary Guo <gary@garyguo.net>
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> ---
>>  drivers/gpu/nova-core/dma.rs      | 19 +++++------
>>  drivers/gpu/nova-core/falcon.rs   |  7 ++--
>>  drivers/gpu/nova-core/firmware.rs | 10 ++----
>>  drivers/gpu/nova-core/gsp.rs      | 18 ++++++----
>>  drivers/gpu/nova-core/gsp/cmdq.rs | 55 ++++++++++++-------------------
>>  5 files changed, 46 insertions(+), 63 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/dma.rs b/drivers/gpu/nova-core/dma.rs
>> index 7215398969da..3c19d5ffcfe8 100644
>> --- a/drivers/gpu/nova-core/dma.rs
>> +++ b/drivers/gpu/nova-core/dma.rs
>> @@ -9,13 +9,13 @@
>>  
>>  use kernel::{
>>      device,
>> -    dma::CoherentAllocation,
>> +    dma::Coherent,
>>      page::PAGE_SIZE,
>>      prelude::*, //
>>  };
>>  
>>  pub(crate) struct DmaObject {
>> -    dma: CoherentAllocation<u8>,
>> +    dma: Coherent<[u8]>,
>>  }
>
> Actually, do we even still have a need for `DmaObject` now that
> `Coherent` is available? It would be nice to check (as a follow-up
> patch, I can look into it) whether we can remove that whole nova-local
> `dma` module.
>
>>  
>>  impl DmaObject {
>> @@ -24,23 +24,22 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, len: usize) -> Result<Sel
>>              .map_err(|_| EINVAL)?
>>              .pad_to_align()
>>              .size();
>> -        let dma = CoherentAllocation::alloc_coherent(dev, len, GFP_KERNEL | __GFP_ZERO)?;
>> +        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> {
>> -        Self::new(dev, data.len()).and_then(|mut dma_obj| {
>> -            // 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.write(data, 0)? }
>> -            Ok(dma_obj)
>> -        })
>> +        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 = CoherentAllocation<u8>;
>> +    type Target = Coherent<[u8]>;
>>  
>>      fn deref(&self) -> &Self::Target {
>>          &self.dma
>> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
>> index 37bfee1d0949..39f5df568ddb 100644
>> --- a/drivers/gpu/nova-core/falcon.rs
>> +++ b/drivers/gpu/nova-core/falcon.rs
>> @@ -25,10 +25,7 @@
>>      driver::Bar0,
>>      falcon::hal::LoadMethod,
>>      gpu::Chipset,
>> -    num::{
>> -        FromSafeCast,
>> -        IntoSafeCast, //
>> -    },
>> +    num::FromSafeCast,
>>      regs,
>>      regs::macros::RegisterBase, //
>>  };
>> @@ -434,7 +431,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
>>              }
>>              FalconMem::Dmem => (
>>                  0,
>> -                fw.dma_handle_with_offset(load_offsets.src_start.into_safe_cast())?,
>> +                fw.dma_handle() + DmaAddress::from(load_offsets.src_start),
>
> Aren't we losing the bounds checking done by `dma_handle_with_offset`?
>
> Mmm, but looking below we are checking that the end of the transfer
> doesn't go beyond the object bounds, so that was probably redundant
> anyway. Maybe we should do a `checked_add` still for safety.

I've had this as `dma_handle_with_offset` no longer make sense in the context of
generic `Coherent` type. Although, I can probably add something like:

impl View<'_, Coherent<...>, ...> {
    fn dma_handle(&self) -> DmaAddress;
}

to replace this so you can get the DMA handle for any projections of the
coherent object (and the bounds checking is done when projecting).

Best,
Gary

Re: [PATCH 0/8] dma::Coherent & dma::CoherentInit API

[Andreas Hindborg]

"Danilo Krummrich" <dakr@kernel.org> writes:

> This patch series introduces the dma::Coherent API Gary worked out in the
> context of his I/O projection work.
>
> Additionally, introduce dma::CoherentInit, a type that encapsulates a
> dma::Coherent object before its DMA address is exposed to the device.
> dma::CoherentInit can guarantee exclusive access to the inner dma::Coherent
> object and implement Deref and DerefMut.
>
> Also add Coherent::init() and Coherent::init_with_attrs() so we can directly
> initialize a new dma::Coherent object through an impl Init<T, E>.
>
> Danilo Krummrich (5):
>   rust: dma: use "kernel vertical" style for imports
>   rust: dma: introduce dma::CoherentInit for memory initialization
>   rust: dma: add Coherent:init() and Coherent::init_with_attrs()
>   gpu: nova-core: use Coherent::init to initialize GspFwWprMeta
>   gpu: nova-core: convert Gsp::new() to use CoherentInit
>
> Gary Guo (3):
>   rust: dma: add generalized container for types other than slices
>   rust: dma: add zeroed constructor to `Coherent`
>   gpu: nova-core: convert to new dma::Coherent API
>
>  drivers/gpu/nova-core/dma.rs      |  19 +-
>  drivers/gpu/nova-core/falcon.rs   |   7 +-
>  drivers/gpu/nova-core/firmware.rs |  10 +-
>  drivers/gpu/nova-core/gsp.rs      |  65 ++--
>  drivers/gpu/nova-core/gsp/boot.rs |   7 +-
>  drivers/gpu/nova-core/gsp/cmdq.rs |  55 +--
>  drivers/gpu/nova-core/gsp/fw.rs   |  82 ++--
>  rust/kernel/device.rs             |   4 +-
>  rust/kernel/dma.rs                | 626 +++++++++++++++++++++++-------
>  samples/rust/rust_dma.rs          |   8 +-
>  10 files changed, 619 insertions(+), 264 deletions(-)
>
>
> base-commit: 1195fcbda62f12108dc9be56fa4173897905b90c

What did you base this on?


Best regards,
Andreas Hindborg

Re: [PATCH 0/8] dma::Coherent & dma::CoherentInit API

[Danilo Krummrich]

On 3/24/26 1:33 PM, Andreas Hindborg wrote:
> What did you base this on?

drm-rust-next

Re: [PATCH 0/8] dma::Coherent & dma::CoherentInit API

[Andreas Hindborg]

"Danilo Krummrich" <dakr@kernel.org> writes:

> On 3/24/26 1:33 PM, Andreas Hindborg wrote:
>> What did you base this on?
>
> drm-rust-next

I don't think 1195fcbda62f12108dc9be56fa4173897905b90c is in
drm-rust-next - or any of the other branches in of
https://gitlab.freedesktop.org/drm/rust/kernel.git .


Best regards,
Andreas Hindborg

Re: [PATCH 0/8] dma::Coherent & dma::CoherentInit API

[Danilo Krummrich]

On 3/24/26 1:46 PM, Andreas Hindborg wrote:
> "Danilo Krummrich" <dakr@kernel.org> writes:
> 
>> On 3/24/26 1:33 PM, Andreas Hindborg wrote:
>>> What did you base this on?
>>
>> drm-rust-next
> 
> I don't think 1195fcbda62f12108dc9be56fa4173897905b90c is in
> drm-rust-next - or any of the other branches in of
> https://gitlab.freedesktop.org/drm/rust/kernel.git .

Oh, I did not notice that you replied to v1, there's already a v2.

[1] https://lore.kernel.org/driver-core/20260320194626.36263-1-dakr@kernel.org/