Re: [PATCH v2] x86/video: Only fall back to vga_default_device() without screen info

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch unconditionally returns `false` if `screen_info` has memory resources that do not match any standard PCI BAR, breaking primary device detection for systems booting in legacy VGA text modes.
--

commit a8c04b0c2e4edb24a29bd696ebde27323036eb98
Author: Mario Limonciello <mario.limonciello@amd.com>

x86/video: Only fall back to vga_default_device() without screen info

This commit changes the primary video device detection on x86 to rely
exclusively on screen_info if available, and only falls back to
vga_default_device() when no screen info resources are present. This
prevents multiple devices from erroneously claiming the primary
boot_display attribute.

Closes: https://lore.kernel.org/linux-pci/20260618081803.2790848-1-aaron.ma@canonical.com/#t

> diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video-common.c
> index e0aeee99bc99e..e5fb3cf885fbd 100644
> --- a/arch/x86/video/video-common.c
> +++ b/arch/x86/video/video-common.c
[ ... ]
> @@ -43,21 +43,26 @@ bool video_is_primary_device(struct device *dev)
>  	if (!pci_is_display(pdev))
>  		return false;
>  
> -	if (pdev == vga_default_device())
> -		return true;
> -
>  #ifdef CONFIG_SCREEN_INFO
>  	numres = screen_info_resources(si, res, ARRAY_SIZE(res));
> -	for (i = 0; i < numres; ++i) {
> -		if (!(res[i].flags & IORESOURCE_MEM))
> -			continue;
> +	if (numres > 0) {
> +		for (i = 0; i < numres; ++i) {
> +			if (!(res[i].flags & IORESOURCE_MEM))
> +				continue;
> +
> +			if (pci_find_resource(pdev, &res[i]))
> +				return true;
> +		}
>  
> -		if (pci_find_resource(pdev, &res[i]))
> -			return true;
> +		return false;

[Severity: High]
Does this unconditional return break primary display detection for systems
booting via legacy BIOS or VGA text mode?

When booting in legacy VGA text mode, screen_info_resources() populates
resources with legacy memory ranges like 0xa0000 or 0xb8000. Since numres > 0,
the code enters this block.

However, legacy VGA windows are implicitly routed and not explicitly
represented as standard PCI BARs. Because of this, pci_find_resource() will
fail to match.

By returning false here, the code completely bypasses the fallback to
vga_default_device() for these legacy systems. Should this fall through to the
vga_default_device() check instead of returning false when the resources do
not match any standard PCI BAR?

>  	}
>  #endif
>  
> -	return false;
> +	/*
> +	 * No framebuffer was set up by the firmware/bootloader, so fall back
> +	 * to the default VGA device.
> +	 */
> +	return pdev == vga_default_device();
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623141505.1816786-1-mario.limonciello@amd.com?part=1