Re: [PATCH v11 18/22] gpu: nova-core: Hopper/Blackwell: add FspCotVersion type

[John Hubbard]

On 6/1/26 7:07 AM, Alexandre Courbot wrote:
> On Sat May 30, 2026 at 12:09 PM JST, John Hubbard wrote:
...
>> +impl FspCotVersion {
>> +    /// Creates a new FSP CoT version.
>> +    pub(crate) const fn new(version: u16) -> Self {
>> +        Self(version)
>> +    }
>> +
>> +    /// Returns the raw protocol version number for the wire format.
>> +    #[expect(dead_code)]
>> +    pub(crate) const fn raw(self) -> u16 {
>> +        self.0
>> +    }
>> +}
> 
> This type seems to just wrap a `u16`, and return its raw value without
> any limitation for its range, or other functionality. It is just created
> in `Chipset::fsp_cot_version`, to be immediately unpacked by
> `Fsp::boot_fmc`. Why not just use a `u16` directly?

OK, will do that.

> 
>> +
>>  /// FSP message timeout in milliseconds.
>>  const FSP_MSG_TIMEOUT_MS: i64 = 2000;
>>  
>> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
>> index 7dd736e5b190..6cdface3c618 100644
>> --- a/drivers/gpu/nova-core/gpu.rs
>> +++ b/drivers/gpu/nova-core/gpu.rs
>> @@ -21,6 +21,7 @@
>>          Falcon, //
>>      },
>>      fb::SysmemFlush,
>> +    fsp::FspCotVersion,
>>      gsp::{
>>          self,
>>          Gsp, //
>> @@ -141,6 +142,21 @@ pub(crate) const fn needs_fwsec_bootloader(self) -> bool {
>>      pub(crate) fn pci_config_mirror_range(self) -> Range<u32> {
>>          hal::gpu_hal(self).pci_config_mirror_range()
>>      }
>> +
>> +    /// Returns the FSP Chain of Trust (CoT) protocol version for this chipset.
>> +    ///
>> +    /// Hopper (GH100) uses version 1, Blackwell uses version 2.
>> +    /// Returns `None` for architectures that do not use FSP.
>> +    #[expect(dead_code)]
>> +    pub(crate) const fn fsp_cot_version(self) -> Option<FspCotVersion> {
>> +        match self.arch() {
>> +            Architecture::Hopper => Some(FspCotVersion::new(1)),
>> +            Architecture::BlackwellGB10x | Architecture::BlackwellGB20x => {
>> +                Some(FspCotVersion::new(2))
>> +            }
>> +            _ => None,
>> +        }
>> +    }
> 
> This is only used in the `fsp` module - can we turn this into a FSP HAL
> method? That way it also won't need to return an `Option` since
> `fsp_hal` will already have filtered architectures that don't support
> FSP.

Yes. This was one of those snippets that I had flagged as "might need
to be a HAL", so I'll go ahead and make it a HAL now that you've also
suggested that.

thanks,
-- 
John Hubbard