[PATCH] gpu: nova-core: vbios: use offset_of in PmuLookupTableHeader::new

[PATCH] gpu: nova-core: vbios: use offset_of in PmuLookupTableHeader::new

[Rhys Lloyd]

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

base-commit: 14ae91a81ec8fa0bc23170d4aa16dd2a20d54105
prerequisite-patch-id: 6f9311de987d56f4313d5fbdd85ed8c48a44e78c
-- 
2.50.1

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

[Rhys Lloyd]

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

Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
---
Changes in v2:
- Added `#[repr(C)]` to `BitHeader`

---
 drivers/gpu/nova-core/vbios.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 5b5d9f38cbb3..912698308102 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -346,6 +346,7 @@ fn image_size_bytes(&self) -> usize {
 /// [`FwSecBiosImage`].
 #[derive(Debug, Clone, Copy)]
 #[expect(dead_code)]
+#[repr(C)]
 struct BitHeader {
     /// 0h: BIT Header Identifier (BMP=0x7FFF/BIT=0xB8FF)
     id: u16,
@@ -365,7 +366,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: 14ae91a81ec8fa0bc23170d4aa16dd2a20d54105
--
2.50.1

[PATCH v3] gpu: nova-core: vbios: change PmuLookupTableEntry to use size_of

[Rhys Lloyd]

Annotate the PmuLookupTableEntry with an `#[repr(C, packed)]` attribute.
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>()`

Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
---
Changes in v2:
- get_data helper function renamed to data
Changes in v3:
- Change PmuLookupTableEntry.data back to u32
- Remove helper function
- Annotate `PmuLookupTableEntry` with `#[repr(C, packed)]`

---
 drivers/gpu/nova-core/vbios.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 5b5d9f38cbb3..4c8368946bd6 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, packed)]
 struct PmuLookupTableEntry {
     application_id: u8,
     target_id: u8,
@@ -901,7 +902,7 @@ struct PmuLookupTableEntry {

 impl PmuLookupTableEntry {
     fn new(data: &[u8]) -> Result<Self> {
-        if data.len() < 6 {
+        if data.len() < core::mem::size_of::<Self>() {
             return Err(EINVAL);
         }


base-commit: 14ae91a81ec8fa0bc23170d4aa16dd2a20d54105
--
2.50.1

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

[Alexandre Courbot]

On Fri Jul 18, 2025 at 4:36 PM JST, Rhys Lloyd wrote:
> 12 is identical to the value of `size_of::<BitHeader>()`,
> so use the latter instead.
>
> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>

Looking good - I'll push this to nova-next unless someone complains.
Thanks!

Re: [PATCH v3] gpu: nova-core: vbios: change PmuLookupTableEntry to use size_of

[Alexandre Courbot]

On Fri Jul 18, 2025 at 4:36 PM JST, Rhys Lloyd wrote:
> Annotate the PmuLookupTableEntry with an `#[repr(C, packed)]` attribute.
> 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>()`
>
> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>

Looking good - I'll also push this one after a short grace period. Thanks!

Re: [PATCH] gpu: nova-core: vbios: use offset_of in PmuLookupTableHeader::new

[Alexandre Courbot]

On Fri Jul 18, 2025 at 4:36 PM JST, Rhys Lloyd wrote:
> 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) }],

This chunk does not apply - on nova-next PmuLookupTableHeader does not
seem to exist. I think I remember you split PmuLookupTableHeader in
another patch, so can you send all the relevant patches as a series that
applies cleanly on top of nova-next?

Re: [PATCH] gpu: nova-core: vbios: use offset_of in PmuLookupTableHeader::new

[Danilo Krummrich]

On 7/25/25 3:35 PM, Alexandre Courbot wrote:
> This chunk does not apply - on nova-next PmuLookupTableHeader does not
> seem to exist. I think I remember you split PmuLookupTableHeader in
> another patch, so can you send all the relevant patches as a series that
> applies cleanly on top of nova-next?

If otherwise the series is ready, please wait for -rc1 to be out, and rebase on
-rc1.

Thanks,
Danilo

Re: [PATCH] gpu: nova-core: vbios: use offset_of in PmuLookupTableHeader::new

[Rhys Lloyd]

On Fri, Jul 25, 2025 at 7:04 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On 7/25/25 3:35 PM, Alexandre Courbot wrote:
> > This chunk does not apply - on nova-next PmuLookupTableHeader does not
> > seem to exist. I think I remember you split PmuLookupTableHeader in
> > another patch, so can you send all the relevant patches as a series that
> > applies cleanly on top of nova-next?
>
> If otherwise the series is ready, please wait for -rc1 to be out, and rebase on
> -rc1.
>
> Thanks,
> Danilo

Strange that it does not apply.  I'll figure out how to send a patch
series and resend it without any changes.