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 <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Danilo Krummrich <[email protected]>
>> ---
>> 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
