Re: [PATCH v2 5/5] gpu: nova-core: run Booter Unloader and FWSEC-SB upon unbinding

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

On Tue Apr 21, 2026 at 3:16 PM JST, Alexandre Courbot wrote:
> When probing the driver, the FWSEC-FRTS firmware creates a WPR2 secure
> memory region to store the GSP firmware, and the Booter Loader loads and
> starts that firmware into the GSP, making it run in RISC-V mode.
>
> These operations need to be reverted upon unloading, particularly the
> WPR2 secure region creation, as its presence prevents the driver from
> subsequently probing.
>
> Thus, load and run the Booter Unloader and FWSEC-SB firmwares at unbind
> time to put the GPU into a state where it can be probed again.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/firmware/booter.rs |  1 -
>  drivers/gpu/nova-core/firmware/fwsec.rs  |  1 -
>  drivers/gpu/nova-core/gpu.rs             |  8 +++++-
>  drivers/gpu/nova-core/gsp/boot.rs        | 43 ++++++++++++++++++++++++++++++++
>  drivers/gpu/nova-core/regs.rs            |  5 ++++
>  5 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
> index de2a4536b532..771b018ba580 100644
> --- a/drivers/gpu/nova-core/firmware/booter.rs
> +++ b/drivers/gpu/nova-core/firmware/booter.rs
> @@ -280,7 +280,6 @@ fn new_booter(data: &[u8]) -> Result<Self> {
>  #[derive(Copy, Clone, Debug, PartialEq)]
>  pub(crate) enum BooterKind {
>      Loader,
> -    #[expect(unused)]
>      Unloader,
>  }
>  
> diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
> index 8810cb49db67..4108f28cd338 100644
> --- a/drivers/gpu/nova-core/firmware/fwsec.rs
> +++ b/drivers/gpu/nova-core/firmware/fwsec.rs
> @@ -144,7 +144,6 @@ pub(crate) enum FwsecCommand {
>      /// image into it.
>      Frts { frts_addr: u64, frts_size: u64 },
>      /// Asks [`FwsecFirmware`] to load pre-OS apps on the PMU.
> -    #[expect(dead_code)]
>      Sb,
>  }
>  
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index 8f2ae9e8a519..37d0e4587ed3 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -286,7 +286,13 @@ pub(crate) fn unbind(&self, dev: &device::Device<device::Core>) {
>              return;
>          };
>  
> -        let _ = kernel::warn_on_err!(self.gsp.unload(dev, bar, &self.gsp_falcon));
> +        let _ = kernel::warn_on_err!(self.gsp.unload(
> +            dev,
> +            bar,
> +            self.spec.chipset,
> +            &self.gsp_falcon,
> +            &self.sec2_falcon,
> +        ));
>  
>          self.sysmem_flush.unregister(bar);
>      }
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> index 3f4e99b2497b..e00cfebe5d11 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -267,7 +267,9 @@ pub(crate) fn unload(
>          &self,
>          dev: &device::Device<device::Bound>,
>          bar: &Bar0,
> +        chipset: Chipset,
>          gsp_falcon: &Falcon<Gsp>,
> +        sec2_falcon: &Falcon<Sec2>,
>      ) -> Result {
>          // Shut down the GSP.
>  
> @@ -275,6 +277,47 @@ pub(crate) fn unload(
>              .inspect_err(|e| dev_err!(dev, "unload guest driver failed: {:?}", e))?;
>          dev_dbg!(dev, "GSP shut down\n");
>  
> +        // Run FWSEC-SB to reset the GSP falcon to its pre-libos state.
> +
> +        let bios = Vbios::new(dev, bar)?;
> +        let fwsec_sb = FwsecFirmware::new(dev, gsp_falcon, bar, &bios, FwsecCommand::Sb)?;
> +
> +        if chipset.needs_fwsec_bootloader() {
> +            let fwsec_sb_bl = FwsecFirmwareWithBl::new(fwsec_sb, dev, chipset)?;
> +            // Load and run the bootloader, which will load FWSEC-SB and run it.
> +            fwsec_sb_bl.run(dev, gsp_falcon, bar)?;
> +        } else {
> +            // Load and run FWSEC-SB directly.
> +            fwsec_sb.run(dev, gsp_falcon, bar)?;
> +        }
> +        dev_dbg!(dev, "FWSEC SB completed\n");
> +
> +        // Remove WPR2 region if set.
> +
> +        let wpr2_hi = bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_HI);
> +        if wpr2_hi.is_wpr2_set() {
> +            let booter_unloader = BooterFirmware::new(
> +                dev,
> +                BooterKind::Unloader,
> +                chipset,
> +                FIRMWARE_VERSION,
> +                sec2_falcon,
> +                bar,
> +            )?;
> +
> +            sec2_falcon.reset(bar)?;
> +            sec2_falcon.load(dev, bar, &booter_unloader)?;
> +            let _ = sec2_falcon.boot(bar, Some(0xff), Some(0xff))?;

What about a named constant if you can think of a good name for 0xff or
a comment explaining why we need to write 0xff into the two mailboxes?
Presumably we don't care about the return value here since we check
success using the register read below.

Thanks for working on this, this will be a great help for avoiding
reboots during development (since pcie reset sometimes has issues).

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

> +
> +            let wpr2_hi = bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_HI);
> +            if wpr2_hi.is_wpr2_set() {
> +                dev_err!(dev, "WPR2 region still set after Booter Unloader ran\n");
> +                return Err(EBUSY);
> +            }
> +        }
> +
> +        dev_info!(dev, "successfully unloaded\n");
> +
>          Ok(())
>      }
>  }
> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
> index 2f171a4ff9ba..21ff5d15f648 100644
> --- a/drivers/gpu/nova-core/regs.rs
> +++ b/drivers/gpu/nova-core/regs.rs
> @@ -176,6 +176,11 @@ impl NV_PFB_PRI_MMU_WPR2_ADDR_HI {
>      pub(crate) fn higher_bound(self) -> u64 {
>          u64::from(self.hi_val()) << 12
>      }
> +
> +    /// Returns whether the WPR2 region is currently set.
> +    pub(crate) fn is_wpr2_set(self) -> bool {
> +        self.hi_val() != 0
> +    }
>  }
>  
>  // PGSP