Re: [PATCH 02/11] gpu: nova-core: add ImemNs section infrastructure

[Joel Fernandes <joelagnelf@nvidia.com>]

On 11/19/2025 2:54 PM, Timur Tabi wrote:
> On Wed, 2025-11-19 at 15:55 +0900, Alexandre Courbot wrote:
>> On Wed Nov 19, 2025 at 3:30 PM JST, John Hubbard wrote:
>>> On 11/18/25 5:54 PM, Alexandre Courbot wrote:
>>>> On Sat Nov 15, 2025 at 8:30 AM JST, Timur Tabi wrote:
>>>>> The GSP booter firmware in Turing and GA100 includes a third memory
>>>>> section called ImemNs, which is non-secure IMEM.  This section must
>>>>> be loaded separately from DMEM and secure IMEM, but only if it
>>>>> actually exists.
>>>>>
>>>>> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
>>>>> ---
>>>>>   drivers/gpu/nova-core/falcon.rs          | 18 ++++++++++++++++--
>>>>>   drivers/gpu/nova-core/firmware/booter.rs |  9 +++++++++
>>>>>   drivers/gpu/nova-core/firmware/fwsec.rs  |  5 +++++
>>>>>   3 files changed, 30 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
>>>>> index 0e0935dbb927..ece8b92a627e 100644
>>>>> --- a/drivers/gpu/nova-core/falcon.rs
>>>>> +++ b/drivers/gpu/nova-core/falcon.rs
>>>>> @@ -239,6 +239,8 @@ fn from(value: PeregrineCoreSelect) -> Self {
>>>>>   pub(crate) enum FalconMem {
>>>>>       /// Secure Instruction Memory.
>>>>>       ImemSec,
>>>>> +    /// Non-Secure Instruction Memory.
>>>>> +    ImemNs,
>>>>
>>>> So, seeing how this is taking shape I now think we should just have one
>>>> Imem variant:
>>>>
>>>>      Imem { secure: bool },
>>>
>>> ohhh, boolean args are usually not a good idea, because they make the
>>> callsite non-self-documenting.
>>>
>>> That's also true even in magical languages such as Rust. :)
>>
>> I fully agree; that's why I made the field named so its name needs to be
>> specified every time. :) Maybe `is_secure` would have been better
>> though.
>>
>>>
>>> Let's prefer enum args over bools, generally, please. So for example
>>> (there are other ways to structure things, and this is just the
>>> enum aspect of it):
>>>
>>>      enum ImemSecurity {
>>>          Secure,
>>>          NonSecure,
>>>      }
>>>
>>>     Imem { security: ImemSecurity },
>>
>> That would change
>>
>>     FalconMem::Imem { secure: true }
>>
>> into
>>
>>     FalconMem::Imem {security: ImemSecurity::Secure }
>>
>> If we want to use an enum I think we can remove the name:
>>
>>      Imem(ImemSecurity),
>>
>> So we can specify `Imem` as
>>
>>     FalconMem::Imem(ImemSecurity::Secure)
>>
>> which is as explicit, and a bit shorter.
A struct could be another option? You have 2 entities here, the location of the
memory (instruction memory or data memory) and the secure aspect.

struct FalconMem {
   type: FalconMemType,  // enum which can be instruction or data
   security: FalconMemSecurity, // enum can be secure or insecure.
}

That documents everything. But it is just an option I am putting out to consider
if it helps.

Thanks.