All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Esther Shimanovich <eshimanovich@chromium.org>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>,
	Mario Limonciello <mario.limonciello@amd.com>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	Rajat Jain <rajatja@google.com>
Subject: Re: [PATCH v4] PCI: Relabel JHL6540 on Lenovo X1 Carbon 7,8
Date: Wed, 8 May 2024 07:14:37 +0200	[thread overview]
Message-ID: <ZjsKPSgV39SF0gdX@wunner.de> (raw)
In-Reply-To: <CA+Y6NJE8hA+wt+auW1wJBWA6EGMc6CGpmdExr3475E_Yys-Zdw@mail.gmail.com>

On Wed, May 01, 2024 at 06:23:28PM -0400, Esther Shimanovich wrote:
> On Sat, Apr 27, 2024 at 3:17AM Lukas Wunner <lukas@wunner.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@google.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

  parent reply	other threads:[~2024-05-08  5:14 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-21 20:53 [PATCH v4] PCI: Relabel JHL6540 on Lenovo X1 Carbon 7,8 Esther Shimanovich
2023-12-21 23:15 ` Dmitry Torokhov
2023-12-27  0:15 ` Bjorn Helgaas
2023-12-28 13:25 ` Lukas Wunner
2023-12-28 13:39   ` Mika Westerberg
2024-01-17 21:21     ` Esther Shimanovich
2024-01-18  6:00       ` Mika Westerberg
2024-01-18 15:47         ` Mario Limonciello
2024-01-18 16:12           ` Dmitry Torokhov
2024-01-18 16:21             ` Dmitry Torokhov
2024-01-19  5:37             ` Mika Westerberg
2024-01-19  7:48               ` Mika Westerberg
2024-01-19 10:22                 ` Mika Westerberg
2024-01-19 16:03                   ` Esther Shimanovich
2024-01-22  6:10                     ` Mika Westerberg
2024-01-22 23:50                   ` Mario Limonciello
2024-01-23  6:18                     ` Mika Westerberg
2024-01-25 23:45                       ` Esther Shimanovich
2024-04-15 22:34                         ` Esther Shimanovich
2024-04-16  5:03                           ` Mika Westerberg
2024-04-18 19:43                             ` Esther Shimanovich
2024-04-19  4:49                               ` Mika Westerberg
2024-04-22 19:17                                 ` Esther Shimanovich
2024-04-22 19:21                                   ` Mario Limonciello
2024-04-23  5:33                                     ` Mika Westerberg
2024-04-23  8:31                                       ` Lukas Wunner
2024-04-23  8:40                                         ` Mika Westerberg
2024-04-23 16:59                                       ` Mario Limonciello
2024-04-24  8:56                                         ` Mika Westerberg
2024-04-25 21:16                                           ` Esther Shimanovich
2024-04-26  4:52                                             ` Mika Westerberg
2024-04-26 15:58                                               ` Esther Shimanovich
2024-04-27  5:35                                               ` Lukas Wunner
2024-04-27  7:41                                                 ` Mika Westerberg
2024-04-27  7:08                                             ` Lukas Wunner
2024-04-27 15:09                                             ` Lukas Wunner
2024-05-01 22:23                                               ` Esther Shimanovich
2024-05-02  4:38                                                 ` Mika Westerberg
2024-05-02  9:54                                                   ` Mario Limonciello
2024-05-02 10:07                                                     ` Mika Westerberg
2024-05-08  5:14                                                 ` Lukas Wunner [this message]
2024-05-10  5:26                                                   ` Mika Westerberg
2024-05-10 15:44                                                     ` Esther Shimanovich
2024-05-11  4:38                                                       ` Mika Westerberg
2024-05-11  5:43                                                         ` Mika Westerberg
2024-05-15 18:53                                                           ` Esther Shimanovich
2024-05-15 20:35                                                             ` Lukas Wunner
2024-05-15 20:51                                                               ` Lukas Wunner
2024-05-15 21:44                                                                 ` Esther Shimanovich
2024-05-16  8:30                                                               ` Mika Westerberg
2024-05-16 10:03                                                                 ` Mika Westerberg
2024-05-16  9:16                                                             ` Mika Westerberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZjsKPSgV39SF0gdX@wunner.de \
    --to=lukas@wunner.de \
    --cc=bhelgaas@google.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=eshimanovich@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=mika.westerberg@linux.intel.com \
    --cc=rajatja@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.