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

Andrew Jones
2022-10-29T09:56:31Z

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).

Thanks,
drew