Messages in this thread | | | From | Atish Patra <> | Date | Sun, 22 May 2022 00:33:12 -0700 | Subject | Re: [PATCH 1/2] riscv: introduce unified static key mechanism for ISA extensions |
| |
On Tue, May 17, 2022 at 11:53 AM Jisheng Zhang <jszhang@kernel.org> wrote: > > Currently, riscv has several extensions which may not be supported on > all riscv platforms, for example, FPU and so on. To support unified > kernel Image style, we need to check whether the feature is supported
/s/suportted/supported
> or not. If the check sits at hot code path, then performance will be > impacted a lot. static key can be used to solve the issue. In the past > FPU support has been converted to use static key mechanism. I believe > we will have similar cases in the future. > > this patch tries to add an unified mechanism to use static keys for > some ISA extensions by implementing an array of default-false static keys > and enabling them when detected. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > arch/riscv/include/asm/hwcap.h | 40 ++++++++++++++++++++++++++++++++++ > arch/riscv/kernel/cpufeature.c | 7 ++++++ > 2 files changed, 47 insertions(+) > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h > index 0734e42f74f2..b0433d2b880d 100644 > --- a/arch/riscv/include/asm/hwcap.h > +++ b/arch/riscv/include/asm/hwcap.h > @@ -12,6 +12,7 @@ > #include <uapi/asm/hwcap.h> > > #ifndef __ASSEMBLY__ > +#include <linux/jump_label.h> > /* > * This yields a mask that user programs can use to figure out what > * instruction set this cpu supports. > @@ -55,6 +56,16 @@ enum riscv_isa_ext_id { > RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, > }; > > +/* > + * This enum represents the logical ID for each RISC-V ISA extension static > + * keys. We can use static key to optimize code path if some ISA extensions > + * are available. > + */ > +enum riscv_isa_ext_key { > + RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ > + RISCV_ISA_EXT_KEY_MAX, > +}; > + > struct riscv_isa_ext_data { > /* Name of the extension displayed to userspace via /proc/cpuinfo */ > char uprop[RISCV_ISA_EXT_NAME_LEN_MAX]; > @@ -62,6 +73,35 @@ struct riscv_isa_ext_data { > unsigned int isa_ext_id; > }; > > +extern struct static_key_false riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_MAX]; > + > +static __always_inline int riscv_isa_ext2key(int num) > +{ > + switch (num) { > + case RISCV_ISA_EXT_f: > + return RISCV_ISA_EXT_KEY_FPU; > + case RISCV_ISA_EXT_d: > + return RISCV_ISA_EXT_KEY_FPU; > + default: > + return -EINVAL; > + } > +} > + > +/* > + * @num must be a compile-time constant. > + */ > +static __always_inline bool riscv_isa_have_key_extension(int num) > +{ > + if (RISCV_ISA_EXT_ID_MAX <= num) > + return false; > + > + num = riscv_isa_ext2key(num); > + if (RISCV_ISA_EXT_KEY_MAX <= num || num < 0) > + return false; > +
Why do you need the additional check in the hot path ? riscv_isa_ext_keys array can be directly accessed at the caller instead of calling this function.
> + return static_branch_likely(&riscv_isa_ext_keys[num]); > +} > +
> unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap); > > #define riscv_isa_extension_mask(ext) BIT_MASK(RISCV_ISA_EXT_##ext) > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > index 1b2d42d7f589..89f886b35357 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -24,6 +24,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly; > #ifdef CONFIG_FPU > __ro_after_init DEFINE_STATIC_KEY_FALSE(cpu_hwcap_fpu); > #endif > +__ro_after_init DEFINE_STATIC_KEY_ARRAY_FALSE(riscv_isa_ext_keys, RISCV_ISA_EXT_KEY_MAX); > +EXPORT_SYMBOL(riscv_isa_ext_keys); > > /** > * riscv_isa_extension_base() - Get base extension word > @@ -232,6 +234,11 @@ void __init riscv_fill_hwcap(void) > print_str[j++] = (char)('a' + i); > pr_info("riscv: ELF capabilities %s\n", print_str); > > + for_each_set_bit(i, riscv_isa, RISCV_ISA_EXT_MAX) { > + j = riscv_isa_ext2key(i); > + if (j >= 0) > + static_branch_enable(&riscv_isa_ext_keys[j]); > + } > #ifdef CONFIG_FPU > if (elf_hwcap & (COMPAT_HWCAP_ISA_F | COMPAT_HWCAP_ISA_D)) > static_branch_enable(&cpu_hwcap_fpu); > -- > 2.34.1 >
-- Regards, Atish
| |