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

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

On Tue Nov 25, 2025 at 8:24 AM JST, Timur Tabi wrote:
> On Wed, 2025-11-19 at 16:03 +0900, Alexandre Courbot wrote:
>> If it comes down to "This is how things are pre and post GA102" (and the
>> evidence I have seen to far suggests that unfortunately), then so be it
>> - we at the very least encode this as a method of `Chipset` to avoid
>> hardcoding chipset versions in several places.
>
> I could add this method to Chipset:
>
>     pub(crate) fn supports_dma(&self) -> bool {
> 	*self > Self::GA100
>     }
>
> But shouldn't this be added to the HAL instead?

Agreed, that's a better place for that.

>
> There are a few things that are arbitrarily conditional:
>
> 1) whether to use PIO or DMA to transfer firmware images to Falcons

As you said this one could be abstracted in the falcon HAL, by having a
(public) load method that calls back into the (now private) DMA or PIO
methods of `falcon.rs`.

> 2) whether a Generic Bootloader is needed for load FWSEC

I suspect this one is correlated to the FWSEC header version? But maybe
I am just seeing patterns.

> 3) whether a Non-Secure IMEM section exists

This is the one that gives us headaches currently...

> 4) what the name of the signature section in the GSP-RM ELF image is

This one is appropriately dealt with with the match statement in the
code - after all, these ARE dependent on the chipset.

>
> If I'm reading it right, booter.rs currently has no concept of a HAL, which is why I gated on
> Chipset.  I agree that doing stuff like
>
>                 FalconLoadTarget {
>                     src_start: app0.offset,
>                     dst_start: if chipset > Chipset::GA100 { 0 } else { app0.offset },
>                     len: app0.len,
>                 },

`booter.rs` won't have a HAL because it is not a hardware part. So worst
case, if there is really nothing better, relying on the chipset is
acceptable as this is just how the firmware is designed.

>
> is not ideal.  To be honest, it's hard to wrap my head around all of these offsets.  I'm convinced
> that most of the confusion is the result of RM developers trying to shoehorn complex firmware images
> into legacy file formats.  Just looking at the above:
>
> On Turing, the secure IMEM image is copied from app0.offset in the file to -> app0.offset (256) in
> IMEM.
> On Ampere, the secure IMEM image is copied from app0.offset in the file to -> 0 in IMEM.
>
> Why? Apparently, because on Ampere we copy the Secure image to offset 0 and ignore the Non-Secure
> image.  On Turing, we copy the Non-Secure image to offset 0 and copy the Secure image to offset 256.
> It would have been much easier to copy the Secure image to 256 on Ampere also, but that's just not
> what happens.
>
> For some reason, os_code_offset and os_code_size contain the same values on both Turing and Ampere,
> but those values are used only on Turing to hold the Non-Secure section.  So the firmware image
> headers say that there's a non-secure IMEM section on Ampere, but it's a lie.

Yup, I was hoping that we could discriminate between the two using the
size of the non-secure image (which I expected to be zero on Ampere+),
but ... no. :')

Now with moving the loading abstraction to the HAL, I guess that would
leave only `booter.rs` as the place where we need to match on the
chipset "because that's the way it is". If that's the case, let's not
spend too much time looking for patterns that don't exist, a comment
explaining the situation will do and we can try and fix this in future
firmware versions.

Thanks for looking into it!