Le 11/12/2020 à 17:55, David Laight a écrit :
From: Christophe Leroy
Sent: 11 December 2020 16:43
Le 11/12/2020 à 17:07, David Laight a écrit :
From: Christophe Leroy
Sent: 11 December 2020 15:22
Le 11/12/2020 à 09:52, Xu Wang a écrit :
Remove casting the values returned by dma_alloc_coherent.
Can you explain more in the commit log ?
As far as I can see, dma_alloc_coherent() doesn't return __iomem, and ring_base
member is __iomem
Which is probably wrong - that is the kernel address of kernel memory.
So it shouldn't have the __iomem marker.
That's where the buffer descriptors are, the driver accesses to the content of
the buffer
descriptors using the IO accessors in_be16()/out_be16(). Is it not correct ?
I've just been looking at the crap in there.
My understanding is that IO accessors are for IO devices (eg addresses
from io_remap() etc).
Buffers allocated by dma_alloc_coherent() are normal kernel memory
and don't need any accessors.
Now you might need some barriers - mostly because an ethernet chip
can typically read a ring entry without being prodded.
IIRC there is a barrier in writel() to ensure the dma master will
'see' all memory writes done before the IO write that kicks it into
doing some processing.
As far as I can see, writel() is using __iomem memory, see
https://elixir.bootlin.com/linux/v5.12-rc2/source/include/asm-generic/io.h#L221
The fact that the driver contains so many __iomem casts (eg in
tx_restart) is an indication that something is badly awry.
__iomem exists to check you are using the correct type of pointer.
Any __iomem casts are dubious.
I agree, but what else can we do to guarantee proper access to that memory ?
Christophe
