linux-kernel - Re: [PATCH 1/1] riscv: set ARCH_DMA_DEFAULT_COHERENT if RISCV_DMA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c53c22dd-c482-4808-bdd7-e81c01c04f9e@inbox.ru>
Date: Fri, 22 Dec 2023 17:39:34 +0300
From: Maxim Kochetkov <fido_max@...ox.ru>
To: Christoph Hellwig <hch@....de>, Jiaxun Yang <jiaxun.yang@...goat.com>
Cc: linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
 robh@...nel.org, mpe@...erman.id.au, aou@...s.berkeley.edu,
 palmer@...belt.com, paul.walmsley@...ive.com, Conor Dooley <conor@...nel.org>
Subject: Re: [PATCH 1/1] riscv: set ARCH_DMA_DEFAULT_COHERENT if
 RISCV_DMA_NONCOHERENT is not set



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?