Re: [PATCH v10 18/28] gpu: nova-core: Hopper/Blackwell: add FMC signature extraction

["Alexandre Courbot" <acourbot@nvidia.com>]

On Sat Apr 11, 2026 at 11:49 AM JST, John Hubbard wrote:
> Add extract_fmc_signatures() which extracts SHA-384 hash, RSA public
> key, and RSA signature from FMC ELF32 firmware sections. These are
> needed for FSP Chain of Trust verification.
>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  drivers/gpu/nova-core/firmware.rs |  3 +-
>  drivers/gpu/nova-core/fsp.rs      | 78 +++++++++++++++++++++++++++++++
>  drivers/gpu/nova-core/gsp/boot.rs |  3 +-
>  3 files changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
> index bc26807116e4..6d07715b3a49 100644
> --- a/drivers/gpu/nova-core/firmware.rs
> +++ b/drivers/gpu/nova-core/firmware.rs
> @@ -26,6 +26,7 @@
>      },
>  };
>  
> +pub(crate) use elf::elf_section;
>  pub(crate) mod booter;
>  pub(crate) mod fsp;
>  pub(crate) mod fwsec;
> @@ -646,7 +647,7 @@ fn elf32_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a [u8]> {
>      }
>  
>      /// Automatically detects ELF32 vs ELF64 based on the ELF header.
> -    pub(super) fn elf_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a [u8]> {
> +    pub(crate) fn elf_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a [u8]> {

We don't want to change the visibility of this method - thankfully there
is an easy way to achieve this - please read on.

>          // Check ELF magic.
>          if elf.len() < 5 || elf.get(0..4)? != b"\x7fELF" {
>              return None;
> diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs
> index 55e543e80de8..8287bda795ca 100644
> --- a/drivers/gpu/nova-core/fsp.rs
> +++ b/drivers/gpu/nova-core/fsp.rs
> @@ -18,6 +18,18 @@
>  /// FSP secure boot completion timeout in milliseconds.
>  const FSP_SECURE_BOOT_TIMEOUT_MS: i64 = 5000;
>  
> +/// Size constraints for FSP security signatures (Hopper/Blackwell).

This doccomment is about the 3 following items but will only appears on
the first one. You want one doccomment per const that reads something
like "/// Expected size for X".

> +const FSP_HASH_SIZE: usize = 48; // SHA-384 hash
> +const FSP_PKEY_SIZE: usize = 384; // RSA-3072 public key
> +const FSP_SIG_SIZE: usize = 384; // RSA-3072 signature
> +
> +/// Structure to hold FMC signatures.
> +#[derive(Debug, Clone, Copy)]
> +pub(crate) struct FmcSignatures {
> +    hash384: [u8; FSP_HASH_SIZE],
> +    public_key: [u8; FSP_PKEY_SIZE],
> +    signature: [u8; FSP_SIG_SIZE],
> +}
>  /// FSP interface for Hopper/Blackwell GPUs.
>  pub(crate) struct Fsp;
>  
> @@ -50,4 +62,70 @@ pub(crate) fn wait_secure_boot(
>          })
>          .map(|_| ())
>      }
> +
> +    /// Extract FMC firmware signatures for Chain of Trust verification.
> +    ///
> +    /// Extracts real cryptographic signatures from FMC ELF32 firmware sections.
> +    /// Returns signatures in a heap-allocated structure to prevent stack overflow.
> +    pub(crate) fn extract_fmc_signatures(

If you make this a method of `FmcFirmware`, you can keep the current
visibility of `elf_section`, while also simplifying the caller (as it is
only ever called on the data of a `FmcFirmware`).

> +        dev: &device::Device<device::Bound>,
> +        fmc_fw_data: &[u8],
> +    ) -> Result<KBox<FmcSignatures>> {
> +        let hash_section = crate::firmware::elf_section(fmc_fw_data, "hash")
> +            .ok_or(EINVAL)
> +            .inspect_err(|_| dev_err!(dev, "FMC firmware missing 'hash' section\n"))?;
> +
> +        let pkey_section = crate::firmware::elf_section(fmc_fw_data, "publickey")
> +            .ok_or(EINVAL)
> +            .inspect_err(|_| dev_err!(dev, "FMC firmware missing 'publickey' section\n"))?;
> +
> +        let sig_section = crate::firmware::elf_section(fmc_fw_data, "signature")
> +            .ok_or(EINVAL)
> +            .inspect_err(|_| dev_err!(dev, "FMC firmware missing 'signature' section\n"))?;
> +
> +        if hash_section.len() != FSP_HASH_SIZE {
> +            dev_err!(
> +                dev,
> +                "FMC hash section size {} != expected {}\n",
> +                hash_section.len(),
> +                FSP_HASH_SIZE
> +            );
> +            return Err(EINVAL);
> +        }
> +
> +        if pkey_section.len() > FSP_PKEY_SIZE {
> +            dev_err!(
> +                dev,
> +                "FMC publickey section size {} > maximum {}\n",
> +                pkey_section.len(),
> +                FSP_PKEY_SIZE
> +            );
> +            return Err(EINVAL);
> +        }
> +
> +        if sig_section.len() > FSP_SIG_SIZE {
> +            dev_err!(
> +                dev,
> +                "FMC signature section size {} > maximum {}\n",
> +                sig_section.len(),
> +                FSP_SIG_SIZE
> +            );
> +            return Err(EINVAL);
> +        }

That's quite a bit of repeating code. I'd like to factor this out into a
closure, but first a question: the length of `hash_section` is required
to be exactly `FSP_HASH_SIZE`, but the other two sections are only
required to be smaller. Does that really make sense or should they all
be strictly equal to their expected size? Because AFAICT 384 is the only
size that makes sense for them.

Assuming we can apply the same length test for all 3, you can replace
all this code with just:

    /// Returns the section `name` of size `expected_len`, or EINVAL if
    /// the section doesn't exist or doesn't have the expected length.
    let get_section = |name, expected_len| {
        crate::firmware::elf_section(fmc_fw_data, name)
            .ok_or(EINVAL)
            .inspect_err(|_| dev_err!(dev, "FMC firmware missing '{}' section\n", name))
            .and_then(|section| {
                if section.len() > expected_len {
                    dev_err!(
                        dev,
                        "FMC {} section size {} != expected {}\n",
                        name,
                        section.len(),
                        expected_len
                    );
                    Err(EINVAL)
                } else {
                    Ok(section)
                }
            })
    };

    let hash_section = get_section("hash", FSP_HASH_SIZE)?;
    let pkey_section = get_section("publickey", FSP_PKEY_SIZE)?;
    let sig_section = get_section("signature", FSP_SIG_SIZE)?;

> +
> +        let mut signatures = KBox::new(
> +            FmcSignatures {
> +                hash384: [0u8; FSP_HASH_SIZE],
> +                public_key: [0u8; FSP_PKEY_SIZE],
> +                signature: [0u8; FSP_SIG_SIZE],
> +            },
> +            GFP_KERNEL,
> +        )?;
> +
> +        signatures.hash384.copy_from_slice(hash_section);
> +        signatures.public_key[..pkey_section.len()].copy_from_slice(pkey_section);
> +        signatures.signature[..sig_section.len()].copy_from_slice(sig_section);

If the size assumption I make above is correct, you can remove the
slices on the destination.

Also note that `copy_from_slice` can panic, so these statements should
have a `//PANIC:` comment justifying why it cannot happen.

> +
> +        Ok(signatures)
> +    }
>  }
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> index 9609cef3ff51..739624af1cef 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -208,7 +208,8 @@ fn boot_via_fsp(
>      ) -> Result {
>          let _fsp_falcon = Falcon::<FspEngine>::new(dev, chipset)?;
>  
> -        let _fsp_fw = FspFirmware::new(dev, chipset, FIRMWARE_VERSION)?;
> +        let fsp_fw = FspFirmware::new(dev, chipset, FIRMWARE_VERSION)?;
> +        let _signatures = Fsp::extract_fmc_signatures(dev, fsp_fw.fmc_elf.data())?;

Once `extract_fmc_signatures` is a method of `FspFirmware`, we can
remove that new line and pass `fsp_fw` to `FmcBootArgs::new`. It is
more logical, and will allow it to access both the image and the
signatures from this single argument.