Skip to contentSkip to conversation

gpu: nova-core: vbios: change PmuLookupTableEntry to relax alignment

PATCH Superseded Rhys Lloyd 3 participants 6 messages Jul 14, 2025 – Jul 16, 2025

Subsystems (4)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Rhys Lloyd

author
#0 lore raw

PATCH 1/1 gpu: nova-core: vbios: change PmuLookupTableEntry to relax alignment

Instead of the data field containing a u32 and changing the alignment, change data to [u8; 4] and convert to u32 with a helper function. Removes another magic number by making the struct the same size as the data it needs to read, allowing the use of size_of::<PmuLookupTableEntry>()

---
 drivers/gpu/nova-core/vbios.rs | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
drivers/gpu/nova-core/vbios.rs +9 -4
index 5b5d9f38cbb3..40244485283c 100644
@@ -896,21 +896,26 @@ fn try_from(base: BiosImageBase) -> Result<Self> {
 struct PmuLookupTableEntry {
     application_id: u8,
     target_id: u8,
-    data: u32,
+    data: [u8; 4],
 }
 
 impl PmuLookupTableEntry {
     fn new(data: &[u8]) -> Result<Self> {
-        if data.len() < 6 {
+        if data.len() < core::mem::size_of::<Self>() {
             return Err(EINVAL);
         }
 
         Ok(PmuLookupTableEntry {
             application_id: data[0],
             target_id: data[1],
-            data: u32::from_le_bytes(data[2..6].try_into().map_err(|_| EINVAL)?),
+            data: [data[2], data[3], data[4], data[5]],
         })
     }
+
+    /// Construct a u32 from `self.data`.
+    fn get_data(&self) -> u32 {
+        u32::from_le_bytes(self.data)
+    }
 }
 
 /// The [`PmuLookupTableEntry`] structure is used to find the [`PmuLookupTableEntry`] for a given
@@ -1037,7 +1042,7 @@ fn setup_falcon_data(
             .find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD)
         {
             Ok(entry) => {
-                let mut ucode_offset = entry.data as usize;
+                let mut ucode_offset = entry.get_data() as usize;
                 ucode_offset -= pci_at_image.base.data.len();
                 if ucode_offset < first_fwsec.base.data.len() {
                     dev_err!(pdev.as_ref(), "Falcon Ucode offset not in second Fwsec.\n");

base-commit: 215a3f91713383a3c0d2da82d223a608a3c17ac1
prerequisite-patch-id: d80f92d314a0693d4c89ffb7810d9ab6990336fa

Signature · 1 line
2.50.1
author
#1 lore raw

PATCH 1/1 gpu: nova-core: vbios: split out PmuLookupTableHeader from PmuLookupTable

Separating the header allows the use of size_of::<PmuLookupTableHeader>() instead of the magic number 4.

---
 drivers/gpu/nova-core/vbios.rs | 56 +++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 21 deletions(-)
drivers/gpu/nova-core/vbios.rs +35 -21
index 663fc50e8b66..20011c5c9bbc 100644
@@ -889,6 +889,32 @@ fn try_from(base: BiosImageBase) -> Result<Self> {
     }
 }
 
+/// The [`PmuLookupTableHeader`] structure is header information for [`PmuLookupTable`].
+///
+/// See the [`PmuLookupTable`] description for more information.
+#[expect(dead_code)]
+struct PmuLookupTableHeader {
+    version: u8,
+    header_len: u8,
+    entry_len: u8,
+    entry_count: u8,
+}
+
+impl PmuLookupTableHeader {
+    fn new(data: &[u8]) -> Result<Self> {
+        if data.len() < core::mem::size_of::<Self>() {
+            return Err(EINVAL);
+        }
+
+        Ok(PmuLookupTableHeader {
+            version: data[0],
+            header_len: data[1],
+            entry_len: data[2],
+            entry_count: data[3],
+        })
+    }
+}
+
 /// The [`PmuLookupTableEntry`] structure is a single entry in the [`PmuLookupTable`].
 ///
 /// See the [`PmuLookupTable`] description for more information.
@@ -918,24 +944,18 @@ fn new(data: &[u8]) -> Result<Self> {
 ///
 /// 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(pdev: &pci::Device, data: &[u8]) -> Result<Self> {
-        if data.len() < 4 {
-            return Err(EINVAL);
-        }
+        let header = PmuLookupTableHeader::new(data)?;
 
-        let header_len = data[1] as usize;
-        let entry_len = data[2] as usize;
-        let entry_count = data[3] as usize;
+        let header_len = header.header_len as usize;
+        let entry_len = header.entry_len as usize;
+        let entry_count = header.entry_count as usize;
 
         let required_bytes = header_len + (entry_count * entry_len);
 
@@ -963,27 +983,21 @@ fn new(pdev: &pci::Device, data: &[u8]) -> Result<Self> {
             );
         }
 
-        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 = (idx as usize) * self.entry_len as usize;
+        let index = (idx as usize) * self.header.entry_len as usize;
         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);

base-commit: 215a3f91713383a3c0d2da82d223a608a3c17ac1

Signature · 1 line
2.50.1
#2 lore raw

Re: [PATCH] gpu: nova-core: vbios: split out PmuLookupTableHeader from PmuLookupTable

Quoted 18 lines · Rhys Lloyd

Separating the header allows the use of size_of::<PmuLookupTableHeader>() instead of the magic number 4.

---
 drivers/gpu/nova-core/vbios.rs | 56 +++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 21 deletions(-)
drivers/gpu/nova-core/vbios.rs +2 -0
index 663fc50e8b66..20011c5c9bbc 100644
@@ -889,6 +889,32 @@ fn try_from(base: BiosImageBase) -> Result<Self> {
     }
 }
 
+/// The [`PmuLookupTableHeader`] structure is header information for [`PmuLookupTable`].
+///

drivers/gpu/nova-core/vbios.rs fn try_from(base: BiosImageBase) -> Result<Self> { · line 903

Quoted 10 lines
drivers/gpu/nova-core/vbios.rs +10 -0
+/// See the [`PmuLookupTable`] description for more information.
+#[expect(dead_code)]
+struct PmuLookupTableHeader {
+    version: u8,
+    header_len: u8,
+    entry_len: u8,
+    entry_count: u8,
+}
+
+impl PmuLookupTableHeader {

Can you add a

// TODO[TRSM]: use FromBytes::from_bytes when it becomes available.

as ultimately that's what we want to do.

(we are using these markers to keep track of tasks to complete as Rust features land, see Documentation/gpu/nova/core/todo.rst)

drivers/gpu/nova-core/vbios.rs fn try_from(base: BiosImageBase) -> Result<Self> { · line 910

Quoted 7 lines
drivers/gpu/nova-core/vbios.rs +7 -0
+    fn new(data: &[u8]) -> Result<Self> {
+        if data.len() < core::mem::size_of::<Self>() {
+            return Err(EINVAL);
+        }
+
+        Ok(PmuLookupTableHeader {
+            version: data[0],

How about making PmuLookupTableHeader #[repr(C)] and using offset_of!? This will also set the stage for the transition to using FromBytes::from_bytes.

author
#3 lore raw

PATCH 1/1 gpu: nova-core: vbios: use size_of instead of magic number

In reply to Rhys Lloyd

12 is identical to the value of size_of::<BitHeader>(), so use the latter instead.

---
 drivers/gpu/nova-core/vbios.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/nova-core/vbios.rs +1 -1
index 663fc50e8b66..ac35415b4ffb 100644
@@ -365,7 +365,7 @@ struct BitHeader {
 
 impl BitHeader {
     fn new(data: &[u8]) -> Result<Self> {
-        if data.len() < 12 {
+        if data.len() < core::mem::size_of::<Self>() {
             return Err(EINVAL);
         }
 

base-commit: 215a3f91713383a3c0d2da82d223a608a3c17ac1

Signature · 1 line
2.50.1
#4 lore raw

Re: [PATCH] gpu: nova-core: vbios: use size_of instead of magic number

Quoted 13 lines · Rhys Lloyd

12 is identical to the value of size_of::<BitHeader>(), so use the latter instead.

---
 drivers/gpu/nova-core/vbios.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/nova-core/vbios.rs +0 -0
index 663fc50e8b66..ac35415b4ffb 100644

drivers/gpu/nova-core/vbios.rs struct BitHeader { · line 368

Quoted 5 lines
drivers/gpu/nova-core/vbios.rs +1 -1
@@ -365,7 +365,7 @@ struct BitHeader {
 
 impl BitHeader {
     fn new(data: &[u8]) -> Result<Self> {
-        if data.len() < 12 {
+        if data.len() < core::mem::size_of::<Self>() {

I agree, but would feel better if we also made BitHeader #[repr(C)] to really guarantee this.

(or please educate me if this is unneeded :))

reviewer
#5 lore raw
In reply to Rhys Lloyd

drivers/gpu/nova-core/vbios.rs fn try_from(base: BiosImageBase) -> Result<Self> { · line 916

Quoted 3 lines · Rhys Lloyd
drivers/gpu/nova-core/vbios.rs +3 -0
+
+    /// Construct a u32 from `self.data`.
+    fn get_data(&self) -> u32 {

Getters in Rust usually don't start with get_. Using data(&self) here also looks better IMO.

---
Cheers,
Benno

Quoted 3 lines
drivers/gpu/nova-core/vbios.rs +2 -0
+        u32::from_le_bytes(self.data)
+    }
 }