Re: [PATCH 18/27] gpu: nova-core: separate the generic falcon bootloader from FWSEC

On Tue, 18 Aug 2026 20:52:11 -0700
John Hubbard <[email protected]> wrote:

Looking good to me.

Reviewed-by: Zhi Wang <[email protected]>

> The r000 GSP boot protocol sends a load-and-execute event for the
> generic falcon bootloader.
> 
> Nova-core stored the image and its load parameters in the FWSEC
> firmware wrapper, which kept the r000 boot path from loading it
> independently.
> 
> Separate the bootloader image and load parameters from the FWSEC
> wrapper. Place the bootloader at the end of the target falcon's
> HWCFG-reported IMEM rather than below a fixed 64 KiB ceiling, so its
> placement remains within bounds on falcons with less IMEM.
> 
> Assisted-by: Cursor:claude-opus-5
> Signed-off-by: John Hubbard <[email protected]>
> ---
>  drivers/gpu/nova-core/falcon.rs               |  15 +++
>  drivers/gpu/nova-core/firmware.rs             |   1 +
>  .../nova-core/firmware/fwsec/bootloader.rs    |  64 ++---------
>  .../gpu/nova-core/firmware/gen_bootloader.rs  | 102
> ++++++++++++++++++ drivers/gpu/nova-core/gsp/hal/tu102.rs        |
> 6 +- drivers/gpu/nova-core/regs.rs                 |   5 +
>  6 files changed, 135 insertions(+), 58 deletions(-)
>  create mode 100644 drivers/gpu/nova-core/firmware/gen_bootloader.rs
> 
> diff --git a/drivers/gpu/nova-core/falcon.rs
> b/drivers/gpu/nova-core/falcon.rs index 20a288050c37..b8013d2eb582
> 100644 --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -387,6 +387,21 @@ pub(crate) fn new(
>          })
>      }
>  
> +    /// Returns the size of this falcon's IMEM, in bytes.
> +    ///
> +    /// `NV_PFALCON_FALCON_IMEMC` addresses IMEM with a 16-bit byte
> offset, so the result never
> +    /// exceeds 64KiB.
> +    pub(crate) fn imem_size(&self) -> usize {
> +        let blocks = usize::from_safe_cast(
> +            *self
> +                .bar
> +                .read(regs::NV_PFALCON_FALCON_HWCFG::of::<E>())
> +                .imem_size(),
> +        );
> +
> +        blocks * MEM_BLOCK_ALIGNMENT
> +    }
> +
>      /// Resets DMA-related registers.
>      pub(crate) fn dma_reset(&self) {
>          self.bar.update(regs::NV_PFALCON_FBIF_CTL::of::<E>(), |v| {
> diff --git a/drivers/gpu/nova-core/firmware.rs
> b/drivers/gpu/nova-core/firmware.rs index e0befe84aa3e..9ff764930bcd
> 100644 --- a/drivers/gpu/nova-core/firmware.rs
> +++ b/drivers/gpu/nova-core/firmware.rs
> @@ -26,6 +26,7 @@
>  pub(crate) mod booter;
>  pub(crate) mod fsp;
>  pub(crate) mod fwsec;
> +pub(crate) mod gen_bootloader;
>  pub(crate) mod gsp;
>  pub(crate) mod radix3;
>  pub(crate) mod riscv;
> diff --git a/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
> b/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs index
> ec4d92317a93..6670d17b4eeb 100644 ---
> a/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs +++
> b/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs @@ -14,18 +14,12
> @@ dma::Coherent,
>      io::{register::WithBase, Io},
>      prelude::*,
> -    ptr::{
> -        Alignable,
> -        Alignment, //
> -    },
> -    sizes,
>      transmute::AsBytes,
>  };
>  
>  use crate::{
>      driver::Bar0,
>      falcon::{
> -        self,
>          gsp::Gsp,
>          Falcon,
>          FalconBromParams,
> @@ -39,10 +33,7 @@
>      },
>      firmware::{
>          fwsec::FwsecFirmware,
> -        tlv::{
> -            request_tlv, //
> -            Tlv,
> -        },
> +        gen_bootloader::GenericBootloader, //
>      },
>      gpu::Chipset,
>      num::FromSafeCast, //
> @@ -101,16 +92,12 @@ unsafe impl AsBytes for BootloaderDmemDescV2 {}
>  pub(crate) struct FwsecFirmwareWithBl {
>      /// DMA object the bootloader will copy the firmware from.
>      _firmware_dma: Coherent<[u8]>,
> -    /// Code of the bootloader to be loaded into non-secure IMEM.
> -    ucode: KVec<u8>,
> +    /// Bootloader that performs the load.
> +    bootloader: GenericBootloader,
>      /// Descriptor to be loaded into DMEM for the bootloader to read.
>      dmem_desc: BootloaderDmemDescV2,
> -    /// Range-validated start offset of the firmware code in IMEM.
> -    imem_dst_start: u16,
>      /// BROM parameters of the loaded firmware.
>      brom_params: FalconBromParams,
> -    /// Range-validated `desc.start_tag`.
> -    start_tag: u16,
>  }
>  
>  impl FwsecFirmwareWithBl {
> @@ -120,29 +107,9 @@ pub(crate) fn new(
>          firmware: FwsecFirmware,
>          dev: &Device<device::Bound>,
>          chipset: Chipset,
> +        falcon: &Falcon<'_, Gsp>,
>      ) -> Result<Self> {
> -        let fw = request_tlv(dev, chipset, "gen_bootloader")?;
> -        let tlv = Tlv::new(fw.data())?;
> -        dev_dbg!(
> -            dev,
> -            "loaded generic bootloader firmware v{}\n",
> -            tlv.get_string(b"VERS")?
> -        );
> -
> -        let ucode = {
> -            let blob = tlv.get_bytes(b"BLOB")?;
> -            let code_size =
> usize::from_safe_cast(tlv.get_u32(b"CDSZ")?);
> -            let code = blob.get(..code_size).ok_or(EINVAL)?;
> -            let aligned_code_size = code_size
> -                .align_up(Alignment::new::<{
> falcon::MEM_BLOCK_ALIGNMENT }>())
> -                .ok_or(EINVAL)?;
> -
> -            let mut ucode = KVec::with_capacity(aligned_code_size,
> GFP_KERNEL)?;
> -            ucode.extend_from_slice(code, GFP_KERNEL)?;
> -            ucode.resize(aligned_code_size, 0, GFP_KERNEL)?;
> -
> -            ucode
> -        };
> +        let bootloader = GenericBootloader::new(dev, chipset,
> falcon)?; 
>          // `BootloaderDmemDescV2` expects the source to be a mirror
> image of the destination and // uses the same offset parameter for
> both. @@ -213,21 +180,11 @@ pub(crate) fn new(
>              }
>          };
>  
> -        // The bootloader's code must be loaded in the area right
> below the first 64K of IMEM.
> -        const BOOTLOADER_LOAD_CEILING: usize = sizes::SZ_64K;
> -        let imem_dst_start = BOOTLOADER_LOAD_CEILING
> -            .checked_sub(ucode.len())
> -            .ok_or(EOVERFLOW)?;
> -
> -        let start_tag = u16::try_from(tlv.get_u32(b"STRT")?)?;
> -
>          Ok(Self {
>              _firmware_dma: firmware_dma,
> -            ucode,
> +            bootloader,
>              dmem_desc,
>              brom_params: firmware.brom_params(),
> -            imem_dst_start: u16::try_from(imem_dst_start)?,
> -            start_tag,
>          })
>      }
>  
> @@ -282,7 +239,7 @@ fn brom_params(&self) -> FalconBromParams {
>      fn boot_addr(&self) -> u32 {
>          // On V2 platforms, the boot address is extracted from the
> generic bootloader, because the // gbl is what actually copies FWSEC
> into memory, so that is what needs to be booted.
> -        u32::from(self.start_tag) << 8
> +        self.bootloader.boot_addr()
>      }
>  }
>  
> @@ -292,12 +249,7 @@ fn imem_sec_load_params(&self) ->
> Option<FalconPioImemLoadTarget<'_>> { }
>  
>      fn imem_ns_load_params(&self) ->
> Option<FalconPioImemLoadTarget<'_>> {
> -        Some(FalconPioImemLoadTarget {
> -            data: self.ucode.as_ref(),
> -            dst_start: self.imem_dst_start,
> -            secure: false,
> -            start_tag: self.start_tag,
> -        })
> +        Some(self.bootloader.imem_load_params())
>      }
>  
>      fn dmem_load_params(&self) -> FalconPioDmemLoadTarget<'_> {
> diff --git a/drivers/gpu/nova-core/firmware/gen_bootloader.rs
> b/drivers/gpu/nova-core/firmware/gen_bootloader.rs new file mode
> 100644 index 000000000000..f949223af2d0
> --- /dev/null
> +++ b/drivers/gpu/nova-core/firmware/gen_bootloader.rs
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! The generic falcon bootloader.
> +//!
> +//! A small program loaded into falcon IMEM using PIO, which then
> DMAs a larger image into IMEM and +//! DMEM from a descriptor the
> driver leaves in DMEM at offset 0. Open RM's +//!
> `ksec2GetGenericBlUcode` supplies the same image for both SEC2 and
> GSP. + +use kernel::{
> +    device,
> +    prelude::*,
> +    ptr::{
> +        Alignable,
> +        Alignment, //
> +    },
> +};
> +
> +use crate::{
> +    falcon::{
> +        self,
> +        Falcon,
> +        FalconEngine,
> +        FalconPioImemLoadTarget, //
> +    },
> +    firmware::tlv::{
> +        request_tlv, //
> +        Tlv,
> +    },
> +    gpu::Chipset,
> +    num::FromSafeCast, //
> +};
> +
> +/// The generic falcon bootloader image and the IMEM placement it
> was loaded for. +pub(crate) struct GenericBootloader {
> +    /// Bootloader code, zero-padded to a whole number of falcon
> memory blocks.
> +    ucode: KVec<u8>,
> +    /// Byte offset in IMEM the code is loaded at.
> +    imem_dst_start: u16,
> +    /// Tag the first code block is loaded under.
> +    start_tag: u16,
> +}
> +
> +impl GenericBootloader {
> +    /// Loads the generic bootloader image for `chipset`, placed in
> the last blocks of `falcon`'s
> +    /// IMEM so the image it goes on to load has the rest to itself.
> +    ///
> +    /// # Errors
> +    ///
> +    /// - `EINVAL` if a required TLV field is absent or the image
> does not fit in IMEM.
> +    /// - `ENOMEM` if the padded copy of the code cannot be
> allocated.
> +    pub(crate) fn new<E: FalconEngine + 'static>(
> +        dev: &device::Device<device::Bound>,
> +        chipset: Chipset,
> +        falcon: &Falcon<'_, E>,
> +    ) -> Result<Self> {
> +        let fw = request_tlv(dev, chipset, "gen_bootloader")?;
> +        let tlv = Tlv::new(fw.data())?;
> +        dev_dbg!(
> +            dev,
> +            "loaded generic bootloader firmware v{}\n",
> +            tlv.get_string(b"VERS")?
> +        );
> +
> +        let ucode = {
> +            let blob = tlv.get_bytes(b"BLOB")?;
> +            let code_size =
> usize::from_safe_cast(tlv.get_u32(b"CDSZ")?);
> +            let code = blob.get(..code_size).ok_or(EINVAL)?;
> +            let aligned_code_size = code_size
> +                .align_up(Alignment::new::<{
> falcon::MEM_BLOCK_ALIGNMENT }>())
> +                .ok_or(EINVAL)?;
> +
> +            let mut ucode = KVec::with_capacity(aligned_code_size,
> GFP_KERNEL)?;
> +            ucode.extend_from_slice(code, GFP_KERNEL)?;
> +            ucode.resize(aligned_code_size, 0, GFP_KERNEL)?;
> +
> +            ucode
> +        };
> +
> +        let imem_dst_start =
> falcon.imem_size().checked_sub(ucode.len()).ok_or(EINVAL)?; +
> +        Ok(Self {
> +            ucode,
> +            imem_dst_start: u16::try_from(imem_dst_start)?,
> +            start_tag: u16::try_from(tlv.get_u32(b"STRT")?)?,
> +        })
> +    }
> +
> +    /// Returns the address the falcon must boot from to run this
> bootloader.
> +    pub(crate) fn boot_addr(&self) -> u32 {
> +        u32::from(self.start_tag) << 8
> +    }
> +
> +    /// Returns the PIO parameters that place this bootloader in
> non-secure IMEM.
> +    pub(crate) fn imem_load_params(&self) ->
> FalconPioImemLoadTarget<'_> {
> +        FalconPioImemLoadTarget {
> +            data: self.ucode.as_ref(),
> +            dst_start: self.imem_dst_start,
> +            secure: false,
> +            start_tag: self.start_tag,
> +        }
> +    }
> +}
> diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs
> b/drivers/gpu/nova-core/gsp/hal/tu102.rs index
> a5c0ca355493..68a48c882c0f 100644 ---
> a/drivers/gpu/nova-core/gsp/hal/tu102.rs +++
> b/drivers/gpu/nova-core/gsp/hal/tu102.rs @@ -166,7 +166,7 @@ fn
> run_fwsec_frts( )?;
>  
>          if self.needs_fwsec_bootloader {
> -            let fwsec_frts_bl = FwsecFirmwareWithBl::new(fwsec_frts,
> dev, chipset)?;
> +            let fwsec_frts_bl = FwsecFirmwareWithBl::new(fwsec_frts,
> dev, chipset, falcon)?; // Load and run the bootloader, which will
> load FWSEC-FRTS and run it. fwsec_frts_bl.run(dev, falcon, bar)?;
>          } else {
> @@ -224,7 +224,9 @@ fn build_unload_bundle(
>          // Load the FWSEC SB firmware, as well as its bootloader if
> required. let fwsec_sb = FwsecFirmware::new(dev, gsp_falcon, bios,
> FwsecCommand::Sb)?; let fwsec_sb = if self.needs_fwsec_bootloader {
> -
> FwsecUnloadFirmware::WithBl(FwsecFirmwareWithBl::new(fwsec_sb, dev,
> chipset)?)
> +            FwsecUnloadFirmware::WithBl(FwsecFirmwareWithBl::new(
> +                fwsec_sb, dev, chipset, gsp_falcon,
> +            )?)
>          } else {
>              FwsecUnloadFirmware::WithoutBl(fwsec_sb)
>          };
> diff --git a/drivers/gpu/nova-core/regs.rs
> b/drivers/gpu/nova-core/regs.rs index 5d265b5788e3..5501c36a56af
> 100644 --- a/drivers/gpu/nova-core/regs.rs
> +++ b/drivers/gpu/nova-core/regs.rs
> @@ -247,6 +247,11 @@ pub(crate) fn usable_fb_size(self) -> u64 {
>          31:0    value => u32;
>      }
>  
> +    pub(crate) NV_PFALCON_FALCON_HWCFG(u32) @ PFalconBase +
> 0x00000108 {
> +        /// Size of this falcon's IMEM, in blocks of
> [`crate::falcon::MEM_BLOCK_ALIGNMENT`] bytes.
> +        8:0     imem_size;
> +    }
> +
>      pub(crate) NV_PFALCON_FALCON_DMACTL(u32) @ PFalconBase +
> 0x0000010c { 7:7     secure_stat => bool;
>          6:3     dmaq_num;