Re: [PATCH v11 09/22] gpu: nova-core: add auto-detection of 32-bit, 64-bit firmware images

["Eliot Courtney" <ecourtney@nvidia.com>]

On Sat May 30, 2026 at 12:09 PM JST, John Hubbard wrote:
> A firmware image may be either a 32-bit or a 64-bit ELF, and callers
> should not have to know which. Detect the ELF class from the image
> header at parse time and dispatch to the matching parser, so a single
> entry point handles both layouts.
>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  drivers/gpu/nova-core/firmware.rs     | 22 ++++++++++++++++++----
>  drivers/gpu/nova-core/firmware/gsp.rs |  4 ++--
>  2 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
> index e4dcc9a87b7e..866bc9b3571e 100644
> --- a/drivers/gpu/nova-core/firmware.rs
> +++ b/drivers/gpu/nova-core/firmware.rs
> @@ -629,14 +629,28 @@ fn elf_section_generic<'a, F>(elf: &'a [u8], name: &str) -> Option<&'a [u8]>
>          })
>      }
>  
> -    /// Tries to extract section with name `name` from the ELF64 image `elf`, and returns it.
> -    pub(super) fn elf64_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a [u8]> {
> +    /// Extract the section with name `name` from the ELF64 image `elf`.
> +    fn elf64_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a [u8]> {
>          elf_section_generic::<Elf64Format>(elf, name)
>      }
>  
>      /// Extract the section with name `name` from the ELF32 image `elf`.
> -    #[expect(dead_code)]
> -    pub(super) fn elf32_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a [u8]> {
> +    fn elf32_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a [u8]> {
>          elf_section_generic::<Elf32Format>(elf, name)
>      }
> +
> +    /// Automatically detects ELF32 vs ELF64 based on the ELF header.
> +    pub(super) fn elf_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a [u8]> {
> +        // Check ELF magic.
> +        if elf.len() < 5 || elf.get(0..4)? != b"\x7fELF" {
> +            return None;
> +        }
> +
> +        // Check ELF class: 1 = 32-bit, 2 = 64-bit.
> +        match elf.get(4)? {
> +            1 => elf32_section(elf, name),
> +            2 => elf64_section(elf, name),
> +            _ => None,
> +        }
> +    }
>  }

What about adding named constants (inline in the function) for these
magic numbers?

The elf.len() check looks unnecessary since you are using .get() and
returning an Option rather than a Result. Instead you could do
`if elf.get(0..SELFMAG) != Some(ELFMAG)`.

With those resolved,

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>