linux-kernel - Re: [PATCH 04/31] nova-core: factor .fwsignature* selection into a new get_gsp_sigs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DFNI0EDLI3MD.2YNWZBAWOFTBQ@garyguo.net>
Date: Tue, 13 Jan 2026 13:33:36 +0000
From: "Gary Guo" <gary@...yguo.net>
To: "John Hubbard" <jhubbard@...dia.com>, "Danilo Krummrich"
 <dakr@...nel.org>
Cc: "Alexandre Courbot" <acourbot@...dia.com>, "Joel Fernandes"
 <joelagnelf@...dia.com>, "Timur Tabi" <ttabi@...dia.com>, "Alistair Popple"
 <apopple@...dia.com>, "Edwin Peer" <epeer@...dia.com>, "Zhi Wang"
 <zhiw@...dia.com>, "David Airlie" <airlied@...il.com>, "Simona Vetter"
 <simona@...ll.ch>, "Bjorn Helgaas" <bhelgaas@...gle.com>, "Miguel Ojeda"
 <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>, "Boqun Feng"
 <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Benno Lossin"
 <lossin@...nel.org>, "Andreas Hindborg" <a.hindborg@...nel.org>, "Alice
 Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>,
 <nouveau@...ts.freedesktop.org>, <rust-for-linux@...r.kernel.org>, "LKML"
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 04/31] nova-core: factor .fwsignature* selection into a
 new get_gsp_sigs_section()

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 <jhubbard@...dia.com>
> ---
>  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 <gary@...yguo.net>

> +        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))?;