Re: [PATCH v4 09/13] gpu: nova-core: introduce GspBootMethod

["Alexandre Courbot" <acourbot@nvidia.com>]

On Tue Jul 7, 2026 at 10:21 AM JST, Alexandre Courbot wrote:
> On Thu Jul 2, 2026 at 11:28 AM JST, Eliot Courtney wrote:
>> On Mon Jun 29, 2026 at 11:09 PM JST, Alexandre Courbot wrote:
>>> The GSP boot method is currently determined by two ad-hoc methods of
>>> `Chipset`: `uses_fsp` (a boolean telling whether to use the FSP boot
>>> path or the Sec2 Booter one) and `needs_fwsec_bootloader` (another
>>> boolean valid only for the Sec2 Booter method that tells whether the
>>> FWSEC bootloader must be used).
>>>
>>> This is neither extensible nor sound: the combination `uses_fsp &&
>>> needs_fwsec_bootloader` is invalid, but can still be expressed.
>>>
>>> Thus, unify these two predicates into a single `gsp_boot_method` method
>>> that returns an enum type unambiguously describing the boot method to
>>> use. This ensures that no invalid combination can be expressed, which
>>> makes matching sounder.
>>>
>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>> ---
>>
>> Ideally we would not need to expose bootloader details (needs fwsec
>> bootloader, mapping of architecture to GspBootMethod) outside of the
>> HAL, and also not have to duplicate the logic for determining
>> `needs_fwsec_bootloader` in both the HAL and Chipset. One thing
>> preventing keeping all this in the HALs is that we need to know the set
>> of firmware files in a const context, which can't go through the &dyn
>> for HALs. I also find it a bit odd to go Chipset->GspBootMethod->HAL
>> which relies on the GSP HAL being entirely determined by the boot method
>> which doesn't feel generally correct to me.
>
> So far it is a good mapping though - the only thing abstracted by the
> GSP HAL is how we boot and stop the GSP, which is entirely determined by
> the boot method used.
>
> While I agree that not exposing these details outside of the HAL would
> be nice, at least for the time being we need to - the creation of the
> `Fsp` happens at the `Gpu` level, and is now conditional on the boot
> method. Thus the introduction of `GspBootMethod`. I would be happy to
> remove this in the future if we can, but that doesn't look possible
> without more refactoring, and this series is already doing quite a bit
> of it just to move the `Fsp` instance up. I think going further should
> be a separate effort.

Need to give myself a self-rebuke here. :)

The only thing that still required the boot method leaking out of the
HAL was the instantiation of `Fsp`, and that can also be taken care of
by adding a constructor that returns an `Option<Fsp>`. It integrates
nicely with the already-existing `Fsp` HAL, and combining that with
Eliot's idea of providing the firmware files from the GSP HAL we can
remove the `Chipset` ad-hoc boolean method entirely, without needing to
introduce `GspBootMethod`.

This makes things look cleaner overall, so I'll use this approach for
v5.