Nouveau - Nov 2025 - [PATCH 02/11] gpu: nova-core: add ImemNs section infrastructure

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 at 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.

On Wed, 2025-11-19 at 15:34 -0500, Joel Fernandes wrote:
> 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.
Sure, but the security only applies to Imem, not Dmem.  I didn't want to
come up with a design that
allowed for "Secure Dmem", like your FalconMem struct does. 
That's why I think it makes more sense
to have just two Imem types.