Nouveau - Oct 2025 - [PATCH 0/5] gpu: nova-core: leverage FromBytes for VBIOS structures

This series spawns from the discussion in [1], where it was noticed that
several structures of the VBIOS could just be read using `FromBytes`
instead of being built manually.

While implementing it, I also noticed that we were repeating the
following pattern quite a bit in the code:

    slice
        .get(..size_of::<TargetType>())
        .and_then(TargetType::from_bytes_copy)

... which begs for dedicated methods doing this in the `FromBytes`
trait. This is what the first patch does, taking inspiration from a
similar feature in the `zerocopy` crate.

The remaining patches leverage these new methods to simplify the VBIOS
code a bit, accordingly.

If patch 1 is deemed a valid idea, I hope we can merge it into the DRM
tree along with the rest of this series.

The base for this work is `drm-rust-next`, with [2] applied.

[1] https://lore.kernel.org/rust-for-linux/DDTRW1P2I4PB.10ZTZDY95JBC5 at
nvidia.com/
[2] https://lore.kernel.org/rust-for-linux/20251026-nova-as-v1-1-60c78726462d at
nvidia.com/

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
Alexandre Courbot (5):
      rust: transmute: add `from_bytes_prefix` family of methods
      gpu: nova-core: vbios: use FromBytes for PmuLookupTable header
      gpu: nova-core: vbios: use FromBytes for PcirStruct
      gpu: nova-core: vbios: use FromBytes for BitHeader
      gpu: nova-core: vbios: use FromBytes for NpdeStruct

 drivers/gpu/nova-core/vbios.rs | 137 ++++++++++++++++-------------------------
 rust/kernel/transmute.rs       |  60 ++++++++++++++++++
 2 files changed, 113 insertions(+), 84 deletions(-)
---
base-commit: 639291d7c30cec5cf0d9a79371021c2e4404cfc9
change-id: 20251028-nova-vbios-frombytes-eb0cbb6a2f11

Best regards,
-- 
Alexandre Courbot <acourbot at nvidia.com>

The `from_bytes*` family of functions expect a slice of the exact same
size as the requested type. This can be sometimes cumbersome for callers
that deal with dynamic stream of data that needs to be manually cut
before each invocation of `from_bytes`.

To simplify such callers, introduce a new `from_bytes*_prefix` family of
methods, which split the input slice at the index required for the
equivalent `from_bytes` method to succeed, and return its result
alongside with the remainder of the slice.

This design is inspired by zerocopy's `try_*_from_prefix` family of
methods.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
 rust/kernel/transmute.rs | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
index cfc37d81adf2..00cd3092c7cc 100644
--- a/rust/kernel/transmute.rs
+++ b/rust/kernel/transmute.rs
@@ -58,6 +58,26 @@ fn from_bytes(bytes: &[u8]) -> Option<&Self>
         }
     }
 
+    /// Converts the beginning of `bytes` to a reference to `Self`.
+    ///
+    /// This method is similar to [`Self::from_bytes`], with the difference
that `bytes` does not
+    /// need to be the same size of `Self` - the appropriate portion is cut
from the beginning of
+    /// `bytes`, and the remainder returned alongside the result.
+    fn from_bytes_prefix(bytes: &[u8]) -> Option<(&Self,
&[u8])>
+    where
+        Self: Sized,
+    {
+        if bytes.len() < size_of::<Self>() {
+            None
+        } else {
+            // We checked that `bytes.len() >= size_of::<Self>`, thus
`split_at` cannot panic.
+            // TODO: replace with `split_at_checked` once the MSRV is >=
1.80.
+            let (prefix, remainder) = bytes.split_at(size_of::<Self>());
+
+            Self::from_bytes(prefix).map(|s| (s, remainder))
+        }
+    }
+
     /// Converts a mutable slice of bytes to a reference to `Self`.
     ///
     /// Succeeds if the reference is properly aligned, and the size of `bytes`
is equal to that of
@@ -80,6 +100,26 @@ fn from_bytes_mut(bytes: &mut [u8]) ->
Option<&mut Self>
         }
     }
 
+    /// Converts the beginning of `bytes` to a mutable reference to `Self`.
+    ///
+    /// This method is similar to [`Self::from_bytes_mut`], with the difference
that `bytes` does
+    /// not need to be the same size of `Self` - the appropriate portion is cut
from the beginning
+    /// of `bytes`, and the remainder returned alongside the result.
+    fn from_bytes_mut_prefix(bytes: &mut [u8]) -> Option<(&mut
Self, &mut [u8])>
+    where
+        Self: AsBytes + Sized,
+    {
+        if bytes.len() < size_of::<Self>() {
+            None
+        } else {
+            // We checked that `bytes.len() >= size_of::<Self>`, thus
`split_at_mut` cannot panic.
+            // TODO: replace with `split_at_mut_checked` once the MSRV is >=
1.80.
+            let (prefix, remainder) =
bytes.split_at_mut(size_of::<Self>());
+
+            Self::from_bytes_mut(prefix).map(|s| (s, remainder))
+        }
+    }
+
     /// Creates an owned instance of `Self` by copying `bytes`.
     ///
     /// Unlike [`FromBytes::from_bytes`], which requires aligned input, this
method can be used on
@@ -97,6 +137,26 @@ fn from_bytes_copy(bytes: &[u8]) ->
Option<Self>
             None
         }
     }
+
+    /// Creates an owned instance of `Self` from the beginning of `bytes`.
+    ///
+    /// This method is similar to [`Self::from_bytes_copy`], with the
difference that `bytes` does
+    /// not need to be the same size of `Self` - the appropriate portion is cut
from the beginning
+    /// of `bytes`, and the remainder returned alongside the result.
+    fn from_bytes_copy_prefix(bytes: &[u8]) -> Option<(Self,
&[u8])>
+    where
+        Self: Sized,
+    {
+        if bytes.len() < size_of::<Self>() {
+            None
+        } else {
+            // We checked that `bytes.len() >= size_of::<Self>`, thus
`split_at` cannot panic.
+            // TODO: replace with `split_at_checked` once the MSRV is >=
1.80.
+            let (prefix, remainder) = bytes.split_at(size_of::<Self>());
+
+            Self::from_bytes_copy(prefix).map(|s| (s, remainder))
+        }
+    }
 }
 
 macro_rules! impl_frombytes {

-- 
2.51.0

Use `from_bytes_copy_prefix` to create the `PmuLookupTable` header
instead of building it ourselves from the bytes stream. This lets us
remove a few `as` conversions and array accesses.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 44 ++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 46da51b9f6b0..b6c20627a5e3 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -10,6 +10,7 @@
 use kernel::error::Result;
 use kernel::prelude::*;
 use kernel::ptr::{Alignable, Alignment};
+use kernel::transmute::FromBytes;
 use kernel::types::ARef;
 
 /// The offset of the VBIOS ROM in the BAR0 space.
@@ -866,29 +867,36 @@ fn new(data: &[u8]) -> Result<Self> {
     }
 }
 
+#[repr(C)]
+struct PmuLookupTableHeader {
+    version: u8,
+    header_len: u8,
+    entry_len: u8,
+    entry_count: u8,
+}
+
+// SAFETY: all bit patterns are valid for `PmuLookupTableHeader`.
+unsafe impl FromBytes for PmuLookupTableHeader {}
+
 /// The [`PmuLookupTableEntry`] structure is used to find the
[`PmuLookupTableEntry`] for a given
 /// application ID.
 ///
 /// The table of entries is pointed to by the falcon data pointer in the BIT
table, and is used to
 /// locate the Falcon Ucode.
-#[expect(dead_code)]
 struct PmuLookupTable {
-    version: u8,
-    header_len: u8,
-    entry_len: u8,
-    entry_count: u8,
+    header: PmuLookupTableHeader,
     table_data: KVec<u8>,
 }
 
 impl PmuLookupTable {
     fn new(dev: &device::Device, data: &[u8]) -> Result<Self>
{
-        if data.len() < 4 {
-            return Err(EINVAL);
-        }
+        let header = PmuLookupTableHeader::from_bytes_copy_prefix(data)
+            .ok_or(EINVAL)?
+            .0;
 
-        let header_len = usize::from(data[1]);
-        let entry_len = usize::from(data[2]);
-        let entry_count = usize::from(data[3]);
+        let header_len = usize::from(header.header_len);
+        let entry_len = usize::from(header.entry_len);
+        let entry_count = usize::from(header.entry_count);
 
         let required_bytes = header_len + (entry_count * entry_len);
 
@@ -909,27 +917,21 @@ fn new(dev: &device::Device, data: &[u8]) ->
Result<Self> {
             dev_dbg!(dev, "PMU entry: {:02x?}\n",
&data[i..][..entry_len]);
         }
 
-        Ok(PmuLookupTable {
-            version: data[0],
-            header_len: header_len as u8,
-            entry_len: entry_len as u8,
-            entry_count: entry_count as u8,
-            table_data,
-        })
+        Ok(PmuLookupTable { header, table_data })
     }
 
     fn lookup_index(&self, idx: u8) -> Result<PmuLookupTableEntry>
{
-        if idx >= self.entry_count {
+        if idx >= self.header.entry_count {
             return Err(EINVAL);
         }
 
-        let index = (usize::from(idx)) * usize::from(self.entry_len);
+        let index = (usize::from(idx)) * usize::from(self.header.entry_len);
         PmuLookupTableEntry::new(&self.table_data[index..])
     }
 
     // find entry by type value
     fn find_entry_by_type(&self, entry_type: u8) ->
Result<PmuLookupTableEntry> {
-        for i in 0..self.entry_count {
+        for i in 0..self.header.entry_count {
             let entry = self.lookup_index(i)?;
             if entry.application_id == entry_type {
                 return Ok(entry);

-- 
2.51.0

Use `from_bytes_copy_prefix` to create `PcirStruct` instead of building
it ourselves from the bytes stream. This lets us remove a few array
accesses and results in shorter code.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 40 ++++++++++++----------------------------
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index b6c20627a5e3..b02a1997306f 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -314,45 +314,29 @@ struct PcirStruct {
     max_runtime_image_len: u16,
 }
 
+// SAFETY: all bit patterns are valid for `PcirStruct`.
+unsafe impl FromBytes for PcirStruct {}
+
 impl PcirStruct {
     fn new(dev: &device::Device, data: &[u8]) -> Result<Self>
{
-        if data.len() < core::mem::size_of::<PcirStruct>() {
-            dev_err!(dev, "Not enough data for PcirStruct\n");
-            return Err(EINVAL);
-        }
-
-        let mut signature = [0u8; 4];
-        signature.copy_from_slice(&data[0..4]);
+        let pcir = PcirStruct::from_bytes_copy_prefix(data).ok_or(EINVAL)?.0;
 
         // Signature should be "PCIR" (0x52494350) or
"NPDS" (0x5344504e).
-        if &signature != b"PCIR" && &signature !=
b"NPDS" {
-            dev_err!(dev, "Invalid signature for PcirStruct: {:?}\n",
signature);
+        if &pcir.signature != b"PCIR" &&
&pcir.signature != b"NPDS" {
+            dev_err!(
+                dev,
+                "Invalid signature for PcirStruct: {:?}\n",
+                pcir.signature
+            );
             return Err(EINVAL);
         }
 
-        let mut class_code = [0u8; 3];
-        class_code.copy_from_slice(&data[13..16]);
-
-        let image_len = u16::from_le_bytes([data[16], data[17]]);
-        if image_len == 0 {
+        if pcir.image_len == 0 {
             dev_err!(dev, "Invalid image length: 0\n");
             return Err(EINVAL);
         }
 
-        Ok(PcirStruct {
-            signature,
-            vendor_id: u16::from_le_bytes([data[4], data[5]]),
-            device_id: u16::from_le_bytes([data[6], data[7]]),
-            device_list_ptr: u16::from_le_bytes([data[8], data[9]]),
-            pci_data_struct_len: u16::from_le_bytes([data[10], data[11]]),
-            pci_data_struct_rev: data[12],
-            class_code,
-            image_len,
-            vendor_rom_rev: u16::from_le_bytes([data[18], data[19]]),
-            code_type: data[20],
-            last_image: data[21],
-            max_runtime_image_len: u16::from_le_bytes([data[22], data[23]]),
-        })
+        Ok(pcir)
     }
 
     /// Check if this is the last image in the ROM.

-- 
2.51.0

Use `from_bytes_copy_prefix` to create `BitHeader` instead of building
it ourselves from the bytes stream. This lets us remove a few array
accesses and results in shorter code.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index b02a1997306f..ade99c90dd3d 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -374,30 +374,19 @@ struct BitHeader {
     checksum: u8,
 }
 
+// SAFETY: all bit patterns are valid for `BitHeader`.
+unsafe impl FromBytes for BitHeader {}
+
 impl BitHeader {
     fn new(data: &[u8]) -> Result<Self> {
-        if data.len() < core::mem::size_of::<Self>() {
-            return Err(EINVAL);
-        }
-
-        let mut signature = [0u8; 4];
-        signature.copy_from_slice(&data[2..6]);
+        let header = BitHeader::from_bytes_copy_prefix(data).ok_or(EINVAL)?.0;
 
         // Check header ID and signature
-        let id = u16::from_le_bytes([data[0], data[1]]);
-        if id != 0xB8FF || &signature != b"BIT\0" {
+        if header.id != 0xB8FF || &header.signature != b"BIT\0" {
             return Err(EINVAL);
         }
 
-        Ok(BitHeader {
-            id,
-            signature,
-            bcd_version: u16::from_le_bytes([data[6], data[7]]),
-            header_size: data[8],
-            token_size: data[9],
-            token_entries: data[10],
-            checksum: data[11],
-        })
+        Ok(header)
     }
 }
 

-- 
2.51.0

Use `from_bytes_copy_prefix` to create `NpdeStruct` instead of building
it ourselves from the bytes stream. This lets us remove a few array
accesses and results in shorter code.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index ade99c90dd3d..8d143df46ede 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -537,35 +537,29 @@ struct NpdeStruct {
     last_image: u8,
 }
 
+// SAFETY: all bit patterns are valid for `NpdeStruct`.
+unsafe impl FromBytes for NpdeStruct {}
+
 impl NpdeStruct {
     fn new(dev: &device::Device, data: &[u8]) -> Option<Self>
{
-        if data.len() < core::mem::size_of::<Self>() {
-            dev_dbg!(dev, "Not enough data for NpdeStruct\n");
-            return None;
-        }
-
-        let mut signature = [0u8; 4];
-        signature.copy_from_slice(&data[0..4]);
+        let npde = NpdeStruct::from_bytes_copy_prefix(data)?.0;
 
         // Signature should be "NPDE" (0x4544504E).
-        if &signature != b"NPDE" {
-            dev_dbg!(dev, "Invalid signature for NpdeStruct: {:?}\n",
signature);
+        if &npde.signature != b"NPDE" {
+            dev_dbg!(
+                dev,
+                "Invalid signature for NpdeStruct: {:?}\n",
+                npde.signature
+            );
             return None;
         }
 
-        let subimage_len = u16::from_le_bytes([data[8], data[9]]);
-        if subimage_len == 0 {
+        if npde.subimage_len == 0 {
             dev_dbg!(dev, "Invalid subimage length: 0\n");
             return None;
         }
 
-        Some(NpdeStruct {
-            signature,
-            npci_data_ext_rev: u16::from_le_bytes([data[4], data[5]]),
-            npci_data_ext_len: u16::from_le_bytes([data[6], data[7]]),
-            subimage_len,
-            last_image: data[10],
-        })
+        Some(npde)
     }
 
     /// Check if this is the last image in the ROM.

-- 
2.51.0

On 10/28/25 8:07 AM, Alexandre Courbot wrote:
... 
> The base for this work is `drm-rust-next`, with [2] applied.
Taking a look now, but unable to apply, using those steps. Do you have
anything else perhaps in your tree?


thanks,
John Hubbard
> 
> [1] https://lore.kernel.org/rust-for-linux/DDTRW1P2I4PB.10ZTZDY95JBC5 at
nvidia.com/
> [2]
https://lore.kernel.org/rust-for-linux/20251026-nova-as-v1-1-60c78726462d at
nvidia.com/
> 
> Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
> ---
> Alexandre Courbot (5):
>       rust: transmute: add `from_bytes_prefix` family of methods
>       gpu: nova-core: vbios: use FromBytes for PmuLookupTable header
>       gpu: nova-core: vbios: use FromBytes for PcirStruct
>       gpu: nova-core: vbios: use FromBytes for BitHeader
>       gpu: nova-core: vbios: use FromBytes for NpdeStruct
> 
>  drivers/gpu/nova-core/vbios.rs | 137
++++++++++++++++-------------------------
>  rust/kernel/transmute.rs       |  60 ++++++++++++++++++
>  2 files changed, 113 insertions(+), 84 deletions(-)
> ---
> base-commit: 639291d7c30cec5cf0d9a79371021c2e4404cfc9
> change-id: 20251028-nova-vbios-frombytes-eb0cbb6a2f11
> 
> Best regards,

On Wed, Oct 29, 2025 at 12:07:35AM +0900, Alexandre Courbot
wrote:
> This series spawns from the discussion in [1], where it was noticed that
> several structures of the VBIOS could just be read using `FromBytes`
> instead of being built manually.
> 
> While implementing it, I also noticed that we were repeating the
> following pattern quite a bit in the code:
> 
>     slice
>         .get(..size_of::<TargetType>())
>         .and_then(TargetType::from_bytes_copy)
> 
> ... which begs for dedicated methods doing this in the `FromBytes`
> trait. This is what the first patch does, taking inspiration from a
> similar feature in the `zerocopy` crate.
> 
> The remaining patches leverage these new methods to simplify the VBIOS
> code a bit, accordingly.
> 
> If patch 1 is deemed a valid idea, I hope we can merge it into the DRM
> tree along with the rest of this series.
> 
> The base for this work is `drm-rust-next`, with [2] applied.
> 
> [1] https://lore.kernel.org/rust-for-linux/DDTRW1P2I4PB.10ZTZDY95JBC5 at
nvidia.com/
> [2]
https://lore.kernel.org/rust-for-linux/20251026-nova-as-v1-1-60c78726462d at
nvidia.com/
> 
> Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
Once the nit I suggested in patch 1 is discussed, for all patches:

Reviewed-by: Joel Fernandes <joelagnelf at nvidia.com>

thanks,

 - Joel
> ---
> Alexandre Courbot (5):
>       rust: transmute: add `from_bytes_prefix` family of methods
>       gpu: nova-core: vbios: use FromBytes for PmuLookupTable header
>       gpu: nova-core: vbios: use FromBytes for PcirStruct
>       gpu: nova-core: vbios: use FromBytes for BitHeader
>       gpu: nova-core: vbios: use FromBytes for NpdeStruct
> 
>  drivers/gpu/nova-core/vbios.rs | 137
++++++++++++++++-------------------------
>  rust/kernel/transmute.rs       |  60 ++++++++++++++++++
>  2 files changed, 113 insertions(+), 84 deletions(-)
> ---
> base-commit: 639291d7c30cec5cf0d9a79371021c2e4404cfc9
> change-id: 20251028-nova-vbios-frombytes-eb0cbb6a2f11
> 
> Best regards,
> -- 
> Alexandre Courbot <acourbot at nvidia.com>
>

On Wed Oct 29, 2025 at 12:07 AM JST, Alexandre Courbot
wrote:
> This series spawns from the discussion in [1], where it was noticed that
> several structures of the VBIOS could just be read using `FromBytes`
> instead of being built manually.
>
> While implementing it, I also noticed that we were repeating the
> following pattern quite a bit in the code:
>
>     slice
>         .get(..size_of::<TargetType>())
>         .and_then(TargetType::from_bytes_copy)
>
> ... which begs for dedicated methods doing this in the `FromBytes`
> trait. This is what the first patch does, taking inspiration from a
> similar feature in the `zerocopy` crate.
>
> The remaining patches leverage these new methods to simplify the VBIOS
> code a bit, accordingly.
>
> If patch 1 is deemed a valid idea, I hope we can merge it into the DRM
> tree along with the rest of this series.
>
> The base for this work is `drm-rust-next`, with [2] applied.
>
> [1] https://lore.kernel.org/rust-for-linux/DDTRW1P2I4PB.10ZTZDY95JBC5 at
nvidia.com/
> [2]
https://lore.kernel.org/rust-for-linux/20251026-nova-as-v1-1-60c78726462d at
nvidia.com/
>
> Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
Pushed to drm-rust-next, thanks for the reviews!