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 <[email protected]> >>> --- >> >> 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.
