linux-kernel - Re: [PATCH v3 1/7] drm/nouveau: Switch from pci_is_thunderbolt_attached() to dev_is

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]

Date:   Thu, 16 Nov 2023 14:50:24 +0200 (EET)
From:   Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To:     Mario Limonciello <mario.limonciello@....com>
cc:     Karol Herbst <kherbst@...hat.com>, Lyude Paul <lyude@...hat.com>,
        Alex Deucher <alexander.deucher@....com>,
        Christian König <christian.koenig@....com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Lukas Wunner <lukas@...ner.de>,
        Danilo Krummrich <dakr@...hat.com>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Xinhui Pan <Xinhui.Pan@....com>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Pali Rohár <pali@...nel.org>,
        Marek Behún <kabel@...nel.org>,
        "Maciej W . Rozycki" <macro@...am.me.uk>,
        Manivannan Sadhasivam <mani@...nel.org>,
        "open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS" 
        <dri-devel@...ts.freedesktop.org>,
        "open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS" 
        <nouveau@...ts.freedesktop.org>,
        open list <linux-kernel@...r.kernel.org>,
        "open list:RADEON and AMDGPU DRM DRIVERS" 
        <amd-gfx@...ts.freedesktop.org>,
        "open list:PCI SUBSYSTEM" <linux-pci@...r.kernel.org>,
        "open list:ACPI" <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH v3 1/7] drm/nouveau: Switch from pci_is_thunderbolt_attached()
 to dev_is_removable()

On Tue, 14 Nov 2023, Mario Limonciello wrote:

> pci_is_thunderbolt_attached() looks at the hierarchy of the PCIe device
> to determine if any bridge along the way has the is_thunderbolt bit set.
> This bit will only be set when one of the devices in the hierarchy is an
> Intel Thunderbolt device.
> 
> However PCIe devices can be connected to USB4 hubs and routers which won't
> necessarily set the is_thunderbolt bit. These devices will however be
> marked as externally facing which means they are marked removable by
> pci_set_removable().
> 
> Look whether the device is marked removable to determine it's
> connected to a Thunderbolt controller or USB4 router.

(I hate having to say this) but I still feel the actual reason why you're 
doing this change is missing from the above and has to be read between the 
lines.

Perhaps something along these lines matches what's your intent here:

The external GPU check added in commit 1d3c11030103 ("drm/nouveau: Don't 
register Thunderbolt eGPU with vga_switcheroo") covers only Thunderbolt
devices but removable PCIe devices can also be connected to USB4 hubs and 
routers. Those devices won't necessarily set the is_thunderbolt bit that
pci_is_thunderbolt_attached() checks for.

USB4 hubs and routers are however marked as externally facing which means 
they are marked removable by pci_set_removable().

Extend the check for removable device beyond Thunderbolt devices by 
using dev_is_removable() that covers also USB4 routers.

?

-- 
 i.

> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> ---
> v2->v3:
>  * Update commit message
> ---
>  drivers/gpu/drm/nouveau/nouveau_vga.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c
> index f8bf0ec26844..14215b7ca187 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_vga.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
> @@ -94,8 +94,8 @@ nouveau_vga_init(struct nouveau_drm *drm)
>  
>  	vga_client_register(pdev, nouveau_vga_set_decode);
>  
> -	/* don't register Thunderbolt eGPU with vga_switcheroo */
> -	if (pci_is_thunderbolt_attached(pdev))
> +	/* don't register USB4/Thunderbolt eGPU with vga_switcheroo */
> +	if (dev_is_removable(&pdev->dev))
>  		return;
>  
>  	vga_switcheroo_register_client(pdev, &nouveau_switcheroo_ops, runtime);
> @@ -118,7 +118,7 @@ nouveau_vga_fini(struct nouveau_drm *drm)
>  
>  	vga_client_unregister(pdev);
>  
> -	if (pci_is_thunderbolt_attached(pdev))
> +	if (dev_is_removable(&pdev->dev))
>  		return;
>  
>  	vga_switcheroo_unregister_client(pdev);
>