On Thu, Mar 17, 2022 at 04:17:07PM +0000, Robin Murphy wrote: > Between me trying to get rid of iommu_present() and Mario wanting to > support the AMD equivalent of DMAR_PLATFORM_OPT_IN, scrutiny has shown > that the iommu_dma_protection attribute is being far too optimistic. > Even if an IOMMU might be present for some PCI segment in the system, > that doesn't necessarily mean it provides translation for the device(s) > we care about. Furthermore, all that DMAR_PLATFORM_OPT_IN really does > is tell us that memory was protected before the kernel was loaded, and > prevent the user from disabling the intel-iommu driver entirely. What > actually matters is whether we trust individual devices, based on the > "external facing" property that we expect firmware to describe for > Thunderbolt ports. > > Avoid false positives by looking as close as possible to the same PCI > topology that the IOMMU layer will consider once a Thunderbolt endpoint > appears. Crucially, we can't assume that IOMMU translation being enabled > for any reason is sufficient on its own; full (expensive) DMA protection > will still only be imposed on untrusted devices. > > CC: Mario Limonciello <mario.limonciello@xxxxxxx> > Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx> > --- > > This supersedes my previous attempt just trying to replace > iommu_present() at [1], further to the original discussion at [2]. > > [1] https://lore.kernel.org/linux-iommu/BL1PR12MB515799C0BE396377DBBEF055E2119@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/ > [2] https://lore.kernel.org/linux-iommu/202203160844.lKviWR1Q-lkp@xxxxxxxxx/T/ > > drivers/thunderbolt/domain.c | 12 +++--------- > drivers/thunderbolt/nhi.c | 35 +++++++++++++++++++++++++++++++++++ > include/linux/thunderbolt.h | 2 ++ > 3 files changed, 40 insertions(+), 9 deletions(-) > > diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c > index 7018d959f775..d5c825e84ac8 100644 > --- a/drivers/thunderbolt/domain.c > +++ b/drivers/thunderbolt/domain.c > @@ -7,9 +7,7 @@ > */ > > #include <linux/device.h> > -#include <linux/dmar.h> > #include <linux/idr.h> > -#include <linux/iommu.h> > #include <linux/module.h> > #include <linux/pm_runtime.h> > #include <linux/slab.h> > @@ -257,13 +255,9 @@ static ssize_t iommu_dma_protection_show(struct device *dev, > struct device_attribute *attr, > char *buf) > { > - /* > - * Kernel DMA protection is a feature where Thunderbolt security is > - * handled natively using IOMMU. It is enabled when IOMMU is > - * enabled and ACPI DMAR table has DMAR_PLATFORM_OPT_IN set. > - */ > - return sprintf(buf, "%d\n", > - iommu_present(&pci_bus_type) && dmar_platform_optin()); > + struct tb *tb = container_of(dev, struct tb, dev); > + > + return sprintf(buf, "%d\n", tb->nhi->iommu_dma_protection); sysfs_emit() please. thanks, greg k-h