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@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
Hi Alex,
Nice improvement, a nit:
On Wed, Oct 29, 2025 at 12:07:37AM +0900, Alexandre Courbot wrote:
[..]
> 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;
Can we change to the following, it is easier to read than using `.0` IMO.
let (header, _rest) = PmuLookupTableHeader::from_bytes_copy_prefix(data)
.ok_or(EINVAL)?;
(and similarly in the other patches).
thanks,
- Joel
On Tue Nov 4, 2025 at 5:04 AM JST, Joel Fernandes wrote:
> Hi Alex,
> Nice improvement, a nit:
>
> On Wed, Oct 29, 2025 at 12:07:37AM +0900, Alexandre Courbot wrote:
> [..]
>> 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;
>
> Can we change to the following, it is easier to read than using `.0` IMO.
>
> let (header, _rest) = PmuLookupTableHeader::from_bytes_copy_prefix(data)
> .ok_or(EINVAL)?;
>
> (and similarly in the other patches).
We can use `let (header, _) =` to make sure the unused remainder is not
bound to any variable. That also turns that statement into a one-liner.
Thanks!
On 11/3/2025 6:50 PM, Alexandre Courbot wrote:
> On Tue Nov 4, 2025 at 5:04 AM JST, Joel Fernandes wrote:
>> Hi Alex,
>> Nice improvement, a nit:
>>
>> On Wed, Oct 29, 2025 at 12:07:37AM +0900, Alexandre Courbot wrote:
>> [..]
>>> 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;
>>
>> Can we change to the following, it is easier to read than using `.0` IMO.
>>
>> let (header, _rest) = PmuLookupTableHeader::from_bytes_copy_prefix(data)
>> .ok_or(EINVAL)?;
>>
>> (and similarly in the other patches).
>
> We can use `let (header, _) =` to make sure the unused remainder is not
> bound to any variable. That also turns that statement into a one-liner.
>
Perfect, that's even better, thanks!
- Joel
© 2016 - 2026 Red Hat, Inc.