Alexandre Courbot
2025-Jul-16 01:28 UTC
[PATCH v2] gpu: nova-core: vbios: change PmuLookupTableEntry to relax alignment
On Tue Jul 15, 2025 at 12:22 AM JST, Joel Fernandes wrote:> > > On 7/14/2025 10:53 AM, Alice Ryhl wrote: >> On Mon, Jul 14, 2025 at 4:49?PM Joel Fernandes <joelagnelf at nvidia.com> wrote: >>> >>> Hello, Rhys, >>> >>> On Mon, Jul 14, 2025 at 04:02:23AM -0700, Rhys Lloyd wrote: >>>> 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>()` >>>> >>>> Signed-off-by: Rhys Lloyd <krakow20 at gmail.com> >>>> --- >>>> Changes in v2: >>>> - get_data helper function renamed to data >>>> >>>> --- >>>> drivers/gpu/nova-core/vbios.rs | 13 +++++++++---- >>>> 1 file changed, 9 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs >>>> index 5b5d9f38cbb3..339c66e63c7e 100644 >>>> --- a/drivers/gpu/nova-core/vbios.rs >>>> +++ b/drivers/gpu/nova-core/vbios.rs >>>> @@ -896,21 +896,26 @@ fn try_from(base: BiosImageBase) -> Result<Self> { >>>> struct PmuLookupTableEntry { >>>> application_id: u8, >>>> target_id: u8, >>>> - data: u32, >>>> + data: [u8; 4], >>> >>> Instead of this, could we make the struct as #repr[(C, packed)] or does that >>> not work for some reason? >> >> It would probably, but packed structs aren't very nice to work with >> because Rust has to be really careful to never generate a reference to >> unaligned fields. > Oh, interesting. I am Ok with the [u8; 4] then. Btw, we do have several > #[repr(C, packed)] in vbios.rs already.Yeah, in this particular case this is a module-local struct for which (AFAICT) we don't need to generate references to, so unless there are other issues I think making it packed and storing the properly-ordered u32 at construction time is both simpler and safer.