On Thu, 2025-12-18 at 21:59 +0900, Alexandre Courbot wrote:
> 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.
Hmmm... that would require that I implement FalconLoadParams for
GenericBootloader. That's not a
bad idea. I'm not sure how I would build the Dmem FalconLoadTarget, however,
given that it needs to
reference data from the FRTS FalconFirmware object.
impl FalconLoadParams for GenericBootloader {
fn imem_sec_load_params(&self) -> FalconLoadTarget {
FalconLoadTarget {
}
}
fn imem_ns_load_params(&self) -> Option<FalconLoadTarget> {
FalconLoadTarget {
}
}
fn dmem_load_params(&self) -> FalconLoadTarget {
FalconLoadTarget {
// How do I extract data from the FRTS firmware image here?
}
}
}
> 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.
Yeah, I don't understand why this is being programmed at all in the PIO case,
but that's what
Nouveau does:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/tu102.c#n132
>
> > + // 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
> };
Ok.
>
> > +
> > + 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).
Sorry, I'm a bit confused. What's the difference between GenBootloader and
GenericBootloader?
Can't I just add a new() constructor to GenericBootloader?
