Re: [PATCH v9 24/31] gpu: nova-core: Hopper/Blackwell: add FSP Chain of Trust boot

[John Hubbard]

On 3/30/26 8:11 AM, Alexandre Courbot wrote:
> On Thu Mar 26, 2026 at 10:38 AM JST, John Hubbard wrote:
>> Add boot_fmc() which builds and sends the Chain of Trust message to FSP,
>> and FmcBootArgs which bundles the DMA-coherent boot parameters that FSP
>> reads at boot time. The FspFirmware struct fields become pub(crate) and
>> fmc_full changes from DmaObject to KVec<u8> for CPU-side signature
>> extraction.
>>
>> Co-developed-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
>> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
>> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
>> ---
>>  drivers/gpu/nova-core/firmware/fsp.rs |   8 +-
>>  drivers/gpu/nova-core/fsp.rs          | 170 +++++++++++++++++++++++++-
>>  drivers/gpu/nova-core/gpu.rs          |   1 -
>>  drivers/gpu/nova-core/mctp.rs         |   7 --
>>  4 files changed, 172 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/firmware/fsp.rs b/drivers/gpu/nova-core/firmware/fsp.rs
>> index 028f651553e0..5bd15b644825 100644
>> --- a/drivers/gpu/nova-core/firmware/fsp.rs
>> +++ b/drivers/gpu/nova-core/firmware/fsp.rs
>> @@ -14,16 +14,16 @@
>>      gpu::Chipset, //
>>  };
>>  
>> -#[expect(unused)]
>> +#[expect(dead_code)]
> 
> Why?

`dead_code` is the specific lint that fires here. `unused` is a lint
group that *contains* `dead_code` plus a dozen others (unused_imports,
unused_variables, unused_mut, etc). Since the only warning is "this
struct is never constructed," `dead_code` names the exact lint.

That said, you're right that this change doesn't belong in this patch.
I've moved it to the introducing commit in v10.

> 
>>  pub(crate) struct FspFirmware {
>>      /// FMC firmware image data (only the "image" ELF section).
>> -    fmc_image: DmaObject,
>> -    /// Full FMC ELF (for signature extraction.
>> +    pub(crate) fmc_image: DmaObject,
>> +    /// Full FMC ELF for signature extraction.
>>      pub(crate) fmc_elf: Firmware,
> 
> Let's make the comment for `fmc_elf` correct since its introduction
> instead of fixing it here?

Agreed, done in v10. The introducing commit now has the correct
comment from the start.

> 
>>  }
>>  
>>  impl FspFirmware {
>> -    #[expect(unused)]
>> +    #[expect(dead_code)]
> 
> Same here, I don't see the point. If it's not unused anymore, let's
> remove the `expect` entirely. If it is, let's keep the more accurate
> `unused`.

One small correction: `unused` is actually the *less* specific choice.
It is a lint group containing dead_code, unused_imports,
unused_variables, and about a dozen others. `dead_code` is the single
lint that actually fires on an unreferenced struct or function.
#[expect(...)] is meant to name the expected warning, so using the
specific lint is more precise.

But you're right that this change belonged in the introducing commit,
not here. Fixed in v10: the introducing commit uses
#[expect(dead_code)] from the start, and this patch only changes
fmc_image to pub(crate).

> 
> Btw, this is the kind of stuff you'd normally catch by looking at the
> diff before sending it. I shouldn't have to do this.

I actually spend lots of time fooling around with dead_code and
unused, trying to get it just right. There are a lot of patches that
add and remove these.

In this case, after the dust settled after applying the v8 review
feedback, I did notice this unused-->dead_code diff that we
are discussing, but I just let it go, because it's only a couple
of lines that end up getting vaporized by the end of the series
anyway, and I was anxious to get all the other things (especially
Gary/Sashiko's big discovery about the two Blackwell arches)
posted.


thanks,
-- 
John Hubbard