On Wed Dec 3, 2025 at 5:58 AM GMT, John Hubbard wrote:
> Keep Gsp::new() from getting too cluttered, by factoring out the
> selection of .fwsignature* items. This will continue to grow as we add
> GPUs.
>
> Signed-off-by: John Hubbard <[email protected]>
> ---
> drivers/gpu/nova-core/firmware/gsp.rs | 43 ++++++++++++++-------------
> 1 file changed, 23 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/firmware/gsp.rs
> b/drivers/gpu/nova-core/firmware/gsp.rs
> index 547f46b6655b..86ed4d650d05 100644
> --- a/drivers/gpu/nova-core/firmware/gsp.rs
> +++ b/drivers/gpu/nova-core/firmware/gsp.rs
> @@ -151,39 +151,42 @@ pub(crate) struct GspFirmware {
> }
>
> impl GspFirmware {
> - /// Loads the GSP firmware binaries, map them into `dev`'s
> address-space, and creates the page
> - /// tables expected by the GSP bootloader to load it.
> - pub(crate) fn new<'a, 'b>(
> - dev: &'a device::Device<device::Bound>,
> - chipset: Chipset,
> - ver: &'b str,
> - ) -> Result<impl PinInit<Self, Error> + 'a> {
> - let fw = super::request_firmware(dev, chipset, "gsp", ver)?;
> -
> - let fw_section = elf::elf64_section(fw.data(),
> ".fwimage").ok_or(EINVAL)?;
> -
> - let sigs_section = match chipset.arch() {
> - Architecture::Ampere => ".fwsignature_ga10x",
> - Architecture::Hopper => ".fwsignature_gh10x",
> - Architecture::Ada => ".fwsignature_ad10x",
> + fn get_gsp_sigs_section(chipset: Chipset) -> Result<&'static str> {
I would just return `Option` here, and have
let sigs_section = Self::get_gsp_sigs_section(chipset).ok_or(ENOTSUPP)?;
similar to the ELF line aboe it.
The code looks correct to me regardless if you make the change.
Reviewed-by: Gary Guo <[email protected]>
> + match chipset.arch() {
> + Architecture::Ampere => Ok(".fwsignature_ga10x"),
> + Architecture::Hopper => Ok(".fwsignature_gh10x"),
> + Architecture::Ada => Ok(".fwsignature_ad10x"),
> Architecture::Blackwell => {
> // Distinguish between GB10x and GB20x series
> match chipset {
> // GB10x series: GB100, GB102
> - Chipset::GB100 | Chipset::GB102 => ".fwsignature_gb10x",
> + Chipset::GB100 | Chipset::GB102 =>
> Ok(".fwsignature_gb10x"),
> // GB20x series: GB202, GB203, GB205, GB206, GB207
> Chipset::GB202
> | Chipset::GB203
> | Chipset::GB205
> | Chipset::GB206
> - | Chipset::GB207 => ".fwsignature_gb20x",
> + | Chipset::GB207 => Ok(".fwsignature_gb20x"),
> // Non-Blackwell chipsets, which can't happen here, but
> Rust doesn't know that.
> - _ => return Err(ENOTSUPP),
> + _ => Err(ENOTSUPP),
> }
> }
> + _ => Err(ENOTSUPP),
> + }
> + }
>
> - _ => return Err(ENOTSUPP),
> - };
> + /// Loads the GSP firmware binaries, map them into `dev`'s
> address-space, and creates the page
> + /// tables expected by the GSP bootloader to load it.
> + pub(crate) fn new<'a, 'b>(
> + dev: &'a device::Device<device::Bound>,
> + chipset: Chipset,
> + ver: &'b str,
> + ) -> Result<impl PinInit<Self, Error> + 'a> {
> + let fw = super::request_firmware(dev, chipset, "gsp", ver)?;
> +
> + let fw_section = elf::elf64_section(fw.data(),
> ".fwimage").ok_or(EINVAL)?;
> +
> + let sigs_section = Self::get_gsp_sigs_section(chipset)?;
> let signatures = elf::elf64_section(fw.data(), sigs_section)
> .ok_or(EINVAL)
> .and_then(|data| DmaObject::from_data(dev, data))?;
