Re: [PATCH 1/1] riscv: set ARCH_DMA_DEFAULT_COHERENT if RISCV_DMA_NONCOHERENT is not set

[Maxim Kochetkov]

On 22.12.2023 07:14, Christoph Hellwig wrote:
> On Thu, Dec 21, 2023 at 10:27:33PM +0000, Jiaxun Yang wrote:
> > 在 2023/12/21 20:29, Conor Dooley 写道:
> > > + Christoph
> > >
> > > I don't think this patch is correct. Regardless of whether we support
> > > cache management operations, DMA is assumed to be coherent unless
> > > peripherals etc are specified to otherwise in DT (or however ACPI deals
> > > with that kind of thing).
> > >
> > > What problem are you trying to solve here?
> > >
> > > On Thu, Dec 21, 2023 at 09:51:52PM +0300, Maxim Kochetkov wrote:
> > > > Not all the RISCV are DMA coherent by default.
> >
> > Sorry for chime in here.
> > IMO if your platform is not coherent by default, just insert
> > "dma-noncoherent"
> > at devicetree root node.
>
> Exactly.  ARCH_DMA_DEFAULT_COHERENTis a setting that just says for
> a given architecture assumes coherent unless otherwise specified,
> which has historically been the case for mips.  Not setting it means
> non-coherent unless specified, which has historially been the case
> for arm.
>
> RISC-V starte out without support for non-coherent DMA, and high ups
> in RISCV still told me in 2019 that RISC-V doesn't need cache
> management instructions because no new hardware would ever not be
> dma coherent.  Yeah, right..
>
> Anyay, Linux for RISC-V has historically been coherent only and then
> coherent default, so this option is wrong, and you need to mark
> you platform as non-coherent by inserting dma-noncoherent somewhere.
Conor, Christoph, Jiaxun, thanks for quick feedback!

The problem is very simple:
For non mips platforms dma_default_coherent is used at 
of_dma_is_coherent() and device_initialize().
of_dma_is_coherent() affects only DT devices. And we can override it 
with "dma-coherent"/"dma-noncoherent". ACPI devices can specify by
"attr == DEV_DMA_COHERENT". But all other devices (platform_device, usb, 
etc..) do not have this feature. These devices will use value from 
device_initialize(). And we have no possibility to change 
dma_default_coherent value by disabling ARCH_DMA_DEFAULT_COHERENT.
Moreover, changing dma_default_coherent from false to true may cause 
regression for other devices. That is why I suggest possibility to 
disable ARCH_DMA_DEFAULT_COHERENT by RISCV_DMA_NONCOHERENT option.
It will works like for PPC:
.....
config PPC
	bool
	default y
	#
	# Please keep this list sorted alphabetically.
	#
	select ARCH_32BIT_OFF_T if PPC32
	select ARCH_DISABLE_KASAN_INLINE	if PPC_RADIX_MMU
	select ARCH_DMA_DEFAULT_COHERENT	if !NOT_COHERENT_CACHE
.....
Doesn't the option RISCV_DMA_NONCOHERENT say the DMA should be 
non-coherent by default?