On Sat Jan 24, 2026 at 3:07 AM JST, Gary Guo wrote: > On Thu Jan 22, 2026 at 10:28 PM GMT, Timur Tabi wrote: >> The GSP booter firmware in Turing and GA100 includes a third memory >> section called ImemNonSecure, 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]> >> Reviewed-by: John Hubbard <[email protected]> >> --- >> drivers/gpu/nova-core/falcon.rs | 19 +++++++++++++++++-- >> drivers/gpu/nova-core/firmware/booter.rs | 9 +++++++++ >> drivers/gpu/nova-core/firmware/fwsec.rs | 5 +++++ >> 3 files changed, 31 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/nova-core/falcon.rs >> b/drivers/gpu/nova-core/falcon.rs >> index 310d4e75bad3..0d1a0f86d83c 100644 >> --- a/drivers/gpu/nova-core/falcon.rs >> +++ b/drivers/gpu/nova-core/falcon.rs >> @@ -242,6 +242,9 @@ fn from(value: PeregrineCoreSelect) -> Self { >> pub(crate) enum FalconMem { >> /// Secure Instruction Memory. >> ImemSecure, >> + /// Non-Secure Instruction Memory. >> + #[expect(unused)] >> + ImemNonSecure, >> /// Data Memory. >> Dmem, >> } >> @@ -351,6 +354,10 @@ pub(crate) trait FalconLoadParams { >> /// Returns the load parameters for Secure `IMEM`. >> fn imem_sec_load_params(&self) -> FalconLoadTarget; >> >> + /// Returns the load parameters for Non-Secure `IMEM`, >> + /// used only on Turing and GA100. >> + fn imem_ns_load_params(&self) -> Option<FalconLoadTarget>; >> + >> /// Returns the load parameters for `DMEM`. >> fn dmem_load_params(&self) -> FalconLoadTarget; >> >> @@ -460,7 +467,9 @@ fn dma_wr<F: FalconFirmware<Target = E>>( >> // >> // For DMEM we can fold the start offset into the DMA handle. >> let (src_start, dma_start) = match target_mem { >> - FalconMem::ImemSecure => (load_offsets.src_start, >> fw.dma_handle()), >> + FalconMem::ImemSecure | FalconMem::ImemNonSecure => { >> + (load_offsets.src_start, fw.dma_handle()) >> + } >> FalconMem::Dmem => ( >> 0, >> >> fw.dma_handle_with_offset(load_offsets.src_start.into_safe_cast())?, >> @@ -517,7 +526,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>( >> >> let cmd = regs::NV_PFALCON_FALCON_DMATRFCMD::default() >> .set_size(DmaTrfCmdSize::Size256B) >> - .set_imem(target_mem == FalconMem::ImemSecure) >> + .set_imem(target_mem != FalconMem::Dmem) >> .set_sec(if sec { 1 } else { 0 }); >> >> for pos in (0..num_transfers).map(|i| i * DMA_LEN) { >> @@ -546,6 +555,12 @@ fn dma_wr<F: FalconFirmware<Target = E>>( >> >> /// Perform a DMA load into `IMEM` and `DMEM` of `fw`, and prepare the >> falcon to run it. >> pub(crate) fn dma_load<F: FalconFirmware<Target = E>>(&self, bar: >> &Bar0, fw: &F) -> Result { >> + // The Non-Secure section only exists on firmware used by Turing >> and GA100, and >> + // those platforms do not use DMA. >> + if fw.imem_ns_load_params().is_some() { >> + return Err(EINVAL); >> + } > > As pointed out in last round of review this is unreachable code. > > I am not too against in leaving it in as is, but for checks that assist > debugging only, I would probably use `debug_assert` myself.
I expect this bit of code to go away when we address the remaining issues with the last 2 patches, but let me add a `debug_assert` for now.
