On 5/18/26 7:55 PM, Eliot Courtney wrote: > Move constants and functions to be inside the impls of the types they > are related to. This makes it more obvious what each type and value is > for. > > Signed-off-by: Eliot Courtney <[email protected]> > --- > Documentation/gpu/nova/core/vbios.rst | 2 +- > drivers/gpu/nova-core/vbios.rs | 185 > +++++++++++++++++----------------- > 2 files changed, 96 insertions(+), 91 deletions(-)
Reviewed-by: John Hubbard <[email protected]> thanks, -- John Hubbard > > diff --git a/Documentation/gpu/nova/core/vbios.rst > b/Documentation/gpu/nova/core/vbios.rst > index a4fe63422ede..9d3379ccfb30 100644 > --- a/Documentation/gpu/nova/core/vbios.rst > +++ b/Documentation/gpu/nova/core/vbios.rst > @@ -232,7 +232,7 @@ Falcon data in the VBIOS which contains the PMU lookup > table. This lookup table > used to find the required Falcon ucode based on an application ID. > > The location of the PMU lookup table is found by scanning the BIT (`BIOS > Information Table`_) > -tokens for a token with the id `BIT_TOKEN_ID_FALCON_DATA` (0x70) which > indicates the > +tokens for a token with the Falcon data token id (0x70) which indicates the > offset of the same from the start of the VBIOS image. Unfortunately, the > offset > does not account for the EFI image located between the PciAt and FwSec > images. > The `vbios.rs` code compensates for this with appropriate arithmetic. > diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs > index 9cc2f008bbfb..07b5235faff8 100644 > --- a/drivers/gpu/nova-core/vbios.rs > +++ b/drivers/gpu/nova-core/vbios.rs > @@ -27,16 +27,6 @@ > num::FromSafeCast, > }; > > -/// The offset of the VBIOS ROM in the BAR0 space. > -const ROM_OFFSET: usize = 0x300000; > -/// The maximum length of the VBIOS ROM to scan into. > -const BIOS_MAX_SCAN_LEN: usize = 0x100000; > -/// The size to read ahead when parsing initial BIOS image headers. > -const BIOS_READ_AHEAD_SIZE: usize = 1024; > -/// The bit in the last image indicator byte for the PCI Data Structure that > -/// indicates the last image. Bit 0-6 are reserved, bit 7 is last image bit. > -const LAST_IMAGE_BIT_MASK: u8 = 0x80; > - > /// BIOS Image Type from PCI Data Structure code_type field. > #[derive(Debug, Clone, Copy, PartialEq, Eq)] > #[repr(u8)] > @@ -65,14 +55,6 @@ fn try_from(code: u8) -> Result<Self> { > } > } > > -// PMU lookup table entry types. Used to locate PMU table entries > -// in the Fwsec image, corresponding to falcon ucodes. > -#[expect(dead_code)] > -const FALCON_UCODE_ENTRY_APPID_FIRMWARE_SEC_LIC: u8 = 0x05; > -#[expect(dead_code)] > -const FALCON_UCODE_ENTRY_APPID_FWSEC_DBG: u8 = 0x45; > -const FALCON_UCODE_ENTRY_APPID_FWSEC_PROD: u8 = 0x85; > - > /// Vbios Reader for constructing the VBIOS data. > struct VbiosIterator<'a> { > dev: &'a device::Device, > @@ -110,73 +92,79 @@ struct VbiosIterator<'a> { > } > } > > -/// Return the byte offset where the PCI Expansion ROM images begin in the > GPU's ROM. > -/// > -/// The GPU's ROM may begin with an Init-from-ROM (IFR) header that precedes > -/// the PCI Expansion ROM images (VBIOS). When present, the PROM shadow > -/// method must parse this header to determine the offset where the PCI ROM > -/// images actually begin, and adjust all subsequent reads accordingly. > -/// > -/// On most GPUs this is not needed because the IFR microcode has already > -/// applied the ROM offset so that PROM reads transparently skip the header. > -/// On GA100, for some reason, the IFR offset is not applied to PROM > -/// reads. Therefore, the search for the PCI expansion must skip the IFR > -/// header, if found. > -fn vbios_rom_offset(dev: &device::Device, bar0: &Bar0) -> Result<usize> { > - /// IFR signature. > - const NV_PBUS_IFR_FMT_FIXED0_SIGNATURE_VALUE: u32 = > u32::from_le_bytes(*b"NVGI"); > - /// ROM directory signature. > - const NV_ROM_DIRECTORY_IDENTIFIER: u32 = u32::from_le_bytes(*b"RFRD"); > - /// Offset of the NV_PMGR_ROM_ADDR_OFFSET register in IFR Extended > section. > - const IFR_SW_EXT_ROM_ADDR_OFFSET: usize = 4; > - /// Size of Redundant Firmware Flash Status section. > - const RFW_FLASH_STATUS_SIZE: usize = SZ_4K; > - /// Offset in the ROM Directory of the PCI Option ROM offset > - const PCI_OPTION_ROM_OFFSET: usize = 8; > - > - let signature = bar0.read(NV_PBUS_IFR_FMT_FIXED0).signature(); > - > - if signature == NV_PBUS_IFR_FMT_FIXED0_SIGNATURE_VALUE { > - let fixed1 = bar0.read(NV_PBUS_IFR_FMT_FIXED1); > - > - match fixed1.version() { > - 1 | 2 => { > - let fixed_data_size = usize::from(fixed1.fixed_data_size()); > - let pmgr_rom_addr_offset = fixed_data_size + > IFR_SW_EXT_ROM_ADDR_OFFSET; > - bar0.try_read32(ROM_OFFSET + pmgr_rom_addr_offset) > - .map(usize::from_safe_cast) > - } > - 3 => { > - let fixed2 = bar0.read(NV_PBUS_IFR_FMT_FIXED2); > - let total_data_size = usize::from(fixed2.total_data_size()); > - let flash_status_offset = > - usize::from_safe_cast(bar0.try_read32(ROM_OFFSET + > total_data_size)?); > - let dir_offset = flash_status_offset + RFW_FLASH_STATUS_SIZE; > - let dir_sig = bar0.try_read32(ROM_OFFSET + dir_offset)?; > - if dir_sig != NV_ROM_DIRECTORY_IDENTIFIER { > - dev_err!(dev, "could not find IFR ROM directory\n"); > - return Err(EINVAL); > - } > - bar0.try_read32(ROM_OFFSET + dir_offset + > PCI_OPTION_ROM_OFFSET) > - .map(usize::from_safe_cast) > - } > - _ => { > - dev_err!(dev, "unsupported IFR header version {}\n", > fixed1.version()); > - Err(EINVAL) > - } > - } > - } else { > - Ok(0) > - } > -} > - > impl<'a> VbiosIterator<'a> { > + /// The offset of the VBIOS ROM in the BAR0 space. > + const ROM_OFFSET: usize = 0x300000; > + /// The maximum length of the VBIOS ROM to scan into. > + const BIOS_MAX_SCAN_LEN: usize = 0x100000; > + /// The size to read ahead when parsing initial BIOS image headers. > + const BIOS_READ_AHEAD_SIZE: usize = 1024; > + > + /// Return the byte offset where the PCI Expansion ROM images begin in > the GPU's ROM. > + /// > + /// The GPU's ROM may begin with an Init-from-ROM (IFR) header that > precedes the PCI Expansion > + /// ROM images (VBIOS). When present, the PROM shadow method must parse > this header to determine > + /// the offset where the PCI ROM images actually begin, and adjust all > subsequent reads > + /// accordingly. > + /// > + /// On most GPUs this is not needed because the IFR microcode has > already applied the ROM offset > + /// so that PROM reads transparently skip the header. On GA100, for some > reason, the IFR offset > + /// is not applied to PROM reads. Therefore, the search for the PCI > expansion must skip the IFR > + /// header, if found. > + fn rom_offset(dev: &device::Device, bar0: &Bar0) -> Result<usize> { > + /// IFR signature. > + const NV_PBUS_IFR_FMT_FIXED0_SIGNATURE_VALUE: u32 = > u32::from_le_bytes(*b"NVGI"); > + /// ROM directory signature. > + const NV_ROM_DIRECTORY_IDENTIFIER: u32 = > u32::from_le_bytes(*b"RFRD"); > + /// Offset of the NV_PMGR_ROM_ADDR_OFFSET register in IFR Extended > section. > + const IFR_SW_EXT_ROM_ADDR_OFFSET: usize = 4; > + /// Size of Redundant Firmware Flash Status section. > + const RFW_FLASH_STATUS_SIZE: usize = SZ_4K; > + /// Offset in the ROM Directory of the PCI Option ROM offset. > + const PCI_OPTION_ROM_OFFSET: usize = 8; > + > + let signature = bar0.read(NV_PBUS_IFR_FMT_FIXED0).signature(); > + > + if signature == NV_PBUS_IFR_FMT_FIXED0_SIGNATURE_VALUE { > + let fixed1 = bar0.read(NV_PBUS_IFR_FMT_FIXED1); > + > + match fixed1.version() { > + 1 | 2 => { > + let fixed_data_size = > usize::from(fixed1.fixed_data_size()); > + let pmgr_rom_addr_offset = fixed_data_size + > IFR_SW_EXT_ROM_ADDR_OFFSET; > + bar0.try_read32(Self::ROM_OFFSET + pmgr_rom_addr_offset) > + .map(usize::from_safe_cast) > + } > + 3 => { > + let fixed2 = bar0.read(NV_PBUS_IFR_FMT_FIXED2); > + let total_data_size = > usize::from(fixed2.total_data_size()); > + let flash_status_offset = > + > usize::from_safe_cast(bar0.try_read32(Self::ROM_OFFSET + total_data_size)?); > + let dir_offset = flash_status_offset + > RFW_FLASH_STATUS_SIZE; > + let dir_sig = bar0.try_read32(Self::ROM_OFFSET + > dir_offset)?; > + if dir_sig != NV_ROM_DIRECTORY_IDENTIFIER { > + dev_err!(dev, "could not find IFR ROM directory\n"); > + return Err(EINVAL); > + } > + bar0.try_read32(Self::ROM_OFFSET + dir_offset + > PCI_OPTION_ROM_OFFSET) > + .map(usize::from_safe_cast) > + } > + _ => { > + dev_err!(dev, "unsupported IFR header version {}\n", > fixed1.version()); > + Err(EINVAL) > + } > + } > + } else { > + Ok(0) > + } > + } > + > fn new(dev: &'a device::Device, bar0: &'a Bar0) -> Result<Self> { > Ok(Self { > dev, > bar0, > data: KVec::new(), > - current_offset: vbios_rom_offset(dev, bar0)?, > + current_offset: Self::rom_offset(dev, bar0)?, > last_found: false, > }) > } > @@ -186,7 +174,7 @@ fn read_more(&mut self, len: usize) -> Result { > let start = self.data.len(); > let end = start + len; > > - if end > BIOS_MAX_SCAN_LEN { > + if end > Self::BIOS_MAX_SCAN_LEN { > dev_err!(self.dev, "Error: exceeded BIOS scan limit.\n"); > return Err(EINVAL); > } > @@ -205,7 +193,7 @@ fn read_more(&mut self, len: usize) -> Result { > // Read ROM data bytes and push directly to `data`. > for addr in (start..end).step_by(core::mem::size_of::<u32>()) { > // Read 32-bit word from the VBIOS ROM > - let word = self.bar0.try_read32(ROM_OFFSET + addr)?; > + let word = self.bar0.try_read32(Self::ROM_OFFSET + addr)?; > > // Convert the `u32` to a 4 byte array and push each byte. > word.to_ne_bytes() > @@ -267,7 +255,7 @@ fn next(&mut self) -> Option<Self::Item> { > return None; > } > > - if self.current_offset >= BIOS_MAX_SCAN_LEN { > + if self.current_offset >= Self::BIOS_MAX_SCAN_LEN { > dev_err!(self.dev, "Error: exceeded BIOS scan limit, stopping > scan\n"); > return None; > } > @@ -275,7 +263,7 @@ fn next(&mut self) -> Option<Self::Item> { > // Parse image headers first to get image size. > let image_size = match self.read_bios_image_at_offset( > self.current_offset, > - BIOS_READ_AHEAD_SIZE, > + Self::BIOS_READ_AHEAD_SIZE, > "parse initial BIOS image headers", > ) { > Ok(image) => image.image_size_bytes(), > @@ -416,6 +404,9 @@ struct PcirStruct { > unsafe impl FromBytes for PcirStruct {} > > impl PcirStruct { > + /// The bit in `last_image` that indicates the last image. > + const LAST_IMAGE_BIT_MASK: u8 = 0x80; > + > fn new(dev: &device::Device, data: &[u8]) -> Result<Self> { > let (pcir, _) = > PcirStruct::from_bytes_copy_prefix(data).ok_or(EINVAL)?; > > @@ -439,7 +430,7 @@ fn new(dev: &device::Device, data: &[u8]) -> Result<Self> > { > > /// Check if this is the last image in the ROM. > fn is_last(&self) -> bool { > - self.last_image & LAST_IMAGE_BIT_MASK != 0 > + self.last_image & Self::LAST_IMAGE_BIT_MASK != 0 > } > > /// Calculate image size in bytes from 512-byte blocks. > @@ -505,10 +496,10 @@ struct BitToken { > // SAFETY: all bit patterns are valid for `BitToken`. > unsafe impl FromBytes for BitToken {} > > -// Define the token ID for the Falcon data > -const BIT_TOKEN_ID_FALCON_DATA: u8 = 0x70; > - > impl BitToken { > + /// BIT token ID for Falcon data. > + const ID_FALCON_DATA: u8 = 0x70; > + > /// Find a BIT token entry by BIT ID in a PciAtBiosImage > fn from_id(image: &PciAtBiosImage, token_id: u8) -> Result<Self> { > let header = &image.bit_header; > @@ -604,6 +595,9 @@ struct NpdeStruct { > unsafe impl FromBytes for NpdeStruct {} > > impl NpdeStruct { > + /// The bit in `last_image` that indicates the last image. > + const LAST_IMAGE_BIT_MASK: u8 = 0x80; > + > fn new(dev: &device::Device, data: &[u8]) -> Option<Self> { > let (npde, _) = NpdeStruct::from_bytes_copy_prefix(data)?; > > @@ -627,7 +621,7 @@ fn new(dev: &device::Device, data: &[u8]) -> Option<Self> > { > > /// Check if this is the last image in the ROM. > fn is_last(&self) -> bool { > - self.last_image & LAST_IMAGE_BIT_MASK != 0 > + self.last_image & Self::LAST_IMAGE_BIT_MASK != 0 > } > > /// Calculate image size in bytes from 512-byte blocks. > @@ -799,7 +793,7 @@ fn get_bit_token(&self, token_id: u8) -> Result<BitToken> > { > /// 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)?; > + let token = self.get_bit_token(BitToken::ID_FALCON_DATA)?; > let offset = usize::from(token.data_offset); > > // Read the 4-byte falcon data pointer at the offset specified in > the token. > @@ -846,6 +840,17 @@ struct PmuLookupTableEntry { > // SAFETY: all bit patterns are valid for `PmuLookupTableEntry`. > unsafe impl FromBytes for PmuLookupTableEntry {} > > +impl PmuLookupTableEntry { > + /// PMU lookup table application ID for firmware security license ucode. > + #[expect(dead_code)] > + const APPID_FIRMWARE_SEC_LIC: u8 = 0x05; > + /// PMU lookup table application ID for debug FWSEC ucode. > + #[expect(dead_code)] > + const APPID_FWSEC_DBG: u8 = 0x45; > + /// PMU lookup table application ID for production FWSEC ucode. > + const APPID_FWSEC_PROD: u8 = 0x85; > +} > + > #[repr(C)] > struct PmuLookupTableHeader { > version: u8, > @@ -923,7 +928,7 @@ fn new( > let pmu_lookup_table = PmuLookupTable::new(&second_fwsec.dev, > pmu_lookup_data)?; > > let entry = pmu_lookup_table > - .find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD) > + .find_entry_by_type(PmuLookupTableEntry::APPID_FWSEC_PROD) > .inspect_err(|e| { > dev_err!( > second_fwsec.dev, >
