Re: [PATCH 4/4] thunderbolt: Export IOMMU based DMA protection support to userspace

[Mika Westerberg]

On Mon, Nov 12, 2018 at 06:59:02PM +0200, Yehezkel Bernat wrote:
> On Mon, Nov 12, 2018 at 6:06 PM Mika Westerberg
> <mika.westerberg@xxxxxxxxxxxxxxx> wrote:
> >
> > Recent systems shipping with Windows 10 version 1803 or later may
> > support a feature called Kernel DMA protection [1]. In practice this
> > means that Thunderbolt connected devices are placed behind an IOMMU
> > during the whole time it is connected (including during boot) making
> > Thunderbolt security levels redundant. Some of these systems still have
> > Thunderbolt security level set to "user" in order to support OS
> > downgrade (the older version of the OS might not support IOMMU based DMA
> > protection so connecting a device still relies on user approval then).
> >
> > Export this information to userspace by introducing a new sysfs
> > attribute (iommu_dma_protection). Based on it userspace tools can make
> > more accurate decision whether or not authorize the connected device.
> >
> > In addition update Thunderbolt documentation regarding IOMMU based DMA
> > protection.
> >
> > [1] https://docs.microsoft.com/en-us/windows/security/information-protection/kernel-dma-protection-for-thunderbolt
> >
> > Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> > ---
> 
> I can't comment on the IOMMU side, but the Thunderbolt side looks good to me.

Thanks!

> Just one point:
> Have you considered the option to add this property per (TBT?) device?

No. ;-)

You mean that one device uses security levels and another IOMMU? I don't
think it is possible without having some sort of table in the IOMMU
driver telling which devices it needs identity map and which not. Also
not sure what would be the benefit?

> If the kernel may decide to enable/disable the IOMMU or AST per device, maybe
> it should be on this level.
> Or maybe the IOMMU decision isn't going to change (it's system-wide) and the AST
> decision will be communicated per device by a new sysfs attribute anyway, if
> needed?

Not sure what you mean by "AST"? The IOMMU decision is pretty much
system-wide.