Re: [PATCH 4/7] gpu: nova-core: run the generic bootloader from its own type

[Eliot Courtney]

On Tue Nov 3, 2026 at 7:22 PM JST, Alexandre Courbot wrote:
> From: John Hubbard <jhubbard@xxxxxxxxxx>
>
> The generic falcon bootloader needs a descriptor in DMEM to know what to
> load, so loading it into a falcon means loading the two together by PIO
> and starting the falcon. The FWSEC wrapper did that through its own
> implementation of the PIO loading traits. The r000 load-and-execute
> bootloader event will need to do the same thing.
>
> Give the generic bootloader a run method, which pairs it with the
> descriptor that it receives, loads the pair by PIO, starts the falcon
> with the given MAILBOX0 value and returns the mailboxes once the falcon
> has halted. This will allow the FWSEC and r000 load-and-execute
> bootloader event to share the same code.
>
> No functional changes.
>
> Assisted-by: LLM
> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> Co-developed-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/firmware/fwsec/bootloader.rs | 55 ++-------------
>  drivers/gpu/nova-core/firmware/gen_bootloader.rs   | 80 +++++++++++++++++++++-
>  2 files changed, 84 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs b/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
> index ad798ec5645a..c7bb877fcf20 100644
> --- a/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
> +++ b/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
> @@ -16,22 +16,16 @@
>          register::Array,
>          Io, //
>      },
> -    prelude::*,
> -    transmute::AsBytes,
> +    prelude::*, //
>  };
>  
>  use crate::{
>      falcon::{
>          gsp::Gsp,
>          Falcon,
> -        FalconBromParams,
>          FalconDmaLoadable,
>          FalconFbifMemType,
> -        FalconFbifTarget,
> -        FalconFirmware,
> -        FalconPioDmemLoadTarget,
> -        FalconPioImemLoadTarget,
> -        FalconPioLoadable, //
> +        FalconFbifTarget, //
>      },
>      firmware::{
>          fwsec::FwsecFirmware,
> @@ -50,11 +44,10 @@
>  pub(crate) struct FwsecFirmwareWithBl<'a> {
>      /// DMA object the bootloader will copy the firmware from.
>      _firmware_dma: Coherent<'a, [u8]>,
> +    /// The bootloader that copies the firmware.

nit: doc change looks like it should be in a different patch

>      bootloader: GenericBootloader,
>      /// Descriptor to be loaded into DMEM for the bootloader to read.
>      dmem_desc: BootloaderDmemDescV2,
> -    /// BROM parameters of the loaded firmware.
> -    brom_params: FalconBromParams,
>  }
>  
>  impl<'a> FwsecFirmwareWithBl<'a> {
> @@ -127,7 +120,6 @@ pub(crate) fn new(
>              _firmware_dma: firmware_dma,
>              bootloader,
>              dmem_desc,
> -            brom_params: firmware.brom_params(),
>          })
>      }
>  
> @@ -136,13 +128,9 @@ pub(crate) fn new(
>      /// The bootloader will load the FWSEC firmware and then execute it. This function returns
>      /// after FWSEC has reached completion.
>      pub(crate) fn run(&self, dev: &Device<device::Bound>, falcon: &Falcon<'_, Gsp>) -> Result<()> {
> -        // Reset falcon, load the firmware, and run it.
>          falcon
>              .reset()
>              .inspect_err(|e| dev_err!(dev, "Failed to reset GSP falcon: {:?}\n", e))?;
> -        falcon
> -            .pio_load(self)
> -            .inspect_err(|e| dev_err!(dev, "Failed to load FWSEC firmware: {:?}\n", e))?;
>  
>          // Configure DMA index for the bootloader to fetch the FWSEC firmware from system memory.
>          falcon.pfalcon.update(

Does it make sense for the generic bootloader to also do this step
(falcon.pfalcon.update())? This appears to be something that's part of
what the generic bootloader expects to be done no matter what, and it's
doable just from `BootloaderDmemDescV2` which it already gets.

Modulo answer to above question,

Reviewed-by: Eliot Courtney <ecourtney@xxxxxxxxxx>