Re: revert dma direct internals abuse

From: hch@xxxxxx
Date: Wed Apr 10 2019 - 02:44:13 EST


On Tue, Apr 09, 2019 at 05:24:48PM +0000, Thomas Hellstrom wrote:
> > Note that this only affects external, untrusted devices. But that
> > may include eGPU,
>
> What about discrete graphics cards, like Radeon and Nvidia? Who gets to
> determine what's trusted?

Based on firmware tables. discrete graphics would not qualify unless
they are attached through thunderbolt bridges or external PCIe ports.

> GPU libraries traditionally have been taking care of the CPU mapping
> caching modes since the first AGP drivers. GPU MMU ptes commonly
> support various caching options and pages are changing caching mode
> dynamically.
> So even if the DMA layer needs to do the remapping, couldn't we do that
> on-demand when needed with a simple interface?

The problem is that there is no "simple" interface as the details
depend on the architecture. We have the following base variants
to create coherent memory:

1) do nothing - this works on x86-like platforms where I/O is always
coherent
2) use a special kernel segment, after flushing the caches for the
normal segment, done on platforms like mips that have this
special segment
3) remap the existing kernel direct mapping, after flushing the
caches, done by openrisc and in some cases arm32
4) create a new mapping in vmap or ioremap space after flushing the
caches - done by most architectures with an MMU and non-coherent
devices
5) use a special pool of uncached memory set aside by the hardware
or firmware - done by most architectures without an MMU but with
non-coherent devices

So that is just five major variants with a lot of details on how
it is done in practice. Add to that that many of the operations
are fairly expensive and need to be pre-loaded.

> > That being said: your driver already uses the dma coherent API
> > under various circumstances, so you already have the above issues.
>
> Yes, but they are hidden behind driver options. We can't have someone
> upgrade their kernel and suddenly things don't work anymore, That said,
> I think the SWIOTLB case is rare enough for the below solution to be
> acceptable, although the TTM check for the coherent page pool being
> available still needs to remain.

So can you please respin a version acceptable to you and submit it
for 5.1 ASAP? Otherwise I'll need to move ahead with the simple
revert.