linux-kernel - Re: [PATCH -fixes] dma-mapping: add default implementation to arch_dma_{set|clear}

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: <tencent_30E109FE684F391AE15AC6F4854034C2F00A@qq.com>
Date: Tue, 9 Jul 2024 20:22:35 +0800
From: Yangyu Chen <cyy@...self.name>
To: Christoph Hellwig <hch@....de>
Cc: iommu@...ts.linux.dev,
 Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
 Marek Szyprowski <m.szyprowski@...sung.com>,
 Robin Murphy <robin.murphy@....com>
Subject: Re: [PATCH -fixes] dma-mapping: add default implementation to
 arch_dma_{set|clear}_uncached

> On Jul 9, 2024, at 19:46, Christoph Hellwig <hch@....de> wrote:
> 
> On Tue, Jul 09, 2024 at 07:39:29PM +0800, Yangyu Chen wrote:
>> The reason is that some optimizations failed to apply after adding
>> some passes. I will fix the compiler later. Whatever, we should not
>> rely on this optimization to get the code being successfully compiled.
> 
> The Linux kernel relies on constant propagation and dead code
> eliminiation a lot to make code simpler and more readable.

Actually, the compiler is patched LLVM with -O2 optimization. I didn’t
turn off the optimization.

You can see what we did for the compiler here[1] and compile the
kernel with `-march=rv64imac_zicond_zicldst` added to KBUILD_CFLAGS.
I added conditional load/store pass as Intel did for the x86 APX
extension, which appeared last year (called hoist load/store in
LLVM if you want to search the PR), and then LLVM failed to optimize
this.

The only failed symbol on the kernel with `ARCH=riscv defconfig`
is `arch_dma_set_uncached` since the compiler requires all possible
values to be known. I think a pattern like in kernel/dma/direct.c:349
for symbol `arch_dma_clear_uncached`, which uses `if
(IS_ENABLE(CONFIG_ARCH_HAS_xxx)) xxx` is acceptable. But for the
symbol `arch_dma_set_uncached`, a complex analysis is needed for a
value set in a block of branches. I think we should not rely on such
compiler optimization in such a complex pattern.

In addition, patching this way can also make this symbol safer to use.

[1] https://github.com/cyyself/llvm-project/tree/zicldst-support-bugless-v3

Thanks,
Yangyu Chen