Linux USB
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: "mika.westerberg@linux.intel.com" <mika.westerberg@linux.intel.com>
Cc: "Limonciello, Mario" <Mario.Limonciello@amd.com>,
	"andreas.noever@gmail.com" <andreas.noever@gmail.com>,
	"michael.jamet@intel.com" <michael.jamet@intel.com>,
	"YehezkelShB@gmail.com" <YehezkelShB@gmail.com>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>
Subject: Re: [PATCH] thunderbolt: Make iommu_dma_protection more accurate
Date: Fri, 18 Mar 2022 14:08:16 +0000	[thread overview]
Message-ID: <78fc0426-c22a-ec62-f92b-0019bea5947e@arm.com> (raw)
In-Reply-To: <YjSCWaq7Ej/2iJPp@lahna>

On 2022-03-18 13:25, mika.westerberg@linux.intel.com wrote:
> Hi Robin,
> 
> On Fri, Mar 18, 2022 at 12:01:42PM +0000, Robin Murphy wrote:
>>> This adds quite a lot code and complexity, and honestly I would like to
>>> keep it as simple as possible (and this is not enough because we need to
>>> make sure the DMAR bit is there so that none of the possible connected
>>> devices were able to overwrite our memory already).
>>
>> Shall we forget the standalone sibling check and just make the
>> pdev->untrusted check directly in tb_acpi_add_link() then?
> 
> I think we should leave tb_acpi_add_link() untouched if possible ;-)
> This is because it is used to add the device links from firmware
> description that we need for proper power management of the tunneled
> devices. It has little to do with the identification of the external
> facing DMA-capable PCIe ports.
> 
> Furthermore these links only exists in USB4 software connection manager
> systems so we do not have those in the existing Thunderbolt 3/4 systems
> that use firmware based connection manager (pretty much all out there).
> 
>> On reflection I guess the DMAR bit makes iommu_dma_protection
>> functionally dependent on ACPI already, so we don't actually lose
>> anything (and anyone can come back and revisit firmware-agnostic
>> methods later if a need appears).
> 
> I agree.

OK, so do we have any realistic options for identifying the correct PCI 
devices, if USB4 PCIe adapters might be anywhere relative to their 
associated NHI? Short of maintaining a list of known IDs, the only 
thought I have left is that if we walk the whole PCI segment looking 
specifically for hotplug-capable Gen1 ports, any system modern enough to 
have Thunderbolt is *probably* not going to have any real PCIe Gen1 
hotplug slots, so maybe false negatives might be tolerable, but it still 
feels like a bit of a sketchy heuristic.

I suppose we could just look to see if any device anywhere is marked as 
external-facing, and hope that if firmware's done that much then it's 
done everything right. That's still at least slightly better than what 
we have today, but AFAICS still carries significant risk of a false 
positive for an add-in card that firmware didn't recognise.

I'm satisfied that we've come round to the right conclusion on the DMAR 
opt-in - I'm in the middle or writing up patches for that now - but even 
Microsoft's spec gives that as a separate requirement from the flagging 
of external ports, with both being necessary for Kernel DMA Protection.

Robin.

  reply	other threads:[~2022-03-18 14:08 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-17 16:17 [PATCH] thunderbolt: Make iommu_dma_protection more accurate Robin Murphy
2022-03-17 16:58 ` Greg KH
2022-03-17 17:09 ` Limonciello, Mario
2022-03-17 20:36   ` Limonciello, Mario
2022-03-18 11:38     ` mika.westerberg
2022-03-18 12:01       ` Robin Murphy
2022-03-18 13:25         ` mika.westerberg
2022-03-18 14:08           ` Robin Murphy [this message]
2022-03-18 14:22             ` Limonciello, Mario
2022-03-18 14:47             ` mika.westerberg
2022-03-18 15:15               ` Robin Murphy
2022-03-18 15:23                 ` mika.westerberg
2022-03-18 14:51             ` Lukas Wunner
2022-03-18 15:10               ` Lukas Wunner
2022-03-18 15:11               ` mika.westerberg
2022-03-18  6:44 ` Mika Westerberg
2022-03-18 11:54   ` Robin Murphy

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=78fc0426-c22a-ec62-f92b-0019bea5947e@arm.com \
    --to=robin.murphy@arm.com \
    --cc=Mario.Limonciello@amd.com \
    --cc=YehezkelShB@gmail.com \
    --cc=andreas.noever@gmail.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=michael.jamet@intel.com \
    --cc=mika.westerberg@linux.intel.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