On 30/01/2020 9.53, Christoph Hellwig wrote:

[skipping the DT bits, as I'm everything but an expert on that..]

On Mon, Jan 27, 2020 at 04:00:30PM +0200, Peter Ujfalusi wrote:

quoted

I agree on the phys_to_dma(). It should fail for addresses which does
not fall into any of the ranges.
It is just a that we in Linux don't have the concept atm for ranges, we
have only _one_ range which applies to every memory address.

what does atm here mean?

struct device have only single dma_pfn_offset, one can not have multiple
ranges defined. If we have then only the first is taken and the physical
address and dma address is discarded, only the dma_pfn_offset is stored
and used.

We have needed multi-range support for quite a while, as common broadcom
SOCs do need it.  So patches for that are welcome at least from the
DMA layer perspective (kinda similar to your pseudo code earlier)

But do they have dma_pfn_offset != 0?

quoted

quoted

Nobody's disputing that the current dma_direct_supported()
implementation is broken for the case where ZONE_DMA itself is offset
from PA 0; the more pressing question is why Christoph's diff, which was
trying to take that into account, still didn't work.

I understand that this is a bit more complex than I interpret it, but
the k2g is broken and currently the simplest way to make it work is to
use the arm dma_ops in case the pfn_offset is not 0.
It will be easy to test dma-direct changes trying to address the issue
in hand, but will allow k2g to be usable at the same time.

Well, using the legacy arm dma ops means we can't use swiotlb if there
is an offset, which is also wrong for lots of common cases, including
the Rpi 4.  I'm still curious why my patch didn't work, as I thought
it should.

The dma_pfn_offset is _still_ applied to the mask we are trying to set
(and validate) via dma-direct.

in dma_direct_supported:
mask == 0xffffffff // DMA_BIT_MASK(32)
dev->dma_pfn_offset == 0x780000 // Keystone 2
min_mask == 0xffffff

tmp_mask = __phys_to_dma(dev, min_mask);
tmp_mask == 0xff880ffffff

within __phys_to_dma() converts the min_mask to pfn and calls
pfn_to_dma() which does:
if (dev)
	pfn -= dev->dma_pfn_offset;

the returned pfn is then converted back to address.

the mask (0xffffffff) is well under the tmp_mask (0xff880ffffff) so
dma_direct_supported() will tell us that DMA is not supported for
DMA_BIT_MASK(32), which is not true, because DMA is supporting 32 bits.

We'll need to find the minimum change to make it work
for now without switching ops, even if it isn't the correct one, and
then work from there.

Sure, but can we fix the regression by reverting to arm_ops for now only
if dma_pfn_offset is not 0? It used to work fine in the past at least it
appeared to work on K2 platforms.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel