linux-kernel - Re: [PATCH] PCI/VGA: Don't assume only VGA device found is the boot VGA device

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: <eef7035c-2ffc-485f-b123-fffd3719be28@app.fastmail.com>
Date: Wed, 23 Oct 2024 10:27:49 +0200
From: "Luke Jones" <luke@...nes.dev>
To: "Kai-Heng Feng" <kaihengf@...dia.com>,
 "Alex Deucher" <alexdeucher@...il.com>
Cc: "Mario Limonciello" <superm1@...nel.org>,
 "Bjorn Helgaas" <bhelgaas@...gle.com>,
 "open list:PCI SUBSYSTEM" <linux-pci@...r.kernel.org>,
 "open list" <linux-kernel@...r.kernel.org>, dri-devel@...ts.freedesktop.org,
 "Mario Limonciello" <mario.limonciello@....com>,
 "Alex Deucher" <alexander.deucher@....com>
Subject: Re: [PATCH] PCI/VGA: Don't assume only VGA device found is the boot VGA device



On Wed, 23 Oct 2024, at 3:27 AM, Kai-Heng Feng wrote:
> On 2024/10/22 9:04 PM, Alex Deucher wrote:
>> External email: Use caution opening links or attachments
>> 
>> 
>> On Tue, Oct 22, 2024 at 2:31 AM Kai-Heng Feng <kaihengf@...dia.com> wrote:
>>>
>>> Hi Luke,
>>>
>>> On 2024/10/15 4:04 PM, Luke Jones wrote:
>>>> On Mon, 14 Oct 2024, at 5:25 PM, Mario Limonciello wrote:
>>>>> From: Mario Limonciello <mario.limonciello@....com>
>>>>>
>>>>> The ASUS GA605W has a NVIDIA PCI VGA device and an AMD PCI display device.
>>>>>
>>>>> ```
>>>>> 65:00.0 VGA compatible controller: NVIDIA Corporation AD106M [GeForce
>>>>> RTX 4070 Max-Q / Mobile] (rev a1)
>>>>> 66:00.0 Display controller: Advanced Micro Devices, Inc. [AMD/ATI]
>>>>> Strix [Radeon 880M / 890M] (rev c1)
>>>>> ```
>>>>>
>>>>> The fallback logic in vga_is_boot_device() flags the NVIDIA dGPU as the
>>>>> boot VGA device, but really the eDP is connected to the AMD PCI display
>>>>> device.
>>>>>
>>>>> Drop this case to avoid marking the NVIDIA dGPU as the boot VGA device.
>>>>>
>>>>> Suggested-by: Alex Deucher <alexander.deucher@....com>
>>>>> Reported-by: Luke D. Jones <luke@...nes.dev>
>>>>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3673
>>>>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>>>>> ---
>>>>>    drivers/pci/vgaarb.c | 7 -------
>>>>>    1 file changed, 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
>>>>> index 78748e8d2dba..05ac2b672d4b 100644
>>>>> --- a/drivers/pci/vgaarb.c
>>>>> +++ b/drivers/pci/vgaarb.c
>>>>> @@ -675,13 +675,6 @@ static bool vga_is_boot_device(struct vga_device *vgadev)
>>>>>               return true;
>>>>>       }
>>>>>
>>>>> -    /*
>>>>> -     * Vgadev has neither IO nor MEM enabled.  If we haven't found any
>>>>> -     * other VGA devices, it is the best candidate so far.
>>>>> -     */
>>>>> -    if (!boot_vga)
>>>>> -            return true;
>>>>> -
>>>>>       return false;
>>>>>    }
>>>>>
>>>>> --
>>>>> 2.43.0
>>>>
>>>> Hi Mario,
>>>>
>>>> I can verify that this does leave the `boot_vga` attribute set as 0 for the NVIDIA device.
>>>
>>> Does the following diff work for you?
>>> This variant should be less risky for most systems.
>>>
>>> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
>>> index 78748e8d2dba..3fb734cb9c1b 100644
>>> --- a/drivers/pci/vgaarb.c
>>> +++ b/drivers/pci/vgaarb.c
>>> @@ -675,6 +675,9 @@ static bool vga_is_boot_device(struct vga_device *vgadev)
>>>                   return true;
>>>           }
>>>
>>> +       if (vga_arb_integrated_gpu(&pdev->dev))
>>> +               return true;
>>> +
>> 
>> The problem is that the integrated graphics does not support VGA.
>
> Right, so the check has to be used much earlier.
>
> I wonder does the integrated GFX have _DOD/_DOS while the discrete one doesn't? 
> If that's the case, vga_arb_integrated_gpu() can be used to differentiate which 
> one is the boot GFX.

Discrete might be missing the _DOS? I'm not sure how to interpret the gpu related DSL. You can see the dump here https://gitlab.com/asus-linux/reverse-engineering/-/tree/master/uncategorized/GA605WI?ref_type=heads

ssdt1.dsl contains _DOD and looks to be the discrete unit, does not contain _DOS. the dsdt contains both under "Device (VGA)"

Regards,
Luke.

> Kai-Heng
>
>> 
>> Alex
>> 
>>>           /*
>>>            * Vgadev has neither IO nor MEM enabled.  If we haven't found any
>>>            * other VGA devices, it is the best candidate so far.
>>>
>>>
>>> Kai-Heng
>>>
>>>>
>>>> Tested-by: Luke D. Jones <luke@...nes.dev>
>>>