Re: [PATCH v6 09/11] gpu: nova-core: add FalconUCodeDescV2 support

["Alexandre Courbot" <acourbot@nvidia.com>]

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),
         }
     }