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

["Eliot Courtney" <ecourtney@nvidia.com>]

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.

What if we added a const fn n gsp/hal.rs called like boot_firmware_files
( or whatever) which returns the extra firmware files needed to boot the
GSP based on the architecture. Then we don't need to expose details like
`needs_fwsec_bootloader` or `GspBootMethod` outside of the HALs. We
still need another set of matches but it's unavoidable AFAICT without
being able to know the HAL type in a const context, at least it's
consolidated and in the HAL this way. In the future we might consider
type parameterising `Gpu` with the chipset and types that it uses with
the minimal set of strategies that it needs (e.g. sec2 vs fsp boot) then
we could even make the HALs not use dyn and push a bunch of stuff into
associated types.

That looks like this (on top of patch 8):

WDYT?

diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index 80e948bf7511..1c02d80de7bc 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -423,24 +423,19 @@ const fn make_entry_file(self, chipset: &str, fw: &str) -> Self {
     const fn make_entry_chipset(self, chipset: gpu::Chipset) -> Self {
         let name = chipset.name();
 
-        let this = self
+        let mut this = self
             .make_entry_file(name, "bootloader")
             .make_entry_file(name, "gsp");
 
-        // FSP-based chipsets (Hopper, Blackwell and later) boot the GSP via the FMC image loaded by
-        // FSP. Older chipsets use the SEC2 booter instead.
-        let this = if chipset.uses_fsp() {
-            this.make_entry_file(name, "fmc")
-        } else {
-            this.make_entry_file(name, "booter_load")
-                .make_entry_file(name, "booter_unload")
-        };
-
-        if chipset.needs_fwsec_bootloader() {
-            this.make_entry_file(name, "gen_bootloader")
-        } else {
-            this
+        // Add the firmware files specific to the GSP boot method of `chipset`.
+        let boot_files = crate::gsp::boot_firmware_files(chipset);
+        let mut i = 0;
+        while i < boot_files.len() {
+            this = this.make_entry_file(name, boot_files[i]);
+            i += 1;
         }
+
+        this
     }
 
     pub(crate) const fn create(
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index 95e0dd77746b..7a931f22f629 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -385,9 +385,8 @@ pub(crate) fn new(
 
     /// Loads the FWSEC firmware into `falcon` and execute it.
     ///
-    /// This must only be called on chipsets that do not need the FWSEC bootloader (i.e., where
-    /// [`Chipset::needs_fwsec_bootloader()`](crate::gpu::Chipset::needs_fwsec_bootloader) returns
-    /// `false`). On chipsets that do, use [`bootloader::FwsecFirmwareWithBl`] instead.
+    /// This must only be called on chipsets that do not need the FWSEC bootloader. On chipsets
+    /// where the bootloader is required, use [`bootloader::FwsecFirmwareWithBl`] instead.
     pub(crate) fn run(&self, dev: &Device<device::Bound>, falcon: &Falcon<'_, Gsp>) -> Result<()> {
         // Reset falcon, load the firmware, and run it.
         falcon
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 32bfa0be2357..c04706b60ba8 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -133,22 +133,6 @@ pub(crate) const fn arch(self) -> Architecture {
         }
     }
 
-    /// Returns `true` if this chipset requires the PIO-loaded bootloader in order to boot FWSEC.
-    ///
-    /// This includes all chipsets < GA102.
-    pub(crate) const fn needs_fwsec_bootloader(self) -> bool {
-        matches!(self.arch(), Architecture::Turing) || matches!(self, Self::GA100)
-    }
-
-    /// Returns `true` if this chipset boots via FSP (Hopper and later), which requires the FMC
-    /// firmware image.
-    pub(crate) const fn uses_fsp(self) -> bool {
-        matches!(
-            self.arch(),
-            Architecture::Hopper | Architecture::BlackwellGB10x | Architecture::BlackwellGB20x
-        )
-    }
-
     /// Returns the address range of the PCI config mirror space.
     pub(crate) fn pci_config_mirror_range(self) -> Range<u32> {
         hal::gpu_hal(self).pci_config_mirror_range()
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index b4ac4156056e..d5cca3c8350a 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -30,6 +30,7 @@
     GspFwWprMeta,
     LibosParams, //
 };
+pub(crate) use hal::boot_firmware_files;
 
 use crate::{
     driver::Bar0,
diff --git a/drivers/gpu/nova-core/gsp/hal.rs b/drivers/gpu/nova-core/gsp/hal.rs
index eddb4e8bf510..9da078dd1059 100644
--- a/drivers/gpu/nova-core/gsp/hal.rs
+++ b/drivers/gpu/nova-core/gsp/hal.rs
@@ -57,6 +57,25 @@ fn post_boot(&self, _gsp: &Gsp, _ctx: &GspBootContext<'_>, _gsp_fw: &GspFirmware
     }
 }
 
+/// Returns the names of the firmware files required to boot the GSP of `chipset`, in addition to
+/// the "bootloader" and "gsp" images required by all chipsets.
+pub(crate) const fn boot_firmware_files(chipset: Chipset) -> &'static [&'static str] {
+    match chipset.arch() {
+        // Turing chipsets boot the GSP via the SEC2 Booter, and require the FWSEC bootloader.
+        Architecture::Turing => &["booter_load", "booter_unload", "gen_bootloader"],
+        // GA100 also requires the FWSEC bootloader.
+        Architecture::Ampere if matches!(chipset, Chipset::GA100) => {
+            &["booter_load", "booter_unload", "gen_bootloader"]
+        }
+        // Other Ampere chipsets, as well as Ada chipsets, run FWSEC directly.
+        Architecture::Ampere | Architecture::Ada => &["booter_load", "booter_unload"],
+        // Hopper and later chipsets boot the GSP via the FMC image loaded by FSP.
+        Architecture::Hopper | Architecture::BlackwellGB10x | Architecture::BlackwellGB20x => {
+            &["fmc"]
+        }
+    }
+}
+
 /// Returns the GSP HAL to be used for `chipset`.
 pub(super) fn gsp_hal(chipset: Chipset) -> &'static dyn GspHal {
     match chipset.arch() {