On 5/18/26 7:55 PM, Eliot Courtney wrote: > Push the computation of the falcon data offset into a helper function. > The subtraction to create the offset should be checked, and by doing > this the check can be folded into the existing check in > `falcon_data_ptr`. > > Signed-off-by: Eliot Courtney <[email protected]> > --- > drivers/gpu/nova-core/vbios.rs | 48 > +++++++++++++++++------------------------- > 1 file changed, 19 insertions(+), 29 deletions(-) >
Reviewed-by: John Hubbard <[email protected]> thanks, -- John Hubbard > diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs > index cadc6dcffefb..ca101b2b6095 100644 > --- a/drivers/gpu/nova-core/vbios.rs > +++ b/drivers/gpu/nova-core/vbios.rs > @@ -846,33 +846,29 @@ fn get_bit_token(&self, token_id: u8) -> > Result<BitToken> { > BitToken::from_id(self, token_id) > } > > - /// Find the Falcon data pointer structure in the [`PciAtBiosImage`]. > + /// Find the Falcon data offset from the start of the FWSEC region. > /// > - /// This is just a 4 byte structure that contains a pointer to the > Falcon data in the FWSEC > - /// image. > - fn falcon_data_ptr(&self) -> Result<u32> { > + /// The BIT table contains a 4-byte pointer to the Falcon data. Testing > shows this pointer > + /// treats the PCI-AT and FWSEC images as logically contiguous even when > an EFI image sits in > + /// between them, so subtract the PCI-AT image size here to convert it > to a FWSEC-relative > + /// offset. > + fn falcon_data_offset(&self) -> Result<usize> { > let token = self.get_bit_token(BIT_TOKEN_ID_FALCON_DATA)?; > - > - // Make sure we don't go out of bounds > - if usize::from(token.data_offset) + 4 > self.base.data.len() { > - return Err(EINVAL); > - } > - > - // read the 4 bytes at the offset specified in the token > let offset = usize::from(token.data_offset); > - let bytes: [u8; 4] = self.base.data[offset..offset + > 4].try_into().map_err(|_| { > - dev_err!(self.base.dev, "Failed to convert data slice to > array\n"); > - EINVAL > - })?; > > - let data_ptr = u32::from_le_bytes(bytes); > + // Read the 4-byte falcon data pointer at the offset specified in > the token. > + let data = &self.base.data; > + let (ptr, _) = data > + .get(offset..) > + .and_then(u32::from_bytes_copy_prefix) > + .ok_or(EINVAL)?; > > - if (usize::from_safe_cast(data_ptr)) < self.base.data.len() { > - dev_err!(self.base.dev, "Falcon data pointer out of bounds\n"); > - return Err(EINVAL); > - } > - > - Ok(data_ptr) > + usize::from_safe_cast(ptr) > + .checked_sub(data.len()) > + .ok_or(EINVAL) > + .inspect_err(|_| { > + dev_err!(self.base.dev, "Falcon data pointer out of > bounds\n"); > + }) > } > } > > @@ -989,15 +985,9 @@ fn setup_falcon_data( > pci_at_image: &PciAtBiosImage, > first_fwsec: &FwSecBiosBuilder, > ) -> Result { > - let mut offset = > usize::from_safe_cast(pci_at_image.falcon_data_ptr()?); > + let mut offset = pci_at_image.falcon_data_offset()?; > let mut pmu_in_first_fwsec = false; > > - // The falcon data pointer assumes that the PciAt and FWSEC images > - // are contiguous in memory. However, testing shows the EFI image > sits in > - // between them. So calculate the offset from the end of the PciAt > image > - // rather than the start of it. Compensate. > - offset -= pci_at_image.base.data.len(); > - > // The offset is now from the start of the first Fwsec image, however > // the offset points to a location in the second Fwsec image. Since > // the fwsec images are contiguous, subtract the length of the first > Fwsec >
