On Thu Nov 20, 2025 at 4:54 AM JST, 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. > > I fail to see how any of this is an improvement.
It lets you capture both `Imem` variants with a single match arm instead of having to remember to use `|`, a pattern that is common in this series.
