[Public] > -----Original Message----- > From: Limonciello, Mario > Sent: Wednesday, March 16, 2022 12:54 > To: Robin Murphy <robin.murphy@xxxxxxx>; Mika Westerberg > <mika.westerberg@xxxxxxxxxxxxxxx> > Cc: michael.jamet@xxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; YehezkelShB@xxxxxxxxx; iommu@lists.linux- > foundation.org; andreas.noever@xxxxxxxxx; hch@xxxxxx > Subject: RE: [PATCH] thunderbolt: Stop using iommu_present() > > [Public] > > > >>> > > >>> 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 :( > > > > > Can the USB4 CM make the device links in the DVSEC case perhaps too? I > would > think we want that anyway to control device suspend ordering. > > If I had something discrete to try I'd dust off the DVSEC patch I wrote before > to > try it, but alas all I have is integrated stuff on my hand. > > > >> 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: > > > > > > > > > > I think Microsoft doesn't allow you to turn off the IOMMU though or put it in > passthrough through on the kernel command line. > > > > 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. > > Unless it happened after 5.17-rc8 looking at the code I think that's Intel > specific behavior though at the moment (has_external_pci). I don't see it > in a generic layer. Oh it's via dev_is_untrusted. A few layers through external facing translates to untrusted and then dev_use_swiotlb is used, got it. > > In addition to the point Robin said about firmware not setting external facing > if the IOMMU was disabled on command line then iommu_dma_protection > would be showing the wrong values meaning userspace may choose to > authorize the device automatically in a potentially unsafe scenario. > > Even if the user "knew what they were doing", I would expect that we still > do our best to protect them from themselves and not advertise something > that will cause automatic authorization.
