| 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: <52d9ff04-ef07-4251-b540-e3d3cdcd4c75@app.fastmail.com>
Date: Thu, 31 Oct 2024 11:51:56 +0100
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 Thu, 31 Oct 2024, at 1:58 AM, Kai-Heng Feng wrote:
> On 2024/10/25 8:55 PM, Alex Deucher wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On Fri, Oct 25, 2024 at 3:51 AM Kai-Heng Feng <kaihengf@...dia.com> wrote:
>>>
>>>
>>>
>>> On 2024/10/23 11:27 PM, Alex Deucher wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On Tue, Oct 22, 2024 at 9:27 PM Kai-Heng Feng <kaihengf@...dia.com> 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.
>>>>
>>>> I think the problem is that the boot GPU is being conflated with vga
>>>> arb. In this case the iGPU has no VGA so has no reason to be involved
>>>> in vga arb. Trying to mess with any vga related resources on it could
>>>> be problematic. Do higher levels of the stack look at vga arb to
>>>> determine the "primary" GPU?
>>>
>>> Hmm, I wonder if all those heuristic are needed for EFI based system?
>>>
>>> Can we assume that what being used by UEFI GOP is the primary GFX device?
>>
>> Yes, I believe so. The SBIOS should use the GOP device as determined
>> by the user preference. I.e.., in the bios configuration you can
>> generally select iGPU or PEG for the primary display.
>
> UEFI spec, 10.3.3.1 ACPI _ADR Device Path
>
> "The _ADR device path is used to contain video output device attributes to
> support the Graphics Output Protocol. The device path can contain multiple _ADR
> entries if multiple video output devices are displaying the same output."
>
> Luke, can you please see what are the _ADR values of the iGPU and dGPU?
> Maybe we
> can find which one was used by GOP this way.
I'm not sure what I'm looking at here, but initial search shows:
Device (VGA)
{
Name (_ADR, Zero) // _ADR: Address
Name (DOSA, Zero)
Method (_DOS, 1, NotSerialized) // _DOS: Disable Output Switching
{
DOSA = Arg0
}
Method (_DOD, 0, NotSerialized) // _DOD: Display Output Devices
{
M460 ("PLA-ASL-\\_SB.PCI0.GPPA.VGA._DOD\n", Zero, Zero, Zero, Zero, Zero, Zero)
Return (Package (0x07)
{
0x00010110,
0x00010210,
0x00010220,
0x00010230,
0x00010240,
0x00031000,
0x00032000
})
}
Device (LCD)
{
Name (_ADR, 0x0110) // _ADR: Address
Name (BCLB, Package (0x34)
{
https://gitlab.com/asus-linux/reverse-engineering/-/blob/master/uncategorized/GA605WI/dsdt.dsl?ref_type=heads#L4666
And:
Method (_DOD, 0, NotSerialized) // _DOD: Display Output Devices
{
Return (Package (0x01)
{
0x8000A450
})
}
Device (LCD0)
{
Method (_ADR, 0, Serialized) // _ADR: Address
{
Return (0x8000A450)
}
https://gitlab.com/asus-linux/reverse-engineering/-/blob/master/uncategorized/GA605WI/ssdt1.dsl?ref_type=heads#L411
The links are direct to the lines I thought were relevant in the dumped DSDT.
Luke.
>
> Kai-Heng
>
>>
>> Alex
>>
>>>
>>> Kai-Heng
>>>
>>>>
>>>> Alex
>>>>
>>>>>
>>>>> 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>
>>>>>>>
>>>>>
>>>
Powered by blists - more mailing lists