linux-kernel - Re: [PATCH 0/8] riscv: improve boot time isa extensions handling

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] [day] [month] [year] [list]

Message-ID: <Y16gSDwCWmp3vxaw@xhacker>
Date:   Mon, 31 Oct 2022 00:03:20 +0800
From:   Jisheng Zhang <jszhang@...nel.org>
To:     Andrew Jones <ajones@...tanamicro.com>
Cc:     Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/8] riscv: improve boot time isa extensions handling

On Sat, Oct 29, 2022 at 11:56:09AM +0200, Andrew Jones wrote:
> On Thu, Oct 06, 2022 at 03:08:10PM +0800, Jisheng Zhang wrote:
> > Generally, riscv ISA extensions are fixed for any specific hardware
> > platform, that's to say, the hart features won't change any more
> > after booting, this chacteristic make it straightforward to use
> > static branch to check one specific ISA extension is supported or not
> > to optimize performance.
> > 
> > However, some ISA extensions such as SVPBMT and ZICBOM are handled
> > via. the alternative sequences.
> > 
> > Basically, for ease of maintenance, we prefer to use static branches
> > in C code, but recently, Samuel found that the static branch usage in
> > cpu_relax() breaks building with CONFIG_CC_OPTIMIZE_FOR_SIZE[1]. As
> > Samuel pointed out, "Having a static branch in cpu_relax() is
> > problematic because that function is widely inlined, including in some
> > quite complex functions like in the VDSO. A quick measurement shows
> > this static branch is responsible by itself for around 40% of the jump
> > table."
> > 
> > Samuel's findings pointed out one of a few downsides of static branches
> > usage in C code to handle ISA extensions detected at boot time:
> > static branch's metadata in the __jump_table section, which is not
> > discarded after ISA extensions are finalized, wastes some space.
> > 
> > I want to try to solve the issue for all possible dynamic handling of
> > ISA extensions at boot time. Inspired by Mark[2], this patch introduces
> > riscv_has_extension_*() helpers, which work like static branches but
> > are patched using alternatives, thus the metadata can be freed after
> > patching.
> > 
> > [1]https://lore.kernel.org/linux-riscv/20220922060958.44203-1-samuel@sholland.org/
> > [2]https://lore.kernel.org/linux-arm-kernel/20220912162210.3626215-8-mark.rutland@arm.com/
> > 
> > 
> > Jisheng Zhang (8):
> >   riscv: move riscv_noncoherent_supported() out of ZICBOM probe
> >   riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier
> >   riscv: hwcap: make ISA extension ids can be used in asm
> >   riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA
> >     extensions
> >   riscv: introduce riscv_has_extension_[un]likely()
> >   riscv: fpu: switch has_fpu() to riscv_has_extension_likely()
> >   riscv: cpu_relax: switch to riscv_has_extension_likely()
> >   riscv: remove riscv_isa_ext_keys[] array and related usage
> > 
> >  arch/riscv/include/asm/errata_list.h    |  9 +--
> >  arch/riscv/include/asm/hwcap.h          | 94 ++++++++++++++-----------
> >  arch/riscv/include/asm/switch_to.h      |  3 +-
> >  arch/riscv/include/asm/vdso/processor.h |  2 +-
> >  arch/riscv/kernel/cpufeature.c          | 78 +++-----------------
> >  arch/riscv/kernel/setup.c               |  4 ++
> >  6 files changed, 71 insertions(+), 119 deletions(-)
> > 
> > -- 
> > 2.37.2
> >
> 
> Hi Jisheng,
> 
> I just tried building this with LLVM=1 and fails to compile with messages
> like
> 
>  ld.lld: error: relocation R_RISCV_64 cannot be used against symbol '.Ltmp1'; recompile with -fPIC
>  >>> defined in arch/riscv/kernel/vdso/vgettimeofday.o
>  >>> referenced by vgettimeofday.c
>  >>>               arch/riscv/kernel/vdso/vgettimeofday.o:(.alternative+0x0)
> 
> It does compile and boot with CC=clang and binutils 2.39, where my clang
> version is 14.0.5 (Fedora 14.0.5-1.fc36).

Hi Andrew, 

Below is a quick fix.

PS: I think I need add two or more patches to apply alternatives in the
vDSO. But currently only Zihintpause is affected.

Will send out a new version.

diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
index f2e065671e4d..522b78477bab 100644
--- a/arch/riscv/kernel/vdso/Makefile
+++ b/arch/riscv/kernel/vdso/Makefile
@@ -44,7 +44,7 @@ $(obj)/vdso.o: $(obj)/vdso.so
 # link rule for the .so file, .lds has to be first
 $(obj)/vdso.so.dbg: $(obj)/vdso.lds $(obj-vdso) FORCE
        $(call if_changed,vdsold)
-LDFLAGS_vdso.so.dbg = -shared -S -soname=linux-vdso.so.1 \
+LDFLAGS_vdso.so.dbg = -shared -z notext -S -soname=linux-vdso.so.1 \

> 
> Thanks,
> drew