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