Re: [PATCH v4 5/8] gpu: nova-core: refactor SEC2 booter loading into BooterFirmware::run()

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

On Mon Apr 27, 2026 at 3:57 PM JST, Alexandre Courbot wrote:
> From: John Hubbard <jhubbard@nvidia.com>
>
> Move the SEC2 reset/load/boot sequence into a BooterFirmware::run()
> method, and call it from a thin run_booter() helper on Gsp. This is a
> pure refactoring with no behavior change, done in preparation for adding
> an alternative FSP boot path.

nit: this isn't no behaviour change because it moves a failure in
BooterFirmware::new to after a bunch of messages to GSP and GSP boot.
But assuming that is fine,

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>

>
> Suggested-by: Danilo Krummrich <dakr@kernel.org>
> Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  drivers/gpu/nova-core/firmware/booter.rs | 30 ++++++++++++++++++++++
>  drivers/gpu/nova-core/gsp/boot.rs        | 43 +++++++++++++++-----------------
>  2 files changed, 50 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
> index de2a4536b532..6a41690e72c6 100644
> --- a/drivers/gpu/nova-core/firmware/booter.rs
> +++ b/drivers/gpu/nova-core/firmware/booter.rs
> @@ -8,6 +8,7 @@
>  
>  use kernel::{
>      device,
> +    dma::Coherent,
>      prelude::*,
>      transmute::FromBytes, //
>  };
> @@ -396,6 +397,35 @@ pub(crate) fn new(
>              ucode: ucode_signed,
>          })
>      }
> +
> +    /// Load and run the booter firmware on SEC2.
> +    ///
> +    /// Resets SEC2, loads this firmware image, then boots with the WPR metadata
> +    /// address passed via the SEC2 mailboxes.
> +    pub(crate) fn run<T>(
> +        &self,
> +        dev: &device::Device<device::Bound>,
> +        bar: &Bar0,
> +        sec2_falcon: &Falcon<Sec2>,
> +        wpr_meta: &Coherent<T>,
> +    ) -> Result {
> +        sec2_falcon.reset(bar)?;
> +        sec2_falcon.load(dev, bar, self)?;
> +        let wpr_handle = wpr_meta.dma_handle();
> +        let (mbox0, mbox1) = sec2_falcon.boot(
> +            bar,
> +            Some(wpr_handle as u32),
> +            Some((wpr_handle >> 32) as u32),

nit: should we have // CAST comments here? fine to ignore this since
this this patch is just moving code though.

> +        )?;
> +        dev_dbg!(dev, "SEC2 MBOX0: {:#x}, MBOX1: {:#x}\n", mbox0, mbox1);
> +
> +        if mbox0 != 0 {
> +            dev_err!(dev, "Booter-load failed with error {:#x}\n", mbox0);
> +            return Err(ENODEV);
> +        }
> +
> +        Ok(())
> +    }
>  }
>  
>  impl FalconDmaLoadable for BooterFirmware {
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> index d39da38c8918..e75c4b235aab 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -136,6 +136,25 @@ fn run_fwsec_frts(
>          }
>      }
>  
> +    /// Load and run the booter firmware.
> +    fn run_booter(
> +        dev: &device::Device<device::Bound>,
> +        bar: &Bar0,
> +        chipset: Chipset,
> +        sec2_falcon: &Falcon<Sec2>,
> +        wpr_meta: &Coherent<GspFwWprMeta>,
> +    ) -> Result {
> +        BooterFirmware::new(
> +            dev,
> +            BooterKind::Loader,
> +            chipset,
> +            FIRMWARE_VERSION,
> +            sec2_falcon,
> +            bar,
> +        )?
> +        .run(dev, bar, sec2_falcon, wpr_meta)
> +    }
> +
>      /// Attempt to boot the GSP.
>      ///
>      /// This is a GPU-dependent and complex procedure that involves loading firmware files from
> @@ -173,15 +192,6 @@ pub(crate) fn boot(
>              Self::run_fwsec_frts(dev, chipset, gsp_falcon, bar, &bios, &fb_layout)?;
>          }
>  
> -        let booter_loader = BooterFirmware::new(
> -            dev,
> -            BooterKind::Loader,
> -            chipset,
> -            FIRMWARE_VERSION,
> -            sec2_falcon,
> -            bar,
> -        )?;
> -
>          let wpr_meta = Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
>  
>          self.cmdq
> @@ -203,20 +213,7 @@ pub(crate) fn boot(
>              "Using SEC2 to load and run the booter_load firmware...\n"
>          );
>  
> -        sec2_falcon.reset(bar)?;
> -        sec2_falcon.load(dev, bar, &booter_loader)?;
> -        let wpr_handle = wpr_meta.dma_handle();
> -        let (mbox0, mbox1) = sec2_falcon.boot(
> -            bar,
> -            Some(wpr_handle as u32),
> -            Some((wpr_handle >> 32) as u32),
> -        )?;
> -        dev_dbg!(pdev, "SEC2 MBOX0: {:#x}, MBOX1: {:#x}\n", mbox0, mbox1);
> -
> -        if mbox0 != 0 {
> -            dev_err!(pdev, "Booter-load failed with error {:#x}\n", mbox0);
> -            return Err(ENODEV);
> -        }
> +        Self::run_booter(dev, bar, chipset, sec2_falcon, &wpr_meta)?;
>  
>          gsp_falcon.write_os_version(bar, gsp_fw.bootloader.app_version);
>