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

From: Maxim Kochetkov <fido_max@inbox.ru>
To: Christoph Hellwig <hch@lst.de>, Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	robh@kernel.org, mpe@ellerman.id.au, aou@eecs.berkeley.edu,
	palmer@dabbelt.com, paul.walmsley@sifive.com,
	Conor Dooley <conor@kernel.org>
Subject: Re: [PATCH 1/1] riscv: set ARCH_DMA_DEFAULT_COHERENT if RISCV_DMA_NONCOHERENT is not set
Date: Fri, 22 Dec 2023 17:39:34 +0300	[thread overview]
Message-ID: <c53c22dd-c482-4808-bdd7-e81c01c04f9e@inbox.ru> (raw)
In-Reply-To: <20231222041428.GA2803@lst.de>


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?

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