linux-kernel - Re: [PATCH v4] PCI/VGA: Make the vga_is_firmware

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <bc1e31ca-eb54-9696-792c-9749c0582167@loongson.cn>
Date:   Tue, 22 Aug 2023 09:19:28 +0800
From:   suijingfeng <suijingfeng@...ngson.cn>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, loongson-kernel@...ts.loongnix.cn,
        "Limonciello, Mario" <Mario.Limonciello@....com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v4] PCI/VGA: Make the vga_is_firmware_default() less
 arch-dependent

Hi,


On 2023/8/22 01:33, Bjorn Helgaas wrote:
> On Fri, Aug 18, 2023 at 09:48:46AM +0800, suijingfeng wrote:
>> On 2023/8/18 06:08, Bjorn Helgaas wrote:
>>>> +		if (resource_type(res) != IORESOURCE_MEM)
>>>> +			continue;
>>>> +
>>>> +		if (!res->start || !res->end)
>>>> +			continue;
>>>> +
>>>> +		if (res->start <= fb_start && fb_end <= res->end) {
>>>> +			pdev_boot_vga = pdev;
>>>> +
>>>> +			vgaarb_info(&pdev->dev,
>>>> +				    "BAR %d contains firmware FB\n", i);
>>> Print the BAR with %pR and include the framebuffer region from
>>> screen_info in the same format.
>> I do remember that you already told me to do this in V3, sorry for not
>> replying to you at V3. Most of the time, what you tell me is right.But here,
>> I think I need to explain. Because doing it that way will make the code line
>> too long,and it will exceed 80 characters in the column if we print too
>> much.
>> I believe that the vgaarb_info() at here is already the most compact and
>> simplest form. Printing the BAR with %pR is not absolute necessary, because
>> we can get the additional information by: $ lspci | grep VGA
>>
>> $ dmesg | grep 05:00.0
>> $ dmesg | grep 0000:03:00.0
>> $ dmesg | grep PCI
> Fair enough.  The BAR info is already there.  But I don't think the
> screen_info framebuffer data is in the dmesg log anywhere, and I think
> that would be useful.
>
> It's fine if dmesg lines or kernel printk lines exceed 80 columns.
>
>> Actually, I only add something that is absolute necessary.
>> Printing BAR with %pR and/or Printing the framebuffer region
>> is consider to only for *debugging* purpose.
> I think printing the framebuffer region is important because it makes
> it clear *why* we're selecting the device as the default VGA device.
> It's more than just debugging; it helps make the system more
> transparent and more understandable.

OK, I'm clear what to do next.
The printing will be added at the next version.

> Bjorn