Re: [PATCH v4 5/6] gpu: nova-core: skip the IFR header if present

Alexandre Courbot
2026-04-17T03:52:55Z

On Fri Apr 17, 2026 at 8:31 AM JST, Timur Tabi wrote:
> 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 whatever reason, the IFR offset is not applied to PROM
> reads. Therefore, the search for the PCI expansion must skip the IFR
> header, if found.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
> drivers/gpu/nova-core/regs.rs | 21 ++++++++++++
> drivers/gpu/nova-core/vbios.rs | 60 +++++++++++++++++++++++++++++++++-
> 2 files changed, 80 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
> index 2f171a4ff9ba..0880d52e4527 100644
> --- a/drivers/gpu/nova-core/regs.rs
> +++ b/drivers/gpu/nova-core/regs.rs

Can you add the definitions in `vbios.rs`? They are not used outside of
that module, and are not exactly registers like the other definitions of
`regs.rs`.

Placing them under the `ga100` module also sounds like they are
exclusive to GA100, which is not the case as the vbios code reads them
unconditionally.

> @@ -537,4 +537,25 @@ pub(crate) mod ga100 {
> 0:0 display_disabled => bool;
> }
> }
> +
> + // IFR Header in VBIOS
> +
> + register! {
> + pub(crate) NV_PBUS_IFR_FMT_FIXED0(u32) @ 0x300000 {
> + 31:0 signature;
> + }
> + }
> +
> + register! {
> + pub(crate) NV_PBUS_IFR_FMT_FIXED1(u32) @ 0x300004 {
> + 30:16 fixed_data_size;
> + 15:8 version => u8;
> + }
> + }
> +
> + register! {
> + pub(crate) NV_PBUS_IFR_FMT_FIXED2(u32) @ 0x300008 {
> + 19:0 total_data_size;
> + }
> + }
> }
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index e726594eb130..7ebe58020533 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -25,6 +25,7 @@
> FalconUCodeDescV3, //
> },
> num::FromSafeCast,
> + regs::ga100, //
> };
>
> /// The offset of the VBIOS ROM in the BAR0 space.
> @@ -89,13 +90,70 @@ struct VbiosIterator<'a> {
> last_found: bool,
> }
>
> +/// IFR signature: ASCII "NVGI" as a little-endian u32.
> +const NV_PBUS_IFR_FMT_FIXED0_SIGNATURE_VALUE: u32 = 0x4947564E;

You can use

const NV_PBUS_IFR_FMT_FIXED0_SIGNATURE_VALUE: u32 = u32::from_le_bytes(*b"NVGI");

To make the signature explicit in the code (and simplify the doccomment
a bit).

> +/// ROM directory signature: ASCII "RFRD" as a little-endian u32.
> +const NV_ROM_DIRECTORY_IDENTIFIER: u32 = 0x44524652;

Same here.

> +/// 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 = 4096;

`SZ_4K`?

Also can you move these constants into the `current_offset` method, as
they are not used outside of it?

> +
> +/// 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 whatever 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 current_offset(dev: &device::Device, bar0: &Bar0) -> Result<usize> {

The doccomment is very informative. The function name... not so much. :)
Something like `vbios_rom_offset` maybe?

Also style nit: s/whatever/some

> + let signature = bar0.read(ga100::NV_PBUS_IFR_FMT_FIXED0).signature();
> +
> + if signature == NV_PBUS_IFR_FMT_FIXED0_SIGNATURE_VALUE {
> + let fixed1 = bar0.read(ga100::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(|v| v as usize)

.map(usize::from_safe_cast)

Each `as` should normally come with its own justifying comment, so
better use the infallible converters we introduced.

> + }
> + 3 => {
> + let fixed2 = bar0.read(ga100::NV_PBUS_IFR_FMT_FIXED2);
> + let total_data_size = usize::from(fixed2.total_data_size());
> + let dir_offset =
> + bar0.try_read32(ROM_OFFSET + total_data_size)? as usize + RFW_FLASH_STATUS_SIZE;

usize::from_safe_cast(bar0.try_read32(ROM_OFFSET + total_data_size)?) + RFW_FLASH_STATUS_SIZE;

(or feel free to use an intermediate value if that doesn't read well)

> + 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 + 8)

Where does this `8` come from? This should be a constant at the very least.

> + .map(|v| v as usize)

.map(usize::from_safe_cast)

With these fixed I expect v5 to be in a mergeable state.