On Thu Dec 18, 2025 at 12:29 PM JST, Timur Tabi wrote:
<snip>
> + fn pio_wr<F: FalconFirmware<Target = E>>(
> + &self,
> + bar: &Bar0,
> + fw: &F,
> + target_mem: FalconMem,
> + load_offsets: &FalconLoadTarget,
> + port: u8,
> + tag: u16,
> + ) -> Result {
> + let start = usize::from_safe_cast(load_offsets.src_start);
> + let len = usize::from_safe_cast(load_offsets.len);
> + let mem_base = u16::try_from(load_offsets.dst_start)?;
> +
> + // SAFETY: as_slice() ensures that start+len is within range
That's not the safety concern - check the documentation for `as_slice`.
We need to ensure that there won't be any concurrent access to the DMA
object. Since we are the only user of it at this stage, that's a
guarantee we can indeed provide.
> + let data = unsafe { fw.as_slice(start, len).map_err(|_| EINVAL)? };
> +
> + self.pio_wr_bytes(bar, data, mem_base, target_mem, port, tag)
> + }
> +
> + /// Perform a PIO copy into `IMEM` and `DMEM` of `fw`, and prepare the
> falcon to run it.
> + pub(crate) fn pio_load<F: FalconFirmware<Target = E>>(
> + &self,
> + bar: &Bar0,
> + fw: &F,
> + gbl: Option<&GenericBootloader>,
> + ) -> Result {
> + let imem_sec = fw.imem_sec_load_params();
> + let imem_ns = fw.imem_ns_load_params().ok_or(EINVAL)?;
> + let dmem = fw.dmem_load_params();
> +
> + regs::NV_PFALCON_FBIF_CTL::read(bar, &E::ID)
> + .set_allow_phys_no_ctx(true)
> + .write(bar, &E::ID);
> +
> + regs::NV_PFALCON_FALCON_DMACTL::default().write(bar, &E::ID);
> +
> + // If the Generic Bootloader was passed, then use it to boot FRTS
> + if let Some(gbl) = gbl {
> + let dst_start = u16::try_from(0x10000 - gbl.desc.code_size)?;
> + let data =
> &gbl.ucode[..usize::from_safe_cast(gbl.desc.code_size)];
> + let tag = u16::try_from(gbl.desc.start_tag)?;
> +
> + self.pio_wr_bytes(bar, data, dst_start,
> FalconMem::ImemNonSecure, 0, tag)?;
> +
> + // This structure tells the generic bootloader where to find the
> FWSEC
> + // image.
> + let dmem_desc = BootloaderDmemDescV2 {
> + reserved: [0; 4],
> + signature: [0; 4],
> + ctx_dma: 4, // FALCON_DMAIDX_PHYS_SYS_NCOH
> + code_dma_base: fw.dma_handle(),
> + non_sec_code_off: imem_ns.dst_start,
> + non_sec_code_size: imem_ns.len,
> + sec_code_off: imem_sec.dst_start,
> + sec_code_size: imem_sec.len,
> + code_entry_point: 0,
> + data_dma_base: fw.dma_handle() + u64::from(dmem.src_start),
> + data_size: dmem.len,
> + argc: 0,
> + argv: 0,
> + };
> +
> + regs::NV_PFALCON_FBIF_TRANSCFG::update(bar, &E::ID, 4, |v| {
> + v.set_target(FalconFbifTarget::CoherentSysmem)
> + .set_mem_type(FalconFbifMemType::Physical)
> + });
> +
> + self.pio_wr_bytes(bar, dmem_desc.as_bytes(), 0, FalconMem::Dmem,
> 0, 0)?;
So this `if` branch is really special-casing the generic bootloader. But
at the end of the day it just does these things:
- Write an `ImemNonSecure` section,
- Write an `Dmem` section,
- Program the `TRANSCFG` register so the bootloader can initiate the DMA
transfer.
The first two steps can be expressed as a set of `FalconLoadTarget`s.
That way they can be handled by the non-generic-bootloader path, and we
can remove the `gbl` argument.
So `FwsecFirmware` could have an optional member that contains both the
generic bootloader and the `BootloaderDmemDescV2` corresponding to it.
If that optional member is `Some`, then it returns the `FalconLoadTarget`s
corresponding to the generic bootloader. Otherwise, it behaves as
before.
Interestingly there is no `ImemSecure` section to write so I guess we
will have to make `imem_sec_load_params` return an `Option` as well.
And `NV_PFALCON_FBIF_TRANSCFG` is always programmed as the worst thing
that can happen is that we don't use the DMA engine if there is no
generic bootloader.
> + } else {
> + self.pio_wr(
> + bar,
> + fw,
> + FalconMem::ImemNonSecure,
> + &imem_ns,
> + 0,
> + u16::try_from(imem_ns.dst_start >> 8)?,
> + )?;
> + self.pio_wr(
> + bar,
> + fw,
> + FalconMem::ImemSecure,
> + &imem_sec,
> + 0,
> + u16::try_from(imem_sec.dst_start >> 8)?,
> + )?;
> + self.pio_wr(bar, fw, FalconMem::Dmem, &dmem, 0, 0)?;
> + }
> +
> + self.hal.program_brom(self, bar, &fw.brom_params())?;
> +
> + // Set `BootVec` to start of non-secure code.
> + regs::NV_PFALCON_FALCON_BOOTVEC::default()
> + .set_value(fw.boot_addr())
> + .write(bar, &E::ID);
> +
> + Ok(())
> + }
> +
> /// Perform a DMA write according to `load_offsets` from `dma_handle`
> into the falcon's
> /// `target_mem`.
> ///
> diff --git a/drivers/gpu/nova-core/firmware.rs
> b/drivers/gpu/nova-core/firmware.rs
> index 44897feb82a4..26efbf4f6760 100644
> --- a/drivers/gpu/nova-core/firmware.rs
> +++ b/drivers/gpu/nova-core/firmware.rs
> @@ -31,7 +31,7 @@
> pub(crate) const FIRMWARE_VERSION: &str = "570.144";
>
> /// Requests the GPU firmware `name` suitable for `chipset`, with version
> `ver`.
> -fn request_firmware(
> +pub(crate) fn request_firmware(
There is no need to change the visibility of this function.
> dev: &device::Device,
> chipset: gpu::Chipset,
> name: &str,
> @@ -321,7 +321,7 @@ fn no_patch_signature(self) -> FirmwareDmaObject<F,
> Signed> {
> /// Header common to most firmware files.
> #[repr(C)]
> #[derive(Debug, Clone)]
> -struct BinHdr {
> +pub(crate) struct BinHdr {
Same for this type.
> /// Magic number, must be `0x10de`.
> bin_magic: u32,
> /// Version of the header.
> diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs
> b/drivers/gpu/nova-core/firmware/fwsec.rs
> index 1c1dcdacf5f5..4c26257bbfe0 100644
> --- a/drivers/gpu/nova-core/firmware/fwsec.rs
> +++ b/drivers/gpu/nova-core/firmware/fwsec.rs
> @@ -40,12 +40,15 @@
> FalconLoadTarget, //
> },
> firmware::{
> + BinHdr,
> FalconUCodeDesc,
> FirmwareDmaObject,
> FirmwareSignature,
> Signed,
> Unsigned, //
> + FIRMWARE_VERSION,
> },
> + gpu::Chipset,
> num::{
> FromSafeCast,
> IntoSafeCast, //
> @@ -213,6 +216,72 @@ unsafe fn transmute_mut<T: Sized + FromBytes + AsBytes>(
> T::from_bytes_mut(unsafe { fw.as_slice_mut(offset, size_of::<T>())?
> }).ok_or(EINVAL)
> }
>
> +/// Descriptor used by RM to figure out the requirements of the boot loader.
> +#[repr(C)]
> +#[derive(Debug, Clone)]
> +pub(crate) struct BootloaderDesc {
> + /// Starting tag of bootloader.
> + pub start_tag: u32,
> + /// DMEM offset where [`BootloaderDmemDescV2`] is to be loaded.
> + pub dmem_load_off: u32,
> + /// Offset of code section in the image.
> + pub code_off: u32,
> + /// Size of code section in the image.
> + pub code_size: u32,
> + /// Offset of data section in the image.
> + pub data_off: u32,
> + /// Size of data section in the image.
> + pub data_size: u32,
> +}
> +// SAFETY: any byte sequence is valid for this struct.
> +unsafe impl FromBytes for BootloaderDesc {}
> +// SAFETY: This struct doesn't contain uninitialized bytes and doesn't have
> interior mutability.
> +unsafe impl AsBytes for BootloaderDesc {}
We only need to implement `FromBytes` for this type, `AsBytes` is not
needed.
> +
> +/// Structure used by the boot-loader to load the rest of the code.
> +///
> +/// This has to be filled by the GPU driver and copied into DMEM at offset
> +/// [`BootloaderDesc.dmem_load_off`].
> +#[repr(C, packed)]
> +#[derive(Debug, Clone)]
> +pub(crate) struct BootloaderDmemDescV2 {
> + /// Reserved, should always be first element.
> + pub reserved: [u32; 4],
> + /// 16B signature for secure code, 0s if no secure code.
> + pub signature: [u32; 4],
> + /// DMA context used by the bootloader while loading code/data.
> + pub ctx_dma: u32,
> + /// 256B-aligned physical FB address where code is located.
> + pub code_dma_base: u64,
> + /// Offset from `code_dma_base` where the non-secure code is located
> (must be multiple of 256).
> + pub non_sec_code_off: u32,
> + /// Size of the non-secure code part.
> + pub non_sec_code_size: u32,
> + /// Offset from `code_dma_base` where the secure code is located (must
> be multiple of 256).
> + pub sec_code_off: u32,
> + /// Size of the secure code part.
> + pub sec_code_size: u32,
> + /// Code entry point invoked by the bootloader after code is loaded.
> + pub code_entry_point: u32,
> + /// 256B-aligned physical FB address where data is located.
> + pub data_dma_base: u64,
> + /// Size of data block (should be multiple of 256B).
> + pub data_size: u32,
> + /// Arguments to be passed to the target firmware being loaded.
> + pub argc: u32,
> + /// Number of arguments to be passed to the target firmware being loaded.
> + pub argv: u32,
> +}
> +// SAFETY: any byte sequence is valid for this struct.
> +unsafe impl FromBytes for BootloaderDmemDescV2 {}
> +// SAFETY: This struct doesn't contain uninitialized bytes and doesn't have
> interior mutability.
> +unsafe impl AsBytes for BootloaderDmemDescV2 {}
Here we can do without `FromBytes`.
> +
> +pub(crate) struct GenericBootloader {
> + pub desc: BootloaderDesc,
> + pub ucode: Vec<u8, kernel::alloc::allocator::Kmalloc>,
> +}
> +
> /// The FWSEC microcode, extracted from the BIOS and to be run on the GSP
> falcon.
> ///
> /// It is responsible for e.g. carving out the WPR2 region as the first step
> of the GSP bootflow.
> @@ -221,6 +290,8 @@ pub(crate) struct FwsecFirmware {
> desc: FalconUCodeDesc,
> /// GPU-accessible DMA object containing the firmware.
> ucode: FirmwareDmaObject<Self, Signed>,
> + /// Generic bootloader
> + gen_bootloader: Option<GenericBootloader>,
> }
>
> impl FalconLoadParams for FwsecFirmware {
> @@ -275,7 +346,19 @@ fn brom_params(&self) -> FalconBromParams {
> }
>
> fn boot_addr(&self) -> u32 {
> - 0
> + match &self.desc {
> + FalconUCodeDesc::V2(_v2) => {
> + // 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.
> + if let Some(ref gbl) = self.gen_bootloader {
> + gbl.desc.start_tag << 8
> + } else {
> + 0
> + }
> + }
> + FalconUCodeDesc::V3(_v3) => 0,
> + }
> }
> }
>
> @@ -376,6 +459,7 @@ impl FwsecFirmware {
> /// command.
> pub(crate) fn new(
> dev: &Device<device::Bound>,
> + chipset: Chipset,
> falcon: &Falcon<Gsp>,
> bar: &Bar0,
> bios: &Vbios,
> @@ -432,9 +516,49 @@ pub(crate) fn new(
> ucode_dma.no_patch_signature()
> };
>
> + // The Generic Bootloader exists only on Turing and GA100. To avoid
> a bogus
> + // console error message on other platforms, only try to load it if
> it's
> + // supposed to be there.
> + let gbl_fw = if chipset < Chipset::GA102 {
> + super::request_firmware(dev, chipset, "gen_bootloader",
> FIRMWARE_VERSION)
> + } else {
> + Err(ENOENT)
> + };
Using `Err` to indicate no firmware means that we will proceed even if
`request_firmware` returns an error. This should be:
let gbl_fw = if chipset < Chipset::GA102 {
Some(super::request_firmware(dev, chipset, "gen_bootloader",
FIRMWARE_VERSION)?)
} else {
None
};
> +
> + let gbl = match gbl_fw {
> + Ok(fw) => {
> + let hdr = fw
> + .data()
> + .get(0..size_of::<BinHdr>())
> + .and_then(BinHdr::from_bytes_copy)
> + .ok_or(EINVAL)?;
> +
> + let desc_offset = usize::from_safe_cast(hdr.header_offset);
> + let desc = fw
> + .data()
> + .get(desc_offset..desc_offset +
> size_of::<BootloaderDesc>())
> + .and_then(BootloaderDesc::from_bytes_copy)
> + .ok_or(EINVAL)?;
> +
> + let ucode_start = usize::from_safe_cast(hdr.data_offset);
> + let ucode_size = usize::from_safe_cast(hdr.data_size);
> + let ucode_data = fw
> + .data()
> + .get(ucode_start..ucode_start + ucode_size)
> + .ok_or(EINVAL)?;
> +
> + let mut ucode = KVec::new();
> + ucode.extend_from_slice(ucode_data, GFP_KERNEL)?;
> +
> + Some(GenericBootloader { desc, ucode })
> + }
> + Err(_) => None,
> + };
> +
Actually, let's put that code into a new `GenBootloader` type. You can
follow the example of `BooterFirmware`, which is quite similar (only a
bit more complex).
