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

["Eliot Courtney" <ecourtney@nvidia.com>]

On Sat Apr 11, 2026 at 5:37 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.

I like this commit message a lot because it made it really easy for me
to understand why we are doing this. Could we concisely put a comment in
the code as well explaining why it's necessary?

>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  drivers/gpu/nova-core/vbios.rs | 39 +++++++++++++++++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index e726594eb130..a7ec68448a4a 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -89,13 +89,50 @@ struct VbiosIterator<'a> {
>      last_found: bool,
>  }
>  
> +/// IFR signature: ASCII "NVGI" as a little-endian u32.
> +const IFR_SIGNATURE: u32 = 0x4947564E;
> +/// ROM directory signature: ASCII "RFRD" as a little-endian u32.
> +const ROM_DIRECTORY_SIGNATURE: u32 = 0x44524652;

I wonder if we can use the bindings for some of these constants,
including the fixed value offsets below. I found these values in OpenRM
in dev_bus.h, for example (but it was in tu102 so even though the values
are the same not sure if it's semantically correct):

```
#define NV_PBUS_IFR_FMT_FIXED0                        0x00000000 /*       */
#define NV_PBUS_IFR_FMT_FIXED0_SIGNATURE                    31:0 /*       */
#define NV_PBUS_IFR_FMT_FIXED0_SIGNATURE_VALUE        0x4947564E /*       */
#define NV_PBUS_IFR_FMT_FIXED1                        0x00000004 /*       */
#define NV_PBUS_IFR_FMT_FIXED1_VERSIONSW                    15:8 /*       */
#define NV_PBUS_IFR_FMT_FIXED1_FIXED_DATA_SIZE             30:16 /*       */
#define NV_PBUS_IFR_FMT_FIXED2                        0x00000008 /*       */
#define NV_PBUS_IFR_FMT_FIXED2_TOTAL_DATA_SIZE              19:0 /*       */
```

Unfortunately IIUC we can't use the bitfield range specifies via
bindgen, but the FIXEDX offsets and some of the signature values might
be doable via bindings. WDYT?

> +
>  impl<'a> VbiosIterator<'a> {
>      fn new(dev: &'a device::Device, bar0: &'a Bar0) -> Result<Self> {
> +        let sig = bar0.try_read32(ROM_OFFSET)?;
> +
> +        let current_offset = if sig == IFR_SIGNATURE {
> +            let fixed1 = bar0.try_read32(ROM_OFFSET + 4)?;
> +            let version = ((fixed1 >> 8) & 0xff) as u8;
> +
> +            match version {
> +                // Note: We do not actually expect to see v1 or v2 on these GPUs
> +                1 | 2 => {
> +                    let fixed_data_size = ((fixed1 >> 16) & 0x7fff) as usize;
> +                    bar0.try_read32(ROM_OFFSET + fixed_data_size + 4)? as usize

Should this be an error instead if we don't expect to see it?

> +                }
> +                3 => {
> +                    let fixed2 = bar0.try_read32(ROM_OFFSET + 8)?;
> +                    let total_data_size = (fixed2 & 0x000f_ffff) as usize;

What about using the bitfield! macro to define bitfields for these
things? e.g.

```
bitfield! {
    struct IfrFixed1(u32) {
        15:8 version as u8;
        30:16 fixed_data_size as u16;
    }
}

bitfield! {
    struct IfrFixed2(u32) {
        19:0 total_data_size as u32;
    }
}
```

> +                    let dir_offset = bar0.try_read32(ROM_OFFSET + total_data_size)? as usize + 4096;

What about making an inline constant for 4096?

> +                    let dir_sig = bar0.try_read32(ROM_OFFSET + dir_offset)?;
> +                    if dir_sig != ROM_DIRECTORY_SIGNATURE {
> +                        dev_err!(dev, "could not find IFR ROM directory\n");
> +                        return Err(EINVAL);
> +                    }
> +                    bar0.try_read32(ROM_OFFSET + dir_offset + 8)? as usize
> +                }
> +                _ => {
> +                    dev_err!(dev, "unsupported IFR header version {}\n", version);
> +                    return Err(EINVAL);
> +                }
> +            }
> +        } else {
> +            0
> +        };
> +
>          Ok(Self {
>              dev,
>              bar0,
>              data: KVec::new(),
> -            current_offset: 0,
> +            current_offset,
>              last_found: false,
>          })
>      }