linux-kernel - Re: [PATCH] PCI/sysfs: add write attribute for boot

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1ff8fdfa-754b-312d-cb52-a5c31e3b3792@kylinos.cn>
Date:   Wed, 29 Sep 2021 09:45:58 +0800
From:   李真能 <lizhenneng@...inos.cn>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI/sysfs: add write attribute for boot_vga


在 2021/9/29 上午7:37, Bjorn Helgaas 写道:
> On Mon, Sep 27, 2021 at 11:45:59AM +0800, 李真能 wrote:
>> 在 2021/9/27 上午4:20, Bjorn Helgaas 写道:
>>> On Sun, Sep 26, 2021 at 03:15:39PM +0800, Zhenneng Li wrote:
>>>> Add writing attribute for boot_vga sys node,
>>>> so we can config default video display
>>>> output dynamically when there are two video
>>>> cards on a machine.
>>>>
>>>> Xorg server will determine running on which
>>>> video card based on boot_vga node's value.
>>> When you repost this, please take a look at the git commit log history
>>> and make yours similar.  Specifically, the subject should start with a
>>> capital letter, and the body should be rewrapped to fill 75
>>> characters.
>>>
>>> Please contrast this with the existing VGA arbiter.  See
>>> Documentation/gpu/vgaarbiter.rst.  It sounds like this may overlap
>>> with the VGA arbiter functionality, so this should explain why we need
>>> both and how they interact.
>> "Some "legacy" VGA devices implemented on PCI typically have the same
>> hard-decoded addresses as they did on ISA. When multiple PCI devices are
>> accessed at same time they need some kind of coordination. ", this is the
>> explain of config VGA_ARB, that is to say, some legacy vga devices need use
>> the same pci bus address, if user app(such as xorg) want access card A, but
>> card A and card B have same bus address,  then VGA agaarbiter will determine
>> will card to be accessed.
> Yes.  I think the arbiter also provides an interface for controlling
> the routing of these legacy resources.
>
> Your patch changes the kernel's idea of the default VGA device, but
> doesn't affect the resource routing, AFAICT.
>
>> And xorg will read boot_vga to determine which graphics card is the primary
>> graphics output device.
> Doesn't xorg also have its own mechanism for selecting which graphics
> device to use?
>
> Is the point here that you want to write the sysfs file to select the
> device instead of changing the xorg configuration?  If it's possible
> to configure xorg directly to use different devices, my inclination
> would be to use that instead of doing it via sysfs.
Thanks for reminding, Xorg has the config option "PrimaryGPU", it has 
the same func as boot_vga.
>
>> That is the difference about boot_vga and vgaarbiter.
>>
>>>> Signed-off-by: Zhenneng Li <lizhenneng@...inos.cn>
>>>> ---
>>>>    drivers/pci/pci-sysfs.c | 24 +++++++++++++++++++++++-
>>>>    1 file changed, 23 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
>>>> index 7bbf2673c7f2..a6ba19ce7adb 100644
>>>> --- a/drivers/pci/pci-sysfs.c
>>>> +++ b/drivers/pci/pci-sysfs.c
>>>> @@ -664,7 +664,29 @@ static ssize_t boot_vga_show(struct device *dev, struct device_attribute *attr,
>>>>    			  !!(pdev->resource[PCI_ROM_RESOURCE].flags &
>>>>    			     IORESOURCE_ROM_SHADOW));
>>>>    }
>>>> -static DEVICE_ATTR_RO(boot_vga);
>>>> +
>>>> +static ssize_t boot_vga_store(struct device *dev, struct device_attribute *attr,
>>>> +			      const char *buf, size_t count)
>>>> +{
>>>> +	unsigned long val;
>>>> +	struct pci_dev *pdev = to_pci_dev(dev);
>>>> +	struct pci_dev *vga_dev = vga_default_device();
>>>> +
>>>> +	if (kstrtoul(buf, 0, &val) < 0)
>>>> +		return -EINVAL;
>>>> +
>>>> +	if (val != 1)
>>>> +		return -EINVAL;
>>>> +
>>>> +	if (!capable(CAP_SYS_ADMIN))
>>>> +		return -EPERM;
>>>> +
>>>> +	if (pdev != vga_dev)
>>>> +		vga_set_default_device(pdev);
>>>> +
>>>> +	return count;
>>>> +}
>>>> +static DEVICE_ATTR_RW(boot_vga);
>>>>    static ssize_t pci_read_config(struct file *filp, struct kobject *kobj,
>>>>    			       struct bin_attribute *bin_attr, char *buf,
>>>> -- 
>>>> 2.25.1
>>>>
>>>>
>>>> No virus found
>>>> 		Checked by Hillstone Network AntiVirus