Re: [PATCH v2 11/11] gpu: nova-core: vbios: reject extra PCI-AT and FWSEC images

[Joel Fernandes]

On 4/16/2026 10:34 PM, Eliot Courtney wrote:
> On Wed Apr 15, 2026 at 9:02 AM JST, Joel Fernandes wrote:
>>
>>
>>> On Apr 14, 2026, at 7:40 PM, Timur Tabi <ttabi@xxxxxxxxxx> wrote:
>>>
>>> On Tue, 2026-04-14 at 20:54 +0900, Eliot Courtney wrote:
>>>> Currently, these are silently overwritten. Instead, error if they appear
>>>> twice in the VBIOS.
>>>
>>> How do you know that there can only be one PCI-AT image or no more than 2 FWSEC images?
>>
>> Also, which GPU is this extra PCI-AT observed on? When I wrote all this code, it was on GA102 and there was only one PCI-AT and 2 FWSEC.
> 
> I think that it doesn't make sense for there to be more than one PCI-AT
> image. I haven't observed any, it's just that the current code will
> silently accept it. I think that we should explicitly define the
> behaviour here - erroring seems the most conservative to me, but other
> behaviours could be take the first or take the last.

My opinion is I don't think it is useful to check for that, but I don't
feel strongly either way. PciAt image type is from the standard PCI BIOS
image format and even if it is overwritten, it will take the last one in
current code afaics.

> This is more of a problem for FWSEC images, because the current code
> will take the first image as `first_fwsec_image` and the last image as
> `second_fwsec_image`, which means that if there are more than two, the
> falcon data offset will actually point to the wrong thing, since the
> code assumes that first and second fwsec image are contiguous for the
> purposes of the offset pointer space.
> 
> AFAICT, nouveau and openrm take the first PCI-AT and FWSEC images, but
> they use the first FWSEC image to adjust the offset and then just
> address directly into the whole VBIOS based on that. To match that
> behaviour I can update this this patch to change the nova behaviour to
> match - i.e. take first two FWSEC and first PCI-AT, instead of the
> current behaviour which is last PCI-AT and first+last FWSEC.

My opinion is I don't see a benefit of changing it to that because
everything including Turing is working with current code. Note that vbios
parsing from nova is also legacy for Blackwell and later where the FSP
handles this. And I'd rather not make non-functional changes and risk
breaking working code.

Speaking of Turing, unless we have already done so, we should test this
series on Turing as well. ISTR, Timur had some corner cases with this area
of code.

Thanks.