Re: [PATCH] PCI/VGA: Fix boot device detection after framebuffer relocation

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 29.10.26 um 00:15 schrieb Bjorn Helgaas:
> On Wed, Oct 28, 2026 at 05:46:34PM +0800, wuqianhai@loongson.cn wrote:
>> From: wuqianhai <wuqianhai@loongson.cn>
>>
>> PCI resource assignment can relocate the firmware framebuffer BAR, which
>> leaves the base address in screen_info stale.  The VGA arbiter then calls
>> screen_info_pci_dev() from vga_is_firmware_default(), which fails to match
>> the framebuffer against any PCI device's BARs.  As a result, the boot VGA
>> device is not recognized and the arbiter may select the wrong default
>> device.
>>
>> Fix this by calling screen_info_apply_fixups() in vga_arb_device_init()
>> before adding the VGA devices.  This relocates lfb_base to the assigned
>> BAR address so that vga_is_firmware_default() matches the boot device
>> again.
>>
>> Tested on a LoongArch platform, with vga_is_firmware_default()
>> additionally enabled for CONFIG_LOONGARCH via local changes.
>>
>> Fixes: 337bf13aa9ddab8ae47f2677ab75cf69ae33437b ("PCI/VGA: Replace vga_is_firmware_default() with a screen info check")
>> Signed-off-by: wuqianhai <wuqianhai@loongson.cn>
>> ---
>>   drivers/pci/vgaarb.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
>> index 3e58b2c05c85..aca594651f58 100644
>> --- a/drivers/pci/vgaarb.c
>> +++ b/drivers/pci/vgaarb.c
>> @@ -1527,6 +1527,14 @@ static int __init vga_arb_device_init(void)
>>   	int rc;
>>   	struct pci_dev *pdev;
>>   
>> +	/*
>> +	 * PCI resource assignment may have relocated the firmware
>> +	 * framebuffer BAR. Update screen_info's lfb_base so that
>> +	 * vga_is_firmware_default() can match it against the boot
>> +	 * device's relocated resource.
>> +	 */
>> +	screen_info_apply_fixups();
> I'm really confused about this.  vga_is_firmware_default() relies on
> screen_info_pci_dev(&sysfb_primary_display.screen):
>
>    vga_is_firmware_default
>      screen_info_pci_dev(&sysfb_primary_display.screen)
>        numres = screen_info_resources(&sysfb_primary_display.screen, res, ...)
>        for (i = 0; i < numres; ++i)
>          pdev = __screen_info_pci_dev(&res[i])
>
> screen_info_resources() returns resource structs with PCI bus
> addresses in them.  These are not valid CPU addresses, so that seems
> broken on any system where bus addresses are not the same as CPU
> addresses.

What we get from the firmware for the framebuffer is a memory range.  
  If the underlying PCI memory gets relocated during the boot, we need 
to update the framebuffer data accordingly. This has worked well for the 
last 20 yrs.


>
> screen_info_fixup_lfb() and screen_info_apply_fixups() may fix the
> issue you see, but I think the way this happens is questionable.

>
> screen_info_fixup_lfb() captures the framebuffer PCI bus address in
> screen_info_lfb_res_start.  Then screen_info_apply_fixups() compares
> that PCI bus address with the CPU address in pdev->resource[].  That's
> broken on any system where bus addresses are not the same as CPU
> addresses.  I think it's also broken because screen_info_lfb_res_start
> may not be at the beginning of the BAR.
>
> I think screen_info_fixup_lfb() should only capture the pci_dev, the
> BAR number, and the framebuffer offset into the BAR.  When we need the
> CPU or bus address, we can always compute it from those values.

This is not sufficient, I think. We did this until a while ago and got 
problems with relocated bars. IIRC aarch64 had a number of affected 
systems. See commit 2f29b5c23101 ("video: screen_info: Relocate 
framebuffers behind PCI bridges") for details.

I'm happy to improve that code though, if we can do something better.

Best regards
Thomas

>
>>   	rc = misc_register(&vga_arb_device);
>>   	if (rc < 0)
>>   		pr_err("error %d registering device\n", rc);
>> -- 
>> 2.34.1
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)