Re: [PATCH v9 06/31] gpu: nova-core: Hopper/Blackwell: skip GFW boot waiting

[Joel Fernandes <joelagnelf@nvidia.com>]

On Mar 30, 2026, at 8:19 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> On Tue Mar 31, 2026 at 3:33 AM JST, Joel Fernandes wrote:
>>
>> On 3/30/2026 10:52 AM, Alexandre Courbot wrote:
>>> Please take a look at how other HALs are implemented: each HAL instance
>>> is in its own module. That's not just a cosmetic choice; it allows us to
>>> keep the chipset's specific HAL struct and its helpers completely
>>> private and forces us to make code-sharing explicit. Furthermore, this
>>> particular HAL is bound to grow, so let's split it properly from the
>>> start.
>>> If you do that it also makes more sense to use constants (contrary to
>>> Gary's feedback on v8), if only to align with the rest of the driver.
>>> Once this is done, making `gpu::hal::tu102` absorb the `gfw` module is
>>> trivial, so let's do that while we are at it - having `gfw` as being
>>> driver-wide makes little sense since it has a very limited role for a
>>> specific subset of the chips we support.
>>
>> I feel a HAL might be overkill for this. Looking at the series, this is
>> also the only method. I am doubtful future architectures will have to
>> once again wait for GFW boot (is that expected?).
>>
>> If not, we can just match or conditional on .arch(). We do that already
>> in other places.
>>
>> Something like:
>>  if spec.chipset().arch() < Architecture::Hopper {
>>      gfw::wait_gfw_boot_completion(bar)?;
>>  }
>
> It's not about the size

Simple things can trivially use match/if on chipset/arch, I just don't see
the benefit of a HAL for simple things, and only drawbacks. Even code-wise
doing the simple snippet above is simpler and cleaner, no need separate new
.rs file.

Even if we look at the HAL approach:

Some one reading the code always thinks that a wait is required but the
wait "mechanism" is arch/chipset-specific, not that NO wait is actually
required sometimes. ONLY if they look at the HAL implementation will they
know that on Hopper+ it is not required.
     hal.wait_gfw_boot_completion(bar)
         .inspect_err(|_| dev_err!(pdev, "GFW boot did not complete\n"))?;

Instead in the snippet above, it is absolutely clear to the reader that
Hopper+ does not need to do GFW wait.

> , it's whether the code paths diverge enough. In
> this case they clearly do, and using a HAL lets us get rid of a
> minor module that was at the root of the project (`gfw.rs`) entirely.
>
> I expect this HAL to further grow with the different boot paths as well.
> Not sure if that will happen in this series, but it will eventually.

But just inlining the module in gpu.rs will also get rid of the module?
That's not a strong argument for a HAL. gfw just has one function.

Also, I checked and nouveau does not use HALs for GFW boot.

Other than that, I am concerned about HAL fragmentation (we already have
other more specific HALs like the fb HAL and falcon HAL). So should those
functionalities also go into gpu HAL? Its not clear.

What about other code doing match on chipset -- should all that also go
into this kitchen-sink gpu HAL then? It
needs to be consistent, not "let's put gfw in a HAL because we feel like it
even though we know we're never going to ever have to do per-chipset/arch
gfw wait." There is not much reason for that as far as I can see.

The argument that the HAL will grow in the future also doesn't hold,
since a HAL can be trivially added if and when needed. My understanding is
we don't want to add code until we need it and you haven't provided any
"future" things that will be added to this new common gpu HAL.

Please keep it simple :). The snippet I shared above is just 3 lines, not
new 41 line boiler plate HAL file.

Now if some variant of Blackwell starts doing some custom GFW boot scheme,
I am all for a HAL...

thanks,

-- 
Joel Fernandes