* [PATCH v6 00/11] gpu: nova-core: add Turing support
@ 2026-01-14 19:29 Timur Tabi
2026-01-14 19:29 ` [PATCH v6 01/11] gpu: nova-core: rename Imem to ImemSecure Timur Tabi
` (10 more replies)
0 siblings, 11 replies; 29+ messages in thread
From: Timur Tabi @ 2026-01-14 19:29 UTC (permalink / raw)
To: Alexandre Courbot, John Hubbard, Joel Fernandes, Danilo Krummrich,
nouveau, rust-for-linux
Note: This patchset requires "[PATCH v3 2/7] rust: io: always inline
functions using build_assert with arguments" in order to compile
with CLIPPY.
This patch set adds basic support for pre-booting GSP-RM
on Turing.
There is also partial support for GA100, but it's currently not
fully implemented. GA100 is considered experimental in Nouveau,
and so it hasn't been tested with NovaCore either.
The latest linux-firmware.git is required because it contains the
Generic Bootloader image that has not yet been propogated to
distros.
Summary of changes:
1. Introduce non-secure IMEM support. For GA102 and later, only secure IMEM
is used.
2. Because of non-secure IMEM, Turing booter firmware images need some of
the headers parsed differently for stuff like the load target address.
3. Add support the tu10x firmware signature section in the ELF image.
4. Add several new registers used only on Turing.
5. Some functions that were considered generic Falcon operations are
actually different on Turing vs GA102+, so they are moved to the HAL.
6. The FRTS FWSEC firmware in VBIOS uses a different version of the
descriptor header.
7. On Turing/GA100 LIBOS args struct needs to have its 'size' field
aligned to 4KB. So pad the struct to make it 4K.
8. Turing Falcons do not support DMA, so PIO is used to copy images
into IMEM/DMEM.
Changes from v5:
1. Fixed rustfmt complaints
2. Fixed stray diff in Alex's patch
3. Added checked_sub to imem_ns_load_params()
Alexandre Courbot (1):
gpu: nova-core: align LibosMemoryRegionInitArgument size to page size
Timur Tabi (10):
gpu: nova-core: rename Imem to ImemSecure
gpu: nova-core: add ImemNonSecure section infrastructure
gpu: nova-core: support header parsing on Turing/GA100
gpu: nova-core: add support for Turing/GA100 fwsignature
gpu: nova-core: add NV_PFALCON_FALCON_DMATRFCMD::with_falcon_mem()
gpu: nova-core: move some functions into the HAL
gpu: nova-core: Add basic Turing HAL
gpu: nova-core: add Falcon HAL method supports_dma()
gpu: nova-core: add FalconUCodeDescV2 support
gpu: nova-core: add PIO support for loading firmware images
drivers/gpu/nova-core/falcon.rs | 251 +++++++++++++++++-----
drivers/gpu/nova-core/falcon/hal.rs | 22 ++
drivers/gpu/nova-core/falcon/hal/ga102.rs | 46 ++++
drivers/gpu/nova-core/falcon/hal/tu102.rs | 84 ++++++++
drivers/gpu/nova-core/firmware.rs | 149 ++++++++++++-
drivers/gpu/nova-core/firmware/booter.rs | 43 +++-
drivers/gpu/nova-core/firmware/fwsec.rs | 204 +++++++++++++++---
drivers/gpu/nova-core/firmware/gsp.rs | 6 +-
drivers/gpu/nova-core/gsp.rs | 8 +-
drivers/gpu/nova-core/gsp/boot.rs | 6 +-
drivers/gpu/nova-core/gsp/fw.rs | 14 +-
drivers/gpu/nova-core/regs.rs | 53 +++++
drivers/gpu/nova-core/vbios.rs | 75 ++++---
13 files changed, 838 insertions(+), 123 deletions(-)
create mode 100644 drivers/gpu/nova-core/falcon/hal/tu102.rs
base-commit: 654826aa4a8f25cf825ad9254f37e6cb5092098f
prerequisite-patch-id: a3e23917ec535263604af95194422382f14c2f21
--
2.52.0
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v6 01/11] gpu: nova-core: rename Imem to ImemSecure
2026-01-14 19:29 [PATCH v6 00/11] gpu: nova-core: add Turing support Timur Tabi
@ 2026-01-14 19:29 ` Timur Tabi
2026-01-14 19:29 ` [PATCH v6 02/11] gpu: nova-core: add ImemNonSecure section infrastructure Timur Tabi
` (9 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Timur Tabi @ 2026-01-14 19:29 UTC (permalink / raw)
To: Alexandre Courbot, John Hubbard, Joel Fernandes, Danilo Krummrich,
nouveau, rust-for-linux
Rename FalconMem::Imem to ImemSecure to indicate that it references
Secure Instruction Memory. This change has no functional impact.
On Falcon cores, pages in instruction memory can be tagged as Secure
or Non-Secure. For GA102 and later, only Secure is used, which is why
FalconMem::Imem seems appropriate. However, Turing firmware images
can also contain non-secure sections, and so FalconMem needs to support
that. By renaming Imem to ImemSec now, future patches for Turing support
will be simpler.
Nouveau uses the term "IMEM" to refer both to the Instruction Memory
block on Falcon cores as well as to the images of secure firmware
uploaded to part of IMEM. OpenRM uses the terms "ImemSec" and "ImemNs"
instead, and uses "IMEM" just to refer to the physical memory device.
Renaming these terms allows us to align with OpenRM, avoid confusion
between IMEM and ImemSec, and makes future patches simpler.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
---
drivers/gpu/nova-core/falcon.rs | 20 +++++++++++++-------
drivers/gpu/nova-core/firmware/booter.rs | 12 ++++++------
drivers/gpu/nova-core/firmware/fwsec.rs | 2 +-
3 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 46b02c8a591e..310d4e75bad3 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -240,8 +240,8 @@ fn from(value: PeregrineCoreSelect) -> Self {
/// Different types of memory present in a falcon core.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum FalconMem {
- /// Instruction Memory.
- Imem,
+ /// Secure Instruction Memory.
+ ImemSecure,
/// Data Memory.
Dmem,
}
@@ -348,8 +348,8 @@ pub(crate) struct FalconBromParams {
/// Trait for providing load parameters of falcon firmwares.
pub(crate) trait FalconLoadParams {
- /// Returns the load parameters for `IMEM`.
- fn imem_load_params(&self) -> FalconLoadTarget;
+ /// Returns the load parameters for Secure `IMEM`.
+ fn imem_sec_load_params(&self) -> FalconLoadTarget;
/// Returns the load parameters for `DMEM`.
fn dmem_load_params(&self) -> FalconLoadTarget;
@@ -460,7 +460,7 @@ 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::Imem => (load_offsets.src_start, fw.dma_handle()),
+ FalconMem::ImemSecure => (load_offsets.src_start, fw.dma_handle()),
FalconMem::Dmem => (
0,
fw.dma_handle_with_offset(load_offsets.src_start.into_safe_cast())?,
@@ -517,7 +517,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::Imem)
+ .set_imem(target_mem == FalconMem::ImemSecure)
.set_sec(if sec { 1 } else { 0 });
for pos in (0..num_transfers).map(|i| i * DMA_LEN) {
@@ -552,7 +552,13 @@ pub(crate) fn dma_load<F: FalconFirmware<Target = E>>(&self, bar: &Bar0, fw: &F)
.set_mem_type(FalconFbifMemType::Physical)
});
- self.dma_wr(bar, fw, FalconMem::Imem, fw.imem_load_params(), true)?;
+ self.dma_wr(
+ bar,
+ fw,
+ FalconMem::ImemSecure,
+ fw.imem_sec_load_params(),
+ true,
+ )?;
self.dma_wr(bar, fw, FalconMem::Dmem, fw.dmem_load_params(), true)?;
self.hal.program_brom(self, bar, &fw.brom_params())?;
diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
index f107f753214a..096cd01dbc9d 100644
--- a/drivers/gpu/nova-core/firmware/booter.rs
+++ b/drivers/gpu/nova-core/firmware/booter.rs
@@ -251,8 +251,8 @@ impl<'a> FirmwareSignature<BooterFirmware> for BooterSignature<'a> {}
/// The `Booter` loader firmware, responsible for loading the GSP.
pub(crate) struct BooterFirmware {
- // Load parameters for `IMEM` falcon memory.
- imem_load_target: FalconLoadTarget,
+ // Load parameters for Secure `IMEM` falcon memory.
+ imem_sec_load_target: FalconLoadTarget,
// Load parameters for `DMEM` falcon memory.
dmem_load_target: FalconLoadTarget,
// BROM falcon parameters.
@@ -354,7 +354,7 @@ pub(crate) fn new(
};
Ok(Self {
- imem_load_target: FalconLoadTarget {
+ imem_sec_load_target: FalconLoadTarget {
src_start: app0.offset,
dst_start: 0,
len: app0.len,
@@ -371,8 +371,8 @@ pub(crate) fn new(
}
impl FalconLoadParams for BooterFirmware {
- fn imem_load_params(&self) -> FalconLoadTarget {
- self.imem_load_target.clone()
+ fn imem_sec_load_params(&self) -> FalconLoadTarget {
+ self.imem_sec_load_target.clone()
}
fn dmem_load_params(&self) -> FalconLoadTarget {
@@ -384,7 +384,7 @@ fn brom_params(&self) -> FalconBromParams {
}
fn boot_addr(&self) -> u32 {
- self.imem_load_target.src_start
+ self.imem_sec_load_target.src_start
}
}
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index b28e34d279f4..6a2f5a0d4b15 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -224,7 +224,7 @@ pub(crate) struct FwsecFirmware {
}
impl FalconLoadParams for FwsecFirmware {
- fn imem_load_params(&self) -> FalconLoadTarget {
+ fn imem_sec_load_params(&self) -> FalconLoadTarget {
FalconLoadTarget {
src_start: 0,
dst_start: self.desc.imem_phys_base,
--
2.52.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 02/11] gpu: nova-core: add ImemNonSecure section infrastructure
2026-01-14 19:29 [PATCH v6 00/11] gpu: nova-core: add Turing support Timur Tabi
2026-01-14 19:29 ` [PATCH v6 01/11] gpu: nova-core: rename Imem to ImemSecure Timur Tabi
@ 2026-01-14 19:29 ` Timur Tabi
2026-01-22 12:52 ` Gary Guo
2026-01-14 19:29 ` [PATCH v6 03/11] gpu: nova-core: support header parsing on Turing/GA100 Timur Tabi
` (8 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Timur Tabi @ 2026-01-14 19:29 UTC (permalink / raw)
To: Alexandre Courbot, John Hubbard, Joel Fernandes, Danilo Krummrich,
nouveau, rust-for-linux
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 <ttabi@nvidia.com>
Reviewed-by: John Hubbard <jhubbard@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 310d4e75bad3..60713fecffc9 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -242,6 +242,8 @@ fn from(value: PeregrineCoreSelect) -> Self {
pub(crate) enum FalconMem {
/// Secure Instruction Memory.
ImemSecure,
+ /// Non-Secure Instruction Memory.
+ ImemNonSecure,
/// Data Memory.
Dmem,
}
@@ -351,6 +353,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 +466,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 +525,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 +554,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);
+ }
+
self.dma_reset(bar);
regs::NV_PFALCON_FBIF_TRANSCFG::update(bar, &E::ID, 0, |v| {
v.set_target(FalconFbifTarget::CoherentSysmem)
diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
index 096cd01dbc9d..1b98bb47424c 100644
--- a/drivers/gpu/nova-core/firmware/booter.rs
+++ b/drivers/gpu/nova-core/firmware/booter.rs
@@ -253,6 +253,9 @@ impl<'a> FirmwareSignature<BooterFirmware> for BooterSignature<'a> {}
pub(crate) struct BooterFirmware {
// Load parameters for Secure `IMEM` falcon memory.
imem_sec_load_target: FalconLoadTarget,
+ // Load parameters for Non-Secure `IMEM` falcon memory,
+ // used only on Turing and GA100
+ imem_ns_load_target: Option<FalconLoadTarget>,
// Load parameters for `DMEM` falcon memory.
dmem_load_target: FalconLoadTarget,
// BROM falcon parameters.
@@ -359,6 +362,8 @@ pub(crate) fn new(
dst_start: 0,
len: app0.len,
},
+ // Exists only in the booter image for Turing and GA100
+ imem_ns_load_target: None,
dmem_load_target: FalconLoadTarget {
src_start: load_hdr.os_data_offset,
dst_start: 0,
@@ -375,6 +380,10 @@ fn imem_sec_load_params(&self) -> FalconLoadTarget {
self.imem_sec_load_target.clone()
}
+ fn imem_ns_load_params(&self) -> Option<FalconLoadTarget> {
+ self.imem_ns_load_target.clone()
+ }
+
fn dmem_load_params(&self) -> FalconLoadTarget {
self.dmem_load_target.clone()
}
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index 6a2f5a0d4b15..e4009faba6c5 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -232,6 +232,11 @@ fn imem_sec_load_params(&self) -> FalconLoadTarget {
}
}
+ fn imem_ns_load_params(&self) -> Option<FalconLoadTarget> {
+ // Only used on Turing and GA100, so return None for now
+ None
+ }
+
fn dmem_load_params(&self) -> FalconLoadTarget {
FalconLoadTarget {
src_start: self.desc.imem_load_size,
--
2.52.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 03/11] gpu: nova-core: support header parsing on Turing/GA100
2026-01-14 19:29 [PATCH v6 00/11] gpu: nova-core: add Turing support Timur Tabi
2026-01-14 19:29 ` [PATCH v6 01/11] gpu: nova-core: rename Imem to ImemSecure Timur Tabi
2026-01-14 19:29 ` [PATCH v6 02/11] gpu: nova-core: add ImemNonSecure section infrastructure Timur Tabi
@ 2026-01-14 19:29 ` Timur Tabi
2026-01-14 19:29 ` [PATCH v6 04/11] gpu: nova-core: add support for Turing/GA100 fwsignature Timur Tabi
` (7 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Timur Tabi @ 2026-01-14 19:29 UTC (permalink / raw)
To: Alexandre Courbot, John Hubbard, Joel Fernandes, Danilo Krummrich,
nouveau, rust-for-linux
The Turing/GA100 version of Booter is slightly different from the
GA102+ version. The headers are the same, but different fields of
the headers are used to identify the IMEM section. In addition,
there is an NMEM section on Turing/GA100.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
---
drivers/gpu/nova-core/firmware/booter.rs | 28 ++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
index 1b98bb47424c..86556cee8e67 100644
--- a/drivers/gpu/nova-core/firmware/booter.rs
+++ b/drivers/gpu/nova-core/firmware/booter.rs
@@ -356,14 +356,30 @@ pub(crate) fn new(
}
};
+ // There are two versions of Booter, one for Turing/GA100, and another for
+ // GA102+. The extraction of the IMEM sections differs between the two
+ // versions. Unfortunately, the file names are the same, and the headers
+ // don't indicate the versions. The only way to differentiate is by the Chipset.
+ let (imem_sec_dst_start, imem_ns_load_target) = if chipset <= Chipset::GA100 {
+ (
+ app0.offset,
+ Some(FalconLoadTarget {
+ src_start: 0,
+ dst_start: load_hdr.os_code_offset,
+ len: load_hdr.os_code_size,
+ }),
+ )
+ } else {
+ (0, None)
+ };
+
Ok(Self {
imem_sec_load_target: FalconLoadTarget {
src_start: app0.offset,
- dst_start: 0,
+ dst_start: imem_sec_dst_start,
len: app0.len,
},
- // Exists only in the booter image for Turing and GA100
- imem_ns_load_target: None,
+ imem_ns_load_target,
dmem_load_target: FalconLoadTarget {
src_start: load_hdr.os_data_offset,
dst_start: 0,
@@ -393,7 +409,11 @@ fn brom_params(&self) -> FalconBromParams {
}
fn boot_addr(&self) -> u32 {
- self.imem_sec_load_target.src_start
+ if let Some(ns_target) = &self.imem_ns_load_target {
+ ns_target.dst_start
+ } else {
+ self.imem_sec_load_target.src_start
+ }
}
}
--
2.52.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 04/11] gpu: nova-core: add support for Turing/GA100 fwsignature
2026-01-14 19:29 [PATCH v6 00/11] gpu: nova-core: add Turing support Timur Tabi
` (2 preceding siblings ...)
2026-01-14 19:29 ` [PATCH v6 03/11] gpu: nova-core: support header parsing on Turing/GA100 Timur Tabi
@ 2026-01-14 19:29 ` Timur Tabi
2026-01-14 19:29 ` [PATCH v6 05/11] gpu: nova-core: add NV_PFALCON_FALCON_DMATRFCMD::with_falcon_mem() Timur Tabi
` (6 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Timur Tabi @ 2026-01-14 19:29 UTC (permalink / raw)
To: Alexandre Courbot, John Hubbard, Joel Fernandes, Danilo Krummrich,
nouveau, rust-for-linux
Turing and GA100 share the same GSP-RM firmware binary, but the
signature ELF section is labeled either ".fwsignature_tu10x" or
".fwsignature_tu11x".
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
---
drivers/gpu/nova-core/firmware/gsp.rs | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
index 1025b7f746eb..3f75398954be 100644
--- a/drivers/gpu/nova-core/firmware/gsp.rs
+++ b/drivers/gpu/nova-core/firmware/gsp.rs
@@ -214,9 +214,13 @@ pub(crate) fn new<'a>(
size,
signatures: {
let sigs_section = match chipset.arch() {
+ Architecture::Turing if chipset == Chipset::TU116 || chipset == Chipset::TU117 =>
+ ".fwsignature_tu11x",
+ Architecture::Turing => ".fwsignature_tu10x",
+ // GA100 uses the same firmware as Turing
+ Architecture::Ampere if chipset == Chipset::GA100 => ".fwsignature_tu10x",
Architecture::Ampere => ".fwsignature_ga10x",
Architecture::Ada => ".fwsignature_ad10x",
- _ => return Err(ENOTSUPP),
};
elf::elf64_section(firmware.data(), sigs_section)
--
2.52.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 05/11] gpu: nova-core: add NV_PFALCON_FALCON_DMATRFCMD::with_falcon_mem()
2026-01-14 19:29 [PATCH v6 00/11] gpu: nova-core: add Turing support Timur Tabi
` (3 preceding siblings ...)
2026-01-14 19:29 ` [PATCH v6 04/11] gpu: nova-core: add support for Turing/GA100 fwsignature Timur Tabi
@ 2026-01-14 19:29 ` Timur Tabi
2026-01-14 19:29 ` [PATCH v6 06/11] gpu: nova-core: move some functions into the HAL Timur Tabi
` (5 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Timur Tabi @ 2026-01-14 19:29 UTC (permalink / raw)
To: Alexandre Courbot, John Hubbard, Joel Fernandes, Danilo Krummrich,
nouveau, rust-for-linux
The with_falcon_mem() method initializes the 'imem' and 'sec' fields of
the NV_PFALCON_FALCON_DMATRFCMD register based on the value of
the FalconMem type.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
---
drivers/gpu/nova-core/falcon.rs | 14 +++-----------
drivers/gpu/nova-core/regs.rs | 9 +++++++++
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 60713fecffc9..2a8fb7059a44 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -457,7 +457,6 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
fw: &F,
target_mem: FalconMem,
load_offsets: FalconLoadTarget,
- sec: bool,
) -> Result {
const DMA_LEN: u32 = 256;
@@ -525,8 +524,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::Dmem)
- .set_sec(if sec { 1 } else { 0 });
+ .with_falcon_mem(target_mem);
for pos in (0..num_transfers).map(|i| i * DMA_LEN) {
// Perform a transfer of size `DMA_LEN`.
@@ -566,14 +564,8 @@ pub(crate) fn dma_load<F: FalconFirmware<Target = E>>(&self, bar: &Bar0, fw: &F)
.set_mem_type(FalconFbifMemType::Physical)
});
- self.dma_wr(
- bar,
- fw,
- FalconMem::ImemSecure,
- fw.imem_sec_load_params(),
- true,
- )?;
- self.dma_wr(bar, fw, FalconMem::Dmem, fw.dmem_load_params(), true)?;
+ self.dma_wr(bar, fw, FalconMem::ImemSecure, fw.imem_sec_load_params())?;
+ self.dma_wr(bar, fw, FalconMem::Dmem, fw.dmem_load_params())?;
self.hal.program_brom(self, bar, &fw.brom_params())?;
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 82cc6c0790e5..b8ddfe2e5ae7 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -16,6 +16,7 @@
FalconCoreRevSubversion,
FalconFbifMemType,
FalconFbifTarget,
+ FalconMem,
FalconModSelAlgo,
FalconSecurityModel,
PFalcon2Base,
@@ -325,6 +326,14 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
16:16 set_dmtag as u8;
});
+impl NV_PFALCON_FALCON_DMATRFCMD {
+ /// Programs the `imem` and `sec` fields for the given FalconMem
+ pub(crate) fn with_falcon_mem(self, mem: FalconMem) -> Self {
+ self.set_imem(mem != FalconMem::Dmem)
+ .set_sec(if mem == FalconMem::ImemSecure { 1 } else { 0 })
+ }
+}
+
register!(NV_PFALCON_FALCON_DMATRFFBOFFS @ PFalconBase[0x0000011c] {
31:0 offs as u32;
});
--
2.52.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 06/11] gpu: nova-core: move some functions into the HAL
2026-01-14 19:29 [PATCH v6 00/11] gpu: nova-core: add Turing support Timur Tabi
` (4 preceding siblings ...)
2026-01-14 19:29 ` [PATCH v6 05/11] gpu: nova-core: add NV_PFALCON_FALCON_DMATRFCMD::with_falcon_mem() Timur Tabi
@ 2026-01-14 19:29 ` Timur Tabi
2026-01-16 19:55 ` Danilo Krummrich
2026-01-14 19:29 ` [PATCH v6 07/11] gpu: nova-core: Add basic Turing HAL Timur Tabi
` (4 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Timur Tabi @ 2026-01-14 19:29 UTC (permalink / raw)
To: Alexandre Courbot, John Hubbard, Joel Fernandes, Danilo Krummrich,
nouveau, rust-for-linux
A few Falcon methods are actually GPU-specific, so move them
into the HAL.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
---
drivers/gpu/nova-core/falcon.rs | 45 ++---------------------
drivers/gpu/nova-core/falcon/hal.rs | 10 +++++
drivers/gpu/nova-core/falcon/hal/ga102.rs | 41 +++++++++++++++++++++
3 files changed, 54 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 2a8fb7059a44..d779fcda0e2a 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -16,7 +16,6 @@
prelude::*,
sync::aref::ARef,
time::{
- delay::fsleep,
Delta, //
},
};
@@ -397,48 +396,11 @@ pub(crate) fn dma_reset(&self, bar: &Bar0) {
regs::NV_PFALCON_FALCON_DMACTL::default().write(bar, &E::ID);
}
- /// Wait for memory scrubbing to complete.
- fn reset_wait_mem_scrubbing(&self, bar: &Bar0) -> Result {
- // TIMEOUT: memory scrubbing should complete in less than 20ms.
- read_poll_timeout(
- || Ok(regs::NV_PFALCON_FALCON_HWCFG2::read(bar, &E::ID)),
- |r| r.mem_scrubbing_done(),
- Delta::ZERO,
- Delta::from_millis(20),
- )
- .map(|_| ())
- }
-
- /// Reset the falcon engine.
- fn reset_eng(&self, bar: &Bar0) -> Result {
- let _ = regs::NV_PFALCON_FALCON_HWCFG2::read(bar, &E::ID);
-
- // According to OpenRM's `kflcnPreResetWait_GA102` documentation, HW sometimes does not set
- // RESET_READY so a non-failing timeout is used.
- let _ = read_poll_timeout(
- || Ok(regs::NV_PFALCON_FALCON_HWCFG2::read(bar, &E::ID)),
- |r| r.reset_ready(),
- Delta::ZERO,
- Delta::from_micros(150),
- );
-
- regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(true));
-
- // TIMEOUT: falcon engine should not take more than 10us to reset.
- fsleep(Delta::from_micros(10));
-
- regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(false));
-
- self.reset_wait_mem_scrubbing(bar)?;
-
- Ok(())
- }
-
/// Reset the controller, select the falcon core, and wait for memory scrubbing to complete.
pub(crate) fn reset(&self, bar: &Bar0) -> Result {
- self.reset_eng(bar)?;
+ self.hal.reset_eng(bar)?;
self.hal.select_core(self, bar)?;
- self.reset_wait_mem_scrubbing(bar)?;
+ self.hal.reset_wait_mem_scrubbing(bar)?;
regs::NV_PFALCON_FALCON_RM::default()
.set_value(regs::NV_PMC_BOOT_0::read(bar).into())
@@ -672,8 +634,7 @@ pub(crate) fn signature_reg_fuse_version(
///
/// Returns `true` if the RISC-V core is active, `false` otherwise.
pub(crate) fn is_riscv_active(&self, bar: &Bar0) -> bool {
- let cpuctl = regs::NV_PRISCV_RISCV_CPUCTL::read(bar, &E::ID);
- cpuctl.active_stat()
+ self.hal.is_riscv_active(bar)
}
/// Write the application version to the OS register.
diff --git a/drivers/gpu/nova-core/falcon/hal.rs b/drivers/gpu/nova-core/falcon/hal.rs
index 8dc56a28ad65..c77a1568ea96 100644
--- a/drivers/gpu/nova-core/falcon/hal.rs
+++ b/drivers/gpu/nova-core/falcon/hal.rs
@@ -37,6 +37,16 @@ fn signature_reg_fuse_version(
/// Program the boot ROM registers prior to starting a secure firmware.
fn program_brom(&self, falcon: &Falcon<E>, bar: &Bar0, params: &FalconBromParams) -> Result;
+
+ /// Check if the RISC-V core is active.
+ /// Returns `true` if the RISC-V core is active, `false` otherwise.
+ fn is_riscv_active(&self, bar: &Bar0) -> bool;
+
+ /// Wait for memory scrubbing to complete.
+ fn reset_wait_mem_scrubbing(&self, bar: &Bar0) -> Result;
+
+ /// Reset the falcon engine.
+ fn reset_eng(&self, bar: &Bar0) -> Result;
}
/// Returns a boxed falcon HAL adequate for `chipset`.
diff --git a/drivers/gpu/nova-core/falcon/hal/ga102.rs b/drivers/gpu/nova-core/falcon/hal/ga102.rs
index 0bdfe45a2d03..61cc3d261196 100644
--- a/drivers/gpu/nova-core/falcon/hal/ga102.rs
+++ b/drivers/gpu/nova-core/falcon/hal/ga102.rs
@@ -6,6 +6,7 @@
device,
io::poll::read_poll_timeout,
prelude::*,
+ time::delay::fsleep,
time::Delta, //
};
@@ -117,4 +118,44 @@ fn signature_reg_fuse_version(
fn program_brom(&self, _falcon: &Falcon<E>, bar: &Bar0, params: &FalconBromParams) -> Result {
program_brom_ga102::<E>(bar, params)
}
+
+ fn is_riscv_active(&self, bar: &Bar0) -> bool {
+ let cpuctl = regs::NV_PRISCV_RISCV_CPUCTL::read(bar, &E::ID);
+ cpuctl.active_stat()
+ }
+
+ fn reset_wait_mem_scrubbing(&self, bar: &Bar0) -> Result {
+ // TIMEOUT: memory scrubbing should complete in less than 20ms.
+ read_poll_timeout(
+ || Ok(regs::NV_PFALCON_FALCON_HWCFG2::read(bar, &E::ID)),
+ |r| r.mem_scrubbing_done(),
+ Delta::ZERO,
+ Delta::from_millis(20),
+ )
+ .map(|_| ())
+ }
+
+ fn reset_eng(&self, bar: &Bar0) -> Result {
+ let _ = regs::NV_PFALCON_FALCON_HWCFG2::read(bar, &E::ID);
+
+ // According to OpenRM's `kflcnPreResetWait_GA102` documentation, HW sometimes does not set
+ // RESET_READY so a non-failing timeout is used.
+ let _ = read_poll_timeout(
+ || Ok(regs::NV_PFALCON_FALCON_HWCFG2::read(bar, &E::ID)),
+ |r| r.reset_ready(),
+ Delta::ZERO,
+ Delta::from_micros(150),
+ );
+
+ regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(true));
+
+ // TIMEOUT: falcon engine should not take more than 10us to reset.
+ fsleep(Delta::from_micros(10));
+
+ regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(false));
+
+ self.reset_wait_mem_scrubbing(bar)?;
+
+ Ok(())
+ }
}
--
2.52.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 07/11] gpu: nova-core: Add basic Turing HAL
2026-01-14 19:29 [PATCH v6 00/11] gpu: nova-core: add Turing support Timur Tabi
` (5 preceding siblings ...)
2026-01-14 19:29 ` [PATCH v6 06/11] gpu: nova-core: move some functions into the HAL Timur Tabi
@ 2026-01-14 19:29 ` Timur Tabi
2026-01-14 19:29 ` [PATCH v6 08/11] gpu: nova-core: add Falcon HAL method supports_dma() Timur Tabi
` (3 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Timur Tabi @ 2026-01-14 19:29 UTC (permalink / raw)
To: Alexandre Courbot, John Hubbard, Joel Fernandes, Danilo Krummrich,
nouveau, rust-for-linux
Add the basic HAL for recognizing Turing GPUs. This isn't enough
to support booting GSP-RM on Turing, but it's a start.
Note that GA100, which boots using the same method as Turing, is not
supported yet.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
---
drivers/gpu/nova-core/falcon/hal.rs | 4 ++
drivers/gpu/nova-core/falcon/hal/tu102.rs | 79 +++++++++++++++++++++++
drivers/gpu/nova-core/regs.rs | 14 ++++
3 files changed, 97 insertions(+)
create mode 100644 drivers/gpu/nova-core/falcon/hal/tu102.rs
diff --git a/drivers/gpu/nova-core/falcon/hal.rs b/drivers/gpu/nova-core/falcon/hal.rs
index c77a1568ea96..c886ba03d1f6 100644
--- a/drivers/gpu/nova-core/falcon/hal.rs
+++ b/drivers/gpu/nova-core/falcon/hal.rs
@@ -13,6 +13,7 @@
};
mod ga102;
+mod tu102;
/// Hardware Abstraction Layer for Falcon cores.
///
@@ -60,6 +61,9 @@ pub(super) fn falcon_hal<E: FalconEngine + 'static>(
use Chipset::*;
let hal = match chipset {
+ TU102 | TU104 | TU106 | TU116 | TU117 => {
+ KBox::new(tu102::Tu102::<E>::new(), GFP_KERNEL)? as KBox<dyn FalconHal<E>>
+ }
GA102 | GA103 | GA104 | GA106 | GA107 | AD102 | AD103 | AD104 | AD106 | AD107 => {
KBox::new(ga102::Ga102::<E>::new(), GFP_KERNEL)? as KBox<dyn FalconHal<E>>
}
diff --git a/drivers/gpu/nova-core/falcon/hal/tu102.rs b/drivers/gpu/nova-core/falcon/hal/tu102.rs
new file mode 100644
index 000000000000..586d5dc6b417
--- /dev/null
+++ b/drivers/gpu/nova-core/falcon/hal/tu102.rs
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use core::marker::PhantomData;
+
+use kernel::{
+ io::poll::read_poll_timeout,
+ prelude::*,
+ time::delay::fsleep,
+ time::Delta, //
+};
+
+use crate::{
+ driver::Bar0,
+ falcon::{
+ Falcon,
+ FalconBromParams,
+ FalconEngine, //
+ },
+ regs, //
+};
+
+use super::FalconHal;
+
+pub(super) struct Tu102<E: FalconEngine>(PhantomData<E>);
+
+impl<E: FalconEngine> Tu102<E> {
+ pub(super) fn new() -> Self {
+ Self(PhantomData)
+ }
+}
+
+impl<E: FalconEngine> FalconHal<E> for Tu102<E> {
+ fn select_core(&self, _falcon: &Falcon<E>, _bar: &Bar0) -> Result {
+ Ok(())
+ }
+
+ fn signature_reg_fuse_version(
+ &self,
+ _falcon: &Falcon<E>,
+ _bar: &Bar0,
+ _engine_id_mask: u16,
+ _ucode_id: u8,
+ ) -> Result<u32> {
+ Ok(0)
+ }
+
+ fn program_brom(&self, _falcon: &Falcon<E>, _bar: &Bar0, _params: &FalconBromParams) -> Result {
+ Ok(())
+ }
+
+ fn is_riscv_active(&self, bar: &Bar0) -> bool {
+ let cpuctl = regs::NV_PRISCV_RISCV_CORE_SWITCH_RISCV_STATUS::read(bar, &E::ID);
+ cpuctl.active_stat()
+ }
+
+ fn reset_wait_mem_scrubbing(&self, bar: &Bar0) -> Result {
+ // TIMEOUT: memory scrubbing should complete in less than 10ms.
+ read_poll_timeout(
+ || Ok(regs::NV_PFALCON_FALCON_DMACTL::read(bar, &E::ID)),
+ |r| r.mem_scrubbing_done(),
+ Delta::ZERO,
+ Delta::from_millis(10),
+ )
+ .map(|_| ())
+ }
+
+ fn reset_eng(&self, bar: &Bar0) -> Result {
+ regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(true));
+
+ // TIMEOUT: falcon engine should not take more than 10us to reset.
+ fsleep(Delta::from_micros(10));
+
+ regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(false));
+
+ self.reset_wait_mem_scrubbing(bar)?;
+
+ Ok(())
+ }
+}
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index b8ddfe2e5ae7..cd7b7aa6fc2a 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -307,6 +307,13 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
7:7 secure_stat as bool;
});
+impl NV_PFALCON_FALCON_DMACTL {
+ /// Returns `true` if memory scrubbing is completed.
+ pub(crate) fn mem_scrubbing_done(self) -> bool {
+ !self.dmem_scrubbing() && !self.imem_scrubbing()
+ }
+}
+
register!(NV_PFALCON_FALCON_DMATRFBASE @ PFalconBase[0x00000110] {
31:0 base as u32;
});
@@ -389,6 +396,13 @@ pub(crate) fn with_falcon_mem(self, mem: FalconMem) -> Self {
// PRISCV
+// RISC-V status register for debug (Turing and GA100 only).
+// Reflects current RISC-V core status.
+register!(NV_PRISCV_RISCV_CORE_SWITCH_RISCV_STATUS @ PFalcon2Base[0x00000240] {
+ 0:0 active_stat as bool, "RISC-V core active/inactive status";
+});
+
+// GA102 and later
register!(NV_PRISCV_RISCV_CPUCTL @ PFalcon2Base[0x00000388] {
0:0 halted as bool;
7:7 active_stat as bool;
--
2.52.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 08/11] gpu: nova-core: add Falcon HAL method supports_dma()
2026-01-14 19:29 [PATCH v6 00/11] gpu: nova-core: add Turing support Timur Tabi
` (6 preceding siblings ...)
2026-01-14 19:29 ` [PATCH v6 07/11] gpu: nova-core: Add basic Turing HAL Timur Tabi
@ 2026-01-14 19:29 ` Timur Tabi
2026-01-16 1:53 ` Alexandre Courbot
2026-01-14 19:29 ` [PATCH v6 09/11] gpu: nova-core: add FalconUCodeDescV2 support Timur Tabi
` (2 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Timur Tabi @ 2026-01-14 19:29 UTC (permalink / raw)
To: Alexandre Courbot, John Hubbard, Joel Fernandes, Danilo Krummrich,
nouveau, rust-for-linux
Some GPUs do not support using DMA to transfer code/data from system
memory to Falcon memory, and instead must use programmed I/O (PIO).
Add a function to the Falcon HAL to indicate whether a given GPU's
Falcons support DMA for this purpose.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
drivers/gpu/nova-core/falcon.rs | 7 +++++++
drivers/gpu/nova-core/falcon/hal.rs | 3 +++
drivers/gpu/nova-core/falcon/hal/ga102.rs | 4 ++++
drivers/gpu/nova-core/falcon/hal/tu102.rs | 4 ++++
4 files changed, 18 insertions(+)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index d779fcda0e2a..154e0de22c51 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -637,6 +637,13 @@ pub(crate) fn is_riscv_active(&self, bar: &Bar0) -> bool {
self.hal.is_riscv_active(bar)
}
+ /// Check if this Falcon supports DMA for loading firmware.
+ ///
+ /// Returns `true` if DMA is supported, `false` if PIO must be used instead.
+ pub(crate) fn supports_dma(&self) -> bool {
+ self.hal.supports_dma()
+ }
+
/// Write the application version to the OS register.
pub(crate) fn write_os_version(&self, bar: &Bar0, app_version: u32) {
regs::NV_PFALCON_FALCON_OS::default()
diff --git a/drivers/gpu/nova-core/falcon/hal.rs b/drivers/gpu/nova-core/falcon/hal.rs
index c886ba03d1f6..49501aa6ff7f 100644
--- a/drivers/gpu/nova-core/falcon/hal.rs
+++ b/drivers/gpu/nova-core/falcon/hal.rs
@@ -48,6 +48,9 @@ fn signature_reg_fuse_version(
/// Reset the falcon engine.
fn reset_eng(&self, bar: &Bar0) -> Result;
+
+ /// Returns true of this Falcon supports DMA transfer from system memory to Falcon memory
+ fn supports_dma(&self) -> bool;
}
/// Returns a boxed falcon HAL adequate for `chipset`.
diff --git a/drivers/gpu/nova-core/falcon/hal/ga102.rs b/drivers/gpu/nova-core/falcon/hal/ga102.rs
index 61cc3d261196..6ccbf42ba80c 100644
--- a/drivers/gpu/nova-core/falcon/hal/ga102.rs
+++ b/drivers/gpu/nova-core/falcon/hal/ga102.rs
@@ -158,4 +158,8 @@ fn reset_eng(&self, bar: &Bar0) -> Result {
Ok(())
}
+
+ fn supports_dma(&self) -> bool {
+ true
+ }
}
diff --git a/drivers/gpu/nova-core/falcon/hal/tu102.rs b/drivers/gpu/nova-core/falcon/hal/tu102.rs
index 586d5dc6b417..28dc03db8240 100644
--- a/drivers/gpu/nova-core/falcon/hal/tu102.rs
+++ b/drivers/gpu/nova-core/falcon/hal/tu102.rs
@@ -76,4 +76,8 @@ fn reset_eng(&self, bar: &Bar0) -> Result {
Ok(())
}
+
+ fn supports_dma(&self) -> bool {
+ false
+ }
}
--
2.52.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 09/11] gpu: nova-core: add FalconUCodeDescV2 support
2026-01-14 19:29 [PATCH v6 00/11] gpu: nova-core: add Turing support Timur Tabi
` (7 preceding siblings ...)
2026-01-14 19:29 ` [PATCH v6 08/11] gpu: nova-core: add Falcon HAL method supports_dma() Timur Tabi
@ 2026-01-14 19:29 ` Timur Tabi
2026-01-16 3:11 ` Alexandre Courbot
2026-01-16 3:23 ` Alexandre Courbot
2026-01-14 19:29 ` [PATCH v6 10/11] gpu: nova-core: align LibosMemoryRegionInitArgument size to page size Timur Tabi
2026-01-14 19:29 ` [PATCH v6 11/11] gpu: nova-core: add PIO support for loading firmware images Timur Tabi
10 siblings, 2 replies; 29+ messages in thread
From: Timur Tabi @ 2026-01-14 19:29 UTC (permalink / raw)
To: Alexandre Courbot, John Hubbard, Joel Fernandes, Danilo Krummrich,
nouveau, rust-for-linux
The FRTS firmware in Turing and GA100 VBIOS has an older header
format (v2 instead of v3). To support both v2 and v3 at runtime,
add the FalconUCodeDescV2 struct, and update code that references
the FalconUCodeDescV3 directly with a FalconUCodeDesc enum that
encapsulates both.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
drivers/gpu/nova-core/firmware.rs | 149 +++++++++++++++++++++++-
drivers/gpu/nova-core/firmware/fwsec.rs | 72 ++++++++----
drivers/gpu/nova-core/vbios.rs | 75 +++++++-----
3 files changed, 237 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index 2d2008b33fb4..9f0ad02fbe22 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -4,6 +4,7 @@
//! to be loaded into a given execution unit.
use core::marker::PhantomData;
+use core::ops::Deref;
use kernel::{
device,
@@ -43,6 +44,46 @@ fn request_firmware(
.and_then(|path| firmware::Firmware::request(&path, dev))
}
+/// Structure used to describe some firmwares, notably FWSEC-FRTS.
+#[repr(C)]
+#[derive(Debug, Clone)]
+pub(crate) struct FalconUCodeDescV2 {
+ /// Header defined by 'NV_BIT_FALCON_UCODE_DESC_HEADER_VDESC*' in OpenRM.
+ hdr: u32,
+ /// Stored size of the ucode after the header, compressed or uncompressed
+ stored_size: u32,
+ /// Uncompressed size of the ucode. If store_size == uncompressed_size, then the ucode
+ /// is not compressed.
+ pub(crate) uncompressed_size: u32,
+ /// Code entry point
+ pub(crate) virtual_entry: u32,
+ /// Offset after the code segment at which the Application Interface Table headers are located.
+ pub(crate) interface_offset: u32,
+ /// Base address at which to load the code segment into 'IMEM'.
+ pub(crate) imem_phys_base: u32,
+ /// Size in bytes of the code to copy into 'IMEM'.
+ pub(crate) imem_load_size: u32,
+ /// Virtual 'IMEM' address (i.e. 'tag') at which the code should start.
+ pub(crate) imem_virt_base: u32,
+ /// Virtual address of secure IMEM segment.
+ pub(crate) imem_sec_base: u32,
+ /// Size of secure IMEM segment.
+ pub(crate) imem_sec_size: u32,
+ /// Offset into stored (uncompressed) image at which DMEM begins.
+ pub(crate) dmem_offset: u32,
+ /// Base address at which to load the data segment into 'DMEM'.
+ pub(crate) dmem_phys_base: u32,
+ /// Size in bytes of the data to copy into 'DMEM'.
+ pub(crate) dmem_load_size: u32,
+ /// "Alternate" Size of data to load into IMEM.
+ pub(crate) alt_imem_load_size: u32,
+ /// "Alternate" Size of data to load into DMEM.
+ pub(crate) alt_dmem_load_size: u32,
+}
+
+// SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
+unsafe impl FromBytes for FalconUCodeDescV2 {}
+
/// Structure used to describe some firmwares, notably FWSEC-FRTS.
#[repr(C)]
#[derive(Debug, Clone)]
@@ -76,13 +117,115 @@ pub(crate) struct FalconUCodeDescV3 {
_reserved: u16,
}
-impl FalconUCodeDescV3 {
+// SAFETY: all bit patterns are valid for this type, and it doesn't use
+// interior mutability.
+unsafe impl FromBytes for FalconUCodeDescV3 {}
+
+/// Enum wrapping the different versions of Falcon microcode descriptors.
+///
+/// This allows handling both V2 and V3 descriptor formats through a
+/// unified type, providing version-agnostic access to firmware metadata
+/// via the [`FalconUCodeDescriptor`] trait.
+#[derive(Debug, Clone)]
+pub(crate) enum FalconUCodeDesc {
+ V2(FalconUCodeDescV2),
+ V3(FalconUCodeDescV3),
+}
+
+impl Deref for FalconUCodeDesc {
+ type Target = dyn FalconUCodeDescriptor;
+
+ fn deref(&self) -> &Self::Target {
+ match self {
+ FalconUCodeDesc::V2(v2) => v2,
+ FalconUCodeDesc::V3(v3) => v3,
+ }
+ }
+}
+
+/// Trait providing a common interface for accessing Falcon microcode descriptor fields.
+///
+/// This trait abstracts over the different descriptor versions ([`FalconUCodeDescV2`] and
+/// [`FalconUCodeDescV3`]), allowing code to work with firmware metadata without needing to
+/// know the specific descriptor version. Fields not present return zero.
+pub(crate) trait FalconUCodeDescriptor {
+ fn hdr(&self) -> u32;
+ fn imem_load_size(&self) -> u32;
+ fn interface_offset(&self) -> u32;
+ fn dmem_load_size(&self) -> u32;
+ fn pkc_data_offset(&self) -> u32;
+ fn engine_id_mask(&self) -> u16;
+ fn ucode_id(&self) -> u8;
+ fn signature_count(&self) -> u8;
+ fn signature_versions(&self) -> u16;
+
/// Returns the size in bytes of the header.
- pub(crate) fn size(&self) -> usize {
+ fn size(&self) -> usize {
+ let hdr = self.hdr();
+
const HDR_SIZE_SHIFT: u32 = 16;
const HDR_SIZE_MASK: u32 = 0xffff0000;
+ ((hdr & HDR_SIZE_MASK) >> HDR_SIZE_SHIFT).into_safe_cast()
+ }
+}
+
+impl FalconUCodeDescriptor for FalconUCodeDescV2 {
+ fn hdr(&self) -> u32 {
+ self.hdr
+ }
+ fn imem_load_size(&self) -> u32 {
+ self.imem_load_size
+ }
+ fn interface_offset(&self) -> u32 {
+ self.interface_offset
+ }
+ fn dmem_load_size(&self) -> u32 {
+ self.dmem_load_size
+ }
+ fn pkc_data_offset(&self) -> u32 {
+ 0
+ }
+ fn engine_id_mask(&self) -> u16 {
+ 0
+ }
+ fn ucode_id(&self) -> u8 {
+ 0
+ }
+ fn signature_count(&self) -> u8 {
+ 0
+ }
+ fn signature_versions(&self) -> u16 {
+ 0
+ }
+}
- ((self.hdr & HDR_SIZE_MASK) >> HDR_SIZE_SHIFT).into_safe_cast()
+impl FalconUCodeDescriptor for FalconUCodeDescV3 {
+ fn hdr(&self) -> u32 {
+ self.hdr
+ }
+ fn imem_load_size(&self) -> u32 {
+ self.imem_load_size
+ }
+ fn interface_offset(&self) -> u32 {
+ self.interface_offset
+ }
+ fn dmem_load_size(&self) -> u32 {
+ self.dmem_load_size
+ }
+ fn pkc_data_offset(&self) -> u32 {
+ self.pkc_data_offset
+ }
+ fn engine_id_mask(&self) -> u16 {
+ self.engine_id_mask
+ }
+ fn ucode_id(&self) -> u8 {
+ self.ucode_id
+ }
+ fn signature_count(&self) -> u8 {
+ self.signature_count
+ }
+ fn signature_versions(&self) -> u16 {
+ self.signature_versions
}
}
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index e4009faba6c5..c4fff8b86640 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -40,7 +40,7 @@
FalconLoadTarget, //
},
firmware::{
- FalconUCodeDescV3,
+ FalconUCodeDesc,
FirmwareDmaObject,
FirmwareSignature,
Signed,
@@ -218,38 +218,59 @@ unsafe fn transmute_mut<T: Sized + FromBytes + AsBytes>(
/// It is responsible for e.g. carving out the WPR2 region as the first step of the GSP bootflow.
pub(crate) struct FwsecFirmware {
/// Descriptor of the firmware.
- desc: FalconUCodeDescV3,
+ desc: FalconUCodeDesc,
/// GPU-accessible DMA object containing the firmware.
ucode: FirmwareDmaObject<Self, Signed>,
}
impl FalconLoadParams for FwsecFirmware {
fn imem_sec_load_params(&self) -> FalconLoadTarget {
- FalconLoadTarget {
- src_start: 0,
- dst_start: self.desc.imem_phys_base,
- len: self.desc.imem_load_size,
+ match &self.desc {
+ FalconUCodeDesc::V2(v2) => FalconLoadTarget {
+ src_start: 0,
+ dst_start: v2.imem_sec_base,
+ len: v2.imem_sec_size,
+ },
+ FalconUCodeDesc::V3(v3) => FalconLoadTarget {
+ src_start: 0,
+ dst_start: v3.imem_phys_base,
+ len: v3.imem_load_size,
+ },
}
}
fn imem_ns_load_params(&self) -> Option<FalconLoadTarget> {
- // Only used on Turing and GA100, so return None for now
- None
+ match &self.desc {
+ FalconUCodeDesc::V2(v2) => Some(FalconLoadTarget {
+ src_start: 0,
+ dst_start: v2.imem_phys_base,
+ len: v2.imem_load_size.checked_sub(v2.imem_sec_size)?,
+ }),
+ // Not used on V3 platforms
+ FalconUCodeDesc::V3(_v3) => None,
+ }
}
fn dmem_load_params(&self) -> FalconLoadTarget {
- FalconLoadTarget {
- src_start: self.desc.imem_load_size,
- dst_start: self.desc.dmem_phys_base,
- len: self.desc.dmem_load_size,
+ match &self.desc {
+ FalconUCodeDesc::V2(v2) => FalconLoadTarget {
+ src_start: v2.dmem_offset,
+ dst_start: v2.dmem_phys_base,
+ len: v2.dmem_load_size,
+ },
+ FalconUCodeDesc::V3(v3) => FalconLoadTarget {
+ src_start: v3.imem_load_size,
+ dst_start: v3.dmem_phys_base,
+ len: v3.dmem_load_size,
+ },
}
}
fn brom_params(&self) -> FalconBromParams {
FalconBromParams {
- pkc_data_offset: self.desc.pkc_data_offset,
- engine_id_mask: self.desc.engine_id_mask,
- ucode_id: self.desc.ucode_id,
+ pkc_data_offset: self.desc.pkc_data_offset(),
+ engine_id_mask: self.desc.engine_id_mask(),
+ ucode_id: self.desc.ucode_id(),
}
}
@@ -273,10 +294,10 @@ impl FalconFirmware for FwsecFirmware {
impl FirmwareDmaObject<FwsecFirmware, Unsigned> {
fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Result<Self> {
let desc = bios.fwsec_image().header()?;
- let ucode = bios.fwsec_image().ucode(desc)?;
+ let ucode = bios.fwsec_image().ucode(&desc)?;
let mut dma_object = DmaObject::from_data(dev, ucode)?;
- let hdr_offset = usize::from_safe_cast(desc.imem_load_size + desc.interface_offset);
+ let hdr_offset = usize::from_safe_cast(desc.imem_load_size() + desc.interface_offset());
// SAFETY: we have exclusive access to `dma_object`.
let hdr: &FalconAppifHdrV1 = unsafe { transmute(&dma_object, hdr_offset) }?;
@@ -303,7 +324,7 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
let dmem_mapper: &mut FalconAppifDmemmapperV3 = unsafe {
transmute_mut(
&mut dma_object,
- (desc.imem_load_size + dmem_base).into_safe_cast(),
+ (desc.imem_load_size() + dmem_base).into_safe_cast(),
)
}?;
@@ -317,7 +338,7 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
let frts_cmd: &mut FrtsCmd = unsafe {
transmute_mut(
&mut dma_object,
- (desc.imem_load_size + cmd_in_buffer_offset).into_safe_cast(),
+ (desc.imem_load_size() + cmd_in_buffer_offset).into_safe_cast(),
)
}?;
@@ -364,11 +385,12 @@ pub(crate) fn new(
// Patch signature if needed.
let desc = bios.fwsec_image().header()?;
- let ucode_signed = if desc.signature_count != 0 {
- let sig_base_img = usize::from_safe_cast(desc.imem_load_size + desc.pkc_data_offset);
- let desc_sig_versions = u32::from(desc.signature_versions);
+ let ucode_signed = if desc.signature_count() != 0 {
+ let sig_base_img =
+ usize::from_safe_cast(desc.imem_load_size() + desc.pkc_data_offset());
+ let desc_sig_versions = u32::from(desc.signature_versions());
let reg_fuse_version =
- falcon.signature_reg_fuse_version(bar, desc.engine_id_mask, desc.ucode_id)?;
+ falcon.signature_reg_fuse_version(bar, desc.engine_id_mask(), desc.ucode_id())?;
dev_dbg!(
dev,
"desc_sig_versions: {:#x}, reg_fuse_version: {}\n",
@@ -402,7 +424,7 @@ pub(crate) fn new(
dev_dbg!(dev, "patching signature with index {}\n", signature_idx);
let signature = bios
.fwsec_image()
- .sigs(desc)
+ .sigs(&desc)
.and_then(|sigs| sigs.get(signature_idx).ok_or(EINVAL))?;
ucode_dma.patch_signature(signature, sig_base_img)?
@@ -411,7 +433,7 @@ pub(crate) fn new(
};
Ok(FwsecFirmware {
- desc: desc.clone(),
+ desc,
ucode: ucode_signed,
})
}
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index e59eee2050a8..07a28c4c0837 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -19,6 +19,8 @@
driver::Bar0,
firmware::{
fwsec::Bcrt30Rsa3kSignature,
+ FalconUCodeDesc,
+ FalconUCodeDescV2,
FalconUCodeDescV3, //
},
num::FromSafeCast,
@@ -999,19 +1001,10 @@ fn build(self) -> Result<FwSecBiosImage> {
impl FwSecBiosImage {
/// Get the FwSec header ([`FalconUCodeDescV3`]).
- pub(crate) fn header(&self) -> Result<&FalconUCodeDescV3> {
+ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
// Get the falcon ucode offset that was found in setup_falcon_data.
let falcon_ucode_offset = self.falcon_ucode_offset;
- // Make sure the offset is within the data bounds.
- if falcon_ucode_offset + core::mem::size_of::<FalconUCodeDescV3>() > self.base.data.len() {
- dev_err!(
- self.base.dev,
- "fwsec-frts header not contained within BIOS bounds\n"
- );
- return Err(ERANGE);
- }
-
// Read the first 4 bytes to get the version.
let hdr_bytes: [u8; 4] = self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4]
.try_into()
@@ -1019,33 +1012,49 @@ pub(crate) fn header(&self) -> Result<&FalconUCodeDescV3> {
let hdr = u32::from_le_bytes(hdr_bytes);
let ver = (hdr & 0xff00) >> 8;
- if ver != 3 {
- dev_err!(self.base.dev, "invalid fwsec firmware version: {:?}\n", ver);
- return Err(EINVAL);
+ let hdr_size = match ver {
+ 2 => core::mem::size_of::<FalconUCodeDescV2>(),
+ 3 => core::mem::size_of::<FalconUCodeDescV3>(),
+ _ => {
+ dev_err!(self.base.dev, "invalid fwsec firmware version: {:?}\n", ver);
+ return Err(EINVAL);
+ }
+ };
+ // Make sure the offset is within the data bounds
+ if falcon_ucode_offset + hdr_size > self.base.data.len() {
+ dev_err!(
+ self.base.dev,
+ "fwsec-frts header not contained within BIOS bounds\n"
+ );
+ return Err(ERANGE);
}
- // Return a reference to the FalconUCodeDescV3 structure.
- //
- // SAFETY: We have checked that `falcon_ucode_offset + size_of::<FalconUCodeDescV3>` is
- // within the bounds of `data`. Also, this data vector is from ROM, and the `data` field
- // in `BiosImageBase` is immutable after construction.
- Ok(unsafe {
- &*(self
- .base
- .data
- .as_ptr()
- .add(falcon_ucode_offset)
- .cast::<FalconUCodeDescV3>())
- })
+ let data = self
+ .base
+ .data
+ .get(falcon_ucode_offset..falcon_ucode_offset + hdr_size)
+ .ok_or(EINVAL)?;
+
+ match ver {
+ 2 => {
+ let v2 = FalconUCodeDescV2::from_bytes(data).ok_or(EINVAL)?;
+ Ok(FalconUCodeDesc::V2(v2.clone()))
+ }
+ 3 => {
+ let v3 = FalconUCodeDescV3::from_bytes(data).ok_or(EINVAL)?;
+ Ok(FalconUCodeDesc::V3(v3.clone()))
+ }
+ _ => Err(EINVAL),
+ }
}
/// Get the ucode data as a byte slice
- pub(crate) fn ucode(&self, desc: &FalconUCodeDescV3) -> Result<&[u8]> {
+ pub(crate) fn ucode(&self, desc: &FalconUCodeDesc) -> Result<&[u8]> {
let falcon_ucode_offset = self.falcon_ucode_offset;
// The ucode data follows the descriptor.
let ucode_data_offset = falcon_ucode_offset + desc.size();
- let size = usize::from_safe_cast(desc.imem_load_size + desc.dmem_load_size);
+ let size = usize::from_safe_cast(desc.imem_load_size() + desc.dmem_load_size());
// Get the data slice, checking bounds in a single operation.
self.base
@@ -1061,10 +1070,14 @@ pub(crate) fn ucode(&self, desc: &FalconUCodeDescV3) -> Result<&[u8]> {
}
/// Get the signatures as a byte slice
- pub(crate) fn sigs(&self, desc: &FalconUCodeDescV3) -> Result<&[Bcrt30Rsa3kSignature]> {
+ pub(crate) fn sigs(&self, desc: &FalconUCodeDesc) -> Result<&[Bcrt30Rsa3kSignature]> {
+ let hdr_size = match desc {
+ FalconUCodeDesc::V2(_v2) => core::mem::size_of::<FalconUCodeDescV2>(),
+ FalconUCodeDesc::V3(_v3) => core::mem::size_of::<FalconUCodeDescV3>(),
+ };
// The signatures data follows the descriptor.
- let sigs_data_offset = self.falcon_ucode_offset + core::mem::size_of::<FalconUCodeDescV3>();
- let sigs_count = usize::from(desc.signature_count);
+ let sigs_data_offset = self.falcon_ucode_offset + hdr_size;
+ let sigs_count = usize::from(desc.signature_count());
let sigs_size = sigs_count * core::mem::size_of::<Bcrt30Rsa3kSignature>();
// Make sure the data is within bounds.
--
2.52.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 10/11] gpu: nova-core: align LibosMemoryRegionInitArgument size to page size
2026-01-14 19:29 [PATCH v6 00/11] gpu: nova-core: add Turing support Timur Tabi
` (8 preceding siblings ...)
2026-01-14 19:29 ` [PATCH v6 09/11] gpu: nova-core: add FalconUCodeDescV2 support Timur Tabi
@ 2026-01-14 19:29 ` Timur Tabi
2026-01-14 19:29 ` [PATCH v6 11/11] gpu: nova-core: add PIO support for loading firmware images Timur Tabi
10 siblings, 0 replies; 29+ messages in thread
From: Timur Tabi @ 2026-01-14 19:29 UTC (permalink / raw)
To: Alexandre Courbot, John Hubbard, Joel Fernandes, Danilo Krummrich,
nouveau, rust-for-linux
From: Alexandre Courbot <acourbot@nvidia.com>
On Turing and GA100 (i.e. the versions that use Libos v2), GSP-RM insists
that the 'size' parameter of the LibosMemoryRegionInitArgument struct be
aligned to 4KB. The logging buffers are already aligned to that size, so
only the GSP_ARGUMENTS_CACHED struct needs to be adjusted. Make that
adjustment by adding padding to the end of the struct.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
drivers/gpu/nova-core/gsp.rs | 8 ++++----
drivers/gpu/nova-core/gsp/fw.rs | 14 +++++++++++++-
2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 766fd9905358..bcf6ce18a4a1 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -27,7 +27,7 @@
use crate::{
gsp::cmdq::Cmdq,
gsp::fw::{
- GspArgumentsCached,
+ GspArgumentsAligned,
LibosMemoryRegionInitArgument, //
},
num,
@@ -114,7 +114,7 @@ pub(crate) struct Gsp {
/// Command queue.
pub(crate) cmdq: Cmdq,
/// RM arguments.
- rmargs: CoherentAllocation<GspArgumentsCached>,
+ rmargs: CoherentAllocation<GspArgumentsAligned>,
}
impl Gsp {
@@ -133,7 +133,7 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
logintr: LogBuffer::new(dev)?,
logrm: LogBuffer::new(dev)?,
cmdq: Cmdq::new(dev)?,
- rmargs: CoherentAllocation::<GspArgumentsCached>::alloc_coherent(
+ rmargs: CoherentAllocation::<GspArgumentsAligned>::alloc_coherent(
dev,
1,
GFP_KERNEL | __GFP_ZERO,
@@ -149,7 +149,7 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
libos[1] = LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0)
)?;
dma_write!(libos[2] = LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0))?;
- dma_write!(rmargs[0] = fw::GspArgumentsCached::new(cmdq))?;
+ dma_write!(rmargs[0].inner = fw::GspArgumentsCached::new(cmdq))?;
dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", rmargs))?;
},
}))
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index abffd6beec65..15ca9c183ae1 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -906,9 +906,21 @@ pub(crate) fn new(cmdq: &Cmdq) -> Self {
// SAFETY: Padding is explicit and will not contain uninitialized data.
unsafe impl AsBytes for GspArgumentsCached {}
+/// On Turing and GA100, the entries in the `LibosMemoryRegionInitArgument`
+/// must all be a multiple of GSP_PAGE_SIZE in size, so add padding to force it
+/// to that size.
+#[repr(C)]
+pub(crate) struct GspArgumentsAligned {
+ pub(crate) inner: GspArgumentsCached,
+ _padding: [u8; GSP_PAGE_SIZE - core::mem::size_of::<bindings::GSP_ARGUMENTS_CACHED>()],
+}
+
+// SAFETY: Padding is explicit and will not contain uninitialized data.
+unsafe impl AsBytes for GspArgumentsAligned {}
+
// SAFETY: This struct only contains integer types for which all bit patterns
// are valid.
-unsafe impl FromBytes for GspArgumentsCached {}
+unsafe impl FromBytes for GspArgumentsAligned {}
/// Init arguments for the message queue.
#[repr(transparent)]
--
2.52.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 11/11] gpu: nova-core: add PIO support for loading firmware images
2026-01-14 19:29 [PATCH v6 00/11] gpu: nova-core: add Turing support Timur Tabi
` (9 preceding siblings ...)
2026-01-14 19:29 ` [PATCH v6 10/11] gpu: nova-core: align LibosMemoryRegionInitArgument size to page size Timur Tabi
@ 2026-01-14 19:29 ` Timur Tabi
2026-01-16 15:22 ` Alexandre Courbot
2026-01-16 21:05 ` Danilo Krummrich
10 siblings, 2 replies; 29+ messages in thread
From: Timur Tabi @ 2026-01-14 19:29 UTC (permalink / raw)
To: Alexandre Courbot, John Hubbard, Joel Fernandes, Danilo Krummrich,
nouveau, rust-for-linux
Turing and GA100 use programmed I/O (PIO) instead of DMA to upload
firmware images into Falcon memory.
A new firmware called the Generic Bootloader (as opposed to the
GSP Bootloader) is used to upload FWSEC.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
drivers/gpu/nova-core/falcon.rs | 185 +++++++++++++++++++++-
drivers/gpu/nova-core/falcon/hal.rs | 9 +-
drivers/gpu/nova-core/falcon/hal/ga102.rs | 5 +-
drivers/gpu/nova-core/falcon/hal/tu102.rs | 5 +-
drivers/gpu/nova-core/firmware/fwsec.rs | 129 ++++++++++++++-
drivers/gpu/nova-core/gsp/boot.rs | 6 +-
drivers/gpu/nova-core/regs.rs | 30 ++++
7 files changed, 354 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 154e0de22c51..e7d0b6e87125 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -18,11 +18,17 @@
time::{
Delta, //
},
+ transmute::AsBytes, //
};
use crate::{
dma::DmaObject,
driver::Bar0,
+ falcon::hal::LoadMethod,
+ firmware::fwsec::{
+ BootloaderDmemDescV2,
+ GenericBootloader, //
+ },
gpu::Chipset,
num::{
FromSafeCast,
@@ -409,6 +415,169 @@ pub(crate) fn reset(&self, bar: &Bar0) -> Result {
Ok(())
}
+ /// Write a byte array to Falcon memory using programmed I/O (PIO).
+ ///
+ /// Writes `img` to the specified `target_mem` (IMEM or DMEM) starting at `mem_base`.
+ /// For IMEM writes, tags are set for each 256-byte block starting from `tag`.
+ ///
+ /// Returns `EINVAL` if `img.len()` is not a multiple of 4.
+ fn pio_wr_bytes(
+ &self,
+ bar: &Bar0,
+ img: &[u8],
+ mem_base: u16,
+ target_mem: FalconMem,
+ port: u8,
+ tag: u16,
+ ) -> Result {
+ // Rejecting misaligned images here allows us to avoid checking
+ // inside the loops.
+ if img.len() % 4 != 0 {
+ return Err(EINVAL);
+ }
+
+ let port = usize::from(port);
+
+ match target_mem {
+ FalconMem::ImemSecure | FalconMem::ImemNonSecure => {
+ regs::NV_PFALCON_FALCON_IMEMC::default()
+ .set_secure(target_mem == FalconMem::ImemSecure)
+ .set_aincw(true)
+ .set_offs(mem_base)
+ .write(bar, &E::ID, port);
+
+ let mut tag = tag;
+ for block in img.chunks(256) {
+ regs::NV_PFALCON_FALCON_IMEMT::default()
+ .set_tag(tag)
+ .write(bar, &E::ID, port);
+ for word in block.chunks_exact(4) {
+ let w = [word[0], word[1], word[2], word[3]];
+ regs::NV_PFALCON_FALCON_IMEMD::default()
+ .set_data(u32::from_le_bytes(w))
+ .write(bar, &E::ID, port);
+ }
+ tag += 1;
+ }
+ }
+ FalconMem::Dmem => {
+ regs::NV_PFALCON_FALCON_DMEMC::default()
+ .set_aincw(true)
+ .set_offs(mem_base)
+ .write(bar, &E::ID, port);
+
+ for block in img.chunks(256) {
+ for word in block.chunks_exact(4) {
+ let w = [word[0], word[1], word[2], word[3]];
+ regs::NV_PFALCON_FALCON_DMEMD::default()
+ .set_data(u32::from_le_bytes(w))
+ .write(bar, &E::ID, port);
+ }
+ }
+ }
+ }
+
+ Ok(())
+ }
+
+ fn pio_wr<F: FalconFirmware<Target = E>>(
+ &self,
+ bar: &Bar0,
+ fw: &F,
+ target_mem: FalconMem,
+ load_offsets: &FalconLoadTarget,
+ port: u8,
+ tag: u16,
+ ) -> Result {
+ let start = usize::from_safe_cast(load_offsets.src_start);
+ let len = usize::from_safe_cast(load_offsets.len);
+ let mem_base = u16::try_from(load_offsets.dst_start)?;
+
+ // SAFETY: we are the only user of the firmware image at this stage
+ let data = unsafe { fw.as_slice(start, len).map_err(|_| EINVAL)? };
+
+ self.pio_wr_bytes(bar, data, mem_base, target_mem, port, tag)
+ }
+
+ /// Perform a PIO copy into `IMEM` and `DMEM` of `fw`, and prepare the falcon to run it.
+ pub(crate) fn pio_load<F: FalconFirmware<Target = E>>(
+ &self,
+ bar: &Bar0,
+ fw: &F,
+ gbl: Option<&GenericBootloader>,
+ ) -> Result {
+ let imem_sec = fw.imem_sec_load_params();
+ let imem_ns = fw.imem_ns_load_params().ok_or(EINVAL)?;
+ let dmem = fw.dmem_load_params();
+
+ regs::NV_PFALCON_FBIF_CTL::read(bar, &E::ID)
+ .set_allow_phys_no_ctx(true)
+ .write(bar, &E::ID);
+
+ regs::NV_PFALCON_FALCON_DMACTL::default().write(bar, &E::ID);
+
+ // If the Generic Bootloader was passed, then use it to boot FRTS
+ if let Some(gbl) = gbl {
+ let dst_start = u16::try_from(0x10000 - gbl.desc.code_size)?;
+ let data = &gbl.ucode[..usize::from_safe_cast(gbl.desc.code_size)];
+ let tag = u16::try_from(gbl.desc.start_tag)?;
+
+ self.pio_wr_bytes(bar, data, dst_start, FalconMem::ImemNonSecure, 0, tag)?;
+
+ // This structure tells the generic bootloader where to find the FWSEC
+ // image.
+ let dmem_desc = BootloaderDmemDescV2 {
+ reserved: [0; 4],
+ signature: [0; 4],
+ ctx_dma: 4, // FALCON_DMAIDX_PHYS_SYS_NCOH
+ code_dma_base: fw.dma_handle(),
+ non_sec_code_off: imem_ns.dst_start,
+ non_sec_code_size: imem_ns.len,
+ sec_code_off: imem_sec.dst_start,
+ sec_code_size: imem_sec.len,
+ code_entry_point: 0,
+ data_dma_base: fw.dma_handle() + u64::from(dmem.src_start),
+ data_size: dmem.len,
+ argc: 0,
+ argv: 0,
+ };
+
+ regs::NV_PFALCON_FBIF_TRANSCFG::update(bar, &E::ID, 4, |v| {
+ v.set_target(FalconFbifTarget::CoherentSysmem)
+ .set_mem_type(FalconFbifMemType::Physical)
+ });
+
+ self.pio_wr_bytes(bar, dmem_desc.as_bytes(), 0, FalconMem::Dmem, 0, 0)?;
+ } else {
+ self.pio_wr(
+ bar,
+ fw,
+ FalconMem::ImemNonSecure,
+ &imem_ns,
+ 0,
+ u16::try_from(imem_ns.dst_start >> 8)?,
+ )?;
+ self.pio_wr(
+ bar,
+ fw,
+ FalconMem::ImemSecure,
+ &imem_sec,
+ 0,
+ u16::try_from(imem_sec.dst_start >> 8)?,
+ )?;
+ self.pio_wr(bar, fw, FalconMem::Dmem, &dmem, 0, 0)?;
+ }
+
+ self.hal.program_brom(self, bar, &fw.brom_params())?;
+
+ // Set `BootVec` to start of non-secure code.
+ regs::NV_PFALCON_FALCON_BOOTVEC::default()
+ .set_value(fw.boot_addr())
+ .write(bar, &E::ID);
+
+ Ok(())
+ }
+
/// Perform a DMA write according to `load_offsets` from `dma_handle` into the falcon's
/// `target_mem`.
///
@@ -637,11 +806,17 @@ pub(crate) fn is_riscv_active(&self, bar: &Bar0) -> bool {
self.hal.is_riscv_active(bar)
}
- /// Check if this Falcon supports DMA for loading firmware.
- ///
- /// Returns `true` if DMA is supported, `false` if PIO must be used instead.
- pub(crate) fn supports_dma(&self) -> bool {
- self.hal.supports_dma()
+ // Load a firmware image into Falcon memory
+ pub(crate) fn load<F: FalconFirmware<Target = E>>(
+ &self,
+ bar: &Bar0,
+ fw: &F,
+ gbl: Option<&GenericBootloader>,
+ ) -> Result {
+ match self.hal.load_method() {
+ LoadMethod::Pio => self.pio_load(bar, fw, gbl),
+ LoadMethod::Dma => self.dma_load(bar, fw),
+ }
}
/// Write the application version to the OS register.
diff --git a/drivers/gpu/nova-core/falcon/hal.rs b/drivers/gpu/nova-core/falcon/hal.rs
index 49501aa6ff7f..3a882b7d8aa8 100644
--- a/drivers/gpu/nova-core/falcon/hal.rs
+++ b/drivers/gpu/nova-core/falcon/hal.rs
@@ -15,6 +15,11 @@
mod ga102;
mod tu102;
+pub(crate) enum LoadMethod {
+ Pio,
+ Dma,
+}
+
/// Hardware Abstraction Layer for Falcon cores.
///
/// Implements chipset-specific low-level operations. The trait is generic against [`FalconEngine`]
@@ -49,8 +54,8 @@ fn signature_reg_fuse_version(
/// Reset the falcon engine.
fn reset_eng(&self, bar: &Bar0) -> Result;
- /// Returns true of this Falcon supports DMA transfer from system memory to Falcon memory
- fn supports_dma(&self) -> bool;
+ /// returns the method needed to load data into Falcon memory
+ fn load_method(&self) -> LoadMethod;
}
/// Returns a boxed falcon HAL adequate for `chipset`.
diff --git a/drivers/gpu/nova-core/falcon/hal/ga102.rs b/drivers/gpu/nova-core/falcon/hal/ga102.rs
index 6ccbf42ba80c..c94b3fe537c2 100644
--- a/drivers/gpu/nova-core/falcon/hal/ga102.rs
+++ b/drivers/gpu/nova-core/falcon/hal/ga102.rs
@@ -13,6 +13,7 @@
use crate::{
driver::Bar0,
falcon::{
+ hal::LoadMethod,
Falcon,
FalconBromParams,
FalconEngine,
@@ -159,7 +160,7 @@ fn reset_eng(&self, bar: &Bar0) -> Result {
Ok(())
}
- fn supports_dma(&self) -> bool {
- true
+ fn load_method(&self) -> LoadMethod {
+ LoadMethod::Dma
}
}
diff --git a/drivers/gpu/nova-core/falcon/hal/tu102.rs b/drivers/gpu/nova-core/falcon/hal/tu102.rs
index 28dc03db8240..7bed88511967 100644
--- a/drivers/gpu/nova-core/falcon/hal/tu102.rs
+++ b/drivers/gpu/nova-core/falcon/hal/tu102.rs
@@ -12,6 +12,7 @@
use crate::{
driver::Bar0,
falcon::{
+ hal::LoadMethod,
Falcon,
FalconBromParams,
FalconEngine, //
@@ -77,7 +78,7 @@ fn reset_eng(&self, bar: &Bar0) -> Result {
Ok(())
}
- fn supports_dma(&self) -> bool {
- false
+ fn load_method(&self) -> LoadMethod {
+ LoadMethod::Pio
}
}
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index c4fff8b86640..a5678db8f78c 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -40,12 +40,15 @@
FalconLoadTarget, //
},
firmware::{
+ BinHdr,
FalconUCodeDesc,
FirmwareDmaObject,
FirmwareSignature,
Signed,
Unsigned, //
+ FIRMWARE_VERSION,
},
+ gpu::Chipset,
num::{
FromSafeCast,
IntoSafeCast, //
@@ -213,6 +216,68 @@ unsafe fn transmute_mut<T: Sized + FromBytes + AsBytes>(
T::from_bytes_mut(unsafe { fw.as_slice_mut(offset, size_of::<T>())? }).ok_or(EINVAL)
}
+/// Descriptor used by RM to figure out the requirements of the boot loader.
+#[repr(C)]
+#[derive(Debug, Clone)]
+pub(crate) struct BootloaderDesc {
+ /// Starting tag of bootloader.
+ pub start_tag: u32,
+ /// DMEM offset where [`BootloaderDmemDescV2`] is to be loaded.
+ pub dmem_load_off: u32,
+ /// Offset of code section in the image.
+ pub code_off: u32,
+ /// Size of code section in the image.
+ pub code_size: u32,
+ /// Offset of data section in the image.
+ pub data_off: u32,
+ /// Size of data section in the image.
+ pub data_size: u32,
+}
+// SAFETY: any byte sequence is valid for this struct.
+unsafe impl FromBytes for BootloaderDesc {}
+
+/// Structure used by the boot-loader to load the rest of the code.
+///
+/// This has to be filled by the GPU driver and copied into DMEM at offset
+/// [`BootloaderDesc.dmem_load_off`].
+#[repr(C, packed)]
+#[derive(Debug, Clone)]
+pub(crate) struct BootloaderDmemDescV2 {
+ /// Reserved, should always be first element.
+ pub reserved: [u32; 4],
+ /// 16B signature for secure code, 0s if no secure code.
+ pub signature: [u32; 4],
+ /// DMA context used by the bootloader while loading code/data.
+ pub ctx_dma: u32,
+ /// 256B-aligned physical FB address where code is located.
+ pub code_dma_base: u64,
+ /// Offset from `code_dma_base` where the non-secure code is located (must be multiple of 256).
+ pub non_sec_code_off: u32,
+ /// Size of the non-secure code part.
+ pub non_sec_code_size: u32,
+ /// Offset from `code_dma_base` where the secure code is located (must be multiple of 256).
+ pub sec_code_off: u32,
+ /// Size of the secure code part.
+ pub sec_code_size: u32,
+ /// Code entry point invoked by the bootloader after code is loaded.
+ pub code_entry_point: u32,
+ /// 256B-aligned physical FB address where data is located.
+ pub data_dma_base: u64,
+ /// Size of data block (should be multiple of 256B).
+ pub data_size: u32,
+ /// Arguments to be passed to the target firmware being loaded.
+ pub argc: u32,
+ /// Number of arguments to be passed to the target firmware being loaded.
+ pub argv: u32,
+}
+// SAFETY: This struct doesn't contain uninitialized bytes and doesn't have interior mutability.
+unsafe impl AsBytes for BootloaderDmemDescV2 {}
+
+pub(crate) struct GenericBootloader {
+ pub desc: BootloaderDesc,
+ pub ucode: Vec<u8, kernel::alloc::allocator::Kmalloc>,
+}
+
/// The FWSEC microcode, extracted from the BIOS and to be run on the GSP falcon.
///
/// It is responsible for e.g. carving out the WPR2 region as the first step of the GSP bootflow.
@@ -221,6 +286,8 @@ pub(crate) struct FwsecFirmware {
desc: FalconUCodeDesc,
/// GPU-accessible DMA object containing the firmware.
ucode: FirmwareDmaObject<Self, Signed>,
+ /// Generic bootloader
+ gen_bootloader: Option<GenericBootloader>,
}
impl FalconLoadParams for FwsecFirmware {
@@ -275,7 +342,19 @@ fn brom_params(&self) -> FalconBromParams {
}
fn boot_addr(&self) -> u32 {
- 0
+ match &self.desc {
+ FalconUCodeDesc::V2(_v2) => {
+ // On V2 platforms, the boot address is extracted from the
+ // generic bootloader, because the gbl is what actually copies
+ // FWSEC into memory, so that is what needs to be booted.
+ if let Some(ref gbl) = self.gen_bootloader {
+ gbl.desc.start_tag << 8
+ } else {
+ 0
+ }
+ }
+ FalconUCodeDesc::V3(_v3) => 0,
+ }
}
}
@@ -376,6 +455,7 @@ impl FwsecFirmware {
/// command.
pub(crate) fn new(
dev: &Device<device::Bound>,
+ chipset: Chipset,
falcon: &Falcon<Gsp>,
bar: &Bar0,
bios: &Vbios,
@@ -432,9 +512,54 @@ pub(crate) fn new(
ucode_dma.no_patch_signature()
};
+ // The Generic Bootloader exists only on Turing and GA100. To avoid a bogus
+ // console error message on other platforms, only try to load it if it's
+ // supposed to be there.
+ let gbl_fw = if chipset < Chipset::GA102 {
+ Some(super::request_firmware(
+ dev,
+ chipset,
+ "gen_bootloader",
+ FIRMWARE_VERSION,
+ )?)
+ } else {
+ None
+ };
+
+ let gbl = match gbl_fw {
+ Some(fw) => {
+ let hdr = fw
+ .data()
+ .get(0..size_of::<BinHdr>())
+ .and_then(BinHdr::from_bytes_copy)
+ .ok_or(EINVAL)?;
+
+ let desc_offset = usize::from_safe_cast(hdr.header_offset);
+ let desc = fw
+ .data()
+ .get(desc_offset..desc_offset + size_of::<BootloaderDesc>())
+ .and_then(BootloaderDesc::from_bytes_copy)
+ .ok_or(EINVAL)?;
+
+ let ucode_start = usize::from_safe_cast(hdr.data_offset);
+ let ucode_size = usize::from_safe_cast(hdr.data_size);
+ let ucode_data = fw
+ .data()
+ .get(ucode_start..ucode_start + ucode_size)
+ .ok_or(EINVAL)?;
+
+ let mut ucode = KVec::new();
+ ucode.extend_from_slice(ucode_data, GFP_KERNEL)?;
+
+ Some(GenericBootloader { desc, ucode })
+ }
+ None => None,
+ };
+
Ok(FwsecFirmware {
desc,
ucode: ucode_signed,
+ gen_bootloader: gbl,
})
}
@@ -450,7 +575,7 @@ pub(crate) fn run(
.reset(bar)
.inspect_err(|e| dev_err!(dev, "Failed to reset GSP falcon: {:?}\n", e))?;
falcon
- .dma_load(bar, self)
+ .load(bar, self, self.gen_bootloader.as_ref())
.inspect_err(|e| dev_err!(dev, "Failed to load FWSEC firmware: {:?}\n", e))?;
let (mbox0, _) = falcon
.boot(bar, Some(0), None)
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 581b412554dc..f253d5f12252 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -48,6 +48,7 @@ impl super::Gsp {
/// created the WPR2 region.
fn run_fwsec_frts(
dev: &device::Device<device::Bound>,
+ chipset: Chipset,
falcon: &Falcon<Gsp>,
bar: &Bar0,
bios: &Vbios,
@@ -65,6 +66,7 @@ fn run_fwsec_frts(
let fwsec_frts = FwsecFirmware::new(
dev,
+ chipset,
falcon,
bar,
bios,
@@ -144,7 +146,7 @@ pub(crate) fn boot(
let fb_layout = FbLayout::new(chipset, bar, &gsp_fw)?;
dev_dbg!(dev, "{:#x?}\n", fb_layout);
- Self::run_fwsec_frts(dev, gsp_falcon, bar, &bios, &fb_layout)?;
+ Self::run_fwsec_frts(dev, chipset, gsp_falcon, bar, &bios, &fb_layout)?;
let booter_loader = BooterFirmware::new(
dev,
@@ -183,7 +185,7 @@ pub(crate) fn boot(
);
sec2_falcon.reset(bar)?;
- sec2_falcon.dma_load(bar, &booter_loader)?;
+ sec2_falcon.load(bar, &booter_loader, None)?;
let wpr_handle = wpr_meta.dma_handle();
let (mbox0, mbox1) = sec2_falcon.boot(
bar,
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index cd7b7aa6fc2a..f0129a5f7a7e 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -359,6 +359,36 @@ pub(crate) fn with_falcon_mem(self, mem: FalconMem) -> Self {
1:1 startcpu as bool;
});
+// IMEM access control register. Up to 4 ports are available for IMEM access.
+register!(NV_PFALCON_FALCON_IMEMC @ PFalconBase[0x00000180[4; 16]] {
+ 15:0 offs as u16, "IMEM block and word offset";
+ 24:24 aincw as bool, "Auto-increment on write";
+ 28:28 secure as bool, "Access secure IMEM";
+});
+
+// IMEM data register. Reading/writing this register accesses IMEM at the address
+// specified by the corresponding IMEMC register.
+register!(NV_PFALCON_FALCON_IMEMD @ PFalconBase[0x00000184[4; 16]] {
+ 31:0 data as u32;
+});
+
+// IMEM tag register. Used to set the tag for the current IMEM block.
+register!(NV_PFALCON_FALCON_IMEMT @ PFalconBase[0x00000188[4; 16]] {
+ 15:0 tag as u16;
+});
+
+// DMEM access control register. Up to 8 ports are available for DMEM access.
+register!(NV_PFALCON_FALCON_DMEMC @ PFalconBase[0x000001c0[8; 8]] {
+ 15:0 offs as u16, "DMEM block and word offset";
+ 24:24 aincw as bool, "Auto-increment on write";
+});
+
+// DMEM data register. Reading/writing this register accesses DMEM at the address
+// specified by the corresponding DMEMC register.
+register!(NV_PFALCON_FALCON_DMEMD @ PFalconBase[0x000001c4[8; 8]] {
+ 31:0 data as u32;
+});
+
// Actually known as `NV_PSEC_FALCON_ENGINE` and `NV_PGSP_FALCON_ENGINE` depending on the falcon
// instance.
register!(NV_PFALCON_FALCON_ENGINE @ PFalconBase[0x000003c0] {
--
2.52.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v6 08/11] gpu: nova-core: add Falcon HAL method supports_dma()
2026-01-14 19:29 ` [PATCH v6 08/11] gpu: nova-core: add Falcon HAL method supports_dma() Timur Tabi
@ 2026-01-16 1:53 ` Alexandre Courbot
2026-01-16 3:00 ` Timur Tabi
0 siblings, 1 reply; 29+ messages in thread
From: Alexandre Courbot @ 2026-01-16 1:53 UTC (permalink / raw)
To: Timur Tabi
Cc: John Hubbard, Joel Fernandes, Danilo Krummrich, nouveau,
rust-for-linux
On Thu Jan 15, 2026 at 4:29 AM JST, Timur Tabi wrote:
> Some GPUs do not support using DMA to transfer code/data from system
> memory to Falcon memory, and instead must use programmed I/O (PIO).
> Add a function to the Falcon HAL to indicate whether a given GPU's
> Falcons support DMA for this purpose.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
This patch is completely undone by the last one. Let's just drop it.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 08/11] gpu: nova-core: add Falcon HAL method supports_dma()
2026-01-16 1:53 ` Alexandre Courbot
@ 2026-01-16 3:00 ` Timur Tabi
0 siblings, 0 replies; 29+ messages in thread
From: Timur Tabi @ 2026-01-16 3:00 UTC (permalink / raw)
To: Alexandre Courbot
Cc: nouveau@lists.freedesktop.org, Joel Fernandes, dakr@kernel.org,
John Hubbard, rust-for-linux@vger.kernel.org
On Fri, 2026-01-16 at 10:53 +0900, Alexandre Courbot wrote:
> > Signed-off-by: Timur Tabi <ttabi@nvidia.com>
>
> This patch is completely undone by the last one. Let's just drop it.
I can't believe I didn't notice that. Ugh. I will post a v7 tomorrow.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 09/11] gpu: nova-core: add FalconUCodeDescV2 support
2026-01-14 19:29 ` [PATCH v6 09/11] gpu: nova-core: add FalconUCodeDescV2 support Timur Tabi
@ 2026-01-16 3:11 ` Alexandre Courbot
2026-01-16 3:23 ` Alexandre Courbot
1 sibling, 0 replies; 29+ messages in thread
From: Alexandre Courbot @ 2026-01-16 3:11 UTC (permalink / raw)
To: Timur Tabi
Cc: John Hubbard, Joel Fernandes, Danilo Krummrich, nouveau,
rust-for-linux
On Thu Jan 15, 2026 at 4:29 AM JST, Timur Tabi wrote:
<snip>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index e59eee2050a8..07a28c4c0837 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -19,6 +19,8 @@
> driver::Bar0,
> firmware::{
> fwsec::Bcrt30Rsa3kSignature,
> + FalconUCodeDesc,
> + FalconUCodeDescV2,
> FalconUCodeDescV3, //
> },
> num::FromSafeCast,
> @@ -999,19 +1001,10 @@ fn build(self) -> Result<FwSecBiosImage> {
>
> impl FwSecBiosImage {
> /// Get the FwSec header ([`FalconUCodeDescV3`]).
This comment should also be updated to refer to `FalconUCodeDesc`.
> - pub(crate) fn header(&self) -> Result<&FalconUCodeDescV3> {
> + pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
> // Get the falcon ucode offset that was found in setup_falcon_data.
> let falcon_ucode_offset = self.falcon_ucode_offset;
>
> - // Make sure the offset is within the data bounds.
> - if falcon_ucode_offset + core::mem::size_of::<FalconUCodeDescV3>() > self.base.data.len() {
> - dev_err!(
> - self.base.dev,
> - "fwsec-frts header not contained within BIOS bounds\n"
> - );
> - return Err(ERANGE);
> - }
> -
> // Read the first 4 bytes to get the version.
> let hdr_bytes: [u8; 4] = self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4]
> .try_into()
> @@ -1019,33 +1012,49 @@ pub(crate) fn header(&self) -> Result<&FalconUCodeDescV3> {
> let hdr = u32::from_le_bytes(hdr_bytes);
> let ver = (hdr & 0xff00) >> 8;
>
> - if ver != 3 {
> - dev_err!(self.base.dev, "invalid fwsec firmware version: {:?}\n", ver);
> - return Err(EINVAL);
> + let hdr_size = match ver {
> + 2 => core::mem::size_of::<FalconUCodeDescV2>(),
> + 3 => core::mem::size_of::<FalconUCodeDescV3>(),
> + _ => {
> + dev_err!(self.base.dev, "invalid fwsec firmware version: {:?}\n", ver);
> + return Err(EINVAL);
> + }
> + };
> + // Make sure the offset is within the data bounds
> + if falcon_ucode_offset + hdr_size > self.base.data.len() {
> + dev_err!(
> + self.base.dev,
> + "fwsec-frts header not contained within BIOS bounds\n"
> + );
> + return Err(ERANGE);
> }
>
> - // Return a reference to the FalconUCodeDescV3 structure.
> - //
> - // SAFETY: We have checked that `falcon_ucode_offset + size_of::<FalconUCodeDescV3>` is
> - // within the bounds of `data`. Also, this data vector is from ROM, and the `data` field
> - // in `BiosImageBase` is immutable after construction.
> - Ok(unsafe {
> - &*(self
> - .base
> - .data
> - .as_ptr()
> - .add(falcon_ucode_offset)
> - .cast::<FalconUCodeDescV3>())
> - })
> + let data = self
> + .base
> + .data
> + .get(falcon_ucode_offset..falcon_ucode_offset + hdr_size)
> + .ok_or(EINVAL)?;
> +
> + match ver {
> + 2 => {
> + let v2 = FalconUCodeDescV2::from_bytes(data).ok_or(EINVAL)?;
> + Ok(FalconUCodeDesc::V2(v2.clone()))
> + }
> + 3 => {
> + let v3 = FalconUCodeDescV3::from_bytes(data).ok_or(EINVAL)?;
> + Ok(FalconUCodeDesc::V3(v3.clone()))
> + }
> + _ => Err(EINVAL),
> + }
> }
We can be a little bit shorter here and avoid the redundant by
leveraging the new `from_bytes_copy_prefix`. See if this applies on top
of your patch:
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 07a28c4c0837..72b6bad5964b 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -1012,39 +1013,25 @@ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
let hdr = u32::from_le_bytes(hdr_bytes);
let ver = (hdr & 0xff00) >> 8;
- let hdr_size = match ver {
- 2 => core::mem::size_of::<FalconUCodeDescV2>(),
- 3 => core::mem::size_of::<FalconUCodeDescV3>(),
- _ => {
- dev_err!(self.base.dev, "invalid fwsec firmware version: {:?}\n", ver);
- return Err(EINVAL);
- }
- };
- // Make sure the offset is within the data bounds
- if falcon_ucode_offset + hdr_size > self.base.data.len() {
- dev_err!(
- self.base.dev,
- "fwsec-frts header not contained within BIOS bounds\n"
- );
- return Err(ERANGE);
- }
-
- let data = self
- .base
- .data
- .get(falcon_ucode_offset..falcon_ucode_offset + hdr_size)
- .ok_or(EINVAL)?;
-
+ let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
match ver {
2 => {
- let v2 = FalconUCodeDescV2::from_bytes(data).ok_or(EINVAL)?;
- Ok(FalconUCodeDesc::V2(v2.clone()))
+ let v2 = FalconUCodeDescV2::from_bytes_copy_prefix(data)
+ .ok_or(EINVAL)?
+ .0;
+ Ok(FalconUCodeDesc::V2(v2))
}
3 => {
- let v3 = FalconUCodeDescV3::from_bytes(data).ok_or(EINVAL)?;
- Ok(FalconUCodeDesc::V3(v3.clone()))
+ let v3 = FalconUCodeDescV3::from_bytes_copy_prefix(data)
+ .ok_or(EINVAL)?
+ .0;
+ Ok(FalconUCodeDesc::V3(v3))
+ }
+ _ => {
+ dev_err!(self.base.dev, "invalid fwsec firmware version: {:?}\n", ver);
+ Err(EINVAL)
}
- _ => Err(EINVAL),
}
}
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v6 09/11] gpu: nova-core: add FalconUCodeDescV2 support
2026-01-14 19:29 ` [PATCH v6 09/11] gpu: nova-core: add FalconUCodeDescV2 support Timur Tabi
2026-01-16 3:11 ` Alexandre Courbot
@ 2026-01-16 3:23 ` Alexandre Courbot
2026-01-16 20:09 ` Danilo Krummrich
1 sibling, 1 reply; 29+ messages in thread
From: Alexandre Courbot @ 2026-01-16 3:23 UTC (permalink / raw)
To: Timur Tabi
Cc: John Hubbard, Joel Fernandes, Danilo Krummrich, nouveau,
rust-for-linux
On Thu Jan 15, 2026 at 4:29 AM JST, Timur Tabi wrote:
> The FRTS firmware in Turing and GA100 VBIOS has an older header
> format (v2 instead of v3). To support both v2 and v3 at runtime,
> add the FalconUCodeDescV2 struct, and update code that references
> the FalconUCodeDescV3 directly with a FalconUCodeDesc enum that
> encapsulates both.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
> drivers/gpu/nova-core/firmware.rs | 149 +++++++++++++++++++++++-
> drivers/gpu/nova-core/firmware/fwsec.rs | 72 ++++++++----
> drivers/gpu/nova-core/vbios.rs | 75 +++++++-----
> 3 files changed, 237 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
> index 2d2008b33fb4..9f0ad02fbe22 100644
> --- a/drivers/gpu/nova-core/firmware.rs
> +++ b/drivers/gpu/nova-core/firmware.rs
> @@ -4,6 +4,7 @@
> //! to be loaded into a given execution unit.
>
> use core::marker::PhantomData;
> +use core::ops::Deref;
>
> use kernel::{
> device,
> @@ -43,6 +44,46 @@ fn request_firmware(
> .and_then(|path| firmware::Firmware::request(&path, dev))
> }
>
> +/// Structure used to describe some firmwares, notably FWSEC-FRTS.
> +#[repr(C)]
> +#[derive(Debug, Clone)]
> +pub(crate) struct FalconUCodeDescV2 {
> + /// Header defined by 'NV_BIT_FALCON_UCODE_DESC_HEADER_VDESC*' in OpenRM.
> + hdr: u32,
> + /// Stored size of the ucode after the header, compressed or uncompressed
> + stored_size: u32,
> + /// Uncompressed size of the ucode. If store_size == uncompressed_size, then the ucode
> + /// is not compressed.
> + pub(crate) uncompressed_size: u32,
> + /// Code entry point
> + pub(crate) virtual_entry: u32,
> + /// Offset after the code segment at which the Application Interface Table headers are located.
> + pub(crate) interface_offset: u32,
> + /// Base address at which to load the code segment into 'IMEM'.
> + pub(crate) imem_phys_base: u32,
> + /// Size in bytes of the code to copy into 'IMEM'.
> + pub(crate) imem_load_size: u32,
> + /// Virtual 'IMEM' address (i.e. 'tag') at which the code should start.
> + pub(crate) imem_virt_base: u32,
> + /// Virtual address of secure IMEM segment.
> + pub(crate) imem_sec_base: u32,
> + /// Size of secure IMEM segment.
> + pub(crate) imem_sec_size: u32,
> + /// Offset into stored (uncompressed) image at which DMEM begins.
> + pub(crate) dmem_offset: u32,
> + /// Base address at which to load the data segment into 'DMEM'.
> + pub(crate) dmem_phys_base: u32,
> + /// Size in bytes of the data to copy into 'DMEM'.
> + pub(crate) dmem_load_size: u32,
> + /// "Alternate" Size of data to load into IMEM.
> + pub(crate) alt_imem_load_size: u32,
> + /// "Alternate" Size of data to load into DMEM.
> + pub(crate) alt_dmem_load_size: u32,
> +}
> +
> +// SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
> +unsafe impl FromBytes for FalconUCodeDescV2 {}
> +
> /// Structure used to describe some firmwares, notably FWSEC-FRTS.
> #[repr(C)]
> #[derive(Debug, Clone)]
> @@ -76,13 +117,115 @@ pub(crate) struct FalconUCodeDescV3 {
> _reserved: u16,
> }
>
> -impl FalconUCodeDescV3 {
> +// SAFETY: all bit patterns are valid for this type, and it doesn't use
> +// interior mutability.
> +unsafe impl FromBytes for FalconUCodeDescV3 {}
> +
> +/// Enum wrapping the different versions of Falcon microcode descriptors.
> +///
> +/// This allows handling both V2 and V3 descriptor formats through a
> +/// unified type, providing version-agnostic access to firmware metadata
> +/// via the [`FalconUCodeDescriptor`] trait.
> +#[derive(Debug, Clone)]
> +pub(crate) enum FalconUCodeDesc {
> + V2(FalconUCodeDescV2),
> + V3(FalconUCodeDescV3),
> +}
> +
> +impl Deref for FalconUCodeDesc {
> + type Target = dyn FalconUCodeDescriptor;
> +
> + fn deref(&self) -> &Self::Target {
> + match self {
> + FalconUCodeDesc::V2(v2) => v2,
> + FalconUCodeDesc::V3(v3) => v3,
> + }
> + }
> +}
> +
> +/// Trait providing a common interface for accessing Falcon microcode descriptor fields.
> +///
> +/// This trait abstracts over the different descriptor versions ([`FalconUCodeDescV2`] and
> +/// [`FalconUCodeDescV3`]), allowing code to work with firmware metadata without needing to
> +/// know the specific descriptor version. Fields not present return zero.
> +pub(crate) trait FalconUCodeDescriptor {
> + fn hdr(&self) -> u32;
> + fn imem_load_size(&self) -> u32;
> + fn interface_offset(&self) -> u32;
> + fn dmem_load_size(&self) -> u32;
> + fn pkc_data_offset(&self) -> u32;
> + fn engine_id_mask(&self) -> u16;
> + fn ucode_id(&self) -> u8;
> + fn signature_count(&self) -> u8;
> + fn signature_versions(&self) -> u16;
> +
> /// Returns the size in bytes of the header.
> - pub(crate) fn size(&self) -> usize {
> + fn size(&self) -> usize {
> + let hdr = self.hdr();
> +
> const HDR_SIZE_SHIFT: u32 = 16;
> const HDR_SIZE_MASK: u32 = 0xffff0000;
> + ((hdr & HDR_SIZE_MASK) >> HDR_SIZE_SHIFT).into_safe_cast()
> + }
> +}
> +
> +impl FalconUCodeDescriptor for FalconUCodeDescV2 {
> + fn hdr(&self) -> u32 {
> + self.hdr
> + }
> + fn imem_load_size(&self) -> u32 {
> + self.imem_load_size
> + }
> + fn interface_offset(&self) -> u32 {
> + self.interface_offset
> + }
> + fn dmem_load_size(&self) -> u32 {
> + self.dmem_load_size
> + }
> + fn pkc_data_offset(&self) -> u32 {
> + 0
> + }
> + fn engine_id_mask(&self) -> u16 {
> + 0
> + }
> + fn ucode_id(&self) -> u8 {
> + 0
> + }
> + fn signature_count(&self) -> u8 {
> + 0
> + }
> + fn signature_versions(&self) -> u16 {
> + 0
> + }
> +}
>
> - ((self.hdr & HDR_SIZE_MASK) >> HDR_SIZE_SHIFT).into_safe_cast()
> +impl FalconUCodeDescriptor for FalconUCodeDescV3 {
> + fn hdr(&self) -> u32 {
> + self.hdr
> + }
> + fn imem_load_size(&self) -> u32 {
> + self.imem_load_size
> + }
> + fn interface_offset(&self) -> u32 {
> + self.interface_offset
> + }
> + fn dmem_load_size(&self) -> u32 {
> + self.dmem_load_size
> + }
> + fn pkc_data_offset(&self) -> u32 {
> + self.pkc_data_offset
> + }
> + fn engine_id_mask(&self) -> u16 {
> + self.engine_id_mask
> + }
> + fn ucode_id(&self) -> u8 {
> + self.ucode_id
> + }
> + fn signature_count(&self) -> u8 {
> + self.signature_count
> + }
> + fn signature_versions(&self) -> u16 {
> + self.signature_versions
> }
> }
>
> diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
> index e4009faba6c5..c4fff8b86640 100644
> --- a/drivers/gpu/nova-core/firmware/fwsec.rs
> +++ b/drivers/gpu/nova-core/firmware/fwsec.rs
> @@ -40,7 +40,7 @@
> FalconLoadTarget, //
> },
> firmware::{
> - FalconUCodeDescV3,
> + FalconUCodeDesc,
> FirmwareDmaObject,
> FirmwareSignature,
> Signed,
> @@ -218,38 +218,59 @@ unsafe fn transmute_mut<T: Sized + FromBytes + AsBytes>(
> /// It is responsible for e.g. carving out the WPR2 region as the first step of the GSP bootflow.
> pub(crate) struct FwsecFirmware {
> /// Descriptor of the firmware.
> - desc: FalconUCodeDescV3,
> + desc: FalconUCodeDesc,
> /// GPU-accessible DMA object containing the firmware.
> ucode: FirmwareDmaObject<Self, Signed>,
> }
>
> impl FalconLoadParams for FwsecFirmware {
> fn imem_sec_load_params(&self) -> FalconLoadTarget {
> - FalconLoadTarget {
> - src_start: 0,
> - dst_start: self.desc.imem_phys_base,
> - len: self.desc.imem_load_size,
> + match &self.desc {
> + FalconUCodeDesc::V2(v2) => FalconLoadTarget {
> + src_start: 0,
> + dst_start: v2.imem_sec_base,
> + len: v2.imem_sec_size,
> + },
> + FalconUCodeDesc::V3(v3) => FalconLoadTarget {
> + src_start: 0,
> + dst_start: v3.imem_phys_base,
> + len: v3.imem_load_size,
> + },
> }
> }
>
> fn imem_ns_load_params(&self) -> Option<FalconLoadTarget> {
> - // Only used on Turing and GA100, so return None for now
> - None
> + match &self.desc {
> + FalconUCodeDesc::V2(v2) => Some(FalconLoadTarget {
> + src_start: 0,
> + dst_start: v2.imem_phys_base,
> + len: v2.imem_load_size.checked_sub(v2.imem_sec_size)?,
> + }),
> + // Not used on V3 platforms
> + FalconUCodeDesc::V3(_v3) => None,
> + }
> }
>
> fn dmem_load_params(&self) -> FalconLoadTarget {
> - FalconLoadTarget {
> - src_start: self.desc.imem_load_size,
> - dst_start: self.desc.dmem_phys_base,
> - len: self.desc.dmem_load_size,
> + match &self.desc {
> + FalconUCodeDesc::V2(v2) => FalconLoadTarget {
> + src_start: v2.dmem_offset,
> + dst_start: v2.dmem_phys_base,
> + len: v2.dmem_load_size,
> + },
> + FalconUCodeDesc::V3(v3) => FalconLoadTarget {
> + src_start: v3.imem_load_size,
> + dst_start: v3.dmem_phys_base,
> + len: v3.dmem_load_size,
> + },
> }
> }
>
> fn brom_params(&self) -> FalconBromParams {
> FalconBromParams {
> - pkc_data_offset: self.desc.pkc_data_offset,
> - engine_id_mask: self.desc.engine_id_mask,
> - ucode_id: self.desc.ucode_id,
> + pkc_data_offset: self.desc.pkc_data_offset(),
> + engine_id_mask: self.desc.engine_id_mask(),
> + ucode_id: self.desc.ucode_id(),
> }
> }
It looks like John and I have a different opinion on which approach is
the best to handle the v2/v3 headers (a match in each method vs a new
trait and virtual method calls).
TBH I don't really mind which approach we adopt, but I would like us to
stay consistent through the code. This patch adds the trait and
implements `Deref`, but then the chunk above still performs matches.
I did suggest that we add more methods to the trait in v4, but to be
clear here is the doing what I suggested. However if you prefer to drop
the trait and perform a match in every method of `FalconUCodeDesc`,
that's also acceptable to me - in this case the matches you did above
would be consistent with the general approach and can be kept as-is.
diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index 9f0ad02fbe22..68779540aa28 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -16,7 +16,10 @@
use crate::{
dma::DmaObject,
- falcon::FalconFirmware,
+ falcon::{
+ FalconFirmware,
+ FalconLoadTarget, //
+ },
gpu,
num::{
FromSafeCast,
@@ -167,6 +170,10 @@ fn size(&self) -> usize {
const HDR_SIZE_MASK: u32 = 0xffff0000;
((hdr & HDR_SIZE_MASK) >> HDR_SIZE_SHIFT).into_safe_cast()
}
+
+ fn imem_sec_load_params(&self) -> FalconLoadTarget;
+ fn imem_ns_load_params(&self) -> Option<FalconLoadTarget>;
+ fn dmem_load_params(&self) -> FalconLoadTarget;
}
impl FalconUCodeDescriptor for FalconUCodeDescV2 {
@@ -197,6 +204,30 @@ fn signature_count(&self) -> u8 {
fn signature_versions(&self) -> u16 {
0
}
+
+ fn imem_sec_load_params(&self) -> FalconLoadTarget {
+ FalconLoadTarget {
+ src_start: 0,
+ dst_start: self.imem_sec_base,
+ len: self.imem_sec_size,
+ }
+ }
+
+ fn imem_ns_load_params(&self) -> Option<FalconLoadTarget> {
+ Some(FalconLoadTarget {
+ src_start: 0,
+ dst_start: self.imem_phys_base,
+ len: self.imem_load_size.checked_sub(self.imem_sec_size)?,
+ })
+ }
+
+ fn dmem_load_params(&self) -> FalconLoadTarget {
+ FalconLoadTarget {
+ src_start: self.dmem_offset,
+ dst_start: self.dmem_phys_base,
+ len: self.dmem_load_size,
+ }
+ }
}
impl FalconUCodeDescriptor for FalconUCodeDescV3 {
@@ -227,6 +258,27 @@ fn signature_count(&self) -> u8 {
fn signature_versions(&self) -> u16 {
self.signature_versions
}
+
+ fn imem_sec_load_params(&self) -> FalconLoadTarget {
+ FalconLoadTarget {
+ src_start: 0,
+ dst_start: self.imem_phys_base,
+ len: self.imem_load_size,
+ }
+ }
+
+ fn imem_ns_load_params(&self) -> Option<FalconLoadTarget> {
+ // Not used on V3 platforms
+ None
+ }
+
+ fn dmem_load_params(&self) -> FalconLoadTarget {
+ FalconLoadTarget {
+ src_start: self.imem_load_size,
+ dst_start: self.dmem_phys_base,
+ len: self.dmem_load_size,
+ }
+ }
}
/// Trait implemented by types defining the signed state of a firmware.
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index a5678db8f78c..4efa4711fce1 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -292,45 +292,15 @@ pub(crate) struct FwsecFirmware {
impl FalconLoadParams for FwsecFirmware {
fn imem_sec_load_params(&self) -> FalconLoadTarget {
- match &self.desc {
- FalconUCodeDesc::V2(v2) => FalconLoadTarget {
- src_start: 0,
- dst_start: v2.imem_sec_base,
- len: v2.imem_sec_size,
- },
- FalconUCodeDesc::V3(v3) => FalconLoadTarget {
- src_start: 0,
- dst_start: v3.imem_phys_base,
- len: v3.imem_load_size,
- },
- }
+ self.desc.imem_sec_load_params()
}
fn imem_ns_load_params(&self) -> Option<FalconLoadTarget> {
- match &self.desc {
- FalconUCodeDesc::V2(v2) => Some(FalconLoadTarget {
- src_start: 0,
- dst_start: v2.imem_phys_base,
- len: v2.imem_load_size.checked_sub(v2.imem_sec_size)?,
- }),
- // Not used on V3 platforms
- FalconUCodeDesc::V3(_v3) => None,
- }
+ self.desc.imem_ns_load_params()
}
fn dmem_load_params(&self) -> FalconLoadTarget {
- match &self.desc {
- FalconUCodeDesc::V2(v2) => FalconLoadTarget {
- src_start: v2.dmem_offset,
- dst_start: v2.dmem_phys_base,
- len: v2.dmem_load_size,
- },
- FalconUCodeDesc::V3(v3) => FalconLoadTarget {
- src_start: v3.imem_load_size,
- dst_start: v3.dmem_phys_base,
- len: v3.dmem_load_size,
- },
- }
+ self.desc.dmem_load_params()
}
fn brom_params(&self) -> FalconBromParams {
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v6 11/11] gpu: nova-core: add PIO support for loading firmware images
2026-01-14 19:29 ` [PATCH v6 11/11] gpu: nova-core: add PIO support for loading firmware images Timur Tabi
@ 2026-01-16 15:22 ` Alexandre Courbot
2026-01-16 21:05 ` Danilo Krummrich
1 sibling, 0 replies; 29+ messages in thread
From: Alexandre Courbot @ 2026-01-16 15:22 UTC (permalink / raw)
To: Timur Tabi
Cc: John Hubbard, Joel Fernandes, Danilo Krummrich, nouveau,
rust-for-linux
On Thu Jan 15, 2026 at 4:29 AM JST, Timur Tabi wrote:
> Turing and GA100 use programmed I/O (PIO) instead of DMA to upload
> firmware images into Falcon memory.
>
> A new firmware called the Generic Bootloader (as opposed to the
> GSP Bootloader) is used to upload FWSEC.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Just a heads-up: I have a bunch of comments for this patch, but also
want to write some code to validate them, which is taking a bit of time.
Just letting you know so you hold on v7 until I submit my feedback for
this patch.
Thanks!
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 06/11] gpu: nova-core: move some functions into the HAL
2026-01-14 19:29 ` [PATCH v6 06/11] gpu: nova-core: move some functions into the HAL Timur Tabi
@ 2026-01-16 19:55 ` Danilo Krummrich
2026-01-16 20:08 ` John Hubbard
0 siblings, 1 reply; 29+ messages in thread
From: Danilo Krummrich @ 2026-01-16 19:55 UTC (permalink / raw)
To: Timur Tabi
Cc: Alexandre Courbot, John Hubbard, Joel Fernandes, nouveau,
rust-for-linux
On Wed Jan 14, 2026 at 8:29 PM CET, Timur Tabi wrote:
> + regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(true));
> +
> + // TIMEOUT: falcon engine should not take more than 10us to reset.
> + fsleep(Delta::from_micros(10));
> +
> + regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(false));
This code is just moved, but repeated in the subsequent patch adding the Turing
HAL.
If we always expect the falcon engine to reset within 10us regardless of the GPU
architecture, we could add a regs::NV_PFALCON_FALCON_ENGINE::reset() function
doing the above.
If it the exact timeout differs, we could just pass it as argument to
regs::NV_PFALCON_FALCON_ENGINE::reset().
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 06/11] gpu: nova-core: move some functions into the HAL
2026-01-16 19:55 ` Danilo Krummrich
@ 2026-01-16 20:08 ` John Hubbard
2026-01-16 20:11 ` Danilo Krummrich
0 siblings, 1 reply; 29+ messages in thread
From: John Hubbard @ 2026-01-16 20:08 UTC (permalink / raw)
To: Danilo Krummrich, Timur Tabi
Cc: Alexandre Courbot, Joel Fernandes, nouveau, rust-for-linux
On 1/16/26 11:55 AM, Danilo Krummrich wrote:
> On Wed Jan 14, 2026 at 8:29 PM CET, Timur Tabi wrote:
>> + regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(true));
>> +
>> + // TIMEOUT: falcon engine should not take more than 10us to reset.
>> + fsleep(Delta::from_micros(10));
>> +
>> + regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(false));
>
> This code is just moved, but repeated in the subsequent patch adding the Turing
> HAL.
>
> If we always expect the falcon engine to reset within 10us regardless of the GPU
> architecture, we could add a regs::NV_PFALCON_FALCON_ENGINE::reset() function
On newer GPU architectures, we don't even *do* a falcon reset, fwiw. For
example, on Blackwell, the GPU's FSP reboots the GSP by itself. And we
don't do an FSP reset. So we are all out of the falcon reset business
there. :)
thanks,
--
John Hubbard
> doing the above.
>
> If it the exact timeout differs, we could just pass it as argument to
> regs::NV_PFALCON_FALCON_ENGINE::reset().
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 09/11] gpu: nova-core: add FalconUCodeDescV2 support
2026-01-16 3:23 ` Alexandre Courbot
@ 2026-01-16 20:09 ` Danilo Krummrich
0 siblings, 0 replies; 29+ messages in thread
From: Danilo Krummrich @ 2026-01-16 20:09 UTC (permalink / raw)
To: Alexandre Courbot, Timur Tabi
Cc: John Hubbard, Joel Fernandes, nouveau, rust-for-linux
On Fri Jan 16, 2026 at 4:23 AM CET, Alexandre Courbot wrote:
> It looks like John and I have a different opinion on which approach is
> the best to handle the v2/v3 headers (a match in each method vs a new
> trait and virtual method calls).
Unless performance is a concern (which obviously isn't the case here), I think
the dynamic dispatch is fine and leads to cleaner and more maintainable code.
> TBH I don't really mind which approach we adopt, but I would like us to
> stay consistent through the code. This patch adds the trait and
> implements `Deref`, but then the chunk above still performs matches.
Yes, please be consistent -- Alex' diff looks good to me.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 06/11] gpu: nova-core: move some functions into the HAL
2026-01-16 20:08 ` John Hubbard
@ 2026-01-16 20:11 ` Danilo Krummrich
2026-01-16 20:15 ` John Hubbard
0 siblings, 1 reply; 29+ messages in thread
From: Danilo Krummrich @ 2026-01-16 20:11 UTC (permalink / raw)
To: John Hubbard
Cc: Timur Tabi, Alexandre Courbot, Joel Fernandes, nouveau,
rust-for-linux
On Fri Jan 16, 2026 at 9:08 PM CET, John Hubbard wrote:
> On newer GPU architectures, we don't even *do* a falcon reset, fwiw. For
> example, on Blackwell, the GPU's FSP reboots the GSP by itself. And we
> don't do an FSP reset. So we are all out of the falcon reset business
> there. :)
Great, then let's add the reset() function. :)
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 06/11] gpu: nova-core: move some functions into the HAL
2026-01-16 20:11 ` Danilo Krummrich
@ 2026-01-16 20:15 ` John Hubbard
2026-01-16 20:21 ` Danilo Krummrich
0 siblings, 1 reply; 29+ messages in thread
From: John Hubbard @ 2026-01-16 20:15 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Timur Tabi, Alexandre Courbot, Joel Fernandes, nouveau,
rust-for-linux
On 1/16/26 12:11 PM, Danilo Krummrich wrote:
> On Fri Jan 16, 2026 at 9:08 PM CET, John Hubbard wrote:
>> On newer GPU architectures, we don't even *do* a falcon reset, fwiw. For
>> example, on Blackwell, the GPU's FSP reboots the GSP by itself. And we
>> don't do an FSP reset. So we are all out of the falcon reset business
>> there. :)
>
> Great, then let's add the reset() function. :)
I missed something here. Could you elaborate just a bit more on
your proposal please?
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 06/11] gpu: nova-core: move some functions into the HAL
2026-01-16 20:15 ` John Hubbard
@ 2026-01-16 20:21 ` Danilo Krummrich
2026-01-16 20:27 ` John Hubbard
0 siblings, 1 reply; 29+ messages in thread
From: Danilo Krummrich @ 2026-01-16 20:21 UTC (permalink / raw)
To: John Hubbard
Cc: Timur Tabi, Alexandre Courbot, Joel Fernandes, nouveau,
rust-for-linux
On Fri Jan 16, 2026 at 9:15 PM CET, John Hubbard wrote:
> I missed something here. Could you elaborate just a bit more on
> your proposal please?
Both HALs Ampere and Turing implement the same code to reset the falcon engine:
regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(true));
// TIMEOUT: falcon engine should not take more than 10us to reset.
fsleep(Delta::from_micros(10));
regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(false));
Instead of repeating this code we can just add a new function
regs::NV_PFALCON_FALCON_ENGINE::reset() that toggles the bit with a
corresponding delay.
So far the delay is always the same, should a different delay be required for
another architecture, we can also pass it to
regs::NV_PFALCON_FALCON_ENGINE::reset() as argument, but that doesn't seem to be
necessary.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 06/11] gpu: nova-core: move some functions into the HAL
2026-01-16 20:21 ` Danilo Krummrich
@ 2026-01-16 20:27 ` John Hubbard
0 siblings, 0 replies; 29+ messages in thread
From: John Hubbard @ 2026-01-16 20:27 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Timur Tabi, Alexandre Courbot, Joel Fernandes, nouveau,
rust-for-linux
On 1/16/26 12:21 PM, Danilo Krummrich wrote:
> On Fri Jan 16, 2026 at 9:15 PM CET, John Hubbard wrote:
>> I missed something here. Could you elaborate just a bit more on
>> your proposal please?
>
> Both HALs Ampere and Turing implement the same code to reset the falcon engine:
>
> regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(true));
>
> // TIMEOUT: falcon engine should not take more than 10us to reset.
> fsleep(Delta::from_micros(10));
>
> regs::NV_PFALCON_FALCON_ENGINE::update(bar, &E::ID, |v| v.set_reset(false));
>
> Instead of repeating this code we can just add a new function
> regs::NV_PFALCON_FALCON_ENGINE::reset() that toggles the bit with a
> corresponding delay.
>
> So far the delay is always the same, should a different delay be required for
> another architecture, we can also pass it to
> regs::NV_PFALCON_FALCON_ENGINE::reset() as argument, but that doesn't seem to be
> necessary.
That sounds like exactly the right factoring, yes. Thanks for spelling
it out in detail.
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 11/11] gpu: nova-core: add PIO support for loading firmware images
2026-01-14 19:29 ` [PATCH v6 11/11] gpu: nova-core: add PIO support for loading firmware images Timur Tabi
2026-01-16 15:22 ` Alexandre Courbot
@ 2026-01-16 21:05 ` Danilo Krummrich
2026-01-17 1:55 ` Alexandre Courbot
2026-01-21 0:25 ` Timur Tabi
1 sibling, 2 replies; 29+ messages in thread
From: Danilo Krummrich @ 2026-01-16 21:05 UTC (permalink / raw)
To: Timur Tabi
Cc: Alexandre Courbot, John Hubbard, Joel Fernandes, nouveau,
rust-for-linux
On Wed Jan 14, 2026 at 8:29 PM CET, Timur Tabi wrote:
> + /// Write a byte array to Falcon memory using programmed I/O (PIO).
> + ///
> + /// Writes `img` to the specified `target_mem` (IMEM or DMEM) starting at `mem_base`.
> + /// For IMEM writes, tags are set for each 256-byte block starting from `tag`.
> + ///
> + /// Returns `EINVAL` if `img.len()` is not a multiple of 4.
> + fn pio_wr_bytes(
> + &self,
> + bar: &Bar0,
> + img: &[u8],
> + mem_base: u16,
> + target_mem: FalconMem,
> + port: u8,
This looks like we should use the Bounded type instead.
> + tag: u16,
> + ) -> Result {
> + // Rejecting misaligned images here allows us to avoid checking
> + // inside the loops.
> + if img.len() % 4 != 0 {
> + return Err(EINVAL);
> + }
> +
> + let port = usize::from(port);
> +
> + match target_mem {
> + FalconMem::ImemSecure | FalconMem::ImemNonSecure => {
> + regs::NV_PFALCON_FALCON_IMEMC::default()
> + .set_secure(target_mem == FalconMem::ImemSecure)
> + .set_aincw(true)
> + .set_offs(mem_base)
> + .write(bar, &E::ID, port);
> +
> + let mut tag = tag;
> + for block in img.chunks(256) {
for (n, block) in img.chunks(256).iter().enumerate() {
regs::NV_PFALCON_FALCON_IMEMT::default()
.set_tag(tag + n)
.write(bar, &E::ID, port);
}
This way you don't need the mutable shadow of tag. Besides that, how do we know
this doesn't overflow? Don't we need a checked_add()?
> + regs::NV_PFALCON_FALCON_IMEMT::default()
> + .set_tag(tag)
> + .write(bar, &E::ID, port);
> + for word in block.chunks_exact(4) {
> + let w = [word[0], word[1], word[2], word[3]];
> + regs::NV_PFALCON_FALCON_IMEMD::default()
> + .set_data(u32::from_le_bytes(w))
> + .write(bar, &E::ID, port);
> + }
> + tag += 1;
> + }
> + }
> + FalconMem::Dmem => {
> + regs::NV_PFALCON_FALCON_DMEMC::default()
> + .set_aincw(true)
> + .set_offs(mem_base)
> + .write(bar, &E::ID, port);
> +
> + for block in img.chunks(256) {
> + for word in block.chunks_exact(4) {
> + let w = [word[0], word[1], word[2], word[3]];
> + regs::NV_PFALCON_FALCON_DMEMD::default()
> + .set_data(u32::from_le_bytes(w))
> + .write(bar, &E::ID, port);
> + }
> + }
> + }
> + }
> +
> + Ok(())
> + }
> +
> + fn pio_wr<F: FalconFirmware<Target = E>>(
> + &self,
> + bar: &Bar0,
> + fw: &F,
> + target_mem: FalconMem,
> + load_offsets: &FalconLoadTarget,
> + port: u8,
> + tag: u16,
> + ) -> Result {
> + let start = usize::from_safe_cast(load_offsets.src_start);
> + let len = usize::from_safe_cast(load_offsets.len);
> + let mem_base = u16::try_from(load_offsets.dst_start)?;
> +
> + // SAFETY: we are the only user of the firmware image at this stage
> + let data = unsafe { fw.as_slice(start, len).map_err(|_| EINVAL)? };
Why do we need the firmware image to be backed by a DMA object at this point
when you load the firmware image through PIO anyways?
> diff --git a/drivers/gpu/nova-core/falcon/hal.rs b/drivers/gpu/nova-core/falcon/hal.rs
> index 49501aa6ff7f..3a882b7d8aa8 100644
> --- a/drivers/gpu/nova-core/falcon/hal.rs
> +++ b/drivers/gpu/nova-core/falcon/hal.rs
> @@ -15,6 +15,11 @@
> mod ga102;
> mod tu102;
>
> +pub(crate) enum LoadMethod {
> + Pio,
> + Dma,
> +}
Seems obvious, but still, please add some documentation explaining that this is
the load method for falcon firmware images, etc.
> +pub(crate) struct GenericBootloader {
> + pub desc: BootloaderDesc,
> + pub ucode: Vec<u8, kernel::alloc::allocator::Kmalloc>,
pub ucode: KVec<u8>,
Also, we may want to use KVVec<u8> or just VVec<u8> instead. What's the image
size?
> +}
> +
> /// The FWSEC microcode, extracted from the BIOS and to be run on the GSP falcon.
> ///
> /// It is responsible for e.g. carving out the WPR2 region as the first step of the GSP bootflow.
> @@ -221,6 +286,8 @@ pub(crate) struct FwsecFirmware {
> desc: FalconUCodeDesc,
> /// GPU-accessible DMA object containing the firmware.
> ucode: FirmwareDmaObject<Self, Signed>,
> + /// Generic bootloader
> + gen_bootloader: Option<GenericBootloader>,
I'm not convinced this is a good idea. We probably want a HAL here and have
different FwsecFirmware types:
One with a DMA object and one with a system memory object when the architecture
uses PIO. In the latter case the object can have a GenericBootloader field, i.e.
this also gets us rid of the Option and all the subsequent 'if chipset <
Chipset::GA102' checks and 'match gbl_fw' matches below.
> }
>
> impl FalconLoadParams for FwsecFirmware {
> @@ -275,7 +342,19 @@ fn brom_params(&self) -> FalconBromParams {
> }
>
> fn boot_addr(&self) -> u32 {
> - 0
> + match &self.desc {
> + FalconUCodeDesc::V2(_v2) => {
> + // On V2 platforms, the boot address is extracted from the
> + // generic bootloader, because the gbl is what actually copies
> + // FWSEC into memory, so that is what needs to be booted.
> + if let Some(ref gbl) = self.gen_bootloader {
> + gbl.desc.start_tag << 8
> + } else {
> + 0
> + }
> + }
> + FalconUCodeDesc::V3(_v3) => 0,
> + }
> }
> }
>
> @@ -376,6 +455,7 @@ impl FwsecFirmware {
> /// command.
> pub(crate) fn new(
> dev: &Device<device::Bound>,
> + chipset: Chipset,
> falcon: &Falcon<Gsp>,
> bar: &Bar0,
> bios: &Vbios,
> @@ -432,9 +512,54 @@ pub(crate) fn new(
> ucode_dma.no_patch_signature()
> };
>
> + // The Generic Bootloader exists only on Turing and GA100. To avoid a bogus
> + // console error message on other platforms, only try to load it if it's
> + // supposed to be there.
> + let gbl_fw = if chipset < Chipset::GA102 {
> + Some(super::request_firmware(
> + dev,
> + chipset,
> + "gen_bootloader",
> + FIRMWARE_VERSION,
> + )?)
> + } else {
> + None
> + };
> +
> + let gbl = match gbl_fw {
> + Some(fw) => {
> + let hdr = fw
> + .data()
> + .get(0..size_of::<BinHdr>())
> + .and_then(BinHdr::from_bytes_copy)
> + .ok_or(EINVAL)?;
> +
> + let desc_offset = usize::from_safe_cast(hdr.header_offset);
> + let desc = fw
> + .data()
> + .get(desc_offset..desc_offset + size_of::<BootloaderDesc>())
> + .and_then(BootloaderDesc::from_bytes_copy)
> + .ok_or(EINVAL)?;
> +
> + let ucode_start = usize::from_safe_cast(hdr.data_offset);
> + let ucode_size = usize::from_safe_cast(hdr.data_size);
> + let ucode_data = fw
> + .data()
> + .get(ucode_start..ucode_start + ucode_size)
> + .ok_or(EINVAL)?;
> +
> + let mut ucode = KVec::new();
> + ucode.extend_from_slice(ucode_data, GFP_KERNEL)?;
> +
> + Some(GenericBootloader { desc, ucode })
> + }
> + None => None,
> + };
> +
> Ok(FwsecFirmware {
> desc,
> ucode: ucode_signed,
> + gen_bootloader: gbl,
> })
> }
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 11/11] gpu: nova-core: add PIO support for loading firmware images
2026-01-16 21:05 ` Danilo Krummrich
@ 2026-01-17 1:55 ` Alexandre Courbot
2026-01-21 0:25 ` Timur Tabi
1 sibling, 0 replies; 29+ messages in thread
From: Alexandre Courbot @ 2026-01-17 1:55 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: Timur Tabi, Joel Fernandes, nouveau, rust-for-linux
On Sat Jan 17, 2026 at 6:05 AM JST, Danilo Krummrich wrote:
<snip>
>> + regs::NV_PFALCON_FALCON_IMEMT::default()
>> + .set_tag(tag)
>> + .write(bar, &E::ID, port);
>> + for word in block.chunks_exact(4) {
>> + let w = [word[0], word[1], word[2], word[3]];
>> + regs::NV_PFALCON_FALCON_IMEMD::default()
>> + .set_data(u32::from_le_bytes(w))
>> + .write(bar, &E::ID, port);
>> + }
>> + tag += 1;
>> + }
>> + }
>> + FalconMem::Dmem => {
>> + regs::NV_PFALCON_FALCON_DMEMC::default()
>> + .set_aincw(true)
>> + .set_offs(mem_base)
>> + .write(bar, &E::ID, port);
>> +
>> + for block in img.chunks(256) {
>> + for word in block.chunks_exact(4) {
>> + let w = [word[0], word[1], word[2], word[3]];
>> + regs::NV_PFALCON_FALCON_DMEMD::default()
>> + .set_data(u32::from_le_bytes(w))
>> + .write(bar, &E::ID, port);
>> + }
>> + }
>> + }
>> + }
>> +
>> + Ok(())
>> + }
>> +
>> + fn pio_wr<F: FalconFirmware<Target = E>>(
>> + &self,
>> + bar: &Bar0,
>> + fw: &F,
>> + target_mem: FalconMem,
>> + load_offsets: &FalconLoadTarget,
>> + port: u8,
>> + tag: u16,
>> + ) -> Result {
>> + let start = usize::from_safe_cast(load_offsets.src_start);
>> + let len = usize::from_safe_cast(load_offsets.len);
>> + let mem_base = u16::try_from(load_offsets.dst_start)?;
>> +
>> + // SAFETY: we are the only user of the firmware image at this stage
>> + let data = unsafe { fw.as_slice(start, len).map_err(|_| EINVAL)? };
>
> Why do we need the firmware image to be backed by a DMA object at this point
> when you load the firmware image through PIO anyways?
When we request the firmware, we don't know whether it is going to be
loaded via DMA or PIO. DMA does require a DMA object though, so the safe
route is to always create one and access its slice if it turns out we
will be doing PIO.
Another way would be to put the firmware into a vector and create a DMA
object on-demand in `dma_load`, but that's a more important refactoring
that we can probably keep for after this patchset should we want to do
it.
>> +}
>> +
>> /// The FWSEC microcode, extracted from the BIOS and to be run on the GSP falcon.
>> ///
>> /// It is responsible for e.g. carving out the WPR2 region as the first step of the GSP bootflow.
>> @@ -221,6 +286,8 @@ pub(crate) struct FwsecFirmware {
>> desc: FalconUCodeDesc,
>> /// GPU-accessible DMA object containing the firmware.
>> ucode: FirmwareDmaObject<Self, Signed>,
>> + /// Generic bootloader
>> + gen_bootloader: Option<GenericBootloader>,
>
> I'm not convinced this is a good idea. We probably want a HAL here and have
> different FwsecFirmware types:
>
> One with a DMA object and one with a system memory object when the architecture
> uses PIO. In the latter case the object can have a GenericBootloader field, i.e.
> this also gets us rid of the Option and all the subsequent 'if chipset <
> Chipset::GA102' checks and 'match gbl_fw' matches below.
Yeah that's basically what I have in mind as well - the FW type using
the generic bootloader could wrap the regular FwsecFirmware as its
purpose is simply to load it. I'm trying to put together some code to
that effect but it looks like it would work and would also remove the
special-case code in the PIO loader.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 11/11] gpu: nova-core: add PIO support for loading firmware images
2026-01-16 21:05 ` Danilo Krummrich
2026-01-17 1:55 ` Alexandre Courbot
@ 2026-01-21 0:25 ` Timur Tabi
1 sibling, 0 replies; 29+ messages in thread
From: Timur Tabi @ 2026-01-21 0:25 UTC (permalink / raw)
To: dakr@kernel.org
Cc: nouveau@lists.freedesktop.org, Alexandre Courbot, Joel Fernandes,
John Hubbard, rust-for-linux@vger.kernel.org
On Fri, 2026-01-16 at 22:05 +0100, Danilo Krummrich wrote:
> > + fn pio_wr_bytes(
> > + &self,
> > + bar: &Bar0,
> > + img: &[u8],
> > + mem_base: u16,
> > + target_mem: FalconMem,
> > + port: u8,
>
> This looks like we should use the Bounded type instead.
I would rather just hard-code the port to 0 since that's the only value we ever use, and we always
pass it as a constant anyway. This looks silly to me:
self.pio_wr_bytes(bar, data, dst_start, FalconMem::ImemNonSecure, Bounded::<u8, 2>::new::<0>(),
tag)?;
> for (n, block) in img.chunks(256).iter().enumerate() {
> regs::NV_PFALCON_FALCON_IMEMT::default()
> .set_tag(tag + n)
> .write(bar, &E::ID, port);
> }
>
> This way you don't need the mutable shadow of tag. Besides that, how do we know
> this doesn't overflow? Don't we need a checked_add()?
Ok.
> > + fn pio_wr<F: FalconFirmware<Target = E>>(
> > + &self,
> > + bar: &Bar0,
> > + fw: &F,
> > + target_mem: FalconMem,
> > + load_offsets: &FalconLoadTarget,
> > + port: u8,
> > + tag: u16,
> > + ) -> Result {
> > + let start = usize::from_safe_cast(load_offsets.src_start);
> > + let len = usize::from_safe_cast(load_offsets.len);
> > + let mem_base = u16::try_from(load_offsets.dst_start)?;
> > +
> > + // SAFETY: we are the only user of the firmware image at this stage
> > + let data = unsafe { fw.as_slice(start, len).map_err(|_| EINVAL)? };
>
> Why do we need the firmware image to be backed by a DMA object at this point
> when you load the firmware image through PIO anyways?
I'm not sure I understand. The firmware images are always loaded into DMA buffers. I don't control
that.
pio_wr() is just supposed to be the PIO equivalent to dma_wr(). It's purpose is to load FW images.
I'll add a function comment.
> > diff --git a/drivers/gpu/nova-core/falcon/hal.rs b/drivers/gpu/nova-core/falcon/hal.rs
> > index 49501aa6ff7f..3a882b7d8aa8 100644
> > --- a/drivers/gpu/nova-core/falcon/hal.rs
> > +++ b/drivers/gpu/nova-core/falcon/hal.rs
> > @@ -15,6 +15,11 @@
> > mod ga102;
> > mod tu102;
> >
> > +pub(crate) enum LoadMethod {
> > + Pio,
> > + Dma,
> > +}
>
> Seems obvious, but still, please add some documentation explaining that this is
> the load method for falcon firmware images, etc.
Ok.
>
> > +pub(crate) struct GenericBootloader {
> > + pub desc: BootloaderDesc,
> > + pub ucode: Vec<u8, kernel::alloc::allocator::Kmalloc>,
>
> pub ucode: KVec<u8>,
>
> Also, we may want to use KVVec<u8> or just VVec<u8> instead. What's the image
> size?
The Generic Bootloader is tiny, less than a KB and hasn't changed since 2016.
-rw-r--r-- 1 root root 816 Jan 14 20:47 gen_bootloader-570.144.bin
> > /// The FWSEC microcode, extracted from the BIOS and to be run on the GSP falcon.
> > ///
> > /// It is responsible for e.g. carving out the WPR2 region as the first step of the GSP
> > bootflow.
> > @@ -221,6 +286,8 @@ pub(crate) struct FwsecFirmware {
> > desc: FalconUCodeDesc,
> > /// GPU-accessible DMA object containing the firmware.
> > ucode: FirmwareDmaObject<Self, Signed>,
> > + /// Generic bootloader
> > + gen_bootloader: Option<GenericBootloader>,
>
> I'm not convinced this is a good idea. We probably want a HAL here and have
> different FwsecFirmware types:
>
> One with a DMA object and one with a system memory object when the architecture
> uses PIO. In the latter case the object can have a GenericBootloader field, i.e.
> this also gets us rid of the Option and all the subsequent 'if chipset <
> Chipset::GA102' checks and 'match gbl_fw' matches below.
Please give me more details as to what you're thinking. I know Alex has plans for this code as
well, and I really don't want to try to implement something complicated, just to have it conflict
with his designs.
It also feels like the kind of change that could be made in a follow-up patchset.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 02/11] gpu: nova-core: add ImemNonSecure section infrastructure
2026-01-14 19:29 ` [PATCH v6 02/11] gpu: nova-core: add ImemNonSecure section infrastructure Timur Tabi
@ 2026-01-22 12:52 ` Gary Guo
2026-01-22 19:00 ` Timur Tabi
0 siblings, 1 reply; 29+ messages in thread
From: Gary Guo @ 2026-01-22 12:52 UTC (permalink / raw)
To: Timur Tabi, Alexandre Courbot, John Hubbard, Joel Fernandes,
Danilo Krummrich, nouveau, rust-for-linux
On Wed Jan 14, 2026 at 7:29 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 <ttabi@nvidia.com>
> Reviewed-by: John Hubbard <jhubbard@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 310d4e75bad3..60713fecffc9 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -242,6 +242,8 @@ fn from(value: PeregrineCoreSelect) -> Self {
> pub(crate) enum FalconMem {
> /// Secure Instruction Memory.
> ImemSecure,
> + /// Non-Secure Instruction Memory.
> + ImemNonSecure,
> /// Data Memory.
> Dmem,
> }
> @@ -351,6 +353,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 +466,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 +525,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 +554,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);
> + }
This error path should never be exercised would it?
dma_load should never be called in the first place but pio_load should be called
instead when the platform does not support DMA?
Best,
Gary
> +
> self.dma_reset(bar);
> regs::NV_PFALCON_FBIF_TRANSCFG::update(bar, &E::ID, 0, |v| {
> v.set_target(FalconFbifTarget::CoherentSysmem)
> diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
> index 096cd01dbc9d..1b98bb47424c 100644
> --- a/drivers/gpu/nova-core/firmware/booter.rs
> +++ b/drivers/gpu/nova-core/firmware/booter.rs
> @@ -253,6 +253,9 @@ impl<'a> FirmwareSignature<BooterFirmware> for BooterSignature<'a> {}
> pub(crate) struct BooterFirmware {
> // Load parameters for Secure `IMEM` falcon memory.
> imem_sec_load_target: FalconLoadTarget,
> + // Load parameters for Non-Secure `IMEM` falcon memory,
> + // used only on Turing and GA100
> + imem_ns_load_target: Option<FalconLoadTarget>,
> // Load parameters for `DMEM` falcon memory.
> dmem_load_target: FalconLoadTarget,
> // BROM falcon parameters.
> @@ -359,6 +362,8 @@ pub(crate) fn new(
> dst_start: 0,
> len: app0.len,
> },
> + // Exists only in the booter image for Turing and GA100
> + imem_ns_load_target: None,
> dmem_load_target: FalconLoadTarget {
> src_start: load_hdr.os_data_offset,
> dst_start: 0,
> @@ -375,6 +380,10 @@ fn imem_sec_load_params(&self) -> FalconLoadTarget {
> self.imem_sec_load_target.clone()
> }
>
> + fn imem_ns_load_params(&self) -> Option<FalconLoadTarget> {
> + self.imem_ns_load_target.clone()
> + }
> +
> fn dmem_load_params(&self) -> FalconLoadTarget {
> self.dmem_load_target.clone()
> }
> diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
> index 6a2f5a0d4b15..e4009faba6c5 100644
> --- a/drivers/gpu/nova-core/firmware/fwsec.rs
> +++ b/drivers/gpu/nova-core/firmware/fwsec.rs
> @@ -232,6 +232,11 @@ fn imem_sec_load_params(&self) -> FalconLoadTarget {
> }
> }
>
> + fn imem_ns_load_params(&self) -> Option<FalconLoadTarget> {
> + // Only used on Turing and GA100, so return None for now
> + None
> + }
> +
> fn dmem_load_params(&self) -> FalconLoadTarget {
> FalconLoadTarget {
> src_start: self.desc.imem_load_size,
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 02/11] gpu: nova-core: add ImemNonSecure section infrastructure
2026-01-22 12:52 ` Gary Guo
@ 2026-01-22 19:00 ` Timur Tabi
0 siblings, 0 replies; 29+ messages in thread
From: Timur Tabi @ 2026-01-22 19:00 UTC (permalink / raw)
To: gary@garyguo.net, nouveau@lists.freedesktop.org,
Alexandre Courbot, Joel Fernandes, dakr@kernel.org, John Hubbard,
rust-for-linux@vger.kernel.org
On Thu, 2026-01-22 at 12:52 +0000, Gary Guo wrote:
> > /// 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);
> > + }
>
> This error path should never be exercised would it?
>
> dma_load should never be called in the first place but pio_load should be called
> instead when the platform does not support DMA?
Correct. This was changed between v4 and v5. Previously, I had support for nsec images:
+ if let Some(nmem) = fw.imem_ns_load_params() {
+ // This code should never actual get executed, because the Non-Secure
+ // section only exists on firmware used by Turing and GA100, and
+ // those platforms do not use DMA. But we include this code for
+ // consistency.
+ self.dma_wr(bar, fw, FalconMem::ImemNonSecure, nmem, false)?;
+ }
But John said no, so I just changed it to a if-statement to catch coding mistakes.
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2026-01-22 19:00 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-14 19:29 [PATCH v6 00/11] gpu: nova-core: add Turing support Timur Tabi
2026-01-14 19:29 ` [PATCH v6 01/11] gpu: nova-core: rename Imem to ImemSecure Timur Tabi
2026-01-14 19:29 ` [PATCH v6 02/11] gpu: nova-core: add ImemNonSecure section infrastructure Timur Tabi
2026-01-22 12:52 ` Gary Guo
2026-01-22 19:00 ` Timur Tabi
2026-01-14 19:29 ` [PATCH v6 03/11] gpu: nova-core: support header parsing on Turing/GA100 Timur Tabi
2026-01-14 19:29 ` [PATCH v6 04/11] gpu: nova-core: add support for Turing/GA100 fwsignature Timur Tabi
2026-01-14 19:29 ` [PATCH v6 05/11] gpu: nova-core: add NV_PFALCON_FALCON_DMATRFCMD::with_falcon_mem() Timur Tabi
2026-01-14 19:29 ` [PATCH v6 06/11] gpu: nova-core: move some functions into the HAL Timur Tabi
2026-01-16 19:55 ` Danilo Krummrich
2026-01-16 20:08 ` John Hubbard
2026-01-16 20:11 ` Danilo Krummrich
2026-01-16 20:15 ` John Hubbard
2026-01-16 20:21 ` Danilo Krummrich
2026-01-16 20:27 ` John Hubbard
2026-01-14 19:29 ` [PATCH v6 07/11] gpu: nova-core: Add basic Turing HAL Timur Tabi
2026-01-14 19:29 ` [PATCH v6 08/11] gpu: nova-core: add Falcon HAL method supports_dma() Timur Tabi
2026-01-16 1:53 ` Alexandre Courbot
2026-01-16 3:00 ` Timur Tabi
2026-01-14 19:29 ` [PATCH v6 09/11] gpu: nova-core: add FalconUCodeDescV2 support Timur Tabi
2026-01-16 3:11 ` Alexandre Courbot
2026-01-16 3:23 ` Alexandre Courbot
2026-01-16 20:09 ` Danilo Krummrich
2026-01-14 19:29 ` [PATCH v6 10/11] gpu: nova-core: align LibosMemoryRegionInitArgument size to page size Timur Tabi
2026-01-14 19:29 ` [PATCH v6 11/11] gpu: nova-core: add PIO support for loading firmware images Timur Tabi
2026-01-16 15:22 ` Alexandre Courbot
2026-01-16 21:05 ` Danilo Krummrich
2026-01-17 1:55 ` Alexandre Courbot
2026-01-21 0:25 ` Timur Tabi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox