Re: [PATCH v9 02/31] gpu: nova-core: factor .fwsignature* selection into a new find_gsp_sigs_section()

John Hubbard
2026-03-30T17:51:15Z

On 3/30/26 7:29 AM, Alexandre Courbot wrote:
> On Thu Mar 26, 2026 at 10:38 AM JST, John Hubbard wrote:
>> Keep Gsp::new() from getting too cluttered, by factoring out the
>> selection of .fwsignature* items. This will continue to grow as we add
>> GPUs.
>>
>> Reviewed-by: Gary Guo <gary@garyguo.net>
>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
>> ---
>> drivers/gpu/nova-core/firmware/gsp.rs | 31 +++++++++++++++------------
>> 1 file changed, 17 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
>> index 2bbea1db0238..60ea730d1bd5 100644
>> --- a/drivers/gpu/nova-core/firmware/gsp.rs
>> +++ b/drivers/gpu/nova-core/firmware/gsp.rs
>> @@ -146,6 +146,22 @@ pub(crate) struct GspFirmware {
>> }
>>
>> impl GspFirmware {
>> + fn find_gsp_sigs_section(chipset: Chipset) -> Option<&'static str> {
>> + match chipset.arch() {
>> + Architecture::Turing if matches!(chipset, Chipset::TU116 | Chipset::TU117) => {
>> + Some(".fwsignature_tu11x")
>> + }
>> + Architecture::Turing => Some(".fwsignature_tu10x"),
>> + // GA100 uses the same firmware as Turing
>> + Architecture::Ampere if chipset == Chipset::GA100 => Some(".fwsignature_tu10x"),
>> + Architecture::Ampere => Some(".fwsignature_ga10x"),
>> + Architecture::Ada => Some(".fwsignature_ad10x"),
>> + Architecture::Hopper => Some(".fwsignature_gh10x"),
>> + Architecture::BlackwellGB10x => Some(".fwsignature_gb10x"),
>> + Architecture::BlackwellGB20x => Some(".fwsignature_gb20x"),
>> + }
>> + }
>> +
>
> So in v8 I pointed out, on this very patch, that this method doesn't
> need to return an `Option` [1]. Which you agreed to [2]. And yet this is
> unchanged?
>
> Oh, actually this is fixed in the next patch, for whatever reason. Why?

It's an off-by-one error in chosing which commit to edit during git
rebase.

> The original match statement was even exhaustive to begin with, so I
> don't see why that `Option` was even introduced in the first place.

Because at various points during the many months that this patchset
has existed, there were unsupported Arches. Turing, for example.

thanks,
--
John Hubbard