| 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: <8f9efe23-c774-477d-97ad-8e22532ad6b2@kernel.org>
Date: Mon, 30 Jun 2025 14:37:06 -0400
From: Mario Limonciello <superm1@...nel.org>
To: Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>
Cc: Alex Deucher <alexander.deucher@....com>,
Christian König <christian.koenig@....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>,
Bjorn Helgaas <bhelgaas@...gle.com>
Subject: Re: [PATCH v6 9/9] PCI: Add a new 'boot_display' attribute
On 6/30/2025 2:24 AM, Thomas Zimmermann wrote:
> Hi
>
> Am 27.06.25 um 17:37 schrieb Mario Limonciello:
>> On 6/27/2025 2:07 AM, Thomas Zimmermann wrote:
>>> Hi
>>>
>>> Am 27.06.25 um 06:31 schrieb Mario Limonciello:
>>>> From: Mario Limonciello <mario.limonciello@....com>
>>>>
>>>> On systems with multiple GPUs there can be uncertainty which GPU is the
>>>> primary one used to drive the display at bootup. In order to
>>>> disambiguate
>>>> this add a new sysfs attribute 'boot_display' that uses the output of
>>>> video_is_primary_device() to populate whether a PCI device was used for
>>>> driving the display.
>>>>
>>>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>>>> ---
>>>> v6:
>>>> * Only show for the device that is boot display
>>>> * Only create after PCI device sysfs files are initialized to ensure
>>>> that resources are ready.
>>>> v4:
>>>> * new patch
>>>> ---
>>>> Documentation/ABI/testing/sysfs-bus-pci | 8 +++++
>>>> drivers/pci/pci-sysfs.c | 46 +++++++++++++++++++++
>>>> ++++
>>>
>>> The code looks good. Just one more question: could this be added
>>> independently from the PCI bus (at a reasonable cost)? There are
>>> other busses that can host the boot display. Alternatively, we'd add
>>> this attribute per bus as needed.
>>
>> It depends upon the underlying hardware implementation. On x86 it's
>> always PCI and so I realized there is a requirement that PCI resources
>> are setup before screen_info event works.
>>
>> That is the v5 version of this patch would have had a potential race
>> condition with userspace where boot_display didn't always show '1' if
>> userspace read it too quickly.
>>
>> Other architecture's hardware implementation might have similar problem.
>>
>> So in summary I think it would be better to do it per-bus. If we
>> realize there is indeed code duplication we can always move this to a
>> common helper at that point.
>
> Ok, makes sense. With the kernel test robot's issues fixed:
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@...e.de>
Thanks, I've got a fix locally for it.
>
> I guess that interface also needs some sort of OK from user-space devs?
>
Who needs to OK it? I do have MR's for matching userspace
implementations mentioned in the cover letter already.
> Best regards
> Thomas
>
>>
>>>
>>> Best regards
>>> Thomas
>>>
>>>> 2 files changed, 54 insertions(+)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/
>>>> Documentation/ ABI/testing/sysfs-bus-pci
>>>> index 69f952fffec72..8b455b1a58852 100644
>>>> --- a/Documentation/ABI/testing/sysfs-bus-pci
>>>> +++ b/Documentation/ABI/testing/sysfs-bus-pci
>>>> @@ -612,3 +612,11 @@ Description:
>>>> # ls doe_features
>>>> 0001:01 0001:02 doe_discovery
>>>> +
>>>> +What: /sys/bus/pci/devices/.../boot_display
>>>> +Date: October 2025
>>>> +Contact: Linux PCI developers <linux-pci@...r.kernel.org>
>>>> +Description:
>>>> + This file indicates the device was used as a boot
>>>> + display. If the device was used as the boot display, the file
>>>> + will be present and contain "1".
>>>> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
>>>> index 268c69daa4d57..cc766461de1da 100644
>>>> --- a/drivers/pci/pci-sysfs.c
>>>> +++ b/drivers/pci/pci-sysfs.c
>>>> @@ -30,6 +30,7 @@
>>>> #include <linux/msi.h>
>>>> #include <linux/of.h>
>>>> #include <linux/aperture.h>
>>>> +#include <asm/video.h>
>>>> #include "pci.h"
>>>> #ifndef ARCH_PCI_DEV_GROUPS
>>>> @@ -679,6 +680,13 @@ const struct attribute_group *pcibus_groups[] = {
>>>> NULL,
>>>> };
>>>> +static ssize_t boot_display_show(struct device *dev, struct
>>>> device_attribute *attr,
>>>> + char *buf)
>>>> +{
>>>> + return sysfs_emit(buf, "1\n");
>>>> +}
>>>> +static DEVICE_ATTR_RO(boot_display);
>>>> +
>>>> static ssize_t boot_vga_show(struct device *dev, struct
>>>> device_attribute *attr,
>>>> char *buf)
>>>> {
>>>> @@ -1246,6 +1254,37 @@ static int pci_create_attr(struct pci_dev
>>>> *pdev, int num, int write_combine)
>>>> return 0;
>>>> }
>>>> +/**
>>>> + * pci_create_boot_display_file - create a file in sysfs for @dev
>>>> + * @pdev: dev in question
>>>> + *
>>>> + * Creates a file `boot_display` in sysfs for the PCI device @pdev
>>>> + * if it is the boot display device.
>>>> + */
>>>> +static int pci_create_boot_display_file(struct pci_dev *pdev)
>>>> +{
>>>> +#ifdef CONFIG_VIDEO
>>>> + if (video_is_primary_device(&pdev->dev))
>>>> + return sysfs_create_file(&pdev->dev.kobj,
>>>> &dev_attr_boot_display.attr);
>>>> +#endif
>>>> + return 0;
>>>> +}
>>>> +
>>>> +/**
>>>> + * pci_remove_boot_display_file - remove the boot display file for
>>>> @dev
>>>> + * @pdev: dev in question
>>>> + *
>>>> + * Removes the file `boot_display` in sysfs for the PCI device @pdev
>>>> + * if it is the boot display device.
>>>> + */
>>>> +static void pci_remove_boot_display_file(struct pci_dev *pdev)
>>>> +{
>>>> +#ifdef CONFIG_VIDEO
>>>> + if (video_is_primary_device(&pdev->dev))
>>>> + sysfs_remove_file(&pdev->dev.kobj,
>>>> &dev_attr_boot_display.attr);
>>>> +#endif
>>>> +}
>>>> +
>>>> /**
>>>> * pci_create_resource_files - create resource files in sysfs for
>>>> @dev
>>>> * @pdev: dev in question
>>>> @@ -1654,9 +1693,15 @@ static const struct attribute_group
>>>> pci_dev_resource_resize_group = {
>>>> int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
>>>> {
>>>> + int retval;
>>>> +
>>>> if (!sysfs_initialized)
>>>> return -EACCES;
>>>> + retval = pci_create_boot_display_file(pdev);
>>>> + if (retval)
>>>> + return retval;
>>>> +
>>>> return pci_create_resource_files(pdev);
>>>> }
>>>> @@ -1671,6 +1716,7 @@ void pci_remove_sysfs_dev_files(struct pci_dev
>>>> *pdev)
>>>> if (!sysfs_initialized)
>>>> return;
>>>> + pci_remove_boot_display_file(pdev);
>>>> pci_remove_resource_files(pdev);
>>>> }
>>>
>>
>
Powered by blists - more mailing lists