| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <939129a9-0a93-4539-b651-b08fcddf57b5@kernel.org>
Date: Tue, 24 Jun 2025 11:48:45 -0500
From: Mario Limonciello <superm1@...nel.org>
To: Thomas Zimmermann <tzimmermann@...e.de>,
Bjorn Helgaas <bhelgaas@...gle.com>
Cc: Alex Deucher <alexander.deucher@....com>,
Christian König <christian.koenig@....com>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Lukas Wunner <lukas@...ner.de>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, David Woodhouse <dwmw2@...radead.org>,
Lu Baolu <baolu.lu@...ux.intel.com>, Joerg Roedel <joro@...tes.org>,
Will Deacon <will@...nel.org>, Robin Murphy <robin.murphy@....com>,
Alex Williamson <alex.williamson@...hat.com>,
Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
"open list:DRM DRIVERS" <dri-devel@...ts.freedesktop.org>,
open list <linux-kernel@...r.kernel.org>,
"open list:INTEL IOMMU (VT-d)" <iommu@...ts.linux.dev>,
"open list:PCI SUBSYSTEM" <linux-pci@...r.kernel.org>,
"open list:VFIO DRIVER" <kvm@...r.kernel.org>,
"open list:SOUND" <linux-sound@...r.kernel.org>,
Daniel Dadap <ddadap@...dia.com>,
Mario Limonciello <mario.limonciello@....com>
Subject: Re: [PATCH v4 7/8] PCI/VGA: Move check for firmware default out of
VGA arbiter
On 6/24/25 4:21 AM, Thomas Zimmermann wrote:
>
>
> Am 23.06.25 um 20:47 schrieb Mario Limonciello:
>> From: Mario Limonciello <mario.limonciello@....com>
>>
>> The x86 specific check for whether a framebuffer belongs to a device
>> works for display devices as well as VGA devices. Callers to
>> video_is_primary_device() can benefit from checking non-VGA display
>> devices.
>>
>> Move the x86 specific check into x86 specific code, and adjust VGA
>> arbiter to call that code as well. This allows fbcon to find the
>> right PCI device on systems that don't have VGA devices.
>>
>> Suggested-by: Thomas Zimmermann <tzimmermann@...e.de>
>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>> ---
>> v4:
>> * use helper
>> ---
>> arch/x86/video/video-common.c | 13 ++++++++++++-
>> drivers/pci/vgaarb.c | 36 ++---------------------------------
>> 2 files changed, 14 insertions(+), 35 deletions(-)
>>
>> diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video-
>> common.c
>> index 81fc97a2a837a..917568e4d7fb1 100644
>> --- a/arch/x86/video/video-common.c
>> +++ b/arch/x86/video/video-common.c
>> @@ -9,6 +9,7 @@
>> #include <linux/module.h>
>> #include <linux/pci.h>
>> +#include <linux/screen_info.h>
>> #include <linux/vgaarb.h>
>> #include <asm/video.h>
>> @@ -27,6 +28,7 @@ EXPORT_SYMBOL(pgprot_framebuffer);
>> bool video_is_primary_device(struct device *dev)
>> {
>> + struct screen_info *si = &screen_info;
>> struct pci_dev *pdev;
>> if (!dev_is_pci(dev))
>> @@ -34,7 +36,16 @@ bool video_is_primary_device(struct device *dev)
>> pdev = to_pci_dev(dev);
>> - return (pdev == vga_default_device());
>> + if (!pci_is_display(pdev))
>> + return false;
>> +
>> + if (pdev == vga_default_device())
>> + return true;
>> +
>> + if (pdev == screen_info_pci_dev(si))
>> + return true;
>> +
>> + return false;
>> }
>> EXPORT_SYMBOL(video_is_primary_device);
>> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
>> index 78748e8d2dbae..15ab58c70b016 100644
>> --- a/drivers/pci/vgaarb.c
>> +++ b/drivers/pci/vgaarb.c
>> @@ -26,12 +26,12 @@
>> #include <linux/poll.h>
>> #include <linux/miscdevice.h>
>> #include <linux/slab.h>
>> -#include <linux/screen_info.h>
>> #include <linux/vt.h>
>> #include <linux/console.h>
>> #include <linux/acpi.h>
>> #include <linux/uaccess.h>
>> #include <linux/vgaarb.h>
>> +#include <asm/video.h>
>> static void vga_arbiter_notify_clients(void);
>> @@ -554,38 +554,6 @@ void vga_put(struct pci_dev *pdev, unsigned int
>> rsrc)
>> }
>> EXPORT_SYMBOL(vga_put);
>> -static bool vga_is_firmware_default(struct pci_dev *pdev)
>> -{
>> -#if defined(CONFIG_X86)
>> - u64 base = screen_info.lfb_base;
>> - u64 size = screen_info.lfb_size;
>> - struct resource *r;
>> - u64 limit;
>> -
>> - /* Select the device owning the boot framebuffer if there is one */
>> -
>> - if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
>> - base |= (u64)screen_info.ext_lfb_base << 32;
>> -
>> - limit = base + size;
>> -
>> - /* Does firmware framebuffer belong to us? */
>> - pci_dev_for_each_resource(pdev, r) {
>> - if (resource_type(r) != IORESOURCE_MEM)
>> - continue;
>> -
>> - if (!r->start || !r->end)
>> - continue;
>> -
>> - if (base < r->start || limit >= r->end)
>> - continue;
>> -
>> - return true;
>> - }
>> -#endif
>> - return false;
>> -}
>> -
>> static bool vga_arb_integrated_gpu(struct device *dev)
>> {
>> #if defined(CONFIG_ACPI)
>> @@ -623,7 +591,7 @@ static bool vga_is_boot_device(struct vga_device
>> *vgadev)
>> if (boot_vga && boot_vga->is_firmware_default)
>> return false;
>> - if (vga_is_firmware_default(pdev)) {
>> + if (video_is_primary_device(&pdev->dev)) {
>
> Doesn't this generate a cyclic dependency between vgaarb and video? I
> find this call cycle hard to reason about because
> vgaarb_default_device() depends on the results of these boot-device
> tests. Maybe keep vga_is_firmware_default() and just replace its content
> with a call to screen_info_pci_dev().
> Best regards
> Thomas
OK, I'll do that, thanks.
>
>> vgadev->is_firmware_default = true;
>> return true;
>> }
>
Powered by blists - more mailing lists