diff mbox series

[v3,5/7] PCI: ACPI: Detect PCIe root ports that are used for tunneling

Message ID 20231114200755.14911-6-mario.limonciello@amd.com (mailing list archive)
State New, archived
Headers show
Series Improvements to pcie_bandwidth_available() for eGPUs | expand

Commit Message

Mario Limonciello Nov. 14, 2023, 8:07 p.m. UTC
USB4 routers support a feature called "PCIe tunneling". This
allows PCIe traffic to be transmitted over USB4 fabric.

PCIe root ports that are used in this fashion can be discovered
by device specific data that specifies the USB4 router they are
connected to. For the PCI core, the specific connection information
doesn't matter, but it's interesting to know that this root port is
used for tunneling traffic. This will allow other decisions to be
made based upon it.

Detect the `usb4-host-interface` _DSD and if it's found save it
into a new `is_virtual_link` bit in `struct pci_device`.

Link: https://www.usb.org/document-library/usb4r-specification-v20
      USB4 V2 with Errata and ECN through June 2023
      Section 2.2.10.3
Link: https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/usb4-acpi-requirements#port-mapping-_dsd-for-usb-3x-and-pcie
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v2->v3:
 * Use is_virtual_link to be future proof to other types of virtual
   links.
 * Update commit message
---
 drivers/pci/pci-acpi.c | 16 ++++++++++++++++
 include/linux/pci.h    |  1 +
 2 files changed, 17 insertions(+)

Comments

Mika Westerberg Nov. 15, 2023, 10:40 a.m. UTC | #1
Hi Mario,

On Tue, Nov 14, 2023 at 02:07:53PM -0600, Mario Limonciello wrote:
> USB4 routers support a feature called "PCIe tunneling". This
> allows PCIe traffic to be transmitted over USB4 fabric.
> 
> PCIe root ports that are used in this fashion can be discovered
> by device specific data that specifies the USB4 router they are
> connected to. For the PCI core, the specific connection information
> doesn't matter, but it's interesting to know that this root port is
> used for tunneling traffic. This will allow other decisions to be
> made based upon it.
> 
> Detect the `usb4-host-interface` _DSD and if it's found save it
> into a new `is_virtual_link` bit in `struct pci_device`.

While this is fine for the "first" tunneled link, this does not take
into account possible other "virtual" links that lead to the endpoint in
question. Typically for eGPU it only makes sense to plug it directly to
the host but say there is a USB4 hub (with PCIe tunneling capabilities)
in the middle. Now the link from the hub to the eGPU that is also
"virtual" is not marked as such and the bandwidth calculations may not
get what is expected.

It should be possible to map the PCIe ports that go over USB4 links
through router port operation "Get PCIe Downstream Entry Mapping" and
for the Thunderbolt 3 there is the DROM entries (I believe Lukas has
patches for this part already) but I guess it is outside of the scope of
this series. Out of curiosity I tried to look in Windows documentation
if there is such interface for GPUs as we have in Linux but could not
find (which does not mean it does not exist, though).
Mario Limonciello Nov. 15, 2023, 5:08 p.m. UTC | #2
On 11/15/2023 04:40, Mika Westerberg wrote:
> Hi Mario,
> 
> On Tue, Nov 14, 2023 at 02:07:53PM -0600, Mario Limonciello wrote:
>> USB4 routers support a feature called "PCIe tunneling". This
>> allows PCIe traffic to be transmitted over USB4 fabric.
>>
>> PCIe root ports that are used in this fashion can be discovered
>> by device specific data that specifies the USB4 router they are
>> connected to. For the PCI core, the specific connection information
>> doesn't matter, but it's interesting to know that this root port is
>> used for tunneling traffic. This will allow other decisions to be
>> made based upon it.
>>
>> Detect the `usb4-host-interface` _DSD and if it's found save it
>> into a new `is_virtual_link` bit in `struct pci_device`.
> 
> While this is fine for the "first" tunneled link, this does not take
> into account possible other "virtual" links that lead to the endpoint in
> question. Typically for eGPU it only makes sense to plug it directly to
> the host but say there is a USB4 hub (with PCIe tunneling capabilities)
> in the middle. Now the link from the hub to the eGPU that is also
> "virtual" is not marked as such and the bandwidth calculations may not
> get what is expected.

Right; you mentioned the DVSEC available for hubs in this case.  As I 
don't have one of these to validate it works properly I was thinking 
that should be a follow up.

If you think it should be part of the same series I'll add it, but I'd 
ask if you can please check I did it right on one that reports the DVSEC?

> 
> It should be possible to map the PCIe ports that go over USB4 links
> through router port operation "Get PCIe Downstream Entry Mapping" and
> for the Thunderbolt 3 there is the DROM entries (I believe Lukas has
> patches for this part already) but I guess it is outside of the scope of
> this series. 

Yeah I'd prefer to avoid the kitchen sink for the first pass and then we 
an add more cases to is_virtual_link later.
Mika Westerberg Nov. 16, 2023, 9 a.m. UTC | #3
Hi Mario,

On Wed, Nov 15, 2023 at 11:08:43AM -0600, Mario Limonciello wrote:
> On 11/15/2023 04:40, Mika Westerberg wrote:
> > Hi Mario,
> > 
> > On Tue, Nov 14, 2023 at 02:07:53PM -0600, Mario Limonciello wrote:
> > > USB4 routers support a feature called "PCIe tunneling". This
> > > allows PCIe traffic to be transmitted over USB4 fabric.
> > > 
> > > PCIe root ports that are used in this fashion can be discovered
> > > by device specific data that specifies the USB4 router they are
> > > connected to. For the PCI core, the specific connection information
> > > doesn't matter, but it's interesting to know that this root port is
> > > used for tunneling traffic. This will allow other decisions to be
> > > made based upon it.
> > > 
> > > Detect the `usb4-host-interface` _DSD and if it's found save it
> > > into a new `is_virtual_link` bit in `struct pci_device`.
> > 
> > While this is fine for the "first" tunneled link, this does not take
> > into account possible other "virtual" links that lead to the endpoint in
> > question. Typically for eGPU it only makes sense to plug it directly to
> > the host but say there is a USB4 hub (with PCIe tunneling capabilities)
> > in the middle. Now the link from the hub to the eGPU that is also
> > "virtual" is not marked as such and the bandwidth calculations may not
> > get what is expected.
> 
> Right; you mentioned the DVSEC available for hubs in this case.  As I don't
> have one of these to validate it works properly I was thinking that should
> be a follow up.
> 
> If you think it should be part of the same series I'll add it, but I'd ask
> if you can please check I did it right on one that reports the DVSEC?

I don't think it should be part of this series. I just checked and DVSEC
is only required for hosts so kind of hardware equivalent for the _DSD
property you are using here. For hubs there is no such luxury
unfortunately.

I think I do have hardware here with the DVSEC in place so if you
decide to add it, I should be able to try it.
diff mbox series

Patch

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 004575091596..4a94d2fd8fb9 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1415,12 +1415,28 @@  static void pci_acpi_set_external_facing(struct pci_dev *dev)
 		dev->external_facing = 1;
 }
 
+static void pci_acpi_set_virtual_link(struct pci_dev *dev)
+{
+	if (!pci_is_pcie(dev))
+		return;
+
+	switch (pci_pcie_type(dev)) {
+	case PCI_EXP_TYPE_ROOT_PORT:
+	case PCI_EXP_TYPE_DOWNSTREAM:
+		dev->is_virtual_link = device_property_present(&dev->dev, "usb4-host-interface");
+		break;
+	default:
+		return;
+	}
+}
+
 void pci_acpi_setup(struct device *dev, struct acpi_device *adev)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 
 	pci_acpi_optimize_delay(pci_dev, adev->handle);
 	pci_acpi_set_external_facing(pci_dev);
+	pci_acpi_set_virtual_link(pci_dev);
 	pci_acpi_add_edr_notifier(pci_dev);
 
 	pci_acpi_add_pm_notifier(adev, pci_dev);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 20a6e4fc3060..83fbd0d7040e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -441,6 +441,7 @@  struct pci_dev {
 	unsigned int	is_hotplug_bridge:1;
 	unsigned int	shpc_managed:1;		/* SHPC owned by shpchp */
 	unsigned int	is_thunderbolt:1;	/* Thunderbolt controller */
+	unsigned int	is_virtual_link:1;	/* Tunneled (virtual) PCIe link */
 	unsigned int	no_command_complete:1;	/* No command completion */
 	/*
 	 * Devices marked being untrusted are the ones that can potentially