RE: [PATCH] thunderbolt: Make iommu_dma_protection more accurate

Limonciello, Mario
2022-03-18T14:23:01Z

[Public]

> -----Original Message-----
> From: Robin Murphy <robin.murphy@arm.com>
> Sent: Friday, March 18, 2022 09:08
> To: mika.westerberg@linux.intel.com
> Cc: Limonciello, Mario <Mario.Limonciello@amd.com>;
> andreas.noever@gmail.com; michael.jamet@intel.com;
> YehezkelShB@gmail.com; linux-usb@vger.kernel.org; linux-
> kernel@vger.kernel.org; iommu@lists.linux-foundation.org; linux-
> pci@vger.kernel.org
> Subject: Re: [PATCH] thunderbolt: Make iommu_dma_protection more
> accurate
>
> 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.

The thunderbolt driver already has a good idea whether it's using software
CM or firmware CM. How about if we use that to make a decision?

If running firmware CM presumably that is a fixed quantity of machines that will
dwindle over time as OEMs release new HW with SW CM designs. So maybe
leave things "as is" for those - opt in bit is sufficient for this check.

And then if running software CM then make it mandatory that any created links
are set untrusted AND some pre-boot bits are set to get iommu dma protection
set. We effectively end up with the same requirements as Microsoft has in
their spec for new hardware then, and don't break any old hardware that
doesn't have the links made and relies on firmware for the CM.