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

[Mika Westerberg]

On Tue, Nov 13, 2018 at 01:13:31PM +0200, Yehezkel Bernat wrote:
> On Tue, Nov 13, 2018 at 12:56 PM Mika Westerberg
> <mika.westerberg@xxxxxxxxxxxxxxx> wrote:
> >
> > > 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?
> 
> For performance, of course. If some devices are considered safe (maybe a list
> communicated by platform firmware), the kernel may decide to configure them to
> passthrough the IOMMU (I think I remember there is such an option, but maybe I'm
> wrong.)

At least I'm not aware of such an option. Windows for example enables
IOMMU for everything and I think macOS does the same. In Linux (with
these patches) we put all internal devices already passthrough mode so
things like internal graphics should not be affected. eGPUs are
different thing, though.

> > > 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.
> 
> Sorry, I meant ATS, Address Translation Service, mentioned in patch 3 in this
> series, and possibly be enabled for some devices for performance, as mentioned
> there.

Thanks for clarifying :)

> So if needed, this will be another attribute, and definitely
> per-device, isn't it?

Yes, we may want to add a way enable ATS for external devices (perhaps
global option or per-device attribute) if it turns out to cause
performance issues. However, I think it can be done later if needed. I
have not seen a single device actually supporting ATS (with the
exception of the "hacked" one in the linked thesis of patch 3/4 ;-))