Timur Tabi
2025-Nov-14 23:30 UTC
[PATCH 09/11] gpu: nova-core: add FalconUCodeDescV2 support
The FRTS firmware in Turing and GA100 VBIOS has an older header
format (v2 instead of v3). To support both v2 and v3 at runtime,
add the FalconUCodeDescV2 struct, and update code that references
the FalconUCodeDescV3 directly with a FalconUCodeDesc enum that
encapsulates both.
Signed-off-by: Timur Tabi <ttabi at nvidia.com>
---
drivers/gpu/nova-core/firmware.rs | 108 +++++++++++++++++++++++-
drivers/gpu/nova-core/firmware/fwsec.rs | 72 ++++++++++------
drivers/gpu/nova-core/vbios.rs | 74 ++++++++++------
3 files changed, 202 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/nova-core/firmware.rs
b/drivers/gpu/nova-core/firmware.rs
index 2d2008b33fb4..5ca5bf1fb610 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -43,6 +43,43 @@ 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,
+}
+
/// Structure used to describe some firmwares, notably FWSEC-FRTS.
#[repr(C)]
#[derive(Debug, Clone)]
@@ -76,13 +113,80 @@ pub(crate) struct FalconUCodeDescV3 {
_reserved: u16,
}
-impl FalconUCodeDescV3 {
+#[derive(Debug, Clone)]
+pub(crate) enum FalconUCodeDesc {
+ V2(FalconUCodeDescV2),
+ V3(FalconUCodeDescV3),
+}
+
+impl FalconUCodeDesc {
/// Returns the size in bytes of the header.
pub(crate) fn size(&self) -> usize {
+ let hdr = match self {
+ FalconUCodeDesc::V2(v2) => v2.hdr,
+ FalconUCodeDesc::V3(v3) => v3.hdr,
+ };
+
const HDR_SIZE_SHIFT: u32 = 16;
const HDR_SIZE_MASK: u32 = 0xffff0000;
+ ((hdr & HDR_SIZE_MASK) >> HDR_SIZE_SHIFT).into_safe_cast()
+ }
+
+ pub(crate) fn imem_load_size(&self) -> u32 {
+ match self {
+ FalconUCodeDesc::V2(v2) => v2.imem_load_size,
+ FalconUCodeDesc::V3(v3) => v3.imem_load_size,
+ }
+ }
+
+ pub(crate) fn interface_offset(&self) -> u32 {
+ match self {
+ FalconUCodeDesc::V2(v2) => v2.interface_offset,
+ FalconUCodeDesc::V3(v3) => v3.interface_offset,
+ }
+ }
+
+
+ pub(crate) fn dmem_load_size(&self) -> u32 {
+ match self {
+ FalconUCodeDesc::V2(v2) => v2.dmem_load_size,
+ FalconUCodeDesc::V3(v3) => v3.dmem_load_size,
+ }
+ }
+
+ pub(crate) fn pkc_data_offset(&self) -> u32 {
+ match self {
+ FalconUCodeDesc::V2(_v2) => 0,
+ FalconUCodeDesc::V3(v3) => v3.pkc_data_offset,
+ }
+ }
- ((self.hdr & HDR_SIZE_MASK) >>
HDR_SIZE_SHIFT).into_safe_cast()
+ pub(crate) fn engine_id_mask(&self) -> u16 {
+ match self {
+ FalconUCodeDesc::V2(_v2) => 0,
+ FalconUCodeDesc::V3(v3) => v3.engine_id_mask,
+ }
+ }
+
+ pub(crate) fn ucode_id(&self) -> u8 {
+ match self {
+ FalconUCodeDesc::V2(_v2) => 0,
+ FalconUCodeDesc::V3(v3) => v3.ucode_id,
+ }
+ }
+
+ pub(crate) fn signature_count(&self) -> u8 {
+ match self {
+ FalconUCodeDesc::V2(_v2) => 0,
+ FalconUCodeDesc::V3(v3) => v3.signature_count,
+ }
+ }
+
+ pub(crate) fn signature_versions(&self) -> u16 {
+ match self {
+ FalconUCodeDesc::V2(_v2) => 0,
+ FalconUCodeDesc::V3(v3) => v3.signature_versions,
+ }
}
}
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs
b/drivers/gpu/nova-core/firmware/fwsec.rs
index e4009faba6c5..36ff8ed51c23 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 - v2.imem_sec_size,
+ }),
+ // Not used on V3 platforms
+ FalconUCodeDesc::V3(_v3) => None,
+ }
}
fn dmem_load_params(&self) -> FalconLoadTarget {
- FalconLoadTarget {
- src_start: self.desc.imem_load_size,
- dst_start: self.desc.dmem_phys_base,
- len: self.desc.dmem_load_size,
+ match &self.desc {
+ FalconUCodeDesc::V2(v2) => FalconLoadTarget {
+ src_start: v2.dmem_offset,
+ dst_start: v2.dmem_phys_base,
+ len: v2.dmem_load_size,
+ },
+ FalconUCodeDesc::V3(v3) => FalconLoadTarget {
+ src_start: v3.imem_load_size,
+ dst_start: v3.dmem_phys_base,
+ len: v3.dmem_load_size,
+ }
}
}
fn brom_params(&self) -> FalconBromParams {
FalconBromParams {
- pkc_data_offset: self.desc.pkc_data_offset,
- engine_id_mask: self.desc.engine_id_mask,
- ucode_id: self.desc.ucode_id,
+ pkc_data_offset: self.desc.pkc_data_offset(),
+ engine_id_mask: self.desc.engine_id_mask(),
+ ucode_id: self.desc.ucode_id(),
}
}
@@ -273,10 +294,10 @@ impl FalconFirmware for FwsecFirmware {
impl FirmwareDmaObject<FwsecFirmware, Unsigned> {
fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd:
FwsecCommand) -> Result<Self> {
let desc = bios.fwsec_image().header()?;
- let ucode = bios.fwsec_image().ucode(desc)?;
+ let ucode = bios.fwsec_image().ucode(&desc)?;
let mut dma_object = DmaObject::from_data(dev, ucode)?;
- let hdr_offset = usize::from_safe_cast(desc.imem_load_size +
desc.interface_offset);
+ let hdr_offset = usize::from_safe_cast(desc.imem_load_size() +
desc.interface_offset());
// SAFETY: we have exclusive access to `dma_object`.
let hdr: &FalconAppifHdrV1 = unsafe { transmute(&dma_object,
hdr_offset) }?;
@@ -303,7 +324,7 @@ fn new_fwsec(dev: &Device<device::Bound>, bios:
&Vbios, cmd: FwsecCommand) -> Re
let dmem_mapper: &mut FalconAppifDmemmapperV3 = unsafe {
transmute_mut(
&mut dma_object,
- (desc.imem_load_size + dmem_base).into_safe_cast(),
+ (desc.imem_load_size() + dmem_base).into_safe_cast(),
)
}?;
@@ -317,7 +338,7 @@ fn new_fwsec(dev: &Device<device::Bound>, bios:
&Vbios, cmd: FwsecCommand) -> Re
let frts_cmd: &mut FrtsCmd = unsafe {
transmute_mut(
&mut dma_object,
- (desc.imem_load_size +
cmd_in_buffer_offset).into_safe_cast(),
+ (desc.imem_load_size() +
cmd_in_buffer_offset).into_safe_cast(),
)
}?;
@@ -364,11 +385,12 @@ pub(crate) fn new(
// Patch signature if needed.
let desc = bios.fwsec_image().header()?;
- let ucode_signed = if desc.signature_count != 0 {
- let sig_base_img = usize::from_safe_cast(desc.imem_load_size +
desc.pkc_data_offset);
- let desc_sig_versions = u32::from(desc.signature_versions);
+ let ucode_signed = if desc.signature_count() != 0 {
+ let sig_base_img +
usize::from_safe_cast(desc.imem_load_size() + desc.pkc_data_offset());
+ let desc_sig_versions = u32::from(desc.signature_versions());
let reg_fuse_version -
falcon.signature_reg_fuse_version(bar, desc.engine_id_mask, desc.ucode_id)?;
+ falcon.signature_reg_fuse_version(bar, desc.engine_id_mask(),
desc.ucode_id())?;
dev_dbg!(
dev,
"desc_sig_versions: {:#x}, reg_fuse_version: {}\n",
@@ -402,7 +424,7 @@ pub(crate) fn new(
dev_dbg!(dev, "patching signature with index {}\n",
signature_idx);
let signature = bios
.fwsec_image()
- .sigs(desc)
+ .sigs(&desc)
.and_then(|sigs| sigs.get(signature_idx).ok_or(EINVAL))?;
ucode_dma.patch_signature(signature, sig_base_img)?
@@ -411,7 +433,7 @@ pub(crate) fn new(
};
Ok(FwsecFirmware {
- desc: desc.clone(),
+ desc: desc,
ucode: ucode_signed,
})
}
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index abf423560ff4..860d6fb3f516 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -19,6 +19,8 @@
driver::Bar0,
firmware::{
fwsec::Bcrt30Rsa3kSignature,
+ FalconUCodeDesc,
+ FalconUCodeDescV2,
FalconUCodeDescV3, //
},
num::FromSafeCast,
@@ -1004,19 +1006,10 @@ fn build(self) -> Result<FwSecBiosImage> {
impl FwSecBiosImage {
/// Get the FwSec header ([`FalconUCodeDescV3`]).
- pub(crate) fn header(&self) -> Result<&FalconUCodeDescV3>
{
+ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
// Get the falcon ucode offset that was found in setup_falcon_data.
let falcon_ucode_offset = self.falcon_ucode_offset;
- // Make sure the offset is within the data bounds.
- if falcon_ucode_offset +
core::mem::size_of::<FalconUCodeDescV3>() > self.base.data.len() {
- dev_err!(
- self.base.dev,
- "fwsec-frts header not contained within BIOS
bounds\n"
- );
- return Err(ERANGE);
- }
-
// Read the first 4 bytes to get the version.
let hdr_bytes: [u8; 4] =
self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4]
.try_into()
@@ -1024,33 +1017,60 @@ pub(crate) fn header(&self) ->
Result<&FalconUCodeDescV3> {
let hdr = u32::from_le_bytes(hdr_bytes);
let ver = (hdr & 0xff00) >> 8;
- if ver != 3 {
- dev_err!(self.base.dev, "invalid fwsec firmware version:
{:?}\n", ver);
- return Err(EINVAL);
+ let hdr_size = match ver {
+ 2 => core::mem::size_of::<FalconUCodeDescV2>(),
+ 3 => core::mem::size_of::<FalconUCodeDescV3>(),
+ _ => {
+ dev_err!(self.base.dev, "invalid fwsec firmware version:
{:?}\n", ver);
+ return Err(EINVAL);
+ }
+ };
+ // Make sure the offset is within the data bounds
+ if falcon_ucode_offset + hdr_size > self.base.data.len() {
+ dev_err!(
+ self.base.dev,
+ "fwsec-frts header not contained within BIOS
bounds\n"
+ );
+ return Err(ERANGE);
}
- // Return a reference to the FalconUCodeDescV3 structure.
- //
- // SAFETY: We have checked that `falcon_ucode_offset +
size_of::<FalconUCodeDescV3>` is
- // within the bounds of `data`. Also, this data vector is from ROM, and
the `data` field
- // in `BiosImageBase` is immutable after construction.
- Ok(unsafe {
+ let v2 = unsafe {
+ &*(self
+ .base
+ .data
+ .as_ptr()
+ .add(falcon_ucode_offset)
+ .cast::<FalconUCodeDescV2>())
+ };
+
+ let v3 = unsafe {
&*(self
.base
.data
.as_ptr()
.add(falcon_ucode_offset)
.cast::<FalconUCodeDescV3>())
- })
+ };
+
+ // Return a FalconUCodeDesc structure.
+ //
+ // SAFETY: We have checked that `falcon_ucode_offset +
size_of::<FalconUCodeDesc>` is
+ // within the bounds of `data`. Also, this data vector is from ROM, and
the `data` field
+ // in `BiosImageBase` is immutable after construction.
+ match ver {
+ 2 => Ok(FalconUCodeDesc::V2(v2.clone())),
+ 3 => Ok(FalconUCodeDesc::V3(v3.clone())),
+ _ => Err(EINVAL),
+ }
}
/// Get the ucode data as a byte slice
- pub(crate) fn ucode(&self, desc: &FalconUCodeDescV3) ->
Result<&[u8]> {
+ pub(crate) fn ucode(&self, desc: &FalconUCodeDesc) ->
Result<&[u8]> {
let falcon_ucode_offset = self.falcon_ucode_offset;
// The ucode data follows the descriptor.
let ucode_data_offset = falcon_ucode_offset + desc.size();
- let size = usize::from_safe_cast(desc.imem_load_size +
desc.dmem_load_size);
+ let size = usize::from_safe_cast(desc.imem_load_size() +
desc.dmem_load_size());
// Get the data slice, checking bounds in a single operation.
self.base
@@ -1066,10 +1086,14 @@ pub(crate) fn ucode(&self, desc:
&FalconUCodeDescV3) -> Result<&[u8]> {
}
/// Get the signatures as a byte slice
- pub(crate) fn sigs(&self, desc: &FalconUCodeDescV3) ->
Result<&[Bcrt30Rsa3kSignature]> {
+ pub(crate) fn sigs(&self, desc: &FalconUCodeDesc) ->
Result<&[Bcrt30Rsa3kSignature]> {
+ let hdr_size = match desc {
+ FalconUCodeDesc::V2(_v2) =>
core::mem::size_of::<FalconUCodeDescV2>(),
+ FalconUCodeDesc::V3(_v3) =>
core::mem::size_of::<FalconUCodeDescV3>(),
+ };
// The signatures data follows the descriptor.
- let sigs_data_offset = self.falcon_ucode_offset +
core::mem::size_of::<FalconUCodeDescV3>();
- let sigs_count = usize::from(desc.signature_count);
+ let sigs_data_offset = self.falcon_ucode_offset + hdr_size;
+ let sigs_count = usize::from(desc.signature_count());
let sigs_size = sigs_count *
core::mem::size_of::<Bcrt30Rsa3kSignature>();
// Make sure the data is within bounds.
--
2.51.2
Joel Fernandes
2025-Nov-17 23:10 UTC
[PATCH 09/11] gpu: nova-core: add FalconUCodeDescV2 support
On Fri, Nov 14, 2025 at 05:30:42PM -0600, Timur Tabi wrote:> The FRTS firmware in Turing and GA100 VBIOS has an older header > format (v2 instead of v3). To support both v2 and v3 at runtime, > add the FalconUCodeDescV2 struct, and update code that references > the FalconUCodeDescV3 directly with a FalconUCodeDesc enum that > encapsulates both. > > Signed-off-by: Timur Tabi <ttabi at nvidia.com> > --- > drivers/gpu/nova-core/firmware.rs | 108 +++++++++++++++++++++++- > drivers/gpu/nova-core/firmware/fwsec.rs | 72 ++++++++++------ > drivers/gpu/nova-core/vbios.rs | 74 ++++++++++------ > 3 files changed, 202 insertions(+), 52 deletions(-) > > diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs > index 2d2008b33fb4..5ca5bf1fb610 100644 > --- a/drivers/gpu/nova-core/firmware.rs > +++ b/drivers/gpu/nova-core/firmware.rs > @@ -43,6 +43,43 @@ 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, > +} > + > /// Structure used to describe some firmwares, notably FWSEC-FRTS. > #[repr(C)] > #[derive(Debug, Clone)] > @@ -76,13 +113,80 @@ pub(crate) struct FalconUCodeDescV3 { > _reserved: u16, > } > > -impl FalconUCodeDescV3 { > +#[derive(Debug, Clone)] > +pub(crate) enum FalconUCodeDesc { > + V2(FalconUCodeDescV2), > + V3(FalconUCodeDescV3), > +} > + > +impl FalconUCodeDesc { > /// Returns the size in bytes of the header. > pub(crate) fn size(&self) -> usize { > + let hdr = match self { > + FalconUCodeDesc::V2(v2) => v2.hdr, > + FalconUCodeDesc::V3(v3) => v3.hdr, > + }; > + > const HDR_SIZE_SHIFT: u32 = 16; > const HDR_SIZE_MASK: u32 = 0xffff0000; > + ((hdr & HDR_SIZE_MASK) >> HDR_SIZE_SHIFT).into_safe_cast() > + } > + > + pub(crate) fn imem_load_size(&self) -> u32 { > + match self { > + FalconUCodeDesc::V2(v2) => v2.imem_load_size, > + FalconUCodeDesc::V3(v3) => v3.imem_load_size, > + } > + }This looks like the perfect use case for a trait object. You can define a trait, make both descriptors implement the trait and get rid of a lot of the match statements: // First define trait pub(crate) trait FalconUCodeDescriptor { fn imem_load_size(&self) -> u32; fn dmem_load_size(&self) -> u32; fn engine_id_mask(&self) -> u16; // V3-only field, V2 returns 0 ... } // Implement trait for both versions impl FalconUCodeDescriptor for FalconUCodeDescV2 { fn imem_load_size(&self) -> u32 { self.imem_load_size } fn dmem_load_size(&self) -> u32 { self.dmem_load_size } fn engine_id_mask(&self) -> u16 { 0 } // V2 doesn't have this field ... } impl FalconUCodeDescriptor for FalconUCodeDescV3 { fn imem_load_size(&self) -> u32 { self.imem_load_size } fn dmem_load_size(&self) -> u32 { self.dmem_load_size } fn engine_id_mask(&self) -> u16 { self.engine_id_mask } ... } // Keep the same enum. If you want to get rid of the enum, you'll need Box, // but then that requires allocation. pub(crate) enum FalconUCodeDesc { V2(FalconUCodeDescV2), V3(FalconUCodeDescV3), } impl FalconUCodeDesc { // Return trait object, the only match needed. pub(crate) fn as_descriptor(&self) -> &dyn FalconUCodeDescriptor { match self { FalconUCodeDesc::V2(v2) => v2, FalconUCodeDesc::V3(v3) => v3, } } // delegate to trait, no match statements! pub(crate) fn imem_load_size(&self) -> u32 { self.as_descriptor().imem_load_size() } pub(crate) fn dmem_load_size(&self) -> u32 { self.as_descriptor().dmem_load_size() } } // Usage example - no more match statements needed! impl FalconLoadParams for FwsecFirmware { fn dmem_load_params(&self) -> FalconLoadTarget { FalconLoadTarget { src_start: 0, dst_start: 0, len: self.desc.dmem_load_size(), } } } thanks, - Joel> + > + pub(crate) fn interface_offset(&self) -> u32 { > + match self { > + FalconUCodeDesc::V2(v2) => v2.interface_offset, > + FalconUCodeDesc::V3(v3) => v3.interface_offset, > + } > + } > + > + > + pub(crate) fn dmem_load_size(&self) -> u32 { > + match self { > + FalconUCodeDesc::V2(v2) => v2.dmem_load_size, > + FalconUCodeDesc::V3(v3) => v3.dmem_load_size, > + } > + } > + > + pub(crate) fn pkc_data_offset(&self) -> u32 { > + match self { > + FalconUCodeDesc::V2(_v2) => 0, > + FalconUCodeDesc::V3(v3) => v3.pkc_data_offset, > + } > + } > > - ((self.hdr & HDR_SIZE_MASK) >> HDR_SIZE_SHIFT).into_safe_cast() > + pub(crate) fn engine_id_mask(&self) -> u16 { > + match self { > + FalconUCodeDesc::V2(_v2) => 0, > + FalconUCodeDesc::V3(v3) => v3.engine_id_mask, > + } > + } > + > + pub(crate) fn ucode_id(&self) -> u8 { > + match self { > + FalconUCodeDesc::V2(_v2) => 0, > + FalconUCodeDesc::V3(v3) => v3.ucode_id, > + } > + } > + > + pub(crate) fn signature_count(&self) -> u8 { > + match self { > + FalconUCodeDesc::V2(_v2) => 0, > + FalconUCodeDesc::V3(v3) => v3.signature_count, > + } > + } > + > + pub(crate) fn signature_versions(&self) -> u16 { > + match self { > + FalconUCodeDesc::V2(_v2) => 0, > + FalconUCodeDesc::V3(v3) => v3.signature_versions, > + } > } > } > > diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs > index e4009faba6c5..36ff8ed51c23 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 - v2.imem_sec_size, > + }), > + // Not used on V3 platforms > + FalconUCodeDesc::V3(_v3) => None, > + } > } > > fn dmem_load_params(&self) -> FalconLoadTarget { > - FalconLoadTarget { > - src_start: self.desc.imem_load_size, > - dst_start: self.desc.dmem_phys_base, > - len: self.desc.dmem_load_size, > + match &self.desc { > + FalconUCodeDesc::V2(v2) => FalconLoadTarget { > + src_start: v2.dmem_offset, > + dst_start: v2.dmem_phys_base, > + len: v2.dmem_load_size, > + }, > + FalconUCodeDesc::V3(v3) => FalconLoadTarget { > + src_start: v3.imem_load_size, > + dst_start: v3.dmem_phys_base, > + len: v3.dmem_load_size, > + } > } > } > > fn brom_params(&self) -> FalconBromParams { > FalconBromParams { > - pkc_data_offset: self.desc.pkc_data_offset, > - engine_id_mask: self.desc.engine_id_mask, > - ucode_id: self.desc.ucode_id, > + pkc_data_offset: self.desc.pkc_data_offset(), > + engine_id_mask: self.desc.engine_id_mask(), > + ucode_id: self.desc.ucode_id(), > } > } > > @@ -273,10 +294,10 @@ impl FalconFirmware for FwsecFirmware { > impl FirmwareDmaObject<FwsecFirmware, Unsigned> { > fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Result<Self> { > let desc = bios.fwsec_image().header()?; > - let ucode = bios.fwsec_image().ucode(desc)?; > + let ucode = bios.fwsec_image().ucode(&desc)?; > let mut dma_object = DmaObject::from_data(dev, ucode)?; > > - let hdr_offset = usize::from_safe_cast(desc.imem_load_size + desc.interface_offset); > + let hdr_offset = usize::from_safe_cast(desc.imem_load_size() + desc.interface_offset()); > // SAFETY: we have exclusive access to `dma_object`. > let hdr: &FalconAppifHdrV1 = unsafe { transmute(&dma_object, hdr_offset) }?; > > @@ -303,7 +324,7 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re > let dmem_mapper: &mut FalconAppifDmemmapperV3 = unsafe { > transmute_mut( > &mut dma_object, > - (desc.imem_load_size + dmem_base).into_safe_cast(), > + (desc.imem_load_size() + dmem_base).into_safe_cast(), > ) > }?; > > @@ -317,7 +338,7 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re > let frts_cmd: &mut FrtsCmd = unsafe { > transmute_mut( > &mut dma_object, > - (desc.imem_load_size + cmd_in_buffer_offset).into_safe_cast(), > + (desc.imem_load_size() + cmd_in_buffer_offset).into_safe_cast(), > ) > }?; > > @@ -364,11 +385,12 @@ pub(crate) fn new( > > // Patch signature if needed. > let desc = bios.fwsec_image().header()?; > - let ucode_signed = if desc.signature_count != 0 { > - let sig_base_img = usize::from_safe_cast(desc.imem_load_size + desc.pkc_data_offset); > - let desc_sig_versions = u32::from(desc.signature_versions); > + let ucode_signed = if desc.signature_count() != 0 { > + let sig_base_img > + usize::from_safe_cast(desc.imem_load_size() + desc.pkc_data_offset()); > + let desc_sig_versions = u32::from(desc.signature_versions()); > let reg_fuse_version > - falcon.signature_reg_fuse_version(bar, desc.engine_id_mask, desc.ucode_id)?; > + falcon.signature_reg_fuse_version(bar, desc.engine_id_mask(), desc.ucode_id())?; > dev_dbg!( > dev, > "desc_sig_versions: {:#x}, reg_fuse_version: {}\n", > @@ -402,7 +424,7 @@ pub(crate) fn new( > dev_dbg!(dev, "patching signature with index {}\n", signature_idx); > let signature = bios > .fwsec_image() > - .sigs(desc) > + .sigs(&desc) > .and_then(|sigs| sigs.get(signature_idx).ok_or(EINVAL))?; > > ucode_dma.patch_signature(signature, sig_base_img)? > @@ -411,7 +433,7 @@ pub(crate) fn new( > }; > > Ok(FwsecFirmware { > - desc: desc.clone(), > + desc: desc, > ucode: ucode_signed, > }) > } > diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs > index abf423560ff4..860d6fb3f516 100644 > --- a/drivers/gpu/nova-core/vbios.rs > +++ b/drivers/gpu/nova-core/vbios.rs > @@ -19,6 +19,8 @@ > driver::Bar0, > firmware::{ > fwsec::Bcrt30Rsa3kSignature, > + FalconUCodeDesc, > + FalconUCodeDescV2, > FalconUCodeDescV3, // > }, > num::FromSafeCast, > @@ -1004,19 +1006,10 @@ fn build(self) -> Result<FwSecBiosImage> { > > impl FwSecBiosImage { > /// Get the FwSec header ([`FalconUCodeDescV3`]). > - pub(crate) fn header(&self) -> Result<&FalconUCodeDescV3> { > + pub(crate) fn header(&self) -> Result<FalconUCodeDesc> { > // Get the falcon ucode offset that was found in setup_falcon_data. > let falcon_ucode_offset = self.falcon_ucode_offset; > > - // Make sure the offset is within the data bounds. > - if falcon_ucode_offset + core::mem::size_of::<FalconUCodeDescV3>() > self.base.data.len() { > - dev_err!( > - self.base.dev, > - "fwsec-frts header not contained within BIOS bounds\n" > - ); > - return Err(ERANGE); > - } > - > // Read the first 4 bytes to get the version. > let hdr_bytes: [u8; 4] = self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4] > .try_into() > @@ -1024,33 +1017,60 @@ pub(crate) fn header(&self) -> Result<&FalconUCodeDescV3> { > let hdr = u32::from_le_bytes(hdr_bytes); > let ver = (hdr & 0xff00) >> 8; > > - if ver != 3 { > - dev_err!(self.base.dev, "invalid fwsec firmware version: {:?}\n", ver); > - return Err(EINVAL); > + let hdr_size = match ver { > + 2 => core::mem::size_of::<FalconUCodeDescV2>(), > + 3 => core::mem::size_of::<FalconUCodeDescV3>(), > + _ => { > + dev_err!(self.base.dev, "invalid fwsec firmware version: {:?}\n", ver); > + return Err(EINVAL); > + } > + }; > + // Make sure the offset is within the data bounds > + if falcon_ucode_offset + hdr_size > self.base.data.len() { > + dev_err!( > + self.base.dev, > + "fwsec-frts header not contained within BIOS bounds\n" > + ); > + return Err(ERANGE); > } > > - // Return a reference to the FalconUCodeDescV3 structure. > - // > - // SAFETY: We have checked that `falcon_ucode_offset + size_of::<FalconUCodeDescV3>` is > - // within the bounds of `data`. Also, this data vector is from ROM, and the `data` field > - // in `BiosImageBase` is immutable after construction. > - Ok(unsafe { > + let v2 = unsafe { > + &*(self > + .base > + .data > + .as_ptr() > + .add(falcon_ucode_offset) > + .cast::<FalconUCodeDescV2>()) > + }; > + > + let v3 = unsafe { > &*(self > .base > .data > .as_ptr() > .add(falcon_ucode_offset) > .cast::<FalconUCodeDescV3>()) > - }) > + }; > + > + // Return a FalconUCodeDesc structure. > + // > + // SAFETY: We have checked that `falcon_ucode_offset + size_of::<FalconUCodeDesc>` is > + // within the bounds of `data`. Also, this data vector is from ROM, and the `data` field > + // in `BiosImageBase` is immutable after construction. > + match ver { > + 2 => Ok(FalconUCodeDesc::V2(v2.clone())), > + 3 => Ok(FalconUCodeDesc::V3(v3.clone())), > + _ => Err(EINVAL), > + } > } > > /// Get the ucode data as a byte slice > - pub(crate) fn ucode(&self, desc: &FalconUCodeDescV3) -> Result<&[u8]> { > + pub(crate) fn ucode(&self, desc: &FalconUCodeDesc) -> Result<&[u8]> { > let falcon_ucode_offset = self.falcon_ucode_offset; > > // The ucode data follows the descriptor. > let ucode_data_offset = falcon_ucode_offset + desc.size(); > - let size = usize::from_safe_cast(desc.imem_load_size + desc.dmem_load_size); > + let size = usize::from_safe_cast(desc.imem_load_size() + desc.dmem_load_size()); > > // Get the data slice, checking bounds in a single operation. > self.base > @@ -1066,10 +1086,14 @@ pub(crate) fn ucode(&self, desc: &FalconUCodeDescV3) -> Result<&[u8]> { > } > > /// Get the signatures as a byte slice > - pub(crate) fn sigs(&self, desc: &FalconUCodeDescV3) -> Result<&[Bcrt30Rsa3kSignature]> { > + pub(crate) fn sigs(&self, desc: &FalconUCodeDesc) -> Result<&[Bcrt30Rsa3kSignature]> { > + let hdr_size = match desc { > + FalconUCodeDesc::V2(_v2) => core::mem::size_of::<FalconUCodeDescV2>(), > + FalconUCodeDesc::V3(_v3) => core::mem::size_of::<FalconUCodeDescV3>(), > + }; > // The signatures data follows the descriptor. > - let sigs_data_offset = self.falcon_ucode_offset + core::mem::size_of::<FalconUCodeDescV3>(); > - let sigs_count = usize::from(desc.signature_count); > + let sigs_data_offset = self.falcon_ucode_offset + hdr_size; > + let sigs_count = usize::from(desc.signature_count()); > let sigs_size = sigs_count * core::mem::size_of::<Bcrt30Rsa3kSignature>(); > > // Make sure the data is within bounds. > -- > 2.51.2 >
Alexandre Courbot
2025-Nov-19 03:27 UTC
[PATCH 09/11] gpu: nova-core: add FalconUCodeDescV2 support
On Sat Nov 15, 2025 at 8:30 AM JST, Timur Tabi wrote: <snip>> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs > index abf423560ff4..860d6fb3f516 100644 > --- a/drivers/gpu/nova-core/vbios.rs > +++ b/drivers/gpu/nova-core/vbios.rs > @@ -19,6 +19,8 @@ > driver::Bar0, > firmware::{ > fwsec::Bcrt30Rsa3kSignature, > + FalconUCodeDesc, > + FalconUCodeDescV2, > FalconUCodeDescV3, // > }, > num::FromSafeCast, > @@ -1004,19 +1006,10 @@ fn build(self) -> Result<FwSecBiosImage> { > > impl FwSecBiosImage { > /// Get the FwSec header ([`FalconUCodeDescV3`]). > - pub(crate) fn header(&self) -> Result<&FalconUCodeDescV3> { > + pub(crate) fn header(&self) -> Result<FalconUCodeDesc> { > // Get the falcon ucode offset that was found in setup_falcon_data. > let falcon_ucode_offset = self.falcon_ucode_offset; > > - // Make sure the offset is within the data bounds. > - if falcon_ucode_offset + core::mem::size_of::<FalconUCodeDescV3>() > self.base.data.len() { > - dev_err!( > - self.base.dev, > - "fwsec-frts header not contained within BIOS bounds\n" > - ); > - return Err(ERANGE); > - } > - > // Read the first 4 bytes to get the version. > let hdr_bytes: [u8; 4] = self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4] > .try_into() > @@ -1024,33 +1017,60 @@ pub(crate) fn header(&self) -> Result<&FalconUCodeDescV3> { > let hdr = u32::from_le_bytes(hdr_bytes); > let ver = (hdr & 0xff00) >> 8; > > - if ver != 3 { > - dev_err!(self.base.dev, "invalid fwsec firmware version: {:?}\n", ver); > - return Err(EINVAL); > + let hdr_size = match ver { > + 2 => core::mem::size_of::<FalconUCodeDescV2>(), > + 3 => core::mem::size_of::<FalconUCodeDescV3>(), > + _ => { > + dev_err!(self.base.dev, "invalid fwsec firmware version: {:?}\n", ver); > + return Err(EINVAL); > + } > + }; > + // Make sure the offset is within the data bounds > + if falcon_ucode_offset + hdr_size > self.base.data.len() { > + dev_err!( > + self.base.dev, > + "fwsec-frts header not contained within BIOS bounds\n" > + ); > + return Err(ERANGE); > } > > - // Return a reference to the FalconUCodeDescV3 structure. > - // > - // SAFETY: We have checked that `falcon_ucode_offset + size_of::<FalconUCodeDescV3>` is > - // within the bounds of `data`. Also, this data vector is from ROM, and the `data` field > - // in `BiosImageBase` is immutable after construction. > - Ok(unsafe { > + let v2 = unsafe { > + &*(self > + .base > + .data > + .as_ptr() > + .add(falcon_ucode_offset) > + .cast::<FalconUCodeDescV2>()) > + }; > + > + let v3 = unsafe { > &*(self > .base > .data > .as_ptr() > .add(falcon_ucode_offset) > .cast::<FalconUCodeDescV3>()) > - }) > + };Mmm, we are creating references to both versions of the header, one of which is going to be invalid. Let's do the casting in the match arm below: match ver { 2 => { let v2 = unsafe { ... }; Ok(FalconUCodeDesc::V2(v2.clone())) } 3 => { let v3 = unsafe { ... }; Ok(FalconUCodeDesc::V3(v3.clone())) } }