1. Patchew
  2. linux
  3. [PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core
  4. View patch

[PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent

Alexandre Courbot posted 7 patches 1 week, 6 days ago
There is a newer version of this series
[PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent
Posted by Alexandre Courbot 1 week, 6 days ago 
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
Re: [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent
Posted by Gary Guo 1 week, 6 days ago 
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 {
Re: [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent
Posted by Alexandre Courbot 1 week, 4 days ago 
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.
Re: [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent
Posted by Gary Guo 1 week, 4 days ago 
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
Re: [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent
Posted by Alexandre Courbot 1 week, 4 days ago 
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. :)

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.