gpu: nova-core: vbios: change PmuLookupTableEntry to relax alignment
Subsystems (4)
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
@@ -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.dataas 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
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
@@ -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_lenas usize ; +let entry_len = header.entry_lenas usize ; +let entry_count = header.entry_countas 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_lenas u8 , - entry_len: entry_lenas u8 , - entry_count: entry_countas 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 = (idxas usize ) * self.entry_lenas usize ; +let index = (idxas usize ) * self.header.entry_lenas usize ;PmuLookupTableEntry ::new (&self.table_data[index..]) }// find entry by type value fn find_entry_by_type (&self, entry_type:u8 ) ->Result <PmuLookupTableEntry > { -for iin 0 ..self.entry_count { +for iin 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
Alexandre Courbot
maintainerRe: [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.Signed-off-by Rhys Lloyd <[email protected]>--- drivers/gpu/nova-core/vbios.rs | 56 +++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 21 deletions(-)drivers/gpu/nova-core/vbios.rs +2
-0@@ -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.
PATCH 1/1 gpu: nova-core: vbios: use size_of instead of magic number
In reply to Rhys Lloyd12 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
@@ -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
Alexandre Courbot
maintainerRe: [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.Signed-off-by Rhys Lloyd <[email protected]>--- drivers/gpu/nova-core/vbios.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/nova-core/vbios.rs +0
-0
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 :))
Benno Lossin
reviewerdrivers/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) + } }