Skip to contentSkip to conversation

gpu: nova-core: add Turing support

PATCH In review Timur Tabi 6 participants 29 messages Jan 22 – Jan 24

Subsystems (4)
lore
Share & tools
lore

Timur Tabi

author
#0 lore raw

v8 cover gpu: nova-core: add Turing support

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.
9. Load the Generic Bootloader from disk and use it to boot FWSEC on Turing and GA100.

Changes from v7:
1. Fix unused warnings in some commits that introduce an identifier but don't actually use it until a later commit.
2. Split out the Generic Bootloader code from the PIO code into a separate patch.
3. Removed accidentally included vbios.rs.orig

Alexandre Courbot (1):
gpu: nova-core: align LibosMemoryRegionInitArgument size to page size

Timur Tabi (12):
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 NV_PFALCON_FALCON_ENGINE::reset_engine()
gpu: nova-core: add Falcon HAL method load_method()
gpu: nova-core: add FalconUCodeDescV2 support
gpu: nova-core: add PIO support for loading firmware images
gpu: nova-core: use the Generic Bootloader to boot FWSEC

 drivers/gpu/nova-core/falcon.rs           | 252 +++++++++++++++++-----
 drivers/gpu/nova-core/falcon/hal.rs       |  26 +++
 drivers/gpu/nova-core/falcon/hal/ga102.rs |  39 ++++
 drivers/gpu/nova-core/falcon/hal/tu102.rs |  77 +++++++
 drivers/gpu/nova-core/firmware.rs         | 203 ++++++++++++++++-
 drivers/gpu/nova-core/firmware/booter.rs  |  43 +++-
 drivers/gpu/nova-core/firmware/fwsec.rs   | 180 +++++++++++++---
 drivers/gpu/nova-core/firmware/gsp.rs     |   9 +-
 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             |  72 ++++++-
 drivers/gpu/nova-core/vbios.rs            |  64 +++---
 13 files changed, 863 insertions(+), 130 deletions(-)
 create mode 100644 drivers/gpu/nova-core/falcon/hal/tu102.rs

base-commit: 6ea52b6d8f33ae627f4dcf43b12b6e713a8b9331

Signature · 1 line
2.52.0
author
#1 lore raw

PATCH v8 1/13 gpu: nova-core: rename Imem to ImemSecure

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.

---
 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(-)
drivers/gpu/nova-core/falcon.rs +13 -7
index 46b02c8a591e..310d4e75bad3 100644
@@ -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())?;
drivers/gpu/nova-core/firmware/booter.rs +6 -6
index f107f753214a..096cd01dbc9d 100644
@@ -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
     }
 }
 
drivers/gpu/nova-core/firmware/fwsec.rs +1 -1
index b28e34d279f4..6a2f5a0d4b15 100644
@@ -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,
Signature · 1 line
2.52.0
author
#2 lore raw

PATCH v8 2/13 gpu: nova-core: add ImemNonSecure section infrastructure

In reply to Timur Tabi

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.

---
 drivers/gpu/nova-core/falcon.rs          | 19 +++++++++++++++++--
 drivers/gpu/nova-core/firmware/booter.rs |  9 +++++++++
 drivers/gpu/nova-core/firmware/fwsec.rs  |  5 +++++
 3 files changed, 31 insertions(+), 2 deletions(-)
drivers/gpu/nova-core/falcon.rs +17 -2
index 310d4e75bad3..0d1a0f86d83c 100644
@@ -242,6 +242,9 @@ fn from(value: PeregrineCoreSelect) -> Self {
 pub(crate) enum FalconMem {
     /// Secure Instruction Memory.
     ImemSecure,
+    /// Non-Secure Instruction Memory.
+    #[expect(unused)]
+    ImemNonSecure,
     /// Data Memory.
     Dmem,
 }
@@ -351,6 +354,10 @@ pub(crate) trait FalconLoadParams {
     /// Returns the load parameters for Secure `IMEM`.
     fn imem_sec_load_params(&self) -> FalconLoadTarget;
 
+    /// Returns the load parameters for Non-Secure `IMEM`,
+    /// used only on Turing and GA100.
+    fn imem_ns_load_params(&self) -> Option<FalconLoadTarget>;
+
     /// Returns the load parameters for `DMEM`.
     fn dmem_load_params(&self) -> FalconLoadTarget;
 
@@ -460,7 +467,9 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
         //
         // For DMEM we can fold the start offset into the DMA handle.
         let (src_start, dma_start) = match target_mem {
-            FalconMem::ImemSecure => (load_offsets.src_start, fw.dma_handle()),
+            FalconMem::ImemSecure | FalconMem::ImemNonSecure => {
+                (load_offsets.src_start, fw.dma_handle())
+            }
             FalconMem::Dmem => (
                 0,
                 fw.dma_handle_with_offset(load_offsets.src_start.into_safe_cast())?,
@@ -517,7 +526,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
 
         let cmd = regs::NV_PFALCON_FALCON_DMATRFCMD::default()
             .set_size(DmaTrfCmdSize::Size256B)
-            .set_imem(target_mem == FalconMem::ImemSecure)
+            .set_imem(target_mem != FalconMem::Dmem)
             .set_sec(if sec { 1 } else { 0 });
 
         for pos in (0..num_transfers).map(|i| i * DMA_LEN) {
@@ -546,6 +555,12 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
 
     /// Perform a DMA load into `IMEM` and `DMEM` of `fw`, and prepare the falcon to run it.
     pub(crate) fn dma_load<F: FalconFirmware<Target = E>>(&self, bar: &Bar0, fw: &F) -> Result {
+        // The Non-Secure section only exists on firmware used by Turing and GA100, and
+        // those platforms do not use DMA.
+        if fw.imem_ns_load_params().is_some() {
+            return Err(EINVAL);
+        }
+
         self.dma_reset(bar);
         regs::NV_PFALCON_FBIF_TRANSCFG::update(bar, &E::ID, 0, |v| {
             v.set_target(FalconFbifTarget::CoherentSysmem)
drivers/gpu/nova-core/firmware/booter.rs +9 -0
index 096cd01dbc9d..1b98bb47424c 100644
@@ -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()
     }
drivers/gpu/nova-core/firmware/fwsec.rs +5 -0
index 6a2f5a0d4b15..e4009faba6c5 100644
@@ -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,
Signature · 1 line
2.52.0
reviewer
#3 lore raw

Re: [PATCH v8 02/13] gpu: nova-core: add ImemNonSecure section infrastructure

Quoted 59 lines · Timur Tabi

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.

---
 drivers/gpu/nova-core/falcon.rs          | 19 +++++++++++++++++--
 drivers/gpu/nova-core/firmware/booter.rs |  9 +++++++++
 drivers/gpu/nova-core/firmware/fwsec.rs  |  5 +++++
 3 files changed, 31 insertions(+), 2 deletions(-)
drivers/gpu/nova-core/falcon.rs +11 -2
index 310d4e75bad3..0d1a0f86d83c 100644
@@ -242,6 +242,9 @@ fn from(value: PeregrineCoreSelect) -> Self {
 pub(crate) enum FalconMem {
     /// Secure Instruction Memory.
     ImemSecure,
+    /// Non-Secure Instruction Memory.
+    #[expect(unused)]
+    ImemNonSecure,
     /// Data Memory.
     Dmem,
 }
@@ -351,6 +354,10 @@ pub(crate) trait FalconLoadParams {
     /// Returns the load parameters for Secure `IMEM`.
     fn imem_sec_load_params(&self) -> FalconLoadTarget;
 
+    /// Returns the load parameters for Non-Secure `IMEM`,
+    /// used only on Turing and GA100.
+    fn imem_ns_load_params(&self) -> Option<FalconLoadTarget>;
+
     /// Returns the load parameters for `DMEM`.
     fn dmem_load_params(&self) -> FalconLoadTarget;
 
@@ -460,7 +467,9 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
         //
         // For DMEM we can fold the start offset into the DMA handle.
         let (src_start, dma_start) = match target_mem {
-            FalconMem::ImemSecure => (load_offsets.src_start, fw.dma_handle()),
+            FalconMem::ImemSecure | FalconMem::ImemNonSecure => {
+                (load_offsets.src_start, fw.dma_handle())
+            }
             FalconMem::Dmem => (
                 0,
                 fw.dma_handle_with_offset(load_offsets.src_start.into_safe_cast())?,
@@ -517,7 +526,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
 
         let cmd = regs::NV_PFALCON_FALCON_DMATRFCMD::default()
             .set_size(DmaTrfCmdSize::Size256B)
-            .set_imem(target_mem == FalconMem::ImemSecure)
+            .set_imem(target_mem != FalconMem::Dmem)
             .set_sec(if sec { 1 } else { 0 });
 
         for pos in (0..num_transfers).map(|i| i * DMA_LEN) {

drivers/gpu/nova-core/falcon.rs fn dma_wr<F: FalconFirmware<Target = E>>( · line 562

Quoted 8 lines
drivers/gpu/nova-core/falcon.rs +5 -0
@@ -546,6 +555,12 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
 
     /// Perform a DMA load into `IMEM` and `DMEM` of `fw`, and prepare the falcon to run it.
     pub(crate) fn dma_load<F: FalconFirmware<Target = E>>(&self, bar: &Bar0, fw: &F) -> Result {
+        // The Non-Secure section only exists on firmware used by Turing and GA100, and
+        // those platforms do not use DMA.
+        if fw.imem_ns_load_params().is_some() {
+            return Err(EINVAL);
+        }

As pointed out in last round of review this is unreachable code.

I am not too against in leaving it in as is, but for checks that assist debugging only, I would probably use debug_assert myself.

Best,
Gary

Quoted 54 lines
drivers/gpu/nova-core/falcon.rs +1 -0
+
         self.dma_reset(bar);
         regs::NV_PFALCON_FBIF_TRANSCFG::update(bar, &E::ID, 0, |v| {
             v.set_target(FalconFbifTarget::CoherentSysmem)
drivers/gpu/nova-core/firmware/booter.rs +9 -0
index 096cd01dbc9d..1b98bb47424c 100644
@@ -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()
     }
drivers/gpu/nova-core/firmware/fwsec.rs +5 -0
index 6a2f5a0d4b15..e4009faba6c5 100644
@@ -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,
#4 lore raw

Re: [PATCH v8 02/13] gpu: nova-core: add ImemNonSecure section infrastructure

Quoted 71 lines · Gary Guo
Quoted 59 lines · Timur Tabi

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.

---
 drivers/gpu/nova-core/falcon.rs          | 19 +++++++++++++++++--
 drivers/gpu/nova-core/firmware/booter.rs |  9 +++++++++
 drivers/gpu/nova-core/firmware/fwsec.rs  |  5 +++++
 3 files changed, 31 insertions(+), 2 deletions(-)
drivers/gpu/nova-core/falcon.rs +11 -2
index 310d4e75bad3..0d1a0f86d83c 100644
@@ -242,6 +242,9 @@ fn from(value: PeregrineCoreSelect) -> Self {
 pub(crate) enum FalconMem {
     /// Secure Instruction Memory.
     ImemSecure,
+    /// Non-Secure Instruction Memory.
+    #[expect(unused)]
+    ImemNonSecure,
     /// Data Memory.
     Dmem,
 }
@@ -351,6 +354,10 @@ pub(crate) trait FalconLoadParams {
     /// Returns the load parameters for Secure `IMEM`.
     fn imem_sec_load_params(&self) -> FalconLoadTarget;
 
+    /// Returns the load parameters for Non-Secure `IMEM`,
+    /// used only on Turing and GA100.
+    fn imem_ns_load_params(&self) -> Option<FalconLoadTarget>;
+
     /// Returns the load parameters for `DMEM`.
     fn dmem_load_params(&self) -> FalconLoadTarget;
 
@@ -460,7 +467,9 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
         //
         // For DMEM we can fold the start offset into the DMA handle.
         let (src_start, dma_start) = match target_mem {
-            FalconMem::ImemSecure => (load_offsets.src_start, fw.dma_handle()),
+            FalconMem::ImemSecure | FalconMem::ImemNonSecure => {
+                (load_offsets.src_start, fw.dma_handle())
+            }
             FalconMem::Dmem => (
                 0,
                 fw.dma_handle_with_offset(load_offsets.src_start.into_safe_cast())?,
@@ -517,7 +526,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
 
         let cmd = regs::NV_PFALCON_FALCON_DMATRFCMD::default()
             .set_size(DmaTrfCmdSize::Size256B)
-            .set_imem(target_mem == FalconMem::ImemSecure)
+            .set_imem(target_mem != FalconMem::Dmem)
             .set_sec(if sec { 1 } else { 0 });
 
         for pos in (0..num_transfers).map(|i| i * DMA_LEN) {

drivers/gpu/nova-core/falcon.rs fn dma_wr<F: FalconFirmware<Target = E>>( · line 562

Quoted 8 lines
drivers/gpu/nova-core/falcon.rs +5 -0
@@ -546,6 +555,12 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
 
     /// Perform a DMA load into `IMEM` and `DMEM` of `fw`, and prepare the falcon to run it.
     pub(crate) fn dma_load<F: FalconFirmware<Target = E>>(&self, bar: &Bar0, fw: &F) -> Result {
+        // The Non-Secure section only exists on firmware used by Turing and GA100, and
+        // those platforms do not use DMA.
+        if fw.imem_ns_load_params().is_some() {
+            return Err(EINVAL);
+        }

As pointed out in last round of review this is unreachable code.

Quoted 2 lines

I am not too against in leaving it in as is, but for checks that assist debugging only, I would probably use debug_assert myself.

I expect this bit of code to go away when we address the remaining issues with the last 2 patches, but let me add a debug_assert for now.

author
#5 lore raw

PATCH v8 3/13 gpu: nova-core: support header parsing on Turing/GA100

In reply to Timur Tabi

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.

---
 drivers/gpu/nova-core/firmware/booter.rs | 28 ++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)
drivers/gpu/nova-core/firmware/booter.rs +24 -4
index 1b98bb47424c..86556cee8e67 100644
@@ -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
+        }
     }
 }
 
Signature · 1 line
2.52.0
author
#6 lore raw

PATCH v8 4/13 gpu: nova-core: add support for Turing/GA100 fwsignature

In reply to Timur Tabi

Turing and GA100 share the same GSP-RM firmware binary, but the signature ELF section is labeled either ".fwsignature_tu10x" or ".fwsignature_tu11x".

---
 drivers/gpu/nova-core/firmware/gsp.rs | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/gpu/nova-core/firmware/gsp.rs +8 -1
index 1025b7f746eb..beabae9a1189 100644
@@ -214,9 +214,16 @@ pub(crate) fn new<'a>(
                 size,
                 signatures: {
                     let sigs_section = match chipset.arch() {
+                        Architecture::Turing
+                            if matches!(chipset, Chipset::TU116 | 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)
Signature · 1 line
2.52.0
author
#7 lore raw

PATCH v8 5/13 gpu: nova-core: add NV_PFALCON_FALCON_DMATRFCMD::with_falcon_mem()

In reply to Timur Tabi

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.

---
 drivers/gpu/nova-core/falcon.rs | 14 +++-----------
 drivers/gpu/nova-core/regs.rs   |  9 +++++++++
 2 files changed, 12 insertions(+), 11 deletions(-)
drivers/gpu/nova-core/falcon.rs +3 -11
index 0d1a0f86d83c..3629422e7a52 100644
@@ -458,7 +458,6 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
         fw: &F,
         target_mem: FalconMem,
         load_offsets: FalconLoadTarget,
-        sec: bool,
     ) -> Result {
         const DMA_LEN: u32 = 256;
 
@@ -526,8 +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::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`.
@@ -567,14 +565,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())?;
 
drivers/gpu/nova-core/regs.rs +9 -0
index 82cc6c0790e5..b8ddfe2e5ae7 100644
@@ -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;
 });
Signature · 1 line
2.52.0
author
#8 lore raw

PATCH v8 6/13 gpu: nova-core: move some functions into the HAL

In reply to Timur Tabi

A few Falcon methods are actually GPU-specific, so move them into the HAL.

---
 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(-)
drivers/gpu/nova-core/falcon.rs +3 -42
index 3629422e7a52..344354ed50b8 100644
@@ -16,7 +16,6 @@
     prelude::*,
     sync::aref::ARef,
     time::{
-        delay::fsleep,
         Delta, //
     },
 };
@@ -398,48 +397,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())
@@ -673,8 +635,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.
drivers/gpu/nova-core/falcon/hal.rs +10 -0
index 8dc56a28ad65..c77a1568ea96 100644
@@ -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`.
drivers/gpu/nova-core/falcon/hal/ga102.rs +41 -0

51 lines not on this page · raw message

Signature · 1 line
2.52.0
author
#9 lore raw

PATCH v8 7/13 gpu: nova-core: Add basic Turing HAL

In reply to Timur Tabi

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.

---
 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
drivers/gpu/nova-core/falcon/hal.rs +4 -0
index c77a1568ea96..c886ba03d1f6 100644
@@ -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>>
         }
drivers/gpu/nova-core/falcon/hal/tu102.rs new file +79 -0

79 lines not on this page · raw message

drivers/gpu/nova-core/regs.rs +14 -0
index b8ddfe2e5ae7..cd7b7aa6fc2a 100644
@@ -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;
Signature · 1 line
2.52.0
author
#10 lore raw

PATCH v8 8/13 gpu: nova-core: add NV_PFALCON_FALCON_ENGINE::reset_engine()

In reply to Timur Tabi

Add a method for the NV_PFALCON_FALCON_ENGINE register that reset the Falcon, and update the reset_eng() HAL functions to use it.

---
 drivers/gpu/nova-core/falcon/hal/ga102.rs |  9 +--------
 drivers/gpu/nova-core/falcon/hal/tu102.rs |  9 +--------
 drivers/gpu/nova-core/regs.rs             | 19 ++++++++++++++++++-
 3 files changed, 20 insertions(+), 17 deletions(-)
drivers/gpu/nova-core/falcon/hal/ga102.rs +1 -8
index 61cc3d261196..39863813a2bf 100644
@@ -6,7 +6,6 @@
     device,
     io::poll::read_poll_timeout,
     prelude::*,
-    time::delay::fsleep,
     time::Delta, //
 };
 
@@ -147,13 +146,7 @@ fn reset_eng(&self, bar: &Bar0) -> Result {
             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));
-
+        regs::NV_PFALCON_FALCON_ENGINE::reset_engine::<E>(bar);
         self.reset_wait_mem_scrubbing(bar)?;
 
         Ok(())
drivers/gpu/nova-core/falcon/hal/tu102.rs +1 -8
index 586d5dc6b417..23fbf6110572 100644
@@ -5,7 +5,6 @@
 use kernel::{
     io::poll::read_poll_timeout,
     prelude::*,
-    time::delay::fsleep,
     time::Delta, //
 };
 
@@ -65,13 +64,7 @@ fn reset_wait_mem_scrubbing(&self, bar: &Bar0) -> Result {
     }
 
     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));
-
+        regs::NV_PFALCON_FALCON_ENGINE::reset_engine::<E>(bar);
         self.reset_wait_mem_scrubbing(bar)?;
 
         Ok(())
drivers/gpu/nova-core/regs.rs +18 -1
index cd7b7aa6fc2a..ea0d32f5396c 100644
@@ -7,13 +7,18 @@
 #[macro_use]
 pub(crate) mod macros;
 
-use kernel::prelude::*;
+use kernel::{
+    prelude::*,
+    time, //
+};
 
 use crate::{
+    driver::Bar0,
     falcon::{
         DmaTrfCmdSize,
         FalconCoreRev,
         FalconCoreRevSubversion,
+        FalconEngine,
         FalconFbifMemType,
         FalconFbifTarget,
         FalconMem,
@@ -365,6 +370,18 @@ pub(crate) fn with_falcon_mem(self, mem: FalconMem) -> Self {
     0:0     reset as bool;
 });
 
+impl NV_PFALCON_FALCON_ENGINE {
+    /// Resets the falcon
+    pub(crate) fn reset_engine<E: FalconEngine>(bar: &Bar0) {
+        Self::read(bar, &E::ID).set_reset(true).write(bar, &E::ID);
+
+        // TIMEOUT: falcon engine should not take more than 10us to reset.
+        time::delay::fsleep(time::Delta::from_micros(10));
+
+        Self::read(bar, &E::ID).set_reset(false).write(bar, &E::ID);
+    }
+}
+
 register!(NV_PFALCON_FBIF_TRANSCFG @ PFalconBase[0x00000600[8]] {
     1:0     target as u8 ?=> FalconFbifTarget;
     2:2     mem_type as bool => FalconFbifMemType;
Signature · 1 line
2.52.0
author
#11 lore raw

PATCH v8 9/13 gpu: nova-core: add Falcon HAL method load_method()

In reply to Timur Tabi

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.

---
 drivers/gpu/nova-core/falcon/hal.rs       | 14 ++++++++++++++
 drivers/gpu/nova-core/falcon/hal/ga102.rs |  5 +++++
 drivers/gpu/nova-core/falcon/hal/tu102.rs |  5 +++++
 3 files changed, 24 insertions(+)
drivers/gpu/nova-core/falcon/hal.rs +14 -0
index c886ba03d1f6..fe6de900e8b0 100644
@@ -15,6 +15,16 @@
 mod ga102;
 mod tu102;
 
+/// Method used to load data into falcon memory. Some GPU architectures need
+/// PIO and others can use DMA.
+#[expect(unused)]
+pub(crate) enum LoadMethod {
+    /// Programmed I/O
+    Pio,
+    /// Direct Memory Access
+    Dma,
+}
+
 /// Hardware Abstraction Layer for Falcon cores.
 ///
 /// Implements chipset-specific low-level operations. The trait is generic against [`FalconEngine`]
@@ -48,6 +58,10 @@ fn signature_reg_fuse_version(
 
     /// Reset the falcon engine.
     fn reset_eng(&self, bar: &Bar0) -> Result;
+
+    /// returns the method needed to load data into Falcon memory
+    #[expect(unused)]
+    fn load_method(&self) -> LoadMethod;
 }
 
 /// Returns a boxed falcon HAL adequate for `chipset`.
drivers/gpu/nova-core/falcon/hal/ga102.rs +5 -0
index 39863813a2bf..8f62df10da0a 100644
@@ -12,6 +12,7 @@
 use crate::{
     driver::Bar0,
     falcon::{
+        hal::LoadMethod,
         Falcon,
         FalconBromParams,
         FalconEngine,
@@ -151,4 +152,8 @@ fn reset_eng(&self, bar: &Bar0) -> Result {
 
         Ok(())
     }
+
+    fn load_method(&self) -> LoadMethod {
+        LoadMethod::Dma
+    }
 }
drivers/gpu/nova-core/falcon/hal/tu102.rs +5 -0
index 23fbf6110572..7de6f24cc0a0 100644
@@ -11,6 +11,7 @@
 use crate::{
     driver::Bar0,
     falcon::{
+        hal::LoadMethod,
         Falcon,
         FalconBromParams,
         FalconEngine, //
@@ -69,4 +70,8 @@ fn reset_eng(&self, bar: &Bar0) -> Result {
 
         Ok(())
     }
+
+    fn load_method(&self) -> LoadMethod {
+        LoadMethod::Pio
+    }
 }
Signature · 1 line
2.52.0
#12 lore raw

Re: [PATCH v8 09/13] gpu: nova-core: add Falcon HAL method load_method()

Quoted 6 lines · Timur Tabi

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.

rustc 1.78 didn't like the unused items, so I have picked a small chunk of code from patch 12 to exercise the HAL.

Also made load_dma private as I've seen this hasn't been addressed.

author
#13 lore raw

Re: [PATCH v8 09/13] gpu: nova-core: add Falcon HAL method load_method()

Quoted 2 lines · Alexandre Courbot

rustc 1.78 didn't like the unused items, so I have picked a small chunk of code from patch 12 to exercise the HAL.

Should I downgrade my compiler? I ran everything through clippy, rustfmt, and checkpatch multiple times.

Quoted 1 line

Also made load_dma private as I've seen this hasn't been addressed.

Sorry about that. There was so many back-and-forth review comments, I'm not surprised that I missed something.

#14 lore raw

Re: [PATCH v8 09/13] gpu: nova-core: add Falcon HAL method load_method()

Quoted 3 lines · Timur Tabi

Should I downgrade my compiler? I ran everything through clippy, rustfmt, and checkpatch multiple times.

We support a range of compilers, so for complex changes / new code, please aim to build with at least the minimum supported by the kernel (currently 1.78, in the future 1.85) and the latest upstream stable (currently 1.93) before submitting the patches:

https://rust-for-linux.com/contributing#submit-checklist-addendum

There are developers and CIs using different versions, so eventually it will be caught by someone (likely the maintainer), but it can save time to do it early.

Thanks!

Cheers,
Miguel

maintainer
#15 lore raw

Re: [PATCH v8 09/13] gpu: nova-core: add Falcon HAL method load_method()

Quoted 3 lines · Miguel Ojeda

There are developers and CIs using different versions, so eventually it will be caught by someone (likely the maintainer), but it can save time to do it early.

For patches that target the drm-rust tree, we have committer guidelines with a submit-checklist [1] (that eventually points to both, the generic kernel one and the Rust one linked by Miguel).

However, it also lists the most important steps that should be taken before submitting a patch series.

[1] https://drm.pages.freedesktop.org/maintainer-tools/committer/committer-drm-rust.html#submit-checklist

#16 lore raw

Re: [PATCH v8 09/13] gpu: nova-core: add Falcon HAL method load_method()

In reply to Timur Tabi
Quoted 6 lines · Timur Tabi
Quoted 2 lines · Alexandre Courbot

rustc 1.78 didn't like the unused items, so I have picked a small chunk of code from patch 12 to exercise the HAL.

Should I downgrade my compiler? I ran everything through clippy, rustfmt, and checkpatch multiple times.

Thankfully several rustc versions can coexist and it is easy to switch between them.

To install 1.78:

  $ rustup toolchain add 1.78

To switch to 1.78 (in the kernel tree):

  $ rustup override set 1.78

(you can then check that the correct version is used with rustc --version)

To switch back to the stable compiler:

  $ rustup override set stable

Having said that, checking against the MSRV is something Danilo and I will always do before pushing, and problems should only arise on rare occasions - so that's the kind of stuff I don't mind taking care of if it is too much of a burden.

author
#17 lore raw

PATCH v8 10/13 gpu: nova-core: add FalconUCodeDescV2 support

In reply to Timur Tabi

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.

---
 drivers/gpu/nova-core/firmware.rs       | 203 +++++++++++++++++++++++-
 drivers/gpu/nova-core/firmware/fwsec.rs |  46 +++---
 drivers/gpu/nova-core/vbios.rs          |  64 ++++----
 3 files changed, 249 insertions(+), 64 deletions(-)
drivers/gpu/nova-core/firmware.rs +199 -4
index 2d2008b33fb4..68779540aa28 100644
@@ -4,6 +4,7 @@
 //! to be loaded into a given execution unit.
 
 use core::marker::PhantomData;
+use core::ops::Deref;
 
 use kernel::{
     device,
@@ -15,7 +16,10 @@
 
 use crate::{
     dma::DmaObject,
-    falcon::FalconFirmware,
+    falcon::{
+        FalconFirmware,
+        FalconLoadTarget, //
+    },
     gpu,
     num::{
         FromSafeCast,
@@ -43,6 +47,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 +120,164 @@ 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()
+    }
 
-        ((self.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 {
+    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
+    }
+
+    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 {
+    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
+    }
+
+    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,
+        }
     }
 }
 
drivers/gpu/nova-core/firmware/fwsec.rs +19 -27

113 lines not on this page · raw message

drivers/gpu/nova-core/vbios.rs +31 -33

100 lines not on this page · raw message

Signature · 1 line
2.52.0
author
#18 lore raw

PATCH v8 11/13 gpu: nova-core: align LibosMemoryRegionInitArgument size to page size

In reply to Timur Tabi

From: Alexandre Courbot <[email protected]>

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.

---
 drivers/gpu/nova-core/gsp.rs    |  8 ++++----
 drivers/gpu/nova-core/gsp/fw.rs | 14 +++++++++++++-
 2 files changed, 17 insertions(+), 5 deletions(-)
drivers/gpu/nova-core/gsp.rs +4 -4
index 766fd9905358..bcf6ce18a4a1 100644
@@ -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))?;
                 },
             }))
drivers/gpu/nova-core/gsp/fw.rs +13 -1
index caeb0d251fe5..89be26b597d1 100644
@@ -904,9 +904,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)]
Signature · 1 line
2.52.0
reviewer
#19 lore raw

Re: [PATCH v8 11/13] gpu: nova-core: align LibosMemoryRegionInitArgument size to page size

Quoted 60 lines · Timur Tabi

From: Alexandre Courbot <[email protected]>

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.

---
 drivers/gpu/nova-core/gsp.rs    |  8 ++++----
 drivers/gpu/nova-core/gsp/fw.rs | 14 +++++++++++++-
 2 files changed, 17 insertions(+), 5 deletions(-)
drivers/gpu/nova-core/gsp.rs +4 -4
index 766fd9905358..bcf6ce18a4a1 100644
@@ -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))?;
                 },
             }))
drivers/gpu/nova-core/gsp/fw.rs +0 -0
index caeb0d251fe5..89be26b597d1 100644

drivers/gpu/nova-core/gsp/fw.rs pub(crate) fn new(cmdq: &Cmdq) -> Self { · line 911

Quoted 8 lines
drivers/gpu/nova-core/gsp/fw.rs +5 -0
@@ -904,9 +904,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 {

Hi Timir,

This is not "aligned" to GSP_PAGE_SIZE, but rather just "padded" to GSP_PAGE_SIZE, so calling it GspArgumentsAligned would be a misnomer.

GspArgumentsPadded, perhaps?

Best,
Gary

Quoted 14 lines
drivers/gpu/nova-core/gsp/fw.rs +8 -1
+    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)]
author
#20 lore raw

Re: [PATCH v8 11/13] gpu: nova-core: align LibosMemoryRegionInitArgument size to page size

Quoted 8 lines · Gary Guo

drivers/gpu/nova-core/gsp/fw.rs pub(crate) fn new(cmdq: &Cmdq) -> Self { · line 911

Quoted 6 lines
drivers/gpu/nova-core/gsp/fw.rs +5 -0
+/// 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 {

Hi Timir,

Timur

Quoted 4 lines

This is not "aligned" to GSP_PAGE_SIZE, but rather just "padded" to GSP_PAGE_SIZE, so calling it GspArgumentsAligned would be a misnomer.

GspArgumentsPadded, perhaps?

Well, this is Alex's patch, so if he wants to rename it if/when he pulls it in, that's up to him. This is already v8, I'm not keen on any more bike shedding.

#21 lore raw

Re: [PATCH v8 11/13] gpu: nova-core: align LibosMemoryRegionInitArgument size to page size

Quoted 12 lines · Timur Tabi
Quoted 8 lines · Gary Guo

drivers/gpu/nova-core/gsp/fw.rs pub(crate) fn new(cmdq: &Cmdq) -> Self { · line 911

Quoted 6 lines
drivers/gpu/nova-core/gsp/fw.rs +5 -0
+/// 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 {

Hi Timir,

Timur

Quoted 8 lines
Quoted 4 lines

This is not "aligned" to GSP_PAGE_SIZE, but rather just "padded" to GSP_PAGE_SIZE, so calling it GspArgumentsAligned would be a misnomer.

GspArgumentsPadded, perhaps?

Well, this is Alex's patch, so if he wants to rename it if/when he pulls it in, that's up to him. This is already v8, I'm not keen on any more bike shedding.

To respond to Gary's feedback (as opposed to when and how to apply it), I'd classify this as "polishing", rather than using the pejorative "bike shedding". Because polishing clearly makes something a little bit better, and I think there is no argument that the "padded" name makes things objectively better: more accurate name, not longer, all good.

As for the when and how: yes, it's true that we may defer some improvements, and I agree that we could do that in this case.

And...let's try to avoid (perhaps inadvertently) discouraging our reviewers; we are lucky to have them.

thanks,

Signature · 1 line
John Hubbard
#22 lore raw

Re: [PATCH v8 11/13] gpu: nova-core: align LibosMemoryRegionInitArgument size to page size

In reply to Gary Guo
Quoted 72 lines · Gary Guo
Quoted 60 lines · Timur Tabi

From: Alexandre Courbot <[email protected]>

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.

---
 drivers/gpu/nova-core/gsp.rs    |  8 ++++----
 drivers/gpu/nova-core/gsp/fw.rs | 14 +++++++++++++-
 2 files changed, 17 insertions(+), 5 deletions(-)
drivers/gpu/nova-core/gsp.rs +4 -4
index 766fd9905358..bcf6ce18a4a1 100644
@@ -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))?;
                 },
             }))
drivers/gpu/nova-core/gsp/fw.rs +0 -0
index caeb0d251fe5..89be26b597d1 100644

drivers/gpu/nova-core/gsp/fw.rs pub(crate) fn new(cmdq: &Cmdq) -> Self { · line 911

Quoted 8 lines
drivers/gpu/nova-core/gsp/fw.rs +5 -0
@@ -904,9 +904,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 {

Hi Timir,

Quoted 4 lines

This is not "aligned" to GSP_PAGE_SIZE, but rather just "padded" to GSP_PAGE_SIZE, so calling it GspArgumentsAligned would be a misnomer.

GspArgumentsPadded, perhaps?

Sounds good, let me convert when applying.

author
#23 lore raw

PATCH v8 12/13 gpu: nova-core: add PIO support for loading firmware images

In reply to Timur Tabi

Turing and GA100 use programmed I/O (PIO) instead of DMA to upload firmware images into Falcon memory.

---
 drivers/gpu/nova-core/falcon.rs         | 135 +++++++++++++++++++++++-
 drivers/gpu/nova-core/falcon/hal.rs     |   2 -
 drivers/gpu/nova-core/firmware/fwsec.rs |   2 +-
 drivers/gpu/nova-core/gsp/boot.rs       |   2 +-
 drivers/gpu/nova-core/regs.rs           |  30 ++++++
 5 files changed, 166 insertions(+), 5 deletions(-)
drivers/gpu/nova-core/falcon.rs +134 -1
index 344354ed50b8..4f3a3b002725 100644
@@ -23,6 +23,7 @@
 use crate::{
     dma::DmaObject,
     driver::Bar0,
+    falcon::hal::LoadMethod,
     gpu::Chipset,
     num::{
         FromSafeCast,
@@ -242,7 +243,6 @@ pub(crate) enum FalconMem {
     /// Secure Instruction Memory.
     ImemSecure,
     /// Non-Secure Instruction Memory.
-    #[expect(unused)]
     ImemNonSecure,
     /// Data Memory.
     Dmem,
@@ -410,6 +410,131 @@ pub(crate) fn reset(&self, bar: &Bar0) -> Result {
         Ok(())
     }
 
+    /// Write a slice 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 `start_tag`.
+    /// For DMEM, start_tag is ignored.
+    ///
+    /// Returns `EINVAL` if `img.len()` is not a multiple of 4.
+    fn pio_wr_slice(
+        &self,
+        bar: &Bar0,
+        img: &[u8],
+        mem_base: u16,
+        target_mem: FalconMem,
+        start_tag: u16,
+    ) -> Result {
+        // Rejecting misaligned images here allows us to avoid checking
+        // inside the loops.
+        if img.len() % 4 != 0 {
+            return Err(EINVAL);
+        }
+
+        // NV_PFALCON_FALCON_IMEMC supports up to four ports,
+        // but we only ever use one, so just hard-code it.
+        const PORT: usize = 0;
+
+        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);
+
+                for (n, block) in img.chunks(256).enumerate() {
+                    let n = u16::try_from(n)?;
+                    let tag: u16 = start_tag.checked_add(n).ok_or(ERANGE)?;
+                    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);
+                    }
+                }
+            }
+            FalconMem::Dmem => {
+                regs::NV_PFALCON_FALCON_DMEMC::default()
+                    .set_aincw(true)
+                    .set_offs(mem_base)
+                    .write(bar, &E::ID, PORT);
+
+                for word in img.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(())
+    }
+
+    /// Perform a PIO write of a firmware section to falcon memory.
+    ///
+    /// Extracts the data slice specified by `load_offsets` from `fw` and writes it to
+    /// `target_mem` using the given port and tag.
+    fn pio_wr<F: FalconFirmware<Target = E>>(
+        &self,
+        bar: &Bar0,
+        fw: &F,
+        target_mem: FalconMem,
+        load_offsets: &FalconLoadTarget,
+        start_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_slice(bar, data, mem_base, target_mem, start_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) -> 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);
+
+        self.pio_wr(
+            bar,
+            fw,
+            FalconMem::ImemNonSecure,
+            &imem_ns,
+            u16::try_from(imem_ns.dst_start >> 8)?,
+        )?;
+        self.pio_wr(
+            bar,
+            fw,
+            FalconMem::ImemSecure,
+            &imem_sec,
+            u16::try_from(imem_sec.dst_start >> 8)?,
+        )?;
+        self.pio_wr(bar, fw, FalconMem::Dmem, &dmem, 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`.
     ///
@@ -638,6 +763,14 @@ pub(crate) fn is_riscv_active(&self, bar: &Bar0) -> bool {
         self.hal.is_riscv_active(bar)
     }
 
+    // Load a firmware image into Falcon memory
+    pub(crate) fn load<F: FalconFirmware<Target = E>>(&self, bar: &Bar0, fw: &F) -> Result {
+        match self.hal.load_method() {
+            LoadMethod::Pio => self.pio_load(bar, fw),
+            LoadMethod::Dma => self.dma_load(bar, fw),
+        }
+    }
+
     /// 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()
drivers/gpu/nova-core/falcon/hal.rs +0 -2
index fe6de900e8b0..89babd5f9325 100644
@@ -17,7 +17,6 @@
 
 /// Method used to load data into falcon memory. Some GPU architectures need
 /// PIO and others can use DMA.
-#[expect(unused)]
 pub(crate) enum LoadMethod {
     /// Programmed I/O
     Pio,
@@ -60,7 +59,6 @@ fn signature_reg_fuse_version(
     fn reset_eng(&self, bar: &Bar0) -> Result;
 
     /// returns the method needed to load data into Falcon memory
-    #[expect(unused)]
     fn load_method(&self) -> LoadMethod;
 }
 
drivers/gpu/nova-core/firmware/fwsec.rs +1 -1
index 89dc4526041b..a8ec08a500ac 100644
@@ -420,7 +420,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)
             .inspect_err(|e| dev_err!(dev, "Failed to load FWSEC firmware: {:?}\n", e))?;
         let (mbox0, _) = falcon
             .boot(bar, Some(0), None)
drivers/gpu/nova-core/gsp/boot.rs +1 -1
index 581b412554dc..be427fe26a58 100644
@@ -183,7 +183,7 @@ pub(crate) fn boot(
         );
 
         sec2_falcon.reset(bar)?;
-        sec2_falcon.dma_load(bar, &booter_loader)?;
+        sec2_falcon.load(bar, &booter_loader)?;
         let wpr_handle = wpr_meta.dma_handle();
         let (mbox0, mbox1) = sec2_falcon.boot(
             bar,
drivers/gpu/nova-core/regs.rs +30 -0
index ea0d32f5396c..53f412f0ca32 100644
@@ -364,6 +364,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] {
Signature · 1 line
2.52.0
author
#24 lore raw

PATCH v8 13/13 gpu: nova-core: use the Generic Bootloader to boot FWSEC

In reply to Timur Tabi

On Turing and GA100, a new firmware image called the Generic Bootloader (gen_bootloader, or gbl for short) must be used to load FWSEC into Falcon memory. The driver loads gbl into Falcon IMEM, passes a descriptor that points to FWSEC, and then boots the gbl. The gbl will then load FWSEC into IMEM and boot it.

---
 drivers/gpu/nova-core/falcon.rs         |  85 +++++++++++----
 drivers/gpu/nova-core/firmware/fwsec.rs | 131 +++++++++++++++++++++++-
 drivers/gpu/nova-core/gsp/boot.rs       |   6 +-
 3 files changed, 199 insertions(+), 23 deletions(-)
drivers/gpu/nova-core/falcon.rs +67 -18
index 4f3a3b002725..d18483df2d5e 100644
@@ -18,12 +18,17 @@
     time::{
         Delta, //
     },
+    transmute::AsBytes, //
 };
 
 use crate::{
     dma::DmaObject,
     driver::Bar0,
     falcon::hal::LoadMethod,
+    firmware::fwsec::{
+        BootloaderDmemDescV2,
+        GenericBootloader, //
+    },
     gpu::Chipset,
     num::{
         FromSafeCast,
@@ -498,7 +503,12 @@ fn pio_wr<F: FalconFirmware<Target = E>>(
     }
 
     /// 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) -> Result {
+    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();
@@ -509,21 +519,55 @@ pub(crate) fn pio_load<F: FalconFirmware<Target = E>>(&self, bar: &Bar0, fw: &F)
 
         regs::NV_PFALCON_FALCON_DMACTL::default().write(bar, &E::ID);
 
-        self.pio_wr(
-            bar,
-            fw,
-            FalconMem::ImemNonSecure,
-            &imem_ns,
-            u16::try_from(imem_ns.dst_start >> 8)?,
-        )?;
-        self.pio_wr(
-            bar,
-            fw,
-            FalconMem::ImemSecure,
-            &imem_sec,
-            u16::try_from(imem_sec.dst_start >> 8)?,
-        )?;
-        self.pio_wr(bar, fw, FalconMem::Dmem, &dmem, 0)?;
+        // 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_slice(bar, data, dst_start, FalconMem::ImemNonSecure, 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_slice(bar, dmem_desc.as_bytes(), 0, FalconMem::Dmem, 0)?;
+        } else {
+            self.pio_wr(
+                bar,
+                fw,
+                FalconMem::ImemNonSecure,
+                &imem_ns,
+                u16::try_from(imem_ns.dst_start >> 8)?,
+            )?;
+            self.pio_wr(
+                bar,
+                fw,
+                FalconMem::ImemSecure,
+                &imem_sec,
+                u16::try_from(imem_sec.dst_start >> 8)?,
+            )?;
+            self.pio_wr(bar, fw, FalconMem::Dmem, &dmem, 0)?;
+        }
 
         self.hal.program_brom(self, bar, &fw.brom_params())?;
 
@@ -764,9 +808,14 @@ pub(crate) fn is_riscv_active(&self, bar: &Bar0) -> bool {
     }
 
     // Load a firmware image into Falcon memory
-    pub(crate) fn load<F: FalconFirmware<Target = E>>(&self, bar: &Bar0, fw: &F) -> Result {
+    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),
+            LoadMethod::Pio => self.pio_load(bar, fw, gbl),
             LoadMethod::Dma => self.dma_load(bar, fw),
         }
     }
drivers/gpu/nova-core/firmware/fwsec.rs +128 -3

181 lines not on this page · raw message

drivers/gpu/nova-core/gsp/boot.rs +4 -2
index be427fe26a58..f253d5f12252 100644
@@ -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.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,
Signature · 1 line
2.52.0
maintainer Acked-by on the v8 series
#25 lore raw
In reply to Timur Tabi
Quoted 14 lines · Timur Tabi

Alexandre Courbot (1):
gpu: nova-core: align LibosMemoryRegionInitArgument size to page size

Timur Tabi (12):
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 NV_PFALCON_FALCON_ENGINE::reset_engine()
gpu: nova-core: add Falcon HAL method load_method()
gpu: nova-core: add FalconUCodeDescV2 support

Up until here,

Quoted 2 lines

gpu: nova-core: add PIO support for loading firmware images
gpu: nova-core: use the Generic Bootloader to boot FWSEC

#26 lore raw
Quoted 16 lines · Danilo Krummrich
Quoted 14 lines · Timur Tabi

Alexandre Courbot (1):
gpu: nova-core: align LibosMemoryRegionInitArgument size to page size

Timur Tabi (12):
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 NV_PFALCON_FALCON_ENGINE::reset_engine()
gpu: nova-core: add Falcon HAL method load_method()
gpu: nova-core: add FalconUCodeDescV2 support

Quoted 3 lines

Up until here,

Thanks! Pushed to drm-rust-next after checking Ampere was still probing.

reviewer
#27 lore raw
In reply to Timur Tabi
Quoted 34 lines · Timur Tabi

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.
9. Load the Generic Bootloader from disk and use it to boot FWSEC on Turing and GA100.

Quoted 6 lines

Changes from v7:
1. Fix unused warnings in some commits that introduce an identifier but don't actually use it until a later commit.
2. Split out the Generic Bootloader code from the PIO code into a separate patch.
3. Removed accidentally included vbios.rs.orig

I've checked the Rust language aspect of the series and on the internal API design and it looks good. I haven't checked that the actually hardware operations are functioning as intended, though.

Danilo/Alex, feel free to convert this to an Reviewed-by or Acked-by, whichever you see fit.

Best,
Gary

Quoted 36 lines

Alexandre Courbot (1):
gpu: nova-core: align LibosMemoryRegionInitArgument size to page size

Timur Tabi (12):
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 NV_PFALCON_FALCON_ENGINE::reset_engine()
gpu: nova-core: add Falcon HAL method load_method()
gpu: nova-core: add FalconUCodeDescV2 support
gpu: nova-core: add PIO support for loading firmware images
gpu: nova-core: use the Generic Bootloader to boot FWSEC

 drivers/gpu/nova-core/falcon.rs           | 252 +++++++++++++++++-----
 drivers/gpu/nova-core/falcon/hal.rs       |  26 +++
 drivers/gpu/nova-core/falcon/hal/ga102.rs |  39 ++++
 drivers/gpu/nova-core/falcon/hal/tu102.rs |  77 +++++++
 drivers/gpu/nova-core/firmware.rs         | 203 ++++++++++++++++-
 drivers/gpu/nova-core/firmware/booter.rs  |  43 +++-
 drivers/gpu/nova-core/firmware/fwsec.rs   | 180 +++++++++++++---
 drivers/gpu/nova-core/firmware/gsp.rs     |   9 +-
 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             |  72 ++++++-
 drivers/gpu/nova-core/vbios.rs            |  64 +++---
 13 files changed, 863 insertions(+), 130 deletions(-)
 create mode 100644 drivers/gpu/nova-core/falcon/hal/tu102.rs

base-commit: 6ea52b6d8f33ae627f4dcf43b12b6e713a8b9331

#28 lore raw
Quoted 42 lines · Gary Guo
Quoted 34 lines · Timur Tabi

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.
9. Load the Generic Bootloader from disk and use it to boot FWSEC on Turing and GA100.

Quoted 6 lines

Changes from v7:
1. Fix unused warnings in some commits that introduce an identifier but don't actually use it until a later commit.
2. Split out the Generic Bootloader code from the PIO code into a separate patch.
3. Removed accidentally included vbios.rs.orig

Quoted 6 lines

I've checked the Rust language aspect of the series and on the internal API design and it looks good. I haven't checked that the actually hardware operations are functioning as intended, though.

Danilo/Alex, feel free to convert this to an Reviewed-by or Acked-by, whichever you see fit.

Gratefully adding your Reviewed-by! :) Thanks for the review.