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 <[email protected]>
>>>>> ---
>>>>> 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.
