Re: [Nouveau] [PATCH v2 6/9] PCI: Rename is_thunderbolt to is_tunneled
On Mon, Nov 06, 2023 at 10:59:13AM -0600, Mario Limonciello wrote: > On 11/5/2023 11:39, Lukas Wunner wrote: > > On Fri, Nov 03, 2023 at 02:07:55PM -0500, Mario Limonciello wrote: > > > The `is_thunderbolt` bit has been used to indicate that a PCIe device > > > contained the Intel VSEC which is used by various parts of the kernel > > > to change behavior. To later allow usage with USB4 controllers as well, > > > rename this to `is_tunneled`. > > > > This doesn't seem to make sense. is_thunderbolt indicates that a device > > is part of a Thunderbolt controller. See the code comment: > > > > > - unsigned intis_thunderbolt:1; /* Thunderbolt controller */ > > > > A Thunderbolt controller is not necessarily tunneled. The PCIe switch, > > NHI and XHCI of the Thunderbolt host controller are not tunneled at all. > > I could really use some clarification which PCIe devices actually contain > the Intel VSEC. > > Is it in all 3 of those PCIe devices and not just the switch? Yes, I've just double-checked Light Ridge, Cactus Ridge, Alpine Ridge. Thanks, Lukas
Re: [Nouveau] [PATCH v2 6/9] PCI: Rename is_thunderbolt to is_tunneled
On 11/5/2023 11:39, Lukas Wunner wrote: On Fri, Nov 03, 2023 at 02:07:55PM -0500, Mario Limonciello wrote: The `is_thunderbolt` bit has been used to indicate that a PCIe device contained the Intel VSEC which is used by various parts of the kernel to change behavior. To later allow usage with USB4 controllers as well, rename this to `is_tunneled`. This doesn't seem to make sense. is_thunderbolt indicates that a device is part of a Thunderbolt controller. See the code comment: - unsigned intis_thunderbolt:1; /* Thunderbolt controller */ A Thunderbolt controller is not necessarily tunneled. The PCIe switch, NHI and XHCI of the Thunderbolt host controller are not tunneled at all. Thanks, Lukas I could really use some clarification which PCIe devices actually contain the Intel VSEC. Is it in all 3 of those PCIe devices and not just the switch? If so, I think I would rather introduce a separate bit. So after this series we would have: is_tunneled:1 is_thunderbolt:1 no_command_complete:1 * TBT1 devices would set no_command_complete - The consumer would be pcie_init() * All devices with the Intel VSEC would set is_thunderbolt and the two consumers would be: - apple-gmux.c - pci_bridge_d3_possible() * USB4 devices and PCIe switches with the VSEC would set is_tunneled.
Re: [Nouveau] [PATCH v2 6/9] PCI: Rename is_thunderbolt to is_tunneled
On Fri, Nov 03, 2023 at 02:07:55PM -0500, Mario Limonciello wrote: > The `is_thunderbolt` bit has been used to indicate that a PCIe device > contained the Intel VSEC which is used by various parts of the kernel > to change behavior. To later allow usage with USB4 controllers as well, > rename this to `is_tunneled`. This doesn't seem to make sense. is_thunderbolt indicates that a device is part of a Thunderbolt controller. See the code comment: > - unsigned intis_thunderbolt:1; /* Thunderbolt controller */ A Thunderbolt controller is not necessarily tunneled. The PCIe switch, NHI and XHCI of the Thunderbolt host controller are not tunneled at all. Thanks, Lukas
Re: [Nouveau] [PATCH v2 6/9] PCI: Rename is_thunderbolt to is_tunneled
Hi,
On 11/3/23 20:07, Mario Limonciello wrote:
> The `is_thunderbolt` bit has been used to indicate that a PCIe device
> contained the Intel VSEC which is used by various parts of the kernel
> to change behavior. To later allow usage with USB4 controllers as well,
> rename this to `is_tunneled`.
>
> Signed-off-by: Mario Limonciello
Here is my ack for the trivial drivers/platform/x86/apple-gmux.c change:
Acked-by: Hans de Goede
Bjorn, feel free to route this through the PCI tree.
Regards,
Hans
> ---
> drivers/pci/pci.c | 2 +-
> drivers/pci/probe.c | 2 +-
> drivers/platform/x86/apple-gmux.c | 2 +-
> include/linux/pci.h | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 59c01d68c6d5..d9aa5a39f585 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3032,7 +3032,7 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
> return true;
>
> /* Even the oldest 2010 Thunderbolt controller supports D3. */
> - if (bridge->is_thunderbolt)
> + if (bridge->is_tunneled)
> return true;
>
> /* Platform might know better if the bridge supports D3 */
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 795534589b98..518413d15402 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1597,7 +1597,7 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
> /* Is the device part of a Thunderbolt controller? */
> vsec = pci_find_vsec_capability(dev, PCI_VENDOR_ID_INTEL,
> PCI_VSEC_ID_INTEL_TBT);
> if (vsec)
> - dev->is_thunderbolt = 1;
> + dev->is_tunneled = 1;
> }
>
> static void set_pcie_untrusted(struct pci_dev *dev)
> diff --git a/drivers/platform/x86/apple-gmux.c
> b/drivers/platform/x86/apple-gmux.c
> index 1417e230edbd..20315aa4463a 100644
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c
> @@ -774,7 +774,7 @@ static int gmux_resume(struct device *dev)
>
> static int is_thunderbolt(struct device *dev, void *data)
> {
> - return to_pci_dev(dev)->is_thunderbolt;
> + return to_pci_dev(dev)->is_tunneled;
> }
>
> static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 439c2dac8a3e..b1724f25fb02 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -440,7 +440,7 @@ struct pci_dev {
> unsigned intis_virtfn:1;
> unsigned intis_hotplug_bridge:1;
> unsigned intshpc_managed:1; /* SHPC owned by shpchp */
> - unsigned intis_thunderbolt:1; /* Thunderbolt controller */
> + unsigned intis_tunneled:1; /* Tunneled TBT or USB4 link */
> unsigned intno_command_complete:1; /* No command completion */
> /*
>* Devices marked being untrusted are the ones that can potentially
