mbox series

[0/4] Add device links between tunneled USB3 devices and USB4 Host

Message ID 20240619130305.2617784-1-mathias.nyman@linux.intel.com (mailing list archive)
Headers show
Series Add device links between tunneled USB3 devices and USB4 Host | expand

Message

Mathias Nyman June 19, 2024, 1:03 p.m. UTC
The relationship between a USB4 Host Interface providing USB3 tunnels,
and tunneled USB3 devices is not represented in device hierarchy.

This caused issues with power managment as devices may suspend and
resume in incorrect order.
A device link between the USB4 Host Interface and the USB3 xHCI was
originally added to solve this, preventing the USB4 Host Interface from
suspending if the USB3 xHCI Host was still active.
This unfortunately also prevents USB4 Host Interface from suspending even
if the USB3 xHCI Host is only serving native non-tunneled USB devices.

Improve the current powermanagement situation by creating device links
directly from tunneled USB3 devices to the USB4 Host Interface they depend
on instead of a device link between the hosts.
This way USB4 host may suspend when the last tunneled device is
disconnected.

Intel xHCI hosts are capable of reporting if connected USB3 devices are
tunneled via vendor specific capabilities.
Use this until a standard way is available.

Mathias Nyman (4):
  xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
  usb: Add tunneled parameter to usb device structure
  usb: acpi: add device link between tunneled USB3 device and USB4 Host
    Interface
  thunderbolt: Don't create device link from USB4 Host Interface to USB3
    xHC host

 drivers/thunderbolt/acpi.c       | 40 ++++++------------------
 drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci-ext-caps.h |  5 +++
 drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
 drivers/usb/host/xhci.c          | 12 ++++++++
 drivers/usb/host/xhci.h          |  1 +
 include/linux/usb.h              |  2 ++
 7 files changed, 111 insertions(+), 30 deletions(-)

Comments

Mika Westerberg June 20, 2024, 6:41 a.m. UTC | #1
+CC Mario from AMD side to check that we are good and don't break
anything accidentally.

On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
> The relationship between a USB4 Host Interface providing USB3 tunnels,
> and tunneled USB3 devices is not represented in device hierarchy.
> 
> This caused issues with power managment as devices may suspend and
> resume in incorrect order.
> A device link between the USB4 Host Interface and the USB3 xHCI was
> originally added to solve this, preventing the USB4 Host Interface from
> suspending if the USB3 xHCI Host was still active.
> This unfortunately also prevents USB4 Host Interface from suspending even
> if the USB3 xHCI Host is only serving native non-tunneled USB devices.
> 
> Improve the current powermanagement situation by creating device links
> directly from tunneled USB3 devices to the USB4 Host Interface they depend
> on instead of a device link between the hosts.
> This way USB4 host may suspend when the last tunneled device is
> disconnected.
> 
> Intel xHCI hosts are capable of reporting if connected USB3 devices are
> tunneled via vendor specific capabilities.
> Use this until a standard way is available.
> 
> Mathias Nyman (4):
>   xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
>   usb: Add tunneled parameter to usb device structure
>   usb: acpi: add device link between tunneled USB3 device and USB4 Host
>     Interface
>   thunderbolt: Don't create device link from USB4 Host Interface to USB3
>     xHC host
> 
>  drivers/thunderbolt/acpi.c       | 40 ++++++------------------
>  drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
>  drivers/usb/host/xhci-ext-caps.h |  5 +++
>  drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
>  drivers/usb/host/xhci.c          | 12 ++++++++
>  drivers/usb/host/xhci.h          |  1 +
>  include/linux/usb.h              |  2 ++
>  7 files changed, 111 insertions(+), 30 deletions(-)
> 
> -- 
> 2.25.1
Mario Limonciello June 20, 2024, 6:36 p.m. UTC | #2
On 6/20/2024 01:41, Mika Westerberg wrote:
> +CC Mario from AMD side to check that we are good and don't break
> anything accidentally.
> 
> On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
>> The relationship between a USB4 Host Interface providing USB3 tunnels,
>> and tunneled USB3 devices is not represented in device hierarchy.
>>
>> This caused issues with power managment as devices may suspend and
>> resume in incorrect order.
>> A device link between the USB4 Host Interface and the USB3 xHCI was
>> originally added to solve this, preventing the USB4 Host Interface from
>> suspending if the USB3 xHCI Host was still active.
>> This unfortunately also prevents USB4 Host Interface from suspending even
>> if the USB3 xHCI Host is only serving native non-tunneled USB devices.
>>
>> Improve the current powermanagement situation by creating device links
>> directly from tunneled USB3 devices to the USB4 Host Interface they depend
>> on instead of a device link between the hosts.
>> This way USB4 host may suspend when the last tunneled device is
>> disconnected.
>>
>> Intel xHCI hosts are capable of reporting if connected USB3 devices are
>> tunneled via vendor specific capabilities.
>> Use this until a standard way is available.
>>
>> Mathias Nyman (4):
>>    xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
>>    usb: Add tunneled parameter to usb device structure
>>    usb: acpi: add device link between tunneled USB3 device and USB4 Host
>>      Interface
>>    thunderbolt: Don't create device link from USB4 Host Interface to USB3
>>      xHC host
>>
>>   drivers/thunderbolt/acpi.c       | 40 ++++++------------------
>>   drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
>>   drivers/usb/host/xhci-ext-caps.h |  5 +++
>>   drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
>>   drivers/usb/host/xhci.c          | 12 ++++++++
>>   drivers/usb/host/xhci.h          |  1 +
>>   include/linux/usb.h              |  2 ++
>>   7 files changed, 111 insertions(+), 30 deletions(-)
>>
>> -- 
>> 2.25.1

Hi Mika,

Thanks for looping me in.  Unfortunately with this is appears the XHCI 
controller link never gets created.  I've not checked functional impact 
from this, but I'd guess there "should" be some functional problems too.

I grabbed a tree output on an AMD Phoenix system using:

# tree -l /sys/bus/pci/drivers/thunderbolt/ -L 5

I'll attach the before/after patch.  Also here are the relevant devices 
from lspci -nn output:

00:03.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Family 19h 
USB4/Thunderbolt PCIe tunnel [1022:14ef]
00:04.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Family 19h 
USB4/Thunderbolt PCIe tunnel [1022:14ef]
.
.
.
c4:00.3 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:15c0]
c4:00.4 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:15c1]
c4:00.5 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Pink 
Sardine USB4/Thunderbolt NHI controller #1 [1022:1668]
c4:00.6 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Pink 
Sardine USB4/Thunderbolt NHI controller #2 [1022:1669]
/sys/bus/pci/drivers/thunderbolt/
├── 0000:c4:00.5 -> ../../../../devices/pci0000:00/0000:00:08.3/0000:c4:00.5
│   ├── ari_enabled
│   ├── broken_parity_status
│   ├── class
│   ├── config
│   ├── consistent_dma_mask_bits
│   ├── consumer:pci:0000:00:03.1 -> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1
│   │   ├── auto_remove_on
│   │   ├── consumer -> ../../../pci0000:00/0000:00:03.1
│   │   │   ├── 0000:00:03.1:pcie001
│   │   │   │   ├── driver -> ../../../../bus/pci_express/drivers/pcie_pme
│   │   │   │   ├── power
│   │   │   │   ├── subsystem -> ../../../../bus/pci_express
│   │   │   │   └── uevent
│   │   │   ├── 0000:00:03.1:pcie004
│   │   │   │   ├── driver -> ../../../../bus/pci_express/drivers/pciehp
│   │   │   │   ├── power
│   │   │   │   ├── subsystem -> ../../../../bus/pci_express  [recursive, not followed]
│   │   │   │   └── uevent
│   │   │   ├── aer_dev_correctable
│   │   │   ├── aer_dev_fatal
│   │   │   ├── aer_dev_nonfatal
│   │   │   ├── aer_rootport_total_err_cor
│   │   │   ├── aer_rootport_total_err_fatal
│   │   │   ├── aer_rootport_total_err_nonfatal
│   │   │   ├── ari_enabled
│   │   │   ├── broken_parity_status
│   │   │   ├── class
│   │   │   ├── config
│   │   │   ├── consistent_dma_mask_bits
│   │   │   ├── current_link_speed
│   │   │   ├── current_link_width
│   │   │   ├── d3cold_allowed
│   │   │   ├── device
│   │   │   ├── dma_mask_bits
│   │   │   ├── driver -> ../../../bus/pci/drivers/pcieport
│   │   │   │   ├── 0000:00:01.3 -> ../../../../devices/pci0000:00/0000:00:01.3
│   │   │   │   ├── 0000:00:02.1 -> ../../../../devices/pci0000:00/0000:00:02.1
│   │   │   │   ├── 0000:00:02.4 -> ../../../../devices/pci0000:00/0000:00:02.4
│   │   │   │   ├── 0000:00:03.1 -> ../../../../devices/pci0000:00/0000:00:03.1  [recursive, not followed]
│   │   │   │   ├── 0000:00:04.1 -> ../../../../devices/pci0000:00/0000:00:04.1
│   │   │   │   ├── 0000:00:08.1 -> ../../../../devices/pci0000:00/0000:00:08.1
│   │   │   │   ├── 0000:00:08.2 -> ../../../../devices/pci0000:00/0000:00:08.2
│   │   │   │   ├── 0000:00:08.3 -> ../../../../devices/pci0000:00/0000:00:08.3
│   │   │   │   ├── bind
│   │   │   │   ├── new_id
│   │   │   │   ├── remove_id
│   │   │   │   ├── uevent
│   │   │   │   └── unbind
│   │   │   ├── driver_override
│   │   │   ├── enable
│   │   │   ├── firmware_node -> ../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:13
│   │   │   │   ├── adr
│   │   │   │   ├── device:14
│   │   │   │   ├── LNXPOWER:04
│   │   │   │   ├── path
│   │   │   │   ├── physical_node -> ../../../../pci0000:00/0000:00:03.1  [recursive, not followed]
│   │   │   │   ├── power
│   │   │   │   ├── power_resources_D0
│   │   │   │   ├── power_resources_D3hot
│   │   │   │   ├── power_state
│   │   │   │   ├── real_power_state
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../../bus/acpi
│   │   │   │   ├── uevent
│   │   │   │   └── wakeup
│   │   │   ├── iommu -> ../0000:00:00.2/iommu/ivhd0
│   │   │   │   ├── amd-iommu
│   │   │   │   ├── device -> ../../../0000:00:00.2
│   │   │   │   ├── devices
│   │   │   │   ├── power
│   │   │   │   ├── subsystem -> ../../../../../class/iommu
│   │   │   │   └── uevent
│   │   │   ├── iommu_group -> ../../../kernel/iommu_groups/5
│   │   │   │   ├── devices
│   │   │   │   ├── reserved_regions
│   │   │   │   └── type
│   │   │   ├── irq
│   │   │   ├── link
│   │   │   ├── local_cpulist
│   │   │   ├── local_cpus
│   │   │   ├── max_link_speed
│   │   │   ├── max_link_width
│   │   │   ├── modalias
│   │   │   ├── msi_bus
│   │   │   ├── msi_irqs
│   │   │   │   └── 41
│   │   │   ├── numa_node
│   │   │   ├── pci_bus
│   │   │   │   └── 0000:04
│   │   │   ├── power
│   │   │   │   ├── async
│   │   │   │   ├── autosuspend_delay_ms
│   │   │   │   ├── control
│   │   │   │   ├── runtime_active_kids
│   │   │   │   ├── runtime_active_time
│   │   │   │   ├── runtime_enabled
│   │   │   │   ├── runtime_status
│   │   │   │   ├── runtime_suspended_time
│   │   │   │   ├── runtime_usage
│   │   │   │   ├── wakeup
│   │   │   │   ├── wakeup_abort_count
│   │   │   │   ├── wakeup_active
│   │   │   │   ├── wakeup_active_count
│   │   │   │   ├── wakeup_count
│   │   │   │   ├── wakeup_expire_count
│   │   │   │   ├── wakeup_last_time_ms
│   │   │   │   ├── wakeup_max_time_ms
│   │   │   │   └── wakeup_total_time_ms
│   │   │   ├── power_state
│   │   │   ├── remove
│   │   │   ├── rescan
│   │   │   ├── reset
│   │   │   ├── reset_method
│   │   │   ├── resource
│   │   │   ├── revision
│   │   │   ├── secondary_bus_number
│   │   │   ├── subordinate_bus_number
│   │   │   ├── subsystem -> ../../../bus/pci
│   │   │   │   ├── devices
│   │   │   │   ├── drivers
│   │   │   │   ├── drivers_autoprobe
│   │   │   │   ├── drivers_probe
│   │   │   │   ├── rescan
│   │   │   │   ├── resource_alignment
│   │   │   │   ├── slots
│   │   │   │   └── uevent
│   │   │   ├── subsystem_device
│   │   │   ├── subsystem_vendor
│   │   │   ├── supplier:pci:0000:c4:00.5 -> ../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1  [recursive, not followed]
│   │   │   ├── uevent
│   │   │   ├── vendor
│   │   │   └── wakeup
│   │   │       └── wakeup6
│   │   ├── runtime_pm
│   │   ├── status
│   │   ├── subsystem -> ../../../../class/devlink
│   │   │   ├── pci:0000:c2:00.0--pci:0000:c2:00.1 -> ../../devices/virtual/devlink/pci:0000:c2:00.0--pci:0000:c2:00.1
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── consumer -> ../../../pci0000:00/0000:00:08.1/0000:c2:00.1
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── supplier -> ../../../pci0000:00/0000:00:08.1/0000:c2:00.0
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   ├── pci:0000:c4:00.5--pci:0000:00:03.1 -> ../../devices/virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1  [recursive, not followed]
│   │   │   ├── pci:0000:c4:00.6--pci:0000:00:04.1 -> ../../devices/virtual/devlink/pci:0000:c4:00.6--pci:0000:00:04.1
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── consumer -> ../../../pci0000:00/0000:00:04.1  [recursive, not followed]
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── supplier -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.6
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   ├── platform:PNP0C14:00--wmi:05901221-D566-11D1-B2F0-00A0C9062910 -> ../../devices/virtual/devlink/platform:PNP0C14:00--wmi:05901221-D566-11D1-B2F0-00A0C9062910
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   └── platform:PNP0C14:00--wmi:ABBC0F6A-8EA1-11D1-00A0-C90629100000 -> ../../devices/virtual/devlink/platform:PNP0C14:00--wmi:ABBC0F6A-8EA1-11D1-00A0-C90629100000
│   │   │       ├── auto_remove_on
│   │   │       ├── runtime_pm
│   │   │       ├── status
│   │   │       ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │       ├── sync_state_only
│   │   │       └── uevent
│   │   ├── supplier -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   ├── sync_state_only
│   │   └── uevent
│   ├── current_link_speed
│   ├── current_link_width
│   ├── d3cold_allowed
│   ├── device
│   ├── dma_mask_bits
│   ├── domain0
│   │   ├── 0-0
│   │   │   ├── authorized
│   │   │   ├── generation
│   │   │   ├── power
│   │   │   │   ├── async
│   │   │   │   ├── autosuspend_delay_ms
│   │   │   │   ├── control
│   │   │   │   ├── runtime_active_kids
│   │   │   │   ├── runtime_active_time
│   │   │   │   ├── runtime_enabled
│   │   │   │   ├── runtime_status
│   │   │   │   ├── runtime_suspended_time
│   │   │   │   ├── runtime_usage
│   │   │   │   ├── wakeup
│   │   │   │   ├── wakeup_abort_count
│   │   │   │   ├── wakeup_active
│   │   │   │   ├── wakeup_active_count
│   │   │   │   ├── wakeup_count
│   │   │   │   ├── wakeup_expire_count
│   │   │   │   ├── wakeup_last_time_ms
│   │   │   │   ├── wakeup_max_time_ms
│   │   │   │   └── wakeup_total_time_ms
│   │   │   ├── subsystem -> ../../../../../../bus/thunderbolt
│   │   │   │   ├── devices
│   │   │   │   ├── drivers
│   │   │   │   ├── drivers_autoprobe
│   │   │   │   ├── drivers_probe
│   │   │   │   └── uevent
│   │   │   ├── uevent
│   │   │   ├── unique_id
│   │   │   ├── usb4_port2
│   │   │   │   ├── link
│   │   │   │   ├── power
│   │   │   │   └── uevent
│   │   │   └── wakeup
│   │   │       └── wakeup47
│   │   ├── deauthorization
│   │   ├── iommu_dma_protection
│   │   ├── power
│   │   │   ├── async
│   │   │   ├── runtime_active_kids
│   │   │   ├── runtime_enabled
│   │   │   ├── runtime_status
│   │   │   ├── runtime_usage
│   │   │   ├── wakeup
│   │   │   ├── wakeup_abort_count
│   │   │   ├── wakeup_active
│   │   │   ├── wakeup_active_count
│   │   │   ├── wakeup_count
│   │   │   ├── wakeup_expire_count
│   │   │   ├── wakeup_last_time_ms
│   │   │   ├── wakeup_max_time_ms
│   │   │   └── wakeup_total_time_ms
│   │   ├── security
│   │   ├── subsystem -> ../../../../../bus/thunderbolt  [recursive, not followed]
│   │   ├── uevent
│   │   └── wakeup
│   │       └── wakeup48
│   │           ├── active_count
│   │           ├── active_time_ms
│   │           ├── device -> ../../../domain0  [recursive, not followed]
│   │           ├── event_count
│   │           ├── expire_count
│   │           ├── last_change_ms
│   │           ├── max_time_ms
│   │           ├── name
│   │           ├── prevent_suspend_time_ms
│   │           ├── subsystem -> ../../../../../../../class/wakeup
│   │           ├── total_time_ms
│   │           ├── uevent
│   │           └── wakeup_count
│   ├── driver -> ../../../../bus/pci/drivers/thunderbolt  [recursive, not followed]
│   ├── driver_override
│   ├── enable
│   ├── firmware_node -> ../../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:36/device:3b
│   │   ├── adr
│   │   ├── path
│   │   ├── physical_node -> ../../../../../pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   ├── power
│   │   │   ├── async
│   │   │   ├── autosuspend_delay_ms
│   │   │   ├── control
│   │   │   ├── runtime_active_kids
│   │   │   ├── runtime_active_time
│   │   │   ├── runtime_enabled
│   │   │   ├── runtime_status
│   │   │   ├── runtime_suspended_time
│   │   │   └── runtime_usage
│   │   ├── power_resources_D0
│   │   │   └── LNXPOWER:0f -> ../../LNXPOWER:0f
│   │   │       ├── hid
│   │   │       ├── modalias
│   │   │       ├── path
│   │   │       ├── power
│   │   │       ├── resource_in_use
│   │   │       ├── status
│   │   │       ├── subsystem -> ../../../../../../bus/acpi  [recursive, not followed]
│   │   │       └── uevent
│   │   ├── power_resources_D3hot
│   │   │   └── LNXPOWER:0f -> ../../LNXPOWER:0f  [recursive, not followed]
│   │   ├── power_state
│   │   ├── real_power_state
│   │   ├── status
│   │   ├── subsystem -> ../../../../../../bus/acpi  [recursive, not followed]
│   │   ├── uevent
│   │   └── wakeup
│   │       └── wakeup29
│   │           ├── active_count
│   │           ├── active_time_ms
│   │           ├── device -> ../../../device:3b  [recursive, not followed]
│   │           ├── event_count
│   │           ├── expire_count
│   │           ├── last_change_ms
│   │           ├── max_time_ms
│   │           ├── name
│   │           ├── prevent_suspend_time_ms
│   │           ├── subsystem -> ../../../../../../../../class/wakeup  [recursive, not followed]
│   │           ├── total_time_ms
│   │           ├── uevent
│   │           └── wakeup_count
│   ├── iommu -> ../../0000:00:00.2/iommu/ivhd0  [recursive, not followed]
│   ├── iommu_group -> ../../../../kernel/iommu_groups/29
│   │   ├── devices
│   │   │   └── 0000:c4:00.5 -> ../../../../devices/pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   ├── reserved_regions
│   │   └── type
│   ├── irq
│   ├── link
│   │   ├── l0s_aspm
│   │   └── l1_aspm
│   ├── local_cpulist
│   ├── local_cpus
│   ├── max_link_speed
│   ├── max_link_width
│   ├── modalias
│   ├── msi_bus
│   ├── msi_irqs
│   │   ├── 50
│   │   ├── 51
│   │   ├── 53
│   │   ├── 54
│   │   ├── 55
│   │   ├── 56
│   │   ├── 57
│   │   ├── 58
│   │   ├── 59
│   │   ├── 61
│   │   ├── 62
│   │   ├── 64
│   │   ├── 65
│   │   ├── 66
│   │   ├── 67
│   │   └── 68
│   ├── numa_node
│   ├── pools
│   ├── power
│   │   ├── async
│   │   ├── autosuspend_delay_ms
│   │   ├── control
│   │   ├── runtime_active_kids
│   │   ├── runtime_active_time
│   │   ├── runtime_enabled
│   │   ├── runtime_status
│   │   ├── runtime_suspended_time
│   │   ├── runtime_usage
│   │   ├── wakeup
│   │   ├── wakeup_abort_count
│   │   ├── wakeup_active
│   │   ├── wakeup_active_count
│   │   ├── wakeup_count
│   │   ├── wakeup_expire_count
│   │   ├── wakeup_last_time_ms
│   │   ├── wakeup_max_time_ms
│   │   └── wakeup_total_time_ms
│   ├── power_state
│   ├── remove
│   ├── rescan
│   ├── reset
│   ├── reset_method
│   ├── resource
│   ├── resource0
│   ├── revision
│   ├── subsystem -> ../../../../bus/pci  [recursive, not followed]
│   ├── subsystem_device
│   ├── subsystem_vendor
│   ├── uevent
│   ├── vendor
│   └── wakeup
│       └── wakeup49
│           ├── active_count
│           ├── active_time_ms
│           ├── device -> ../../../0000:c4:00.5  [recursive, not followed]
│           ├── event_count
│           ├── expire_count
│           ├── last_change_ms
│           ├── max_time_ms
│           ├── name
│           ├── prevent_suspend_time_ms
│           ├── subsystem -> ../../../../../../class/wakeup  [recursive, not followed]
│           ├── total_time_ms
│           ├── uevent
│           └── wakeup_count
├── 0000:c4:00.6 -> ../../../../devices/pci0000:00/0000:00:08.3/0000:c4:00.6  [recursive, not followed]
├── bind
├── module -> ../../../../module/thunderbolt
│   ├── coresize
│   ├── drivers
│   │   └── pci:thunderbolt -> ../../../bus/pci/drivers/thunderbolt  [recursive, not followed]
│   ├── holders
│   ├── initsize
│   ├── initstate
│   ├── notes
│   ├── parameters
│   │   ├── asym_threshold
│   │   ├── bw_alloc_mode
│   │   ├── clx
│   │   ├── dma_credits
│   │   ├── host_reset
│   │   ├── start_icm
│   │   └── xdomain
│   ├── refcnt
│   ├── sections
│   │   ├── __bpf_raw_tp_map
│   │   ├── __bug_table
│   │   ├── __dyndbg
│   │   ├── _ftrace_events
│   │   ├── __jump_table
│   │   ├── __kcrctab_gpl
│   │   ├── __ksymtab_gpl
│   │   ├── __ksymtab_strings
│   │   ├── __mcount_loc
│   │   ├── __param
│   │   ├── __patchable_function_entries
│   │   ├── __tracepoints
│   │   ├── __tracepoints_ptrs
│   │   └── __tracepoints_strings
│   ├── srcversion
│   ├── taint
│   └── uevent
├── new_id
├── remove_id
├── uevent
└── unbind

116 directories, 307 files
/sys/bus/pci/drivers/thunderbolt/
├── 0000:c4:00.5 -> ../../../../devices/pci0000:00/0000:00:08.3/0000:c4:00.5
│   ├── ari_enabled
│   ├── broken_parity_status
│   ├── class
│   ├── config
│   ├── consistent_dma_mask_bits
│   ├── consumer:pci:0000:00:03.1 -> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1
│   │   ├── auto_remove_on
│   │   ├── consumer -> ../../../pci0000:00/0000:00:03.1
│   │   │   ├── 0000:00:03.1:pcie001
│   │   │   │   ├── driver -> ../../../../bus/pci_express/drivers/pcie_pme
│   │   │   │   ├── power
│   │   │   │   ├── subsystem -> ../../../../bus/pci_express
│   │   │   │   └── uevent
│   │   │   ├── 0000:00:03.1:pcie004
│   │   │   │   ├── driver -> ../../../../bus/pci_express/drivers/pciehp
│   │   │   │   ├── power
│   │   │   │   ├── subsystem -> ../../../../bus/pci_express  [recursive, not followed]
│   │   │   │   └── uevent
│   │   │   ├── aer_dev_correctable
│   │   │   ├── aer_dev_fatal
│   │   │   ├── aer_dev_nonfatal
│   │   │   ├── aer_rootport_total_err_cor
│   │   │   ├── aer_rootport_total_err_fatal
│   │   │   ├── aer_rootport_total_err_nonfatal
│   │   │   ├── ari_enabled
│   │   │   ├── broken_parity_status
│   │   │   ├── class
│   │   │   ├── config
│   │   │   ├── consistent_dma_mask_bits
│   │   │   ├── current_link_speed
│   │   │   ├── current_link_width
│   │   │   ├── d3cold_allowed
│   │   │   ├── device
│   │   │   ├── dma_mask_bits
│   │   │   ├── driver -> ../../../bus/pci/drivers/pcieport
│   │   │   │   ├── 0000:00:01.3 -> ../../../../devices/pci0000:00/0000:00:01.3
│   │   │   │   ├── 0000:00:02.1 -> ../../../../devices/pci0000:00/0000:00:02.1
│   │   │   │   ├── 0000:00:02.4 -> ../../../../devices/pci0000:00/0000:00:02.4
│   │   │   │   ├── 0000:00:03.1 -> ../../../../devices/pci0000:00/0000:00:03.1  [recursive, not followed]
│   │   │   │   ├── 0000:00:04.1 -> ../../../../devices/pci0000:00/0000:00:04.1
│   │   │   │   ├── 0000:00:08.1 -> ../../../../devices/pci0000:00/0000:00:08.1
│   │   │   │   ├── 0000:00:08.2 -> ../../../../devices/pci0000:00/0000:00:08.2
│   │   │   │   ├── 0000:00:08.3 -> ../../../../devices/pci0000:00/0000:00:08.3
│   │   │   │   ├── bind
│   │   │   │   ├── new_id
│   │   │   │   ├── remove_id
│   │   │   │   ├── uevent
│   │   │   │   └── unbind
│   │   │   ├── driver_override
│   │   │   ├── enable
│   │   │   ├── firmware_node -> ../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:13
│   │   │   │   ├── adr
│   │   │   │   ├── device:14
│   │   │   │   ├── LNXPOWER:04
│   │   │   │   ├── path
│   │   │   │   ├── physical_node -> ../../../../pci0000:00/0000:00:03.1  [recursive, not followed]
│   │   │   │   ├── power
│   │   │   │   ├── power_resources_D0
│   │   │   │   ├── power_resources_D3hot
│   │   │   │   ├── power_state
│   │   │   │   ├── real_power_state
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../../bus/acpi
│   │   │   │   ├── uevent
│   │   │   │   └── wakeup
│   │   │   ├── iommu -> ../0000:00:00.2/iommu/ivhd0
│   │   │   │   ├── amd-iommu
│   │   │   │   ├── device -> ../../../0000:00:00.2
│   │   │   │   ├── devices
│   │   │   │   ├── power
│   │   │   │   ├── subsystem -> ../../../../../class/iommu
│   │   │   │   └── uevent
│   │   │   ├── iommu_group -> ../../../kernel/iommu_groups/5
│   │   │   │   ├── devices
│   │   │   │   ├── reserved_regions
│   │   │   │   └── type
│   │   │   ├── irq
│   │   │   ├── link
│   │   │   ├── local_cpulist
│   │   │   ├── local_cpus
│   │   │   ├── max_link_speed
│   │   │   ├── max_link_width
│   │   │   ├── modalias
│   │   │   ├── msi_bus
│   │   │   ├── msi_irqs
│   │   │   │   └── 41
│   │   │   ├── numa_node
│   │   │   ├── pci_bus
│   │   │   │   └── 0000:04
│   │   │   ├── power
│   │   │   │   ├── autosuspend_delay_ms
│   │   │   │   ├── control
│   │   │   │   ├── runtime_active_time
│   │   │   │   ├── runtime_status
│   │   │   │   ├── runtime_suspended_time
│   │   │   │   ├── wakeup
│   │   │   │   ├── wakeup_abort_count
│   │   │   │   ├── wakeup_active
│   │   │   │   ├── wakeup_active_count
│   │   │   │   ├── wakeup_count
│   │   │   │   ├── wakeup_expire_count
│   │   │   │   ├── wakeup_last_time_ms
│   │   │   │   ├── wakeup_max_time_ms
│   │   │   │   └── wakeup_total_time_ms
│   │   │   ├── power_state
│   │   │   ├── remove
│   │   │   ├── rescan
│   │   │   ├── reset
│   │   │   ├── reset_method
│   │   │   ├── resource
│   │   │   ├── revision
│   │   │   ├── secondary_bus_number
│   │   │   ├── subordinate_bus_number
│   │   │   ├── subsystem -> ../../../bus/pci
│   │   │   │   ├── devices
│   │   │   │   ├── drivers
│   │   │   │   ├── drivers_autoprobe
│   │   │   │   ├── drivers_probe
│   │   │   │   ├── rescan
│   │   │   │   ├── resource_alignment
│   │   │   │   ├── slots
│   │   │   │   └── uevent
│   │   │   ├── subsystem_device
│   │   │   ├── subsystem_vendor
│   │   │   ├── supplier:pci:0000:c4:00.5 -> ../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1  [recursive, not followed]
│   │   │   ├── uevent
│   │   │   ├── vendor
│   │   │   └── wakeup
│   │   │       └── wakeup6
│   │   ├── runtime_pm
│   │   ├── status
│   │   ├── subsystem -> ../../../../class/devlink
│   │   │   ├── pci:0000:c2:00.0--pci:0000:c2:00.1 -> ../../devices/virtual/devlink/pci:0000:c2:00.0--pci:0000:c2:00.1
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── consumer -> ../../../pci0000:00/0000:00:08.1/0000:c2:00.1
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── supplier -> ../../../pci0000:00/0000:00:08.1/0000:c2:00.0
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   ├── pci:0000:c4:00.5--pci:0000:00:03.1 -> ../../devices/virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1  [recursive, not followed]
│   │   │   ├── pci:0000:c4:00.5--pci:0000:c4:00.3 -> ../../devices/virtual/devlink/pci:0000:c4:00.5--pci:0000:c4:00.3
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── consumer -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.3
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── supplier -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   ├── pci:0000:c4:00.6--pci:0000:00:04.1 -> ../../devices/virtual/devlink/pci:0000:c4:00.6--pci:0000:00:04.1
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── consumer -> ../../../pci0000:00/0000:00:04.1  [recursive, not followed]
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── supplier -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.6
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   ├── pci:0000:c4:00.6--pci:0000:c4:00.4 -> ../../devices/virtual/devlink/pci:0000:c4:00.6--pci:0000:c4:00.4
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── consumer -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.4
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── supplier -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.6  [recursive, not followed]
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   ├── platform:PNP0C14:00--wmi:05901221-D566-11D1-B2F0-00A0C9062910 -> ../../devices/virtual/devlink/platform:PNP0C14:00--wmi:05901221-D566-11D1-B2F0-00A0C9062910
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   └── platform:PNP0C14:00--wmi:ABBC0F6A-8EA1-11D1-00A0-C90629100000 -> ../../devices/virtual/devlink/platform:PNP0C14:00--wmi:ABBC0F6A-8EA1-11D1-00A0-C90629100000
│   │   │       ├── auto_remove_on
│   │   │       ├── runtime_pm
│   │   │       ├── status
│   │   │       ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │       ├── sync_state_only
│   │   │       └── uevent
│   │   ├── supplier -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   ├── sync_state_only
│   │   └── uevent
│   ├── consumer:pci:0000:c4:00.3 -> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:c4:00.3  [recursive, not followed]
│   ├── current_link_speed
│   ├── current_link_width
│   ├── d3cold_allowed
│   ├── device
│   ├── dma_mask_bits
│   ├── domain0
│   │   ├── 0-0
│   │   │   ├── authorized
│   │   │   ├── generation
│   │   │   ├── power
│   │   │   │   ├── autosuspend_delay_ms
│   │   │   │   ├── control
│   │   │   │   ├── runtime_active_time
│   │   │   │   ├── runtime_status
│   │   │   │   ├── runtime_suspended_time
│   │   │   │   ├── wakeup
│   │   │   │   ├── wakeup_abort_count
│   │   │   │   ├── wakeup_active
│   │   │   │   ├── wakeup_active_count
│   │   │   │   ├── wakeup_count
│   │   │   │   ├── wakeup_expire_count
│   │   │   │   ├── wakeup_last_time_ms
│   │   │   │   ├── wakeup_max_time_ms
│   │   │   │   └── wakeup_total_time_ms
│   │   │   ├── subsystem -> ../../../../../../bus/thunderbolt
│   │   │   │   ├── devices
│   │   │   │   ├── drivers
│   │   │   │   ├── drivers_autoprobe
│   │   │   │   ├── drivers_probe
│   │   │   │   └── uevent
│   │   │   ├── uevent
│   │   │   ├── unique_id
│   │   │   ├── usb4_port2
│   │   │   │   ├── link
│   │   │   │   ├── power
│   │   │   │   └── uevent
│   │   │   └── wakeup
│   │   │       └── wakeup50
│   │   ├── deauthorization
│   │   ├── iommu_dma_protection
│   │   ├── power
│   │   │   ├── wakeup
│   │   │   ├── wakeup_abort_count
│   │   │   ├── wakeup_active
│   │   │   ├── wakeup_active_count
│   │   │   ├── wakeup_count
│   │   │   ├── wakeup_expire_count
│   │   │   ├── wakeup_last_time_ms
│   │   │   ├── wakeup_max_time_ms
│   │   │   └── wakeup_total_time_ms
│   │   ├── security
│   │   ├── subsystem -> ../../../../../bus/thunderbolt  [recursive, not followed]
│   │   ├── uevent
│   │   └── wakeup
│   │       └── wakeup51
│   │           ├── active_count
│   │           ├── active_time_ms
│   │           ├── device -> ../../../domain0  [recursive, not followed]
│   │           ├── event_count
│   │           ├── expire_count
│   │           ├── last_change_ms
│   │           ├── max_time_ms
│   │           ├── name
│   │           ├── prevent_suspend_time_ms
│   │           ├── subsystem -> ../../../../../../../class/wakeup
│   │           ├── total_time_ms
│   │           ├── uevent
│   │           └── wakeup_count
│   ├── driver -> ../../../../bus/pci/drivers/thunderbolt  [recursive, not followed]
│   ├── driver_override
│   ├── enable
│   ├── firmware_node -> ../../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:36/device:3b
│   │   ├── adr
│   │   ├── path
│   │   ├── physical_node -> ../../../../../pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   ├── power
│   │   │   ├── autosuspend_delay_ms
│   │   │   ├── control
│   │   │   ├── runtime_active_time
│   │   │   ├── runtime_status
│   │   │   └── runtime_suspended_time
│   │   ├── power_resources_D0
│   │   │   └── LNXPOWER:0f -> ../../LNXPOWER:0f
│   │   │       ├── hid
│   │   │       ├── modalias
│   │   │       ├── path
│   │   │       ├── power
│   │   │       ├── resource_in_use
│   │   │       ├── status
│   │   │       ├── subsystem -> ../../../../../../bus/acpi  [recursive, not followed]
│   │   │       └── uevent
│   │   ├── power_resources_D3hot
│   │   │   └── LNXPOWER:0f -> ../../LNXPOWER:0f  [recursive, not followed]
│   │   ├── power_state
│   │   ├── real_power_state
│   │   ├── status
│   │   ├── subsystem -> ../../../../../../bus/acpi  [recursive, not followed]
│   │   ├── uevent
│   │   └── wakeup
│   │       └── wakeup29
│   │           ├── active_count
│   │           ├── active_time_ms
│   │           ├── device -> ../../../device:3b  [recursive, not followed]
│   │           ├── event_count
│   │           ├── expire_count
│   │           ├── last_change_ms
│   │           ├── max_time_ms
│   │           ├── name
│   │           ├── prevent_suspend_time_ms
│   │           ├── subsystem -> ../../../../../../../../class/wakeup  [recursive, not followed]
│   │           ├── total_time_ms
│   │           ├── uevent
│   │           └── wakeup_count
│   ├── iommu -> ../../0000:00:00.2/iommu/ivhd0  [recursive, not followed]
│   ├── iommu_group -> ../../../../kernel/iommu_groups/29
│   │   ├── devices
│   │   │   └── 0000:c4:00.5 -> ../../../../devices/pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   ├── reserved_regions
│   │   └── type
│   ├── irq
│   ├── link
│   │   ├── l0s_aspm
│   │   └── l1_aspm
│   ├── local_cpulist
│   ├── local_cpus
│   ├── max_link_speed
│   ├── max_link_width
│   ├── modalias
│   ├── msi_bus
│   ├── msi_irqs
│   │   ├── 57
│   │   ├── 59
│   │   ├── 60
│   │   ├── 61
│   │   ├── 62
│   │   ├── 63
│   │   ├── 64
│   │   ├── 65
│   │   ├── 66
│   │   ├── 67
│   │   ├── 68
│   │   ├── 69
│   │   ├── 70
│   │   ├── 71
│   │   ├── 72
│   │   └── 73
│   ├── numa_node
│   ├── pools
│   ├── power
│   │   ├── autosuspend_delay_ms
│   │   ├── control
│   │   ├── runtime_active_time
│   │   ├── runtime_status
│   │   ├── runtime_suspended_time
│   │   ├── wakeup
│   │   ├── wakeup_abort_count
│   │   ├── wakeup_active
│   │   ├── wakeup_active_count
│   │   ├── wakeup_count
│   │   ├── wakeup_expire_count
│   │   ├── wakeup_last_time_ms
│   │   ├── wakeup_max_time_ms
│   │   └── wakeup_total_time_ms
│   ├── power_state
│   ├── remove
│   ├── rescan
│   ├── reset
│   ├── reset_method
│   ├── resource
│   ├── resource0
│   ├── revision
│   ├── subsystem -> ../../../../bus/pci  [recursive, not followed]
│   ├── subsystem_device
│   ├── subsystem_vendor
│   ├── uevent
│   ├── vendor
│   └── wakeup
│       └── wakeup52
│           ├── active_count
│           ├── active_time_ms
│           ├── device -> ../../../0000:c4:00.5  [recursive, not followed]
│           ├── event_count
│           ├── expire_count
│           ├── last_change_ms
│           ├── max_time_ms
│           ├── name
│           ├── prevent_suspend_time_ms
│           ├── subsystem -> ../../../../../../class/wakeup  [recursive, not followed]
│           ├── total_time_ms
│           ├── uevent
│           └── wakeup_count
├── 0000:c4:00.6 -> ../../../../devices/pci0000:00/0000:00:08.3/0000:c4:00.6  [recursive, not followed]
├── bind
├── module -> ../../../../module/thunderbolt
│   ├── coresize
│   ├── drivers
│   │   └── pci:thunderbolt -> ../../../bus/pci/drivers/thunderbolt  [recursive, not followed]
│   ├── holders
│   ├── initsize
│   ├── initstate
│   ├── notes
│   ├── parameters
│   │   ├── asym_threshold
│   │   ├── bw_alloc_mode
│   │   ├── clx
│   │   ├── dma_credits
│   │   ├── host_reset
│   │   ├── start_icm
│   │   └── xdomain
│   ├── refcnt
│   ├── sections
│   │   ├── __bpf_raw_tp_map
│   │   ├── __bug_table
│   │   ├── __dyndbg
│   │   ├── _ftrace_events
│   │   ├── __jump_table
│   │   ├── __ksymtab_gpl
│   │   ├── __ksymtab_strings
│   │   ├── __mcount_loc
│   │   ├── __param
│   │   ├── __patchable_function_entries
│   │   ├── __tracepoints
│   │   ├── __tracepoints_ptrs
│   │   └── __tracepoints_strings
│   ├── taint
│   └── uevent
├── new_id
├── remove_id
├── uevent
└── unbind

125 directories, 294 files
Mika Westerberg June 21, 2024, 6:19 a.m. UTC | #3
Hi Mario,

On Thu, Jun 20, 2024 at 01:36:56PM -0500, Mario Limonciello wrote:
> On 6/20/2024 01:41, Mika Westerberg wrote:
> > +CC Mario from AMD side to check that we are good and don't break
> > anything accidentally.
> > 
> > On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
> > > The relationship between a USB4 Host Interface providing USB3 tunnels,
> > > and tunneled USB3 devices is not represented in device hierarchy.
> > > 
> > > This caused issues with power managment as devices may suspend and
> > > resume in incorrect order.
> > > A device link between the USB4 Host Interface and the USB3 xHCI was
> > > originally added to solve this, preventing the USB4 Host Interface from
> > > suspending if the USB3 xHCI Host was still active.
> > > This unfortunately also prevents USB4 Host Interface from suspending even
> > > if the USB3 xHCI Host is only serving native non-tunneled USB devices.
> > > 
> > > Improve the current powermanagement situation by creating device links
> > > directly from tunneled USB3 devices to the USB4 Host Interface they depend
> > > on instead of a device link between the hosts.
> > > This way USB4 host may suspend when the last tunneled device is
> > > disconnected.
> > > 
> > > Intel xHCI hosts are capable of reporting if connected USB3 devices are
> > > tunneled via vendor specific capabilities.
> > > Use this until a standard way is available.
> > > 
> > > Mathias Nyman (4):
> > >    xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
> > >    usb: Add tunneled parameter to usb device structure
> > >    usb: acpi: add device link between tunneled USB3 device and USB4 Host
> > >      Interface
> > >    thunderbolt: Don't create device link from USB4 Host Interface to USB3
> > >      xHC host
> > > 
> > >   drivers/thunderbolt/acpi.c       | 40 ++++++------------------
> > >   drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
> > >   drivers/usb/host/xhci-ext-caps.h |  5 +++
> > >   drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
> > >   drivers/usb/host/xhci.c          | 12 ++++++++
> > >   drivers/usb/host/xhci.h          |  1 +
> > >   include/linux/usb.h              |  2 ++
> > >   7 files changed, 111 insertions(+), 30 deletions(-)
> > > 
> > > -- 
> > > 2.25.1
> 
> Hi Mika,
> 
> Thanks for looping me in.  Unfortunately with this is appears the XHCI
> controller link never gets created.  I've not checked functional impact from
> this, but I'd guess there "should" be some functional problems too.

Thanks for checking!

I think the code that sets up the device link based on ->tunneled should
do that always if the hardware cannot tell if this is native or tunneled
link to keep the existing functionality and only do the "optimization"
if the hardware is capable of identifying that.

Perhaps it can be a callback provided by the xHCI controller driver
(->is_tunneled()) that then defaults to true if the
"usb4-host-interface" property is there and in case of Intel hardware
also checks the proprietary register?
Mario Limonciello June 21, 2024, 4:30 p.m. UTC | #4
On 6/21/2024 01:19, Mika Westerberg wrote:
> Hi Mario,
> 
> On Thu, Jun 20, 2024 at 01:36:56PM -0500, Mario Limonciello wrote:
>> On 6/20/2024 01:41, Mika Westerberg wrote:
>>> +CC Mario from AMD side to check that we are good and don't break
>>> anything accidentally.
>>>
>>> On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
>>>> The relationship between a USB4 Host Interface providing USB3 tunnels,
>>>> and tunneled USB3 devices is not represented in device hierarchy.
>>>>
>>>> This caused issues with power managment as devices may suspend and
>>>> resume in incorrect order.
>>>> A device link between the USB4 Host Interface and the USB3 xHCI was
>>>> originally added to solve this, preventing the USB4 Host Interface from
>>>> suspending if the USB3 xHCI Host was still active.
>>>> This unfortunately also prevents USB4 Host Interface from suspending even
>>>> if the USB3 xHCI Host is only serving native non-tunneled USB devices.
>>>>
>>>> Improve the current powermanagement situation by creating device links
>>>> directly from tunneled USB3 devices to the USB4 Host Interface they depend
>>>> on instead of a device link between the hosts.
>>>> This way USB4 host may suspend when the last tunneled device is
>>>> disconnected.
>>>>
>>>> Intel xHCI hosts are capable of reporting if connected USB3 devices are
>>>> tunneled via vendor specific capabilities.
>>>> Use this until a standard way is available.
>>>>
>>>> Mathias Nyman (4):
>>>>     xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
>>>>     usb: Add tunneled parameter to usb device structure
>>>>     usb: acpi: add device link between tunneled USB3 device and USB4 Host
>>>>       Interface
>>>>     thunderbolt: Don't create device link from USB4 Host Interface to USB3
>>>>       xHC host
>>>>
>>>>    drivers/thunderbolt/acpi.c       | 40 ++++++------------------
>>>>    drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
>>>>    drivers/usb/host/xhci-ext-caps.h |  5 +++
>>>>    drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
>>>>    drivers/usb/host/xhci.c          | 12 ++++++++
>>>>    drivers/usb/host/xhci.h          |  1 +
>>>>    include/linux/usb.h              |  2 ++
>>>>    7 files changed, 111 insertions(+), 30 deletions(-)
>>>>
>>>> -- 
>>>> 2.25.1
>>
>> Hi Mika,
>>
>> Thanks for looping me in.  Unfortunately with this is appears the XHCI
>> controller link never gets created.  I've not checked functional impact from
>> this, but I'd guess there "should" be some functional problems too.
> 
> Thanks for checking!
> 
> I think the code that sets up the device link based on ->tunneled should
> do that always if the hardware cannot tell if this is native or tunneled
> link to keep the existing functionality and only do the "optimization"
> if the hardware is capable of identifying that.
> 
> Perhaps it can be a callback provided by the xHCI controller driver
> (->is_tunneled()) that then defaults to true if the
> "usb4-host-interface" property is there and in case of Intel hardware
> also checks the proprietary register?

So I think the problem is you will have an ordering dependency between 
the two drivers for when the link gets created.

Like if thunderbolt.ko loads you would end up with links to PCIe root 
port for tunneling as well as XHCI controller.

Then xhci loads and you end up also adding links to individual ports.
Would you remove the link to the controller?

And if the order is the other way around you end up with a larger state 
machine.

How about PCIe core provides a helper to know whether or not a PCIe 
device will support the proprietary register?

Then thunderbolt.ko can use this to know whether or not to make the XHCI 
link and xhci.ko can use this to know whether or not to make the port links.

Hopefully that means no ordering dependencies.
Mika Westerberg June 24, 2024, 4:59 a.m. UTC | #5
On Fri, Jun 21, 2024 at 11:30:05AM -0500, Mario Limonciello wrote:
> On 6/21/2024 01:19, Mika Westerberg wrote:
> > Hi Mario,
> > 
> > On Thu, Jun 20, 2024 at 01:36:56PM -0500, Mario Limonciello wrote:
> > > On 6/20/2024 01:41, Mika Westerberg wrote:
> > > > +CC Mario from AMD side to check that we are good and don't break
> > > > anything accidentally.
> > > > 
> > > > On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
> > > > > The relationship between a USB4 Host Interface providing USB3 tunnels,
> > > > > and tunneled USB3 devices is not represented in device hierarchy.
> > > > > 
> > > > > This caused issues with power managment as devices may suspend and
> > > > > resume in incorrect order.
> > > > > A device link between the USB4 Host Interface and the USB3 xHCI was
> > > > > originally added to solve this, preventing the USB4 Host Interface from
> > > > > suspending if the USB3 xHCI Host was still active.
> > > > > This unfortunately also prevents USB4 Host Interface from suspending even
> > > > > if the USB3 xHCI Host is only serving native non-tunneled USB devices.
> > > > > 
> > > > > Improve the current powermanagement situation by creating device links
> > > > > directly from tunneled USB3 devices to the USB4 Host Interface they depend
> > > > > on instead of a device link between the hosts.
> > > > > This way USB4 host may suspend when the last tunneled device is
> > > > > disconnected.
> > > > > 
> > > > > Intel xHCI hosts are capable of reporting if connected USB3 devices are
> > > > > tunneled via vendor specific capabilities.
> > > > > Use this until a standard way is available.
> > > > > 
> > > > > Mathias Nyman (4):
> > > > >     xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
> > > > >     usb: Add tunneled parameter to usb device structure
> > > > >     usb: acpi: add device link between tunneled USB3 device and USB4 Host
> > > > >       Interface
> > > > >     thunderbolt: Don't create device link from USB4 Host Interface to USB3
> > > > >       xHC host
> > > > > 
> > > > >    drivers/thunderbolt/acpi.c       | 40 ++++++------------------
> > > > >    drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
> > > > >    drivers/usb/host/xhci-ext-caps.h |  5 +++
> > > > >    drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
> > > > >    drivers/usb/host/xhci.c          | 12 ++++++++
> > > > >    drivers/usb/host/xhci.h          |  1 +
> > > > >    include/linux/usb.h              |  2 ++
> > > > >    7 files changed, 111 insertions(+), 30 deletions(-)
> > > > > 
> > > > > -- 
> > > > > 2.25.1
> > > 
> > > Hi Mika,
> > > 
> > > Thanks for looping me in.  Unfortunately with this is appears the XHCI
> > > controller link never gets created.  I've not checked functional impact from
> > > this, but I'd guess there "should" be some functional problems too.
> > 
> > Thanks for checking!
> > 
> > I think the code that sets up the device link based on ->tunneled should
> > do that always if the hardware cannot tell if this is native or tunneled
> > link to keep the existing functionality and only do the "optimization"
> > if the hardware is capable of identifying that.
> > 
> > Perhaps it can be a callback provided by the xHCI controller driver
> > (->is_tunneled()) that then defaults to true if the
> > "usb4-host-interface" property is there and in case of Intel hardware
> > also checks the proprietary register?
> 
> So I think the problem is you will have an ordering dependency between the
> two drivers for when the link gets created.
> 
> Like if thunderbolt.ko loads you would end up with links to PCIe root port
> for tunneling as well as XHCI controller.

With this patch we only create links to PCIe Root/Downstream ports from
Thunderbolt side and the USB core will deal with the USB ones.

> Then xhci loads and you end up also adding links to individual ports.
> Would you remove the link to the controller?

See above.

> And if the order is the other way around you end up with a larger state
> machine.
> 
> How about PCIe core provides a helper to know whether or not a PCIe device
> will support the proprietary register?

I think the xHCI can be non-PCIe device too (Apple silicon for
instance). The links here are created dynamically and only if there is
need (and support from the hardware) so we can let the USB4 controller
enter D3hot if there is no USB 3.x tunnel needed.
Mario Limonciello June 24, 2024, 6:41 p.m. UTC | #6
>>
>> So I think the problem is you will have an ordering dependency between the
>> two drivers for when the link gets created.
>>
>> Like if thunderbolt.ko loads you would end up with links to PCIe root port
>> for tunneling as well as XHCI controller.
> 
> With this patch we only create links to PCIe Root/Downstream ports from
> Thunderbolt side and the USB core will deal with the USB ones.
> 
>> Then xhci loads and you end up also adding links to individual ports.
>> Would you remove the link to the controller?
> 
> See above.
> 
>> And if the order is the other way around you end up with a larger state
>> machine.
>>
>> How about PCIe core provides a helper to know whether or not a PCIe device
>> will support the proprietary register?
> 
> I think the xHCI can be non-PCIe device too (Apple silicon for
> instance). The links here are created dynamically and only if there is
> need (and support from the hardware) so we can let the USB4 controller
> enter D3hot if there is no USB 3.x tunnel needed.

When I replied I was under the presumption that the next version the 
link creation code for XHCI controller would stay in thunderbolt.ko and 
the XHCI port would be in xhci.ko.  But if you move both non-Intel and 
Intel cases to xhci.ko this should be totally fine.  If you can CC me on 
the next version of the series I'll get that tested for AMD case.

Thanks!
Mika Westerberg June 25, 2024, 5:02 a.m. UTC | #7
On Mon, Jun 24, 2024 at 01:41:09PM -0500, Mario Limonciello wrote:
> > > 
> > > So I think the problem is you will have an ordering dependency between the
> > > two drivers for when the link gets created.
> > > 
> > > Like if thunderbolt.ko loads you would end up with links to PCIe root port
> > > for tunneling as well as XHCI controller.
> > 
> > With this patch we only create links to PCIe Root/Downstream ports from
> > Thunderbolt side and the USB core will deal with the USB ones.
> > 
> > > Then xhci loads and you end up also adding links to individual ports.
> > > Would you remove the link to the controller?
> > 
> > See above.
> > 
> > > And if the order is the other way around you end up with a larger state
> > > machine.
> > > 
> > > How about PCIe core provides a helper to know whether or not a PCIe device
> > > will support the proprietary register?
> > 
> > I think the xHCI can be non-PCIe device too (Apple silicon for
> > instance). The links here are created dynamically and only if there is
> > need (and support from the hardware) so we can let the USB4 controller
> > enter D3hot if there is no USB 3.x tunnel needed.
> 
> When I replied I was under the presumption that the next version the link
> creation code for XHCI controller would stay in thunderbolt.ko and the XHCI
> port would be in xhci.ko.  But if you move both non-Intel and Intel cases to
> xhci.ko this should be totally fine.  If you can CC me on the next version
> of the series I'll get that tested for AMD case.

Sure, we will.
Mathias Nyman June 25, 2024, 2:37 p.m. UTC | #8
On 24.6.2024 7.59, Mika Westerberg wrote:
> On Fri, Jun 21, 2024 at 11:30:05AM -0500, Mario Limonciello wrote:
>> On 6/21/2024 01:19, Mika Westerberg wrote:
>>> Hi Mario,
>>>
>>> On Thu, Jun 20, 2024 at 01:36:56PM -0500, Mario Limonciello wrote:
>>>> On 6/20/2024 01:41, Mika Westerberg wrote:
>>>>> +CC Mario from AMD side to check that we are good and don't break
>>>>> anything accidentally.
>>>>>
>>>>> On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
>>>>>> The relationship between a USB4 Host Interface providing USB3 tunnels,
>>>>>> and tunneled USB3 devices is not represented in device hierarchy.
>>>>>>
>>>>>> This caused issues with power managment as devices may suspend and
>>>>>> resume in incorrect order.
>>>>>> A device link between the USB4 Host Interface and the USB3 xHCI was
>>>>>> originally added to solve this, preventing the USB4 Host Interface from
>>>>>> suspending if the USB3 xHCI Host was still active.
>>>>>> This unfortunately also prevents USB4 Host Interface from suspending even
>>>>>> if the USB3 xHCI Host is only serving native non-tunneled USB devices.
>>>>>>
>>>>>> Improve the current powermanagement situation by creating device links
>>>>>> directly from tunneled USB3 devices to the USB4 Host Interface they depend
>>>>>> on instead of a device link between the hosts.
>>>>>> This way USB4 host may suspend when the last tunneled device is
>>>>>> disconnected.
>>>>>>
>>>>>> Intel xHCI hosts are capable of reporting if connected USB3 devices are
>>>>>> tunneled via vendor specific capabilities.
>>>>>> Use this until a standard way is available.
>>>>>>
>>>>>> Mathias Nyman (4):
>>>>>>      xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
>>>>>>      usb: Add tunneled parameter to usb device structure
>>>>>>      usb: acpi: add device link between tunneled USB3 device and USB4 Host
>>>>>>        Interface
>>>>>>      thunderbolt: Don't create device link from USB4 Host Interface to USB3
>>>>>>        xHC host
>>>>>>
>>>>>>     drivers/thunderbolt/acpi.c       | 40 ++++++------------------
>>>>>>     drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
>>>>>>     drivers/usb/host/xhci-ext-caps.h |  5 +++
>>>>>>     drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
>>>>>>     drivers/usb/host/xhci.c          | 12 ++++++++
>>>>>>     drivers/usb/host/xhci.h          |  1 +
>>>>>>     include/linux/usb.h              |  2 ++
>>>>>>     7 files changed, 111 insertions(+), 30 deletions(-)
>>>>>>
>>>>>> -- 
>>>>>> 2.25.1
>>>>
>>>> Hi Mika,
>>>>
>>>> Thanks for looping me in.  Unfortunately with this is appears the XHCI
>>>> controller link never gets created.  I've not checked functional impact from
>>>> this, but I'd guess there "should" be some functional problems too.
>>>
>>> Thanks for checking!
>>>
>>> I think the code that sets up the device link based on ->tunneled should
>>> do that always if the hardware cannot tell if this is native or tunneled
>>> link to keep the existing functionality and only do the "optimization"
>>> if the hardware is capable of identifying that.
>>>
>>> Perhaps it can be a callback provided by the xHCI controller driver
>>> (->is_tunneled()) that then defaults to true if the
>>> "usb4-host-interface" property is there and in case of Intel hardware
>>> also checks the proprietary register?

How about changing the boolean udev->tunneled into a 3 value enum with
"link_unknown", "link_native", and "link_tunneled" options.

"link_unknown" would be default, xhci driver only changes this to "link_tunneled" or
"link_native" if the host can detect tunnels.

device link to USB4 host would be created for USB3 devices that have
"usb4-host-interface" property and udev->tunneled != native.

Thanks
Mathias
Mika Westerberg June 25, 2024, 2:45 p.m. UTC | #9
On Tue, Jun 25, 2024 at 05:37:27PM +0300, Mathias Nyman wrote:
> On 24.6.2024 7.59, Mika Westerberg wrote:
> > On Fri, Jun 21, 2024 at 11:30:05AM -0500, Mario Limonciello wrote:
> > > On 6/21/2024 01:19, Mika Westerberg wrote:
> > > > Hi Mario,
> > > > 
> > > > On Thu, Jun 20, 2024 at 01:36:56PM -0500, Mario Limonciello wrote:
> > > > > On 6/20/2024 01:41, Mika Westerberg wrote:
> > > > > > +CC Mario from AMD side to check that we are good and don't break
> > > > > > anything accidentally.
> > > > > > 
> > > > > > On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
> > > > > > > The relationship between a USB4 Host Interface providing USB3 tunnels,
> > > > > > > and tunneled USB3 devices is not represented in device hierarchy.
> > > > > > > 
> > > > > > > This caused issues with power managment as devices may suspend and
> > > > > > > resume in incorrect order.
> > > > > > > A device link between the USB4 Host Interface and the USB3 xHCI was
> > > > > > > originally added to solve this, preventing the USB4 Host Interface from
> > > > > > > suspending if the USB3 xHCI Host was still active.
> > > > > > > This unfortunately also prevents USB4 Host Interface from suspending even
> > > > > > > if the USB3 xHCI Host is only serving native non-tunneled USB devices.
> > > > > > > 
> > > > > > > Improve the current powermanagement situation by creating device links
> > > > > > > directly from tunneled USB3 devices to the USB4 Host Interface they depend
> > > > > > > on instead of a device link between the hosts.
> > > > > > > This way USB4 host may suspend when the last tunneled device is
> > > > > > > disconnected.
> > > > > > > 
> > > > > > > Intel xHCI hosts are capable of reporting if connected USB3 devices are
> > > > > > > tunneled via vendor specific capabilities.
> > > > > > > Use this until a standard way is available.
> > > > > > > 
> > > > > > > Mathias Nyman (4):
> > > > > > >      xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
> > > > > > >      usb: Add tunneled parameter to usb device structure
> > > > > > >      usb: acpi: add device link between tunneled USB3 device and USB4 Host
> > > > > > >        Interface
> > > > > > >      thunderbolt: Don't create device link from USB4 Host Interface to USB3
> > > > > > >        xHC host
> > > > > > > 
> > > > > > >     drivers/thunderbolt/acpi.c       | 40 ++++++------------------
> > > > > > >     drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
> > > > > > >     drivers/usb/host/xhci-ext-caps.h |  5 +++
> > > > > > >     drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
> > > > > > >     drivers/usb/host/xhci.c          | 12 ++++++++
> > > > > > >     drivers/usb/host/xhci.h          |  1 +
> > > > > > >     include/linux/usb.h              |  2 ++
> > > > > > >     7 files changed, 111 insertions(+), 30 deletions(-)
> > > > > > > 
> > > > > > > -- 
> > > > > > > 2.25.1
> > > > > 
> > > > > Hi Mika,
> > > > > 
> > > > > Thanks for looping me in.  Unfortunately with this is appears the XHCI
> > > > > controller link never gets created.  I've not checked functional impact from
> > > > > this, but I'd guess there "should" be some functional problems too.
> > > > 
> > > > Thanks for checking!
> > > > 
> > > > I think the code that sets up the device link based on ->tunneled should
> > > > do that always if the hardware cannot tell if this is native or tunneled
> > > > link to keep the existing functionality and only do the "optimization"
> > > > if the hardware is capable of identifying that.
> > > > 
> > > > Perhaps it can be a callback provided by the xHCI controller driver
> > > > (->is_tunneled()) that then defaults to true if the
> > > > "usb4-host-interface" property is there and in case of Intel hardware
> > > > also checks the proprietary register?
> 
> How about changing the boolean udev->tunneled into a 3 value enum with
> "link_unknown", "link_native", and "link_tunneled" options.
> 
> "link_unknown" would be default, xhci driver only changes this to "link_tunneled" or
> "link_native" if the host can detect tunnels.
> 
> device link to USB4 host would be created for USB3 devices that have
> "usb4-host-interface" property and udev->tunneled != native.

Sounds good to me :)
Mario Limonciello June 25, 2024, 2:55 p.m. UTC | #10
On 6/25/2024 09:45, Mika Westerberg wrote:
> On Tue, Jun 25, 2024 at 05:37:27PM +0300, Mathias Nyman wrote:
>> On 24.6.2024 7.59, Mika Westerberg wrote:
>>> On Fri, Jun 21, 2024 at 11:30:05AM -0500, Mario Limonciello wrote:
>>>> On 6/21/2024 01:19, Mika Westerberg wrote:
>>>>> Hi Mario,
>>>>>
>>>>> On Thu, Jun 20, 2024 at 01:36:56PM -0500, Mario Limonciello wrote:
>>>>>> On 6/20/2024 01:41, Mika Westerberg wrote:
>>>>>>> +CC Mario from AMD side to check that we are good and don't break
>>>>>>> anything accidentally.
>>>>>>>
>>>>>>> On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
>>>>>>>> The relationship between a USB4 Host Interface providing USB3 tunnels,
>>>>>>>> and tunneled USB3 devices is not represented in device hierarchy.
>>>>>>>>
>>>>>>>> This caused issues with power managment as devices may suspend and
>>>>>>>> resume in incorrect order.
>>>>>>>> A device link between the USB4 Host Interface and the USB3 xHCI was
>>>>>>>> originally added to solve this, preventing the USB4 Host Interface from
>>>>>>>> suspending if the USB3 xHCI Host was still active.
>>>>>>>> This unfortunately also prevents USB4 Host Interface from suspending even
>>>>>>>> if the USB3 xHCI Host is only serving native non-tunneled USB devices.
>>>>>>>>
>>>>>>>> Improve the current powermanagement situation by creating device links
>>>>>>>> directly from tunneled USB3 devices to the USB4 Host Interface they depend
>>>>>>>> on instead of a device link between the hosts.
>>>>>>>> This way USB4 host may suspend when the last tunneled device is
>>>>>>>> disconnected.
>>>>>>>>
>>>>>>>> Intel xHCI hosts are capable of reporting if connected USB3 devices are
>>>>>>>> tunneled via vendor specific capabilities.
>>>>>>>> Use this until a standard way is available.
>>>>>>>>
>>>>>>>> Mathias Nyman (4):
>>>>>>>>       xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
>>>>>>>>       usb: Add tunneled parameter to usb device structure
>>>>>>>>       usb: acpi: add device link between tunneled USB3 device and USB4 Host
>>>>>>>>         Interface
>>>>>>>>       thunderbolt: Don't create device link from USB4 Host Interface to USB3
>>>>>>>>         xHC host
>>>>>>>>
>>>>>>>>      drivers/thunderbolt/acpi.c       | 40 ++++++------------------
>>>>>>>>      drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
>>>>>>>>      drivers/usb/host/xhci-ext-caps.h |  5 +++
>>>>>>>>      drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
>>>>>>>>      drivers/usb/host/xhci.c          | 12 ++++++++
>>>>>>>>      drivers/usb/host/xhci.h          |  1 +
>>>>>>>>      include/linux/usb.h              |  2 ++
>>>>>>>>      7 files changed, 111 insertions(+), 30 deletions(-)
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> 2.25.1
>>>>>>
>>>>>> Hi Mika,
>>>>>>
>>>>>> Thanks for looping me in.  Unfortunately with this is appears the XHCI
>>>>>> controller link never gets created.  I've not checked functional impact from
>>>>>> this, but I'd guess there "should" be some functional problems too.
>>>>>
>>>>> Thanks for checking!
>>>>>
>>>>> I think the code that sets up the device link based on ->tunneled should
>>>>> do that always if the hardware cannot tell if this is native or tunneled
>>>>> link to keep the existing functionality and only do the "optimization"
>>>>> if the hardware is capable of identifying that.
>>>>>
>>>>> Perhaps it can be a callback provided by the xHCI controller driver
>>>>> (->is_tunneled()) that then defaults to true if the
>>>>> "usb4-host-interface" property is there and in case of Intel hardware
>>>>> also checks the proprietary register?
>>
>> How about changing the boolean udev->tunneled into a 3 value enum with
>> "link_unknown", "link_native", and "link_tunneled" options.
>>
>> "link_unknown" would be default, xhci driver only changes this to "link_tunneled" or
>> "link_native" if the host can detect tunnels.
>>
>> device link to USB4 host would be created for USB3 devices that have
>> "usb4-host-interface" property and udev->tunneled != native.
> 
> Sounds good to me :)

I think it's a good idea as well.  Could you leave a dynamic debugging 
statement with the result?

I think this could be helpful for rooting out unexpected BIOS problems 
with this.