LKML Archive mirror
 help / color / mirror / Atom feed
From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Mario Limonciello <mario.limonciello@amd.com>,
	Esther Shimanovich <eshimanovich@chromium.org>,
	Lukas Wunner <lukas@wunner.de>,
	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: Fri, 19 Jan 2024 09:48:29 +0200	[thread overview]
Message-ID: <20240119074829.GD2543524@black.fi.intel.com> (raw)
In-Reply-To: <20240119053756.GC2543524@black.fi.intel.com>

On Fri, Jan 19, 2024 at 07:37:56AM +0200, Mika Westerberg wrote:
> On Thu, Jan 18, 2024 at 08:12:56AM -0800, Dmitry Torokhov wrote:
> > On Thu, Jan 18, 2024 at 09:47:07AM -0600, Mario Limonciello wrote:
> > > On 1/18/2024 00:00, Mika Westerberg wrote:
> > > > > Before my patch, you see that the JHL6540 controller is inaccurately
> > > > > labeled “removable”:
> > > > > $ udevadm info -a -p /sys/bus/pci/devices/0000:05:00.0 | grep -e
> > > > > {removable} -e {device} -e {vendor} -e looking
> > > > >    looking at device '/devices/pci0000:00/0000:00:1d.4/0000:05:00.0':
> > > > >      ATTR{device}=="0x15d3"
> > > > >      ATTR{removable}=="removable"
> > > > >      ATTR{vendor}=="0x8086"
> > > > 
> > > > This is actually accurate. The Thunderbolt controller is itself
> > > > hot-removable and that BTW happens to be hot-removed when fwupd applies
> > > > firmware upgrades to the device.
> > 
> > This is quite interesting take. Does fwupd rip the controller out of the
> > box to update it? By that account your touchpad is also removable as it
> > may stop functioning when its firmware gets updated.
> 
> The Thunderbolt controller is connected to a hotpluggable PCIe root port
> so it will be dissappear from the userspace so that "removable" in that
> sense is accurate.

There are systems as well where the Thunderbolt (and/or xHCI) controller
only appears if there is anything plugged to the physical Type-C ports
and it gets removed pretty soon after the physical device gets
unplugged. These are also the same Alpine Ridge and Titan Ridge
controllers that this patch is dealing with.

I tried to think about some sort of more generic heuristic how to figure
out that the controller is actually inside the physical system but there
is a problem that the same controller can appear on the bus as well, eg.
you plug in Thunderbolt dock and that one has xHCI controller too. That
device should definitely be "removable". With the "software CM" systems
we have a couple of additional hints in the ACPI tables that can be used
to identify the "tunneled" ports but this does not apply to the older
systems I'm afraid.

Now if I understand the reason behind this patch is actually not about
"removability" that much than about identifying a trusted vs. untrusted
device and attaching a driver to those. I was under impression that
there is already a solution to this in ChromeOS kernel. It has an
allowlist of drivers that are allowed to attach these devices and that
includes the PCIe port drivers, xhci_hcd and the thunderbolt driver,
possibly something else too. Is this not working for your case?

  reply	other threads:[~2024-01-19  7:58 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 [this message]
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
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=20240119074829.GD2543524@black.fi.intel.com \
    --to=mika.westerberg@linux.intel.com \
    --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=lukas@wunner.de \
    --cc=mario.limonciello@amd.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).