On 2022-03-16 17:53, Limonciello, Mario wrote:
> [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.
Ah, it's not necessarily the most obvious thing -
pci_dev->external_facing gets propagated through to pci_dev->untrusted
by set_pcie_untrusted(), and it's that that's then checked by
iommu_get_def_domain_type() to enforce a translation domain regardless
of default passthrough or quirks. It's then further checked by
iommu-dma's dev_is_untrusted() to enforce bounce-buffering to avoid data
leakage in sub-page mappings too.
> 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.
Might it be reasonable for the Thunderbolt core to check early on if any
tunnelled ports are not marked as external facing, and if so just tell
the user that iommu_dma_protection is off the table and anything they
authorise is at their own risk?
Robin.
[Public]
> > 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.
>
> Ah, it's not necessarily the most obvious thing -
> pci_dev->external_facing gets propagated through to pci_dev->untrusted
> by set_pcie_untrusted(), and it's that that's then checked by
> iommu_get_def_domain_type() to enforce a translation domain regardless
> of default passthrough or quirks. It's then further checked by
> iommu-dma's dev_is_untrusted() to enforce bounce-buffering to avoid data
> leakage in sub-page mappings too.
>
Ah thanks for explaining it, that was immediately obvious to me.
> > 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.
>
> Might it be reasonable for the Thunderbolt core to check early on if any
> tunnelled ports are not marked as external facing, and if so just tell
> the user that iommu_dma_protection is off the table and anything they
> authorise is at their own risk?
>
> Robin.
How about in iommu_dma_protection_show to just check that all the device
links to the NHI are marked as untrusted?
Then if there are device links missing we solve that separately (discrete USB4
DVSEC case we just need to make those device links).
Hi Mario,
On Wed, Mar 16, 2022 at 06:34:51PM +0000, Limonciello, Mario wrote:
> > Might it be reasonable for the Thunderbolt core to check early on if any
> > tunnelled ports are not marked as external facing, and if so just tell
> > the user that iommu_dma_protection is off the table and anything they
> > authorise is at their own risk?
> >
> > Robin.
>
> How about in iommu_dma_protection_show to just check that all the device
> links to the NHI are marked as untrusted?
Actually this does not work either because we have pre-USB4 systems out
there that are using firmware based connection manager and do not set
the "device links" (as it is only needed for USB4 software based
connection manager systems).
So only thing we can use is the ->external_facing (and ->untrusted) as
those exists in all these systems (well assuming the BIOS provided them
but this is Microsoft requirement in the same way with the DMAR bit).
[For those who are not familiar with the connection manager, it is the
software or firmware that actually creates the tunnels over the
Thunderbolt/USB4 fabric. In Intel systems up to Alder Lake it used to be
firmware based, and from Alder Lake and beyond it is software based
meaning that the Linux Thunderbolt driver creates the tunnels. Apple
systems have been software based from the beginnning.]