Re: [PATCH 03/11] gpu: nova-core: support header parsing on Turing/GA100

[Joel Fernandes <joelagnelf@nvidia.com>]

On Fri, Nov 14, 2025 at 05:30:36PM -0600, Timur Tabi wrote:
> The Turing/GA100 version of Booter is slightly different from the
> GA102+ version.  The headers are the same, but different fields of
> the headers are used to identify the IMEM section.  In addition,
> there is an NMEM section on Turing/GA100.
> 
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  drivers/gpu/nova-core/firmware/booter.rs | 40 +++++++++++++++++++-----
>  1 file changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
> index 1b98bb47424c..6ac9593504db 100644
> --- a/drivers/gpu/nova-core/firmware/booter.rs
> +++ b/drivers/gpu/nova-core/firmware/booter.rs
> @@ -356,14 +356,34 @@ pub(crate) fn new(
>              }
>          };
>  
> +        // There are two versions of Booter, one for Turing/GA100, and another for
> +        // GA102+.  The extraction of the IMEM sections differs between the two
> +        // versions.  Unfortunately, the file names are the same, and the headers
> +        // don't indicate the versions.  The only way to differentiate is by the Chipset.

Some more doc comments and documentation explaining the header structure
would be great.

> +
>          Ok(Self {
> -            imem_sec_load_target: FalconLoadTarget {
> -                src_start: app0.offset,
> -                dst_start: 0,
> -                len: app0.len,
> +            imem_sec_load_target: if chipset > Chipset::GA100 {
> +                FalconLoadTarget {
> +                    src_start: app0.offset,
> +                    dst_start: 0,
> +                    len: app0.len,
> +                }
> +            } else {
> +                FalconLoadTarget {
> +                    src_start: load_hdr.os_code_size,
> +                    dst_start: app0.offset,
> +                    len: app0.len,
> +                }

Can write more succinctly:

  imem_sec_load_target: FalconLoadTarget {
      src_start: match chipset > Chipset::GA100  {
          true => app0.offset,
          false => load_hdr.os_code_size,
      },
      dst_start: match chipset > Chipset::GA100 {
          true => 0,
          false => app0.offset,
      
      len: app0.len,                                                                            
  },

> +            },
> +            imem_ns_load_target: if chipset > Chipset::GA100 {
> +                None
> +            } else {
> +                Some(FalconLoadTarget {
> +                    src_start: 0,
> +                    dst_start: load_hdr.os_code_offset,
> +                    len: load_hdr.os_code_size,
> +                })
>              },
> -            // Exists only in the booter image for Turing and GA100
> -            imem_ns_load_target: None,
>              dmem_load_target: FalconLoadTarget {
>                  src_start: load_hdr.os_data_offset,
>                  dst_start: 0,
> @@ -393,7 +413,13 @@ fn brom_params(&self) -> FalconBromParams {
>      }
>  
>      fn boot_addr(&self) -> u32 {
> -        self.imem_sec_load_target.src_start
> +        if let Some(ns_target) = &self.imem_ns_load_target {
> +            // Turing and GA100 - use non-secure load target
> +            ns_target.dst_start
> +        } else {
> +            // GA102+ (Ampere) - use secure load target

s/Ampere/Ampere and later/ ? Also missing period at end of comment, here and
elsewhere.

thanks,

 - Joel

> +            self.imem_sec_load_target.src_start
> +        }
>      }
>  }
>  
> -- 
> 2.51.2
>