This patch:
1. Build with _zihintpause if the toolchain has support for it
2. Detects if the platform supports the extension
3. Use PAUSE for cpu_relax if both toolchain and the platform support it
Signed-off-by: Dao Lu <[email protected]>
---
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)) {
--
2.36.0
On Fri, May 13, 2022 at 12:09 AM Atish Patra <[email protected]> wrote:
>
> On Thu, May 12, 2022 at 6:06 PM Samuel Holland <[email protected]> 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 <[email protected]>
> > > ---
> > > 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
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>
>
> --
> Regards,
> Atish
--
Regards,
Atish
On Thu, May 12, 2022 at 6:06 PM Samuel Holland <[email protected]> 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.
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 <[email protected]>
> > ---
> > 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Regards,
Atish
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.
Regards,
Samuel
> 3. Use PAUSE for cpu_relax if both toolchain and the platform support it
>
> Signed-off-by: Dao Lu <[email protected]>
> ---
> 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)) {
>
Hi,
Am Donnerstag, 12. Mai 2022, 05:30:45 CEST schrieb Dao Lu:
> This patch:
> 1. Build with _zihintpause if the toolchain has support for it
> 2. Detects if the platform supports the extension
> 3. Use PAUSE for cpu_relax if both toolchain and the platform support it
This simply explains what the patch does, which is also pretty easy to
see by just reading the patch, so doesn't provide real additional value.
Please use the commit message to provide more background on what
you want to achieve. I.e. a short explanation what it is.
-----
Implement support for the ZiHintPause extension.
The PAUSE instruction is a HINT that indicates the current hart’s rate of
instruction retirement should be temporarily reduced or paused.
-----
The second sentence obviously comes directly from the riscv-spec pdf ;-)
There is one nit below too and with that fixed
Reviewed-by: Heiko Stuebner <[email protected]>
On a Qemu build with your extension patch for it, also
Tested-by: Heiko Stuebner <[email protected]>
> Signed-off-by: Dao Lu <[email protected]>
> ---
> 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);
> + }
> +
You don't really need the braces for the single call to static_branch_enable
> /* 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)) {
>
Heiko
Hi Dao,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linux/master]
[also build test ERROR on linus/master v5.18-rc6 next-20220513]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Dao-Lu/arch-riscv-Add-Zihintpause-extension-support/20220512-113348
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git feb9c5e19e913b53cb536a7aa7c9f20107bb51ec
config: riscv-randconfig-r035-20220512 (https://download.01.org/0day-ci/archive/20220514/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 38189438b69ca27b4c6ce707c52dbd217583d046)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/ebdb085998220eb16e6815608a3a6d8f87711d28
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Dao-Lu/arch-riscv-Add-Zihintpause-extension-support/20220512-113348
git checkout ebdb085998220eb16e6815608a3a6d8f87711d28
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv prepare
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> ld.lld: error: section .text file range overlaps with __jump_table
>>> .text range is [0x800, 0x1277]
>>> __jump_table range is [0x878, 0x8D7]
--
>> ld.lld: error: section .text virtual address range overlaps with __jump_table
>>> .text range is [0x800, 0x1277]
>>> __jump_table range is [0x878, 0x8D7]
--
>> ld.lld: error: section .text load address range overlaps with __jump_table
>>> .text range is [0x800, 0x1277]
>>> __jump_table range is [0x878, 0x8D7]
--
>> ld.lld: error: section .text load address range overlaps with __jump_table
>>> .text range is [0x800, 0x1277]
>>> __jump_table range is [0x878, 0x8D7]
--
>> ld.lld: error: section .text load address range overlaps with __jump_table
>>> .text range is [0x800, 0x1277]
>>> __jump_table range is [0x878, 0x8D7]
llvm-nm: error: arch/riscv/kernel/vdso/vdso.so.dbg: No such file or directory
--
0-DAY CI Kernel Test Service
https://01.org/lkp