linux-kernel - Re: [PATCH v2 5/5] of: address: Always use dma_default

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 for Android: free password hash cracker in your pocket

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

Date:   Fri, 03 Mar 2023 11:11:56 +1100
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Jiaxun Yang <jiaxun.yang@...goat.com>,
        Christoph Hellwig <hch@....de>
Cc:     "linux-mips@...r.kernel.org" <linux-mips@...r.kernel.org>,
        linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        paul.walmsley@...ive.com, palmer@...belt.com,
        Rob Herring <robh+dt@...nel.org>, m.szyprowski@...sung.com,
        robin.murphy@....com, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v2 5/5] of: address: Always use dma_default_coherent for
 default coherency

Jiaxun Yang <jiaxun.yang@...goat.com> writes:
>> 2023年3月1日 13:06，Christoph Hellwig <hch@....de> 写道：
>> 
>>> - select OF_DMA_DEFAULT_COHERENT if !NOT_COHERENT_CACHE
>> 
>> Doesn't powerpc need to select CONFIG_ARCH_DMA_DEFAULT_COHERENT now,
>> or even better should be doing that in the patch adding that
>> symbol?
>
> If I read the code correctly for powerpc OF_DMA_DEFAULT_COHERENT is only selected
> with !NOT_COHERENT_CACHE, which means non-coherent dma support is disabled….

I think you're right, but it's not easy to understand.

powerpc's NOT_COHERENT_CACHE selects:

  select ARCH_HAS_DMA_PREP_COHERENT
  select ARCH_HAS_SYNC_DMA_FOR_DEVICE
  select ARCH_HAS_SYNC_DMA_FOR_CPU


Then in your patch 3 you do:

 #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
	defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
	defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
-bool dma_default_coherent;
+bool dma_default_coherent = IS_ENABLED(CONFIG_ARCH_DMA_DEFAULT_COHERENT);
 #endif

So for powerpc if NOT_COHERENT_CACHE=n, then none of those ARCH_HAS
symbols are defined, and so CONFIG_ARCH_DMA_DEFAULT_COHERENT is never used.

But like I said it's not very obvious, and it also seems fragile to
future changes.

So it seems it would be more future proof, and self documenting for
powerpc to just have:

        select ARCH_DMA_DEFAULT_COHERENT if !NOT_COHERENT_CACHE


cheers