Re: [PATCH] thunderbolt: Stop using iommu_present()

From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: "Limonciello, Mario" <Mario.Limonciello@amd.com>
Cc: Robin Murphy <robin.murphy@arm.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>, "hch@lst.de" <hch@lst.de>
Subject: Re: [PATCH] thunderbolt: Stop using iommu_present()
Date: Wed, 16 Mar 2022 19:37:05 +0200	[thread overview]
Message-ID: <YjIgQfmcw6fydkXd@lahna> (raw)
In-Reply-To: <BL1PR12MB515762E68F3A48A97EB2DC89E2119@BL1PR12MB5157.namprd12.prod.outlook.com>

Hi Mario,

On Wed, Mar 16, 2022 at 05:24:38PM +0000, Limonciello, Mario wrote:
> [Public]
> 
> > On Wed, Mar 16, 2022 at 02:49:09PM +0000, Robin Murphy wrote:
> > > > What we want is to make sure the Tunneled PCIe ports get the full
> > IOMMU
> > > > protection. In case of the discrete above it is also fine if all the
> > > > devices behind the PCIe root port get the full IOMMU protection. Note in
> > > > the integrated all the devices are "siblings".
> > >
> > > Ah, OK, I wasn't aware that the NHI isn't even the right thing in the first
> > > place :(
> > >
> > > Is there an easy way to get from the struct tb to a PCI device representing
> > > the end of its relevant tunnel, or do we have a circular dependency
> > problem
> > > where the latter won't appear until we've authorised it (and thus the
> > IOMMU
> > > layer won't know about it yet either)?
> > 
> > The PCIe root ports (and the PCIe downstream ports) are there already
> > even without "authorization".
> > 
> > There is a way to figure out the "tunneled" PCIe ports by looking at
> > certain properties and we do that already actually. The BIOS has the
> > following under these ports:
> > 
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs
> > .microsoft.com%2Fen-us%2Fwindows-hardware%2Fdrivers%2Fpci%2Fdsd-
> > for-pcie-root-ports%23identifying-externally-exposed-pcie-root-
> > ports&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7C0465d319a
> > 6684335d9c208da07710e7c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7
> > C0%7C637830479402895833%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
> > LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&am
> > p;sdata=z6hpYGpj%2B%2BVvz9d6MXiO4N66PUm4zwhOdI%2Br6l3PjhQ%3D
> > &amp;reserved=0
> > 
> > and the ports will have dev->external_facing set to 1. Perhaps looking
> > at that field helps here?
> 
> External facing isn't a guarantee from the firmware though.  It's something we
> all expect in practice, but I think it's better to look at the ones that are from
> the _DSD usb4-host-interface to be safer.

Right but then we have the discrete ones with the DVSEC that exposes the
tunneled ports :(

> Mika, you might not have seen it yet, but I sent a follow up diff in this thread
> to Robin's patch.  If that looks good Robin can submit a v2 (or I'm happy to do
> so as well as I confirmed it helps my original intent too).

I saw it now and I'm thinking are we making this unnecessary complex? I
mean Microsoft solely depends on the DMAR platform opt-in flag:

  https://docs.microsoft.com/en-us/windows/security/information-protection/kernel-dma-protection-for-thunderbolt

We also do turn on full IOMMU mappings in that case for devices that are
marked as external facing by the same firmware that provided the DMAR
bit. If the user decides to disable IOMMU from command line for instance
then we expect she knows what she is doing.