Messages in this thread | | | From | Atish Patra <> | Date | Fri, 13 May 2022 14:16:09 -0700 | Subject | Re: [PATCH] arch/riscv: Add Zihintpause extension support |
| |
On Fri, May 13, 2022 at 12:09 AM Atish Patra <atishp@atishpatra.org> wrote: > > On Thu, May 12, 2022 at 6:06 PM Samuel Holland <samuel@sholland.org> wrote: > > > > On 5/11/22 10:30 PM, Dao Lu wrote: > > > This patch: > > > 1. Build with _zihintpause if the toolchain has support for it > > > 2. Detects if the platform supports the extension > > > > This instruction is a hint, meaning it is a harmless no-op if the extension is > > unsupported by the CPU. So we can use it as long as the compiler supports it. > > There is no need to probe for it at runtime. > > > > Is it guaranteed that the hardware won't throw any error if it sees a > fence instruction with > (pred=W, succ=0, fm=0, rd=x0, and rs1=x0.) ? I couldn't find anything > specific related to this in the spec. >
Never mind. I found this
"The FENCE encoding currently has nine non-trivial combinations of the four bits PR, PW, SR, and SW, plus one extra encoding FENCE.TSO which facilitates mapping of “acquire+release” or RVTSO semantics. The remaining seven combinations have empty predecessor and/or successor sets and hence are no-ops."
However, we still need the extension availability check to preserve the older platform's behavior. Currently, the stall in cpu_relax is caused by the div. Without the extension probe, it will just execute "nop" which was not the earlier behavior.
> I think using the static key mechanism provides backward compatibility > without any runtime impact. > > > Regards, > > Samuel > > > > > 3. Use PAUSE for cpu_relax if both toolchain and the platform support it > > > > > > Signed-off-by: Dao Lu <daolu@rivosinc.com> > > > --- > > > arch/riscv/Makefile | 4 ++++ > > > arch/riscv/include/asm/hwcap.h | 1 + > > > arch/riscv/include/asm/vdso/processor.h | 19 ++++++++++++++++--- > > > arch/riscv/kernel/cpu.c | 1 + > > > arch/riscv/kernel/cpufeature.c | 7 +++++++ > > > 5 files changed, 29 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > > index 7d81102cffd4..900a8fda1a2d 100644 > > > --- a/arch/riscv/Makefile > > > +++ b/arch/riscv/Makefile > > > @@ -56,6 +56,10 @@ riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c > > > toolchain-need-zicsr-zifencei := $(call cc-option-yn, -march=$(riscv-march-y)_zicsr_zifencei) > > > riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei > > > > > > +# Check if the toolchain supports Zihintpause extension > > > +toolchain-supports-zihintpause := $(call cc-option-yn, -march=$(riscv-march-y)_zihintpause) > > > +riscv-march-$(toolchain-supports-zihintpause) := $(riscv-march-y)_zihintpause > > > + > > > KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y)) > > > KBUILD_AFLAGS += -march=$(riscv-march-y) > > > > > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h > > > index 0734e42f74f2..caa9ee5459b4 100644 > > > --- a/arch/riscv/include/asm/hwcap.h > > > +++ b/arch/riscv/include/asm/hwcap.h > > > @@ -52,6 +52,7 @@ extern unsigned long elf_hwcap; > > > */ > > > enum riscv_isa_ext_id { > > > RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, > > > + RISCV_ISA_EXT_ZIHINTPAUSE, > > > RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, > > > }; > > > > > > diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h > > > index 134388cbaaa1..106b35ba8cac 100644 > > > --- a/arch/riscv/include/asm/vdso/processor.h > > > +++ b/arch/riscv/include/asm/vdso/processor.h > > > @@ -4,15 +4,28 @@ > > > > > > #ifndef __ASSEMBLY__ > > > > > > +#include <linux/jump_label.h> > > > #include <asm/barrier.h> > > > +#include <asm/hwcap.h> > > > > > > +extern struct static_key_false riscv_pause_available; > > > static inline void cpu_relax(void) > > > { > > > + if (!static_branch_likely(&riscv_pause_available)) { > > > #ifdef __riscv_muldiv > > > - int dummy; > > > - /* In lieu of a halt instruction, induce a long-latency stall. */ > > > - __asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy)); > > > + int dummy; > > > + /* In lieu of a halt instruction, induce a long-latency stall. */ > > > + __asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy)); > > > #endif > > > + } else { > > > +#ifdef __riscv_zihintpause > > > + /* > > > + * Reduce instruction retirement. > > > + * This assumes the PC changes. > > > + */ > > > + __asm__ __volatile__ ("pause"); > > > +#endif > > > + } > > > barrier(); > > > } > > > > > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > > > index ccb617791e56..89e563e9c4cc 100644 > > > --- a/arch/riscv/kernel/cpu.c > > > +++ b/arch/riscv/kernel/cpu.c > > > @@ -88,6 +88,7 @@ int riscv_of_parent_hartid(struct device_node *node) > > > */ > > > static struct riscv_isa_ext_data isa_ext_arr[] = { > > > __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF), > > > + __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), > > > __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX), > > > }; > > > > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > > > index 1b2d42d7f589..327c19507dbb 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 > > > +DEFINE_STATIC_KEY_FALSE(riscv_pause_available); > > > +EXPORT_SYMBOL_GPL(riscv_pause_available); > > > > > > /** > > > * riscv_isa_extension_base() - Get base extension word > > > @@ -192,6 +194,7 @@ void __init riscv_fill_hwcap(void) > > > set_bit(*ext - 'a', this_isa); > > > } else { > > > SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); > > > + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); > > > } > > > #undef SET_ISA_EXT_MAP > > > } > > > @@ -213,6 +216,10 @@ void __init riscv_fill_hwcap(void) > > > > > > } > > > > > > + if (__riscv_isa_extension_available(riscv_isa, RISCV_ISA_EXT_ZIHINTPAUSE)) { > > > + static_branch_enable(&riscv_pause_available); > > > + } > > > + > > > /* We don't support systems with F but without D, so mask those out > > > * here. */ > > > if ((elf_hwcap & COMPAT_HWCAP_ISA_F) && !(elf_hwcap & COMPAT_HWCAP_ISA_D)) { > > > > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > -- > Regards, > Atish
-- Regards, Atish
| |