Re: [PATCH v10 2/2] riscv: Add runtime constant support
From: Charlie Jenkins
Date: Tue Apr 01 2025 - 16:43:23 EST
On Tue, Apr 01, 2025 at 12:28:33PM -0700, Nathan Chancellor wrote:
> Hi Charlie,
>
> On Wed, Mar 19, 2025 at 11:35:20AM -0700, Charlie Jenkins wrote:
> > Implement the runtime constant infrastructure for riscv. Use this
> > infrastructure to generate constants to be used by the d_hash()
> > function.
> >
> > This is the riscv variant of commit 94a2bc0f611c ("arm64: add 'runtime
> > constant' support") and commit e3c92e81711d ("runtime constants: add
> > x86 architecture support").
> >
> > Signed-off-by: Charlie Jenkins <charlie@xxxxxxxxxxxx>
> > Reviewed-by: Alexandre Ghiti <alexghiti@xxxxxxxxxxxx>
> ...
> > diff --git a/arch/riscv/include/asm/runtime-const.h b/arch/riscv/include/asm/runtime-const.h
> ...
> > +#define RISCV_RUNTIME_CONST_64_ZBA \
> > + ".option push\n\t" \
> > + ".option arch,+zba\n\t" \
> > + "slli %[__tmp],%[__tmp],32\n\t" \
> > + "add.uw %[__ret],%[__ret],%[__tmp]\n\t" \
> > + "nop\n\t" \
> > + "nop\n\t" \
> > + ".option pop\n\t" \
> ...
> > +#if defined(CONFIG_RISCV_ISA_ZBA) && defined(CONFIG_RISCV_ISA_ZBKB)
> ...
> > +#elif defined(CONFIG_RISCV_ISA_ZBA)
> > +#define runtime_const_ptr(sym) \
> > +({ \
> > + typeof(sym) __ret, __tmp; \
> > + asm_inline(RISCV_RUNTIME_CONST_64_PREAMBLE \
> > + ALTERNATIVE( \
> > + RISCV_RUNTIME_CONST_64_BASE, \
> > + RISCV_RUNTIME_CONST_64_ZBA, \
> > + 0, RISCV_ISA_EXT_ZBA, 1 \
> > + ) \
> > + RISCV_RUNTIME_CONST_64_POSTAMBLE(sym) \
> > + : [__ret] "=r" (__ret), [__tmp] "=r" (__tmp)); \
> > + __ret; \
> > +})
>
> This breaks the build for clang versions 16 and earlier because they do
> not support '.option arch' and it is used in CONFIG_RISCV_ISA_ZBA, which
> has no dependencies and it is default on.
>
> $ make -skj"$(nproc)" ARCH=riscv LLVM=1 mrproper defconfig fs/dcache.o
> fs/dcache.c:117:9: warning: unknown option, expected 'push', 'pop', 'rvc', 'norvc', 'relax' or 'norelax' [-Winline-asm]
> return runtime_const_ptr(dentry_hashtable) +
> ^
> arch/riscv/include/asm/runtime-const.h:103:4: note: expanded from macro 'runtime_const_ptr'
> RISCV_RUNTIME_CONST_64_ZBA, \
> ^
> arch/riscv/include/asm/runtime-const.h:57:17: note: expanded from macro 'RISCV_RUNTIME_CONST_64_ZBA'
> ".option push\n\t" \
> ^
> <inline asm>:32:10: note: instantiated into assembly here
> .option arch,+zba
> ^
> fs/dcache.c:117:9: error: instruction requires the following: 'Zba' (Address Generation Instructions)
> return runtime_const_ptr(dentry_hashtable) +
> ^
> arch/riscv/include/asm/runtime-const.h:103:4: note: expanded from macro 'runtime_const_ptr'
> RISCV_RUNTIME_CONST_64_ZBA, \
> ^
> arch/riscv/include/asm/runtime-const.h:59:30: note: expanded from macro 'RISCV_RUNTIME_CONST_64_ZBA'
> "slli %[__tmp],%[__tmp],32\n\t" \
> ^
> <inline asm>:34:2: note: instantiated into assembly here
> add.uw a2,a2,a3
> ^
> ...
>
> $ rg 'OPTION_ARCH|ZBA' .config
> 364:CONFIG_RISCV_ISA_ZBA=y
>
> Should it grow a dependency on AS_HAS_OPTION_ARCH or should there be a
> different fix?
This should have been fixed by Alex's patch [1]. Zba is in an awkward
state because BPF generates Zba code without the need for toolchain
support.
[1] https://lore.kernel.org/all/20250328115422.253670-1-alexghiti@xxxxxxxxxxxx/
>
> Cheers,
> Nathan