Re: [PATCH v7 07/31] gpu: nova-core: move firmware image parsing code to firmware.rs

[Alexandre Courbot]

On Wed Mar 18, 2026 at 7:53 AM JST, John Hubbard wrote:
> Up until now, only the GSP required parsing of its firmware headers.
> However, upcoming support for Hopper/Blackwell+ adds another firmware
> image (FMC), along with another format (ELF32).
>
> Therefore, the current ELF64 section parsing support needs to be moved
> up a level, so that both of the above can use it.
>
> There are no functional changes. This is pure code movement.
>
> Reviewed-by: Gary Guo <gary@xxxxxxxxxxx>
> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/firmware.rs     | 88 +++++++++++++++++++++++++
>  drivers/gpu/nova-core/firmware/gsp.rs | 93 ++-------------------------
>  2 files changed, 94 insertions(+), 87 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
> index 2bb20081befd..177b8ede151c 100644
> --- a/drivers/gpu/nova-core/firmware.rs
> +++ b/drivers/gpu/nova-core/firmware.rs
> @@ -457,3 +457,91 @@ pub(crate) const fn create(
>          this.0
>      }
>  }
> +
> +/// Ad-hoc and temporary module to extract sections from ELF images.
> +///
> +/// Some firmware images are currently packaged as ELF files, where sections names are used as keys
> +/// to specific and related bits of data. Future firmware versions are scheduled to move away from
> +/// that scheme before nova-core becomes stable, which means this module will eventually be
> +/// removed.
> +mod elf {
> +    use core::mem::size_of;

This import is not needed, `size_of` is already in the prelude.

> +
> +    use kernel::{
> +        bindings,
> +        str::CStr,
> +        transmute::FromBytes, //
> +    };
> +
> +    /// Newtype to provide a [`FromBytes`] implementation.
> +    #[repr(transparent)]
> +    struct Elf64Hdr(bindings::elf64_hdr);
> +    // SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
> +    unsafe impl FromBytes for Elf64Hdr {}
> +
> +    #[repr(transparent)]
> +    struct Elf64SHdr(bindings::elf64_shdr);
> +    // SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
> +    unsafe impl FromBytes for Elf64SHdr {}
> +
> +    /// Tries to extract section with name `name` from the ELF64 image `elf`, and returns it.
> +    pub(super) fn elf64_section<'a, 'b>(elf: &'a [u8], name: &'b str) -> Option<&'a [u8]> {
> +        let hdr = &elf
> +            .get(0..size_of::<bindings::elf64_hdr>())
> +            .and_then(Elf64Hdr::from_bytes)?
> +            .0;
> +
> +        // Get all the section headers.
> +        let mut shdr = {
> +            let shdr_num = usize::from(hdr.e_shnum);
> +            let shdr_start = usize::try_from(hdr.e_shoff).ok()?;
> +            let shdr_end = shdr_num
> +                .checked_mul(size_of::<Elf64SHdr>())
> +                .and_then(|v| v.checked_add(shdr_start))?;
> +
> +            elf.get(shdr_start..shdr_end)
> +                .map(|slice| slice.chunks_exact(size_of::<Elf64SHdr>()))?
> +        };
> +
> +        // Get the strings table.
> +        let strhdr = shdr
> +            .clone()
> +            .nth(usize::from(hdr.e_shstrndx))
> +            .and_then(Elf64SHdr::from_bytes)?;
> +
> +        // Find the section which name matches `name` and return it.
> +        shdr.find(|&sh| {
> +            let Some(hdr) = Elf64SHdr::from_bytes(sh) else {
> +                return false;
> +            };
> +
> +            let Some(name_idx) = strhdr
> +                .0
> +                .sh_offset
> +                .checked_add(u64::from(hdr.0.sh_name))
> +                .and_then(|idx| usize::try_from(idx).ok())
> +            else {
> +                return false;
> +            };
> +
> +            // Get the start of the name.
> +            elf.get(name_idx..)
> +                .and_then(|nstr| CStr::from_bytes_until_nul(nstr).ok())
> +                // Convert into str.
> +                .and_then(|c_str| c_str.to_str().ok())
> +                // Check that the name matches.
> +                .map(|str| str == name)
> +                .unwrap_or(false)
> +        })
> +        // Return the slice containing the section.
> +        .and_then(|sh| {
> +            let hdr = Elf64SHdr::from_bytes(sh)?;
> +            let start = usize::try_from(hdr.0.sh_offset).ok()?;
> +            let end = usize::try_from(hdr.0.sh_size)
> +                .ok()
> +                .and_then(|sh_size| start.checked_add(sh_size))?;
> +
> +            elf.get(start..end)
> +        })
> +    }
> +}
> diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
> index 8bbc3809c640..c6e71339b28e 100644
> --- a/drivers/gpu/nova-core/firmware/gsp.rs
> +++ b/drivers/gpu/nova-core/firmware/gsp.rs
> @@ -1,5 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
> +use core::mem::size_of_val;

And this one is unneeded as well. Actually I mentioned that in my v6
review [1].

[1] https://lore.kernel.org/all/DGZ150DHI878.2YXL15FY7W0GG@xxxxxxxxxx/