From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Esther Shimanovich <eshimanovich@chromium.org>
Cc: Lukas Wunner <lukas@wunner.de>,
Mario Limonciello <mario.limonciello@amd.com>,
Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
Rajat Jain <rajatja@google.com>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>
Subject: Re: [PATCH v4] PCI: Relabel JHL6540 on Lenovo X1 Carbon 7,8
Date: Thu, 18 Jan 2024 08:00:02 +0200 [thread overview]
Message-ID: <20240118060002.GV2543524@black.fi.intel.com> (raw)
In-Reply-To: <CA+Y6NJFQq39WSSwHwm37ZQV8_rwX+6k5r+0uUs_d1+UyGGLqUw@mail.gmail.com>
On Wed, Jan 17, 2024 at 04:21:18PM -0500, Esther Shimanovich wrote:
> Thank you for all your comments! I really appreciate all your help
> with this. I will address the style feedback once we reach a decision
> on how we will fix this bug.
> I first will respond to your comments, and then I will list out the
> possible solutions to this bug, in a way that takes into account all
> of your insights.
>
> On Tue, Dec 26, 2023 at 7:15 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > Can you include a citation (spec name, revision, section) for this
> > DMAR requirement?
> >
> This was my mistake–I misinterpreted what a firmware developer told
> me. This is a firmware ACPI requirement from windows, which is not in
> the DMAR spec. Windows uses it to identify externally exposed PCIE
> root ports.
> https://learn.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-externally-exposed-pcie-root-ports
>
> > But I don't see where the defect is here. And I doubt that this is
> > really a unique situation. So it's likely that this will happen on
> > other systems, and we don't want to have to add quirks every time
> > another one shows up.
> ...
> > don't have the new interface. But we at least need a plan that
> > doesn't require quirks indefinitely.
> ...
> On Thu, Dec 28, 2023 at 8:41 AM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > This is not scalable at all. You would need to include lots of systems
> > here. And there should be no issue at all anyways.
> My team tests hundreds of different devices, and this is the only one
> which exhibited this issue that we’ve seen so far.
> No other devices we’ve seen so far have a discrete internal
> Thunderbolt controller which is treated as a removable device.
> Therefore, we don’t expect that a large number of devices will need
> this quirk.
Well that's pretty much all Intel Titan Ridge and Maple Ridge based
systems. Some early ones did not use IOMMU but all the rest do.
> > There is really nothing "unique" here. It's exactly as specified by
> > this:
> >
> > https://learn.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-externally-exposed-pcie-root-ports
> >
> > and being used in many many system already out there and those have been
> > working just fine.
> I don’t know how many computers have a discrete Thunderbolt chip that
> is separate from their CPU, but this doesn’t seem to be a common
> occurrence.
> These devices were made during a narrow window of time when CPUs
> didn’t have Thunderbolt features yet, so a separate JHL6540 chip had
> to be added so that Lenovo could include Thunderbolt on X1 Carbon Gen
> 7/8.
Before Intel Ice Lake it was all discrete and it is still discrete with
the Barlow Ridge controller who will have exact same ExternalFacing port
as the previous models.
> As you said, these devices do indeed work fine in cases where you
> don’t care if a PCI Thunderbolt device is internal or external, which
> is most cases.
> Problems happen only whenever someone adds a security policy, or some
> other feature that cares about the distinction between a fixed or
> removable PCI device.
Do we have such security policy in the mainline kernel?
> > This has been working just fine so far and as far as I can tell there is
> > no such "policy" in place in the mainline kernel.
> Correct, there is no such policy in the mainline kernel as of now. The
> bug is that the linux kernel’s “removable” property is inaccurate for
> this device.
Or perhaps the "policy" should know this better? IIRC there were some
"exceptions" in the Chrome kernel that allowed to put these devices into
"allowlist" is this not the case anymore?
> > Can you elaborate what the issue is and which mainline kernel you are
> > using to reproduce this?
> Thanks for this question! On a Lenovo Thinkpad Gen 7/Gen 8 computer
> with the linux kernel installed, when you look at the properties of
> the JHL6540 Thunderbolt controller, you see that it is incorrectly
> labeled as removable. I have replicated this bug on the b85ea95d0864
> Linux 6.7-rc1 kernel.
>
> 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.
next prev parent reply other threads:[~2024-01-18 6:01 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 [this message]
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
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=20240118060002.GV2543524@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).