On Thu Jan 15, 2026 at 4:29 AM JST, Timur Tabi wrote:
<snip>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index e59eee2050a8..07a28c4c0837 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -19,6 +19,8 @@
> driver::Bar0,
> firmware::{
> fwsec::Bcrt30Rsa3kSignature,
> + FalconUCodeDesc,
> + FalconUCodeDescV2,
> FalconUCodeDescV3, //
> },
> num::FromSafeCast,
> @@ -999,19 +1001,10 @@ fn build(self) -> Result<FwSecBiosImage> {
>
> impl FwSecBiosImage {
> /// Get the FwSec header ([`FalconUCodeDescV3`]).
This comment should also be updated to refer to `FalconUCodeDesc`.
> - pub(crate) fn header(&self) -> Result<&FalconUCodeDescV3> {
> + pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
> // Get the falcon ucode offset that was found in setup_falcon_data.
> let falcon_ucode_offset = self.falcon_ucode_offset;
>
> - // Make sure the offset is within the data bounds.
> - if falcon_ucode_offset + core::mem::size_of::<FalconUCodeDescV3>() >
> self.base.data.len() {
> - dev_err!(
> - self.base.dev,
> - "fwsec-frts header not contained within BIOS bounds\n"
> - );
> - return Err(ERANGE);
> - }
> -
> // Read the first 4 bytes to get the version.
> let hdr_bytes: [u8; 4] =
> self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4]
> .try_into()
> @@ -1019,33 +1012,49 @@ pub(crate) fn header(&self) ->
> Result<&FalconUCodeDescV3> {
> let hdr = u32::from_le_bytes(hdr_bytes);
> let ver = (hdr & 0xff00) >> 8;
>
> - if ver != 3 {
> - dev_err!(self.base.dev, "invalid fwsec firmware version:
> {:?}\n", ver);
> - return Err(EINVAL);
> + let hdr_size = match ver {
> + 2 => core::mem::size_of::<FalconUCodeDescV2>(),
> + 3 => core::mem::size_of::<FalconUCodeDescV3>(),
> + _ => {
> + dev_err!(self.base.dev, "invalid fwsec firmware version:
> {:?}\n", ver);
> + return Err(EINVAL);
> + }
> + };
> + // Make sure the offset is within the data bounds
> + if falcon_ucode_offset + hdr_size > self.base.data.len() {
> + dev_err!(
> + self.base.dev,
> + "fwsec-frts header not contained within BIOS bounds\n"
> + );
> + return Err(ERANGE);
> }
>
> - // Return a reference to the FalconUCodeDescV3 structure.
> - //
> - // SAFETY: We have checked that `falcon_ucode_offset +
> size_of::<FalconUCodeDescV3>` is
> - // within the bounds of `data`. Also, this data vector is from ROM,
> and the `data` field
> - // in `BiosImageBase` is immutable after construction.
> - Ok(unsafe {
> - &*(self
> - .base
> - .data
> - .as_ptr()
> - .add(falcon_ucode_offset)
> - .cast::<FalconUCodeDescV3>())
> - })
> + let data = self
> + .base
> + .data
> + .get(falcon_ucode_offset..falcon_ucode_offset + hdr_size)
> + .ok_or(EINVAL)?;
> +
> + match ver {
> + 2 => {
> + let v2 = FalconUCodeDescV2::from_bytes(data).ok_or(EINVAL)?;
> + Ok(FalconUCodeDesc::V2(v2.clone()))
> + }
> + 3 => {
> + let v3 = FalconUCodeDescV3::from_bytes(data).ok_or(EINVAL)?;
> + Ok(FalconUCodeDesc::V3(v3.clone()))
> + }
> + _ => Err(EINVAL),
> + }
> }
We can be a little bit shorter here and avoid the redundant by
leveraging the new `from_bytes_copy_prefix`. See if this applies on top
of your patch:
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 07a28c4c0837..72b6bad5964b 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -1012,39 +1013,25 @@ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
let hdr = u32::from_le_bytes(hdr_bytes);
let ver = (hdr & 0xff00) >> 8;
- let hdr_size = match ver {
- 2 => core::mem::size_of::<FalconUCodeDescV2>(),
- 3 => core::mem::size_of::<FalconUCodeDescV3>(),
- _ => {
- dev_err!(self.base.dev, "invalid fwsec firmware version:
{:?}\n", ver);
- return Err(EINVAL);
- }
- };
- // Make sure the offset is within the data bounds
- if falcon_ucode_offset + hdr_size > self.base.data.len() {
- dev_err!(
- self.base.dev,
- "fwsec-frts header not contained within BIOS bounds\n"
- );
- return Err(ERANGE);
- }
-
- let data = self
- .base
- .data
- .get(falcon_ucode_offset..falcon_ucode_offset + hdr_size)
- .ok_or(EINVAL)?;
-
+ let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
match ver {
2 => {
- let v2 = FalconUCodeDescV2::from_bytes(data).ok_or(EINVAL)?;
- Ok(FalconUCodeDesc::V2(v2.clone()))
+ let v2 = FalconUCodeDescV2::from_bytes_copy_prefix(data)
+ .ok_or(EINVAL)?
+ .0;
+ Ok(FalconUCodeDesc::V2(v2))
}
3 => {
- let v3 = FalconUCodeDescV3::from_bytes(data).ok_or(EINVAL)?;
- Ok(FalconUCodeDesc::V3(v3.clone()))
+ let v3 = FalconUCodeDescV3::from_bytes_copy_prefix(data)
+ .ok_or(EINVAL)?
+ .0;
+ Ok(FalconUCodeDesc::V3(v3))
+ }
+ _ => {
+ dev_err!(self.base.dev, "invalid fwsec firmware version:
{:?}\n", ver);
+ Err(EINVAL)
}
- _ => Err(EINVAL),
}
}
