Message-ID: <ZjsKPSgV39SF0gdX@wunner.de>
Date: Wed, 8 May 2024 07:14:37 +0200
From: Lukas Wunner <lukas@...ner.de>
To: Esther Shimanovich <eshimanovich@...omium.org>
Cc: Mika Westerberg <mika.westerberg@...ux.intel.com>,
	Mario Limonciello <mario.limonciello@....com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, Rajat Jain <rajatja@...gle.com>
Subject: Re: [PATCH v4] PCI: Relabel JHL6540 on Lenovo X1 Carbon 7,8

On Wed, May 01, 2024 at 06:23:28PM -0400, Esther Shimanovich wrote:
> On Sat, Apr 27, 2024 at 3:17AM Lukas Wunner <lukas@...ner.de> wrote:
> That is correct, when the user-visible issue occurs, no driver is
> bound to the NHI and XHCI. The discrete JHL chip is not permitted to
> attach to the external-facing root port because of the security
> policy, so the NHI and XHCI are not seen by the computer.

Could you rework your patch to only rectify the NHI's and XHCI's
device properties and leave the bridges untouched?

The thunderbolt driver will then rectify the bridge's properties
using the patches on this branch (particularly the one named
"thunderbolt: Mark PCIe Adapters on Root Switch as non-removable"):

https://github.com/l1k/linux/commits/thunderbolt_associate_v1

This approach keeps most of the code in the thunderbolt driver
(which has a very clear picture which PCI bridges belong to the
Host Router and which to Device Routers).  The footprint in the
PCI core is thus kept minimal, which increases upstream
acceptability of your patch.

You can match the NHI using DECLARE_PCI_FIXUP_CLASS_FINAL():

* Search for PCI_CLASS_SERIAL_USB_USB4 with class shift 0
  to match a USB4 Host Interface from any vendor.
* Seach for PCI_CLASS_SYSTEM_OTHER with class shift 8
  to match a Thunderbolt 1 to 3 Host Interface.
  I recommend checking the is_thunderbolt bit on those devices
  to avoid matching a non-NHI.

Then fixup the device properties of the NHI so that it can bind.

To also rectify the properties of the XHCI, you'd have to use
pci_upstream_bridge() to find the Downstream Port above, check
whether that's non-NULL.  The bus on which the Downstream Port
resides is pdev->bus.  On all Host Routers I know, the XHCI is
below slot 02.0 on that bus, so you could use pci_get_slot()
to find that Downstream Port, then use pci_get_slot() again
to find slot 00.0 on that bridge's subordinate bus.  If that
device has class PCI_CLASS_SERIAL_USB_XHCI, you've found the
XHCI and can rectify its properties.  Device references acquired
with pci_get_*() need to be returned with pci_dev_put().

The quirk should be #ifdef'ed to CONFIG_ACPI.  Alternatively,
it could be declared in pci-acpi.c near pci_acpi_set_external_facing().


> > However that doesn't appear to be sufficient:  I notice that in your
> > patch, you're also clearing the external_facing bit on the Root Port
> > above the discrete host controller.
> 
> Rajat (rajatja@...gle.com) in an internal review had suggested I add
> that, and leave it up to kernel maintainers to decide if it's strictly
> necessary.

I'd recommend to leave the Root Port's properties untouched
unless that's necessary.


> I don???t have this device available at my office. I just saw that
> StarTech sells a universal laptop docking station with chipset-id
> Intel - Alpine Ridge DSL6540. Then I looked up the device, and found
> it here: https://linux-hardware.org/?id=pci:8086-1577-8086-0000
> 
> Therefore, I concluded that the DSL6540 has an NHI component.
> 
> If these logs are important, I could probably make a case to purchase
> that docking station and get the info that you need. Please let me
> know!

Never mind, this scenario is being tested internally at Intel
and the above-linked branch contains a commit to avoid binding
to a Host Interface exposed by a Device Router.

Thanks,

Lukas

Powered by blists - more mailing lists