Re: [PATCH v8 05/31] gpu: nova-core: set DMA mask width based on GPU architecture

[John Hubbard]

On 3/25/26 3:53 AM, Alexandre Courbot wrote:
> On Tue, 24 Mar 2026 20:52:16 -0700, John Hubbard <jhubbard@xxxxxxxxxx> wrote:
>> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
>> index 84b0e1703150..41227d29934e 100644
>> --- a/drivers/gpu/nova-core/driver.rs
>> +++ b/drivers/gpu/nova-core/driver.rs
>> @@ -84,18 +78,20 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
>> [ ... skip 13 lines ... ]
>> +            dev_info!(pdev, "NVIDIA ({})\n", spec);
>> +
>> +            // SAFETY: No concurrent DMA allocations or mappings can be made because
>> +            // the device is still being probed and therefore isn't being used by
>> +            // other threads of execution.
>> +            unsafe { pdev.dma_set_mask_and_coherent(spec.chipset().arch().dma_mask())? };
> 
> To limit the unsafe block to unsafe operations, we should store the DMA
> mask into a local variable.

Done, thanks.

> 
>>
>> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
>> index 3cd7709883be..e7c3860cfb28 100644
>> --- a/drivers/gpu/nova-core/gpu.rs
>> +++ b/drivers/gpu/nova-core/gpu.rs
>> @@ -162,6 +163,16 @@ pub(crate) enum Architecture {
>>      Blackwell = 0x1b,
>>  }
>>  
>> +impl Architecture {
>> +    /// Returns the DMA mask supported by this architecture.
>> +    pub(crate) const fn dma_mask(&self) -> DmaMask {
>> +        match self {
>> +            Self::Turing | Self::Ampere | Self::Ada => DmaMask::new::<47>(),
>> +            Self::Hopper | Self::Blackwell => DmaMask::new::<52>(),
>> +        }
>> +    }
>> +}
> 
> Eventually we may want to move this to the `Gpu` HAL, but let's do that
> as a follow-up patch if we find it is relevant (which I am not sure
> about yet).

Ack. 

> 
>> @@ -295,32 +306,32 @@ pub(crate) fn new<'a>(
>>          pdev: &'a pci::Device<device::Bound>,
>>          devres_bar: Arc<Devres<Bar0>>,
>>          bar: &'a Bar0,
>> +        spec: Spec,
>>      ) -> impl PinInit<Self, Error> + 'a {
>> -        try_pin_init!(Self {
>> -            spec: Spec::new(pdev.as_ref(), bar).inspect(|spec| {
>> -                dev_info!(pdev, "NVIDIA ({})\n", spec);
>> -            })?,
>> +        let chipset = spec.chipset();
> 
> `spec` is now `Copy`, so we don't need this local variable anymore.
> Removing it and accessing `spec.chipset` directly simplifies the
> remainder of the diff.
> 

Fixed, thanks.

thanks,
-- 
John Hubbard