linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] gpu: nova-core: vbios: split out PmuLookupTableHeader from PmuLookupTable
@ 2025-07-27  9:48 Rhys Lloyd
  2025-07-27  9:49 ` [PATCH 2/2] gpu: nova-core: vbios: use offset_of in PmuLookupTableHeader::new Rhys Lloyd
  2025-07-29  4:34 ` [PATCH 1/2] gpu: nova-core: vbios: split out PmuLookupTableHeader from PmuLookupTable Alexandre Courbot
  0 siblings, 2 replies; 3+ messages in thread
From: Rhys Lloyd @ 2025-07-27  9:48 UTC (permalink / raw)
  To: acourbot, dakr
  Cc: Rhys Lloyd, rust-for-linux, airlied, simona, nouveau, dri-devel,
	linux-kernel

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

Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
---
 drivers/gpu/nova-core/vbios.rs | 56 +++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 5b5d9f38cbb3..a77d7a4c8595 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -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: 14ae91a81ec8fa0bc23170d4aa16dd2a20d54105
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] gpu: nova-core: vbios: use offset_of in PmuLookupTableHeader::new
  2025-07-27  9:48 [PATCH 1/2] gpu: nova-core: vbios: split out PmuLookupTableHeader from PmuLookupTable Rhys Lloyd
@ 2025-07-27  9:49 ` Rhys Lloyd
  2025-07-29  4:34 ` [PATCH 1/2] gpu: nova-core: vbios: split out PmuLookupTableHeader from PmuLookupTable Alexandre Courbot
  1 sibling, 0 replies; 3+ messages in thread
From: Rhys Lloyd @ 2025-07-27  9:49 UTC (permalink / raw)
  To: acourbot, dakr
  Cc: Rhys Lloyd, rust-for-linux, airlied, simona, nouveau, dri-devel,
	linux-kernel

Use the offset_of macro for each struct field, annotate the
`PmuLookupTableHeader` struct with `#[repr(C)]` attribute,
and add a TODO message to use FromBytes when available.

Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
---
 drivers/gpu/nova-core/vbios.rs | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index a77d7a4c8595..cedfcf3476bb 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -893,6 +893,7 @@ fn try_from(base: BiosImageBase) -> Result<Self> {
 ///
 /// See the [`PmuLookupTable`] description for more information.
 #[expect(dead_code)]
+#[repr(C)]
 struct PmuLookupTableHeader {
     version: u8,
     header_len: u8,
@@ -901,16 +902,17 @@ struct PmuLookupTableHeader {
 }
 
 impl PmuLookupTableHeader {
+    // TODO[TRSM]: use FromBytes::from_bytes when it becomes available.
     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],
+            version: data[const { core::mem::offset_of!(PmuLookupTableHeader, version) }],
+            header_len: data[const { core::mem::offset_of!(PmuLookupTableHeader, header_len) }],
+            entry_len: data[const { core::mem::offset_of!(PmuLookupTableHeader, entry_len) }],
+            entry_count: data[const { core::mem::offset_of!(PmuLookupTableHeader, entry_count) }],
         })
     }
 }
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] gpu: nova-core: vbios: split out PmuLookupTableHeader from PmuLookupTable
  2025-07-27  9:48 [PATCH 1/2] gpu: nova-core: vbios: split out PmuLookupTableHeader from PmuLookupTable Rhys Lloyd
  2025-07-27  9:49 ` [PATCH 2/2] gpu: nova-core: vbios: use offset_of in PmuLookupTableHeader::new Rhys Lloyd
@ 2025-07-29  4:34 ` Alexandre Courbot
  1 sibling, 0 replies; 3+ messages in thread
From: Alexandre Courbot @ 2025-07-29  4:34 UTC (permalink / raw)
  To: Rhys Lloyd, dakr
  Cc: rust-for-linux, airlied, simona, nouveau, dri-devel, linux-kernel

On Sun Jul 27, 2025 at 6:48 PM JST, Rhys Lloyd wrote:
> Separating the header allows the use of `size_of::<PmuLookupTableHeader>()`
> instead of the magic number 4.
>
> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
> ---
>  drivers/gpu/nova-core/vbios.rs | 56 +++++++++++++++++++++-------------
>  1 file changed, 35 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index 5b5d9f38cbb3..a77d7a4c8595 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -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>() {

We cannot rely on `size_of::<Self>` to be 4 if `Self` is not
`#[repr(C)]`. Since this is done in the second patch, I'd merge these
two into a single one for simplicity.


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-29  4:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-27  9:48 [PATCH 1/2] gpu: nova-core: vbios: split out PmuLookupTableHeader from PmuLookupTable Rhys Lloyd
2025-07-27  9:49 ` [PATCH 2/2] gpu: nova-core: vbios: use offset_of in PmuLookupTableHeader::new Rhys Lloyd
2025-07-29  4:34 ` [PATCH 1/2] gpu: nova-core: vbios: split out PmuLookupTableHeader from PmuLookupTable Alexandre Courbot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).