Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] VBIOS byte stream reconstruction from MMIO uses `to_ne_bytes()` and struct parsing uses native integer types, corrupting VBIOS reading on big-endian architectures. -- commit 1cc33aae3ba2ca65527c5d72b5ee976f3c73ac92 Author: Pedro Yudi Honda <[email protected]> drm/nova: use `zerocopy` in vbios.rs This commit replaces the `transmute::FromBytes` trait with its `zerocopy` equivalent for various VBIOS structures in vbios.rs. It also updates the call sites accordingly to use the new trait methods. > diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs > index e6d144587c4ee..a3c0c06f6818d 100644 > --- a/drivers/gpu/nova-core/vbios.rs > +++ b/drivers/gpu/nova-core/vbios.rs [ ... ] > @@ -359,7 +356,7 @@ pub(crate) fn fwsec_image(&self) -> &FwSecBiosImage { > } > > /// PCI Data Structure as defined in PCI Firmware Specification > -#[derive(Debug, Clone)] > +#[derive(Debug, Clone, FromBytes)] > #[repr(C)] > struct PcirStruct { > /// PCI Data Structure signature ("PCIR" or "NPDS") > signature: [u8; 4], > /// PCI Vendor ID (e.g., 0x10DE for NVIDIA) > vendor_id: u16, [Severity: High] This is a pre-existing issue, but since the VBIOS structures map directly to little-endian VBIOS data, does using native integer types like u16 cause the parsing to fail on big-endian architectures? Also, since VbiosIterator::read_more() converts MMIO 32-bit words back into a byte stream using word.to_ne_bytes(): word.to_ne_bytes() .iter() .try_for_each(|&b| self.data.push(b, GFP_KERNEL))?; doesn't this byte-swap the little-endian hardware register data on big-endian systems? Should explicit little-endian types (e.g., zerocopy::byteorder::little_endian::U16) and word.to_le_bytes() be used to ensure the correct byte order is maintained? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
