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

[Robin Murphy]

On 2022-03-16 17:37, Mika Westerberg wrote:
> 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&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
> > > &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.

Yeah, if external_facing is set correctly then we can safely expect the 
the IOMMU layer to do the right thing, so in that case it probably is OK 
to infer that if an IOMMU is present for the NHI then it'll be managing 
that whole bus hierarchy. What I'm really thinking about here is whether 
we can defend against a case when external_facing *isn't* set, so we 
treat the tunnelled ports as normal PCI buses, assume it's OK since 
we've got an IOMMU and everything else is getting translation domains by 
default, but then a Thunderbolt device shows up masquerading the VID:DID 
of something that gets a passthrough quirk, and thus tricks its way 
through the perceived protection.

Robin.