Message ID | 20220620201530.3929352-1-daolu@rivosinc.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v4] arch/riscv: add Zihintpause support | expand |
ping On Mon, Jun 20, 2022 at 1:15 PM Dao Lu <daolu@rivosinc.com> wrote: > > 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. > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > Tested-by: Heiko Stuebner <heiko@sntech.de> > Signed-off-by: Dao Lu <daolu@rivosinc.com> > --- > > v1 -> v2: > Remove the usage of static branch, use PAUSE if toolchain supports it > v2 -> v3: > Added the static branch back, cpu_relax() behavior is kept the same for > systems that do not support ZiHintPause > v3 -> v4: > Adopted the newly added unified static keys for extensions > --- > arch/riscv/Makefile | 4 ++++ > arch/riscv/include/asm/hwcap.h | 5 +++++ > arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- > arch/riscv/kernel/cpu.c | 1 + > arch/riscv/kernel/cpufeature.c | 1 + > 5 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 > --- a/arch/riscv/include/asm/hwcap.h > +++ b/arch/riscv/include/asm/hwcap.h > @@ -8,6 +8,7 @@ > #ifndef _ASM_RISCV_HWCAP_H > #define _ASM_RISCV_HWCAP_H > > +#include <asm/errno.h> > #include <linux/bits.h> > #include <uapi/asm/hwcap.h> > > @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; > enum riscv_isa_ext_id { > RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, > RISCV_ISA_EXT_SVPBMT, > + RISCV_ISA_EXT_ZIHINTPAUSE, > RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, > }; > > @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { > */ > enum riscv_isa_ext_key { > RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ > + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, > RISCV_ISA_EXT_KEY_MAX, > }; > > @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) > return RISCV_ISA_EXT_KEY_FPU; > case RISCV_ISA_EXT_d: > return RISCV_ISA_EXT_KEY_FPU; > + case RISCV_ISA_EXT_ZIHINTPAUSE: > + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; > default: > return -EINVAL; > } > diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h > index 134388cbaaa1..1e4f8b4aef79 100644 > --- a/arch/riscv/include/asm/vdso/processor.h > +++ b/arch/riscv/include/asm/vdso/processor.h > @@ -4,15 +4,30 @@ > > #ifndef __ASSEMBLY__ > > +#include <linux/jump_label.h> > #include <asm/barrier.h> > +#include <asm/hwcap.h> > > static inline void cpu_relax(void) > { > + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { > #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 { > + /* > + * Reduce instruction retirement. > + * This assumes the PC changes. > + */ > +#ifdef __riscv_zihintpause > + __asm__ __volatile__ ("pause"); > +#else > + /* Encoding of the pause instruction */ > + __asm__ __volatile__ (".4byte 0x100000F"); > +#endif > + } > barrier(); > } > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > index fba9e9f46a8c..a123e92b14dd 100644 > --- a/arch/riscv/kernel/cpu.c > +++ b/arch/riscv/kernel/cpu.c > @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), > + __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 1b3ec44e25f5..708df2c0bc34 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) > } else { > SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); > SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); > + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); > } > #undef SET_ISA_EXT_MAP > } > -- > 2.25.1 >
Reviewed-by: Guo Ren <guoren@kernel.org> On Wed, Jul 6, 2022 at 12:57 AM Dao Lu <daolu@rivosinc.com> wrote: > > ping > > On Mon, Jun 20, 2022 at 1:15 PM Dao Lu <daolu@rivosinc.com> wrote: > > > > 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. > > > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > > Tested-by: Heiko Stuebner <heiko@sntech.de> > > Signed-off-by: Dao Lu <daolu@rivosinc.com> > > --- > > > > v1 -> v2: > > Remove the usage of static branch, use PAUSE if toolchain supports it > > v2 -> v3: > > Added the static branch back, cpu_relax() behavior is kept the same for > > systems that do not support ZiHintPause > > v3 -> v4: > > Adopted the newly added unified static keys for extensions > > --- > > arch/riscv/Makefile | 4 ++++ > > arch/riscv/include/asm/hwcap.h | 5 +++++ > > arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- > > arch/riscv/kernel/cpu.c | 1 + > > arch/riscv/kernel/cpufeature.c | 1 + > > 5 files changed, 29 insertions(+), 3 deletions(-) > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 > > --- a/arch/riscv/include/asm/hwcap.h > > +++ b/arch/riscv/include/asm/hwcap.h > > @@ -8,6 +8,7 @@ > > #ifndef _ASM_RISCV_HWCAP_H > > #define _ASM_RISCV_HWCAP_H > > > > +#include <asm/errno.h> > > #include <linux/bits.h> > > #include <uapi/asm/hwcap.h> > > > > @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; > > enum riscv_isa_ext_id { > > RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, > > RISCV_ISA_EXT_SVPBMT, > > + RISCV_ISA_EXT_ZIHINTPAUSE, > > RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, > > }; > > > > @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { > > */ > > enum riscv_isa_ext_key { > > RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ > > + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, > > RISCV_ISA_EXT_KEY_MAX, > > }; > > > > @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) > > return RISCV_ISA_EXT_KEY_FPU; > > case RISCV_ISA_EXT_d: > > return RISCV_ISA_EXT_KEY_FPU; > > + case RISCV_ISA_EXT_ZIHINTPAUSE: > > + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; > > default: > > return -EINVAL; > > } > > diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h > > index 134388cbaaa1..1e4f8b4aef79 100644 > > --- a/arch/riscv/include/asm/vdso/processor.h > > +++ b/arch/riscv/include/asm/vdso/processor.h > > @@ -4,15 +4,30 @@ > > > > #ifndef __ASSEMBLY__ > > > > +#include <linux/jump_label.h> > > #include <asm/barrier.h> > > +#include <asm/hwcap.h> > > > > static inline void cpu_relax(void) > > { > > + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { > > #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 { > > + /* > > + * Reduce instruction retirement. > > + * This assumes the PC changes. > > + */ > > +#ifdef __riscv_zihintpause > > + __asm__ __volatile__ ("pause"); > > +#else > > + /* Encoding of the pause instruction */ > > + __asm__ __volatile__ (".4byte 0x100000F"); > > +#endif > > + } > > barrier(); > > } > > > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > > index fba9e9f46a8c..a123e92b14dd 100644 > > --- a/arch/riscv/kernel/cpu.c > > +++ b/arch/riscv/kernel/cpu.c > > @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), > > + __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 1b3ec44e25f5..708df2c0bc34 100644 > > --- a/arch/riscv/kernel/cpufeature.c > > +++ b/arch/riscv/kernel/cpufeature.c > > @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) > > } else { > > SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); > > SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); > > + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); > > } > > #undef SET_ISA_EXT_MAP > > } > > -- > > 2.25.1 > >
On Mon, Jun 20, 2022 at 1:15 PM Dao Lu <daolu@rivosinc.com> wrote: > > 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. > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > Tested-by: Heiko Stuebner <heiko@sntech.de> > Signed-off-by: Dao Lu <daolu@rivosinc.com> > --- > > v1 -> v2: > Remove the usage of static branch, use PAUSE if toolchain supports it > v2 -> v3: > Added the static branch back, cpu_relax() behavior is kept the same for > systems that do not support ZiHintPause > v3 -> v4: > Adopted the newly added unified static keys for extensions > --- > arch/riscv/Makefile | 4 ++++ > arch/riscv/include/asm/hwcap.h | 5 +++++ > arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- > arch/riscv/kernel/cpu.c | 1 + > arch/riscv/kernel/cpufeature.c | 1 + > 5 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 > --- a/arch/riscv/include/asm/hwcap.h > +++ b/arch/riscv/include/asm/hwcap.h > @@ -8,6 +8,7 @@ > #ifndef _ASM_RISCV_HWCAP_H > #define _ASM_RISCV_HWCAP_H > > +#include <asm/errno.h> > #include <linux/bits.h> > #include <uapi/asm/hwcap.h> > > @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; > enum riscv_isa_ext_id { > RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, > RISCV_ISA_EXT_SVPBMT, > + RISCV_ISA_EXT_ZIHINTPAUSE, > RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, > }; > > @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { > */ > enum riscv_isa_ext_key { > RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ > + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, > RISCV_ISA_EXT_KEY_MAX, > }; > > @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) > return RISCV_ISA_EXT_KEY_FPU; > case RISCV_ISA_EXT_d: > return RISCV_ISA_EXT_KEY_FPU; > + case RISCV_ISA_EXT_ZIHINTPAUSE: > + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; > default: > return -EINVAL; > } > diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h > index 134388cbaaa1..1e4f8b4aef79 100644 > --- a/arch/riscv/include/asm/vdso/processor.h > +++ b/arch/riscv/include/asm/vdso/processor.h > @@ -4,15 +4,30 @@ > > #ifndef __ASSEMBLY__ > > +#include <linux/jump_label.h> > #include <asm/barrier.h> > +#include <asm/hwcap.h> > > static inline void cpu_relax(void) > { > + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { > #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 { > + /* > + * Reduce instruction retirement. > + * This assumes the PC changes. > + */ > +#ifdef __riscv_zihintpause > + __asm__ __volatile__ ("pause"); > +#else > + /* Encoding of the pause instruction */ > + __asm__ __volatile__ (".4byte 0x100000F"); > +#endif > + } > barrier(); > } > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > index fba9e9f46a8c..a123e92b14dd 100644 > --- a/arch/riscv/kernel/cpu.c > +++ b/arch/riscv/kernel/cpu.c > @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), > + __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 1b3ec44e25f5..708df2c0bc34 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) > } else { > SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); > SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); > + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); > } > #undef SET_ISA_EXT_MAP > } > -- > 2.25.1 > LGTM. Reviewed-by: Atish Patra <atishp@rivosinc.com>
On 20/06/2022 21:15, Dao Lu wrote: > [PATCH v4] arch/riscv: add Zihintpause support A little ornery/pedantic maybe, but the "arch/" isn't needed. Guess it can easily be fixed on application or if there's a v5 so there's prob no need to resend. > 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. > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > Tested-by: Heiko Stuebner <heiko@sntech.de> > Signed-off-by: Dao Lu <daolu@rivosinc.com> > --- > > v1 -> v2: > Remove the usage of static branch, use PAUSE if toolchain supports it > v2 -> v3: > Added the static branch back, cpu_relax() behavior is kept the same for > systems that do not support ZiHintPause > v3 -> v4: > Adopted the newly added unified static keys for extensions > --- > arch/riscv/Makefile | 4 ++++ > arch/riscv/include/asm/hwcap.h | 5 +++++ > arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- > arch/riscv/kernel/cpu.c | 1 + > arch/riscv/kernel/cpufeature.c | 1 + > 5 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 > --- a/arch/riscv/include/asm/hwcap.h > +++ b/arch/riscv/include/asm/hwcap.h > @@ -8,6 +8,7 @@ > #ifndef _ASM_RISCV_HWCAP_H > #define _ASM_RISCV_HWCAP_H > > +#include <asm/errno.h> > #include <linux/bits.h> > #include <uapi/asm/hwcap.h> > > @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; > enum riscv_isa_ext_id { > RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, > RISCV_ISA_EXT_SVPBMT, > + RISCV_ISA_EXT_ZIHINTPAUSE, > RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, > }; > > @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { > */ > enum riscv_isa_ext_key { > RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ > + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, > RISCV_ISA_EXT_KEY_MAX, > }; > > @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) > return RISCV_ISA_EXT_KEY_FPU; > case RISCV_ISA_EXT_d: > return RISCV_ISA_EXT_KEY_FPU; > + case RISCV_ISA_EXT_ZIHINTPAUSE: > + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; > default: > return -EINVAL; > } > diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h > index 134388cbaaa1..1e4f8b4aef79 100644 > --- a/arch/riscv/include/asm/vdso/processor.h > +++ b/arch/riscv/include/asm/vdso/processor.h > @@ -4,15 +4,30 @@ > > #ifndef __ASSEMBLY__ > > +#include <linux/jump_label.h> > #include <asm/barrier.h> > +#include <asm/hwcap.h> > > static inline void cpu_relax(void) > { > + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { > #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 { > + /* > + * Reduce instruction retirement. > + * This assumes the PC changes. > + */ > +#ifdef __riscv_zihintpause > + __asm__ __volatile__ ("pause"); > +#else > + /* Encoding of the pause instruction */ > + __asm__ __volatile__ (".4byte 0x100000F"); > +#endif > + } > barrier(); > } > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > index fba9e9f46a8c..a123e92b14dd 100644 > --- a/arch/riscv/kernel/cpu.c > +++ b/arch/riscv/kernel/cpu.c > @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), > + __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 1b3ec44e25f5..708df2c0bc34 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) > } else { > SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); > SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); > + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); > } > #undef SET_ISA_EXT_MAP > }
On Mon, Jun 20, 2022 at 01:15:25PM -0700, Dao Lu wrote: > 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. > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > Tested-by: Heiko Stuebner <heiko@sntech.de> > Signed-off-by: Dao Lu <daolu@rivosinc.com> Reviewed-by: Jisheng Zhang<jszhang@kernel.org> > --- > > v1 -> v2: > Remove the usage of static branch, use PAUSE if toolchain supports it > v2 -> v3: > Added the static branch back, cpu_relax() behavior is kept the same for > systems that do not support ZiHintPause > v3 -> v4: > Adopted the newly added unified static keys for extensions > --- > arch/riscv/Makefile | 4 ++++ > arch/riscv/include/asm/hwcap.h | 5 +++++ > arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- > arch/riscv/kernel/cpu.c | 1 + > arch/riscv/kernel/cpufeature.c | 1 + > 5 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 > --- a/arch/riscv/include/asm/hwcap.h > +++ b/arch/riscv/include/asm/hwcap.h > @@ -8,6 +8,7 @@ > #ifndef _ASM_RISCV_HWCAP_H > #define _ASM_RISCV_HWCAP_H > > +#include <asm/errno.h> > #include <linux/bits.h> > #include <uapi/asm/hwcap.h> > > @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; > enum riscv_isa_ext_id { > RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, > RISCV_ISA_EXT_SVPBMT, > + RISCV_ISA_EXT_ZIHINTPAUSE, > RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, > }; > > @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { > */ > enum riscv_isa_ext_key { > RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ > + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, > RISCV_ISA_EXT_KEY_MAX, > }; > > @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) > return RISCV_ISA_EXT_KEY_FPU; > case RISCV_ISA_EXT_d: > return RISCV_ISA_EXT_KEY_FPU; > + case RISCV_ISA_EXT_ZIHINTPAUSE: > + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; > default: > return -EINVAL; > } > diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h > index 134388cbaaa1..1e4f8b4aef79 100644 > --- a/arch/riscv/include/asm/vdso/processor.h > +++ b/arch/riscv/include/asm/vdso/processor.h > @@ -4,15 +4,30 @@ > > #ifndef __ASSEMBLY__ > > +#include <linux/jump_label.h> > #include <asm/barrier.h> > +#include <asm/hwcap.h> > > static inline void cpu_relax(void) > { > + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { > #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 { > + /* > + * Reduce instruction retirement. > + * This assumes the PC changes. > + */ > +#ifdef __riscv_zihintpause > + __asm__ __volatile__ ("pause"); > +#else > + /* Encoding of the pause instruction */ > + __asm__ __volatile__ (".4byte 0x100000F"); > +#endif > + } > barrier(); > } > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > index fba9e9f46a8c..a123e92b14dd 100644 > --- a/arch/riscv/kernel/cpu.c > +++ b/arch/riscv/kernel/cpu.c > @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), > + __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 1b3ec44e25f5..708df2c0bc34 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) > } else { > SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); > SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); > + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); > } > #undef SET_ISA_EXT_MAP > } > -- > 2.25.1 >
Am Montag, 11. Juli 2022, 22:44:21 CEST schrieb Conor Dooley: > On 20/06/2022 21:15, Dao Lu wrote: > > [PATCH v4] arch/riscv: add Zihintpause support > > A little ornery/pedantic maybe, but the "arch/" isn't needed. > Guess it can easily be fixed on application or if there's a > v5 so there's prob no need to resend. I noticed that the patch prefix on riscv is all over the place in a lot of cases. There are - RISC-V: -riscv: - now arch/riscv: and probably even more. I guess someone should just decide on one prefix that then gets used (and slightly enforced) all the time. Even just modifying applied patches to one specific prefix should already solve the issue in a short time, as I guess most people will just get the appicable prefix from a "git log" ;-) . Heiko > > 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. > > > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > > Tested-by: Heiko Stuebner <heiko@sntech.de> > > Signed-off-by: Dao Lu <daolu@rivosinc.com> > > --- > > > > v1 -> v2: > > Remove the usage of static branch, use PAUSE if toolchain supports it > > v2 -> v3: > > Added the static branch back, cpu_relax() behavior is kept the same for > > systems that do not support ZiHintPause > > v3 -> v4: > > Adopted the newly added unified static keys for extensions > > --- > > arch/riscv/Makefile | 4 ++++ > > arch/riscv/include/asm/hwcap.h | 5 +++++ > > arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- > > arch/riscv/kernel/cpu.c | 1 + > > arch/riscv/kernel/cpufeature.c | 1 + > > 5 files changed, 29 insertions(+), 3 deletions(-) > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 > > --- a/arch/riscv/include/asm/hwcap.h > > +++ b/arch/riscv/include/asm/hwcap.h > > @@ -8,6 +8,7 @@ > > #ifndef _ASM_RISCV_HWCAP_H > > #define _ASM_RISCV_HWCAP_H > > > > +#include <asm/errno.h> > > #include <linux/bits.h> > > #include <uapi/asm/hwcap.h> > > > > @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; > > enum riscv_isa_ext_id { > > RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, > > RISCV_ISA_EXT_SVPBMT, > > + RISCV_ISA_EXT_ZIHINTPAUSE, > > RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, > > }; > > > > @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { > > */ > > enum riscv_isa_ext_key { > > RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ > > + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, > > RISCV_ISA_EXT_KEY_MAX, > > }; > > > > @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) > > return RISCV_ISA_EXT_KEY_FPU; > > case RISCV_ISA_EXT_d: > > return RISCV_ISA_EXT_KEY_FPU; > > + case RISCV_ISA_EXT_ZIHINTPAUSE: > > + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; > > default: > > return -EINVAL; > > } > > diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h > > index 134388cbaaa1..1e4f8b4aef79 100644 > > --- a/arch/riscv/include/asm/vdso/processor.h > > +++ b/arch/riscv/include/asm/vdso/processor.h > > @@ -4,15 +4,30 @@ > > > > #ifndef __ASSEMBLY__ > > > > +#include <linux/jump_label.h> > > #include <asm/barrier.h> > > +#include <asm/hwcap.h> > > > > static inline void cpu_relax(void) > > { > > + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { > > #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 { > > + /* > > + * Reduce instruction retirement. > > + * This assumes the PC changes. > > + */ > > +#ifdef __riscv_zihintpause > > + __asm__ __volatile__ ("pause"); > > +#else > > + /* Encoding of the pause instruction */ > > + __asm__ __volatile__ (".4byte 0x100000F"); > > +#endif > > + } > > barrier(); > > } > > > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > > index fba9e9f46a8c..a123e92b14dd 100644 > > --- a/arch/riscv/kernel/cpu.c > > +++ b/arch/riscv/kernel/cpu.c > > @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), > > + __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 1b3ec44e25f5..708df2c0bc34 100644 > > --- a/arch/riscv/kernel/cpufeature.c > > +++ b/arch/riscv/kernel/cpufeature.c > > @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) > > } else { > > SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); > > SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); > > + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); > > } > > #undef SET_ISA_EXT_MAP > > } >
On 16/07/2022 15:11, Heiko Stuebner wrote: > Am Montag, 11. Juli 2022, 22:44:21 CEST schrieb Conor Dooley: >> On 20/06/2022 21:15, Dao Lu wrote: >>> [PATCH v4] arch/riscv: add Zihintpause support >> >> A little ornery/pedantic maybe, but the "arch/" isn't needed. >> Guess it can easily be fixed on application or if there's a >> v5 so there's prob no need to resend. > > I noticed that the patch prefix on riscv is all over the place > in a lot of cases. There are > > - RISC-V: > -riscv: > - now arch/riscv: > > and probably even more. > > I guess someone should just decide on one prefix that then > gets used (and slightly enforced) all the time. Yeah, I did see Palmer commenting on RISC-V/riscv once. I think riscv is more common since it matches the arch directory but Palmer himself uses RISC-V I've seen patchsets that use both RISC-V and riscv haha Maybe it is time to pick one? Palmer? > > Even just modifying applied patches to one specific prefix should > already solve the issue in a short time, as I guess most people will > just get the appicable prefix from a "git log" ;-) . Again, that's down to Palmer (and now me I guess to a lesser extent given I've starting doing PRs). My OCD would like a single prefix though ;) Conor. > > > Heiko > >>> 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. >>> >>> Reviewed-by: Heiko Stuebner <heiko@sntech.de> >>> Tested-by: Heiko Stuebner <heiko@sntech.de> >>> Signed-off-by: Dao Lu <daolu@rivosinc.com> >>> --- >>> >>> v1 -> v2: >>> Remove the usage of static branch, use PAUSE if toolchain supports it >>> v2 -> v3: >>> Added the static branch back, cpu_relax() behavior is kept the same for >>> systems that do not support ZiHintPause >>> v3 -> v4: >>> Adopted the newly added unified static keys for extensions >>> --- >>> arch/riscv/Makefile | 4 ++++ >>> arch/riscv/include/asm/hwcap.h | 5 +++++ >>> arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- >>> arch/riscv/kernel/cpu.c | 1 + >>> arch/riscv/kernel/cpufeature.c | 1 + >>> 5 files changed, 29 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile >>> index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 >>> --- a/arch/riscv/include/asm/hwcap.h >>> +++ b/arch/riscv/include/asm/hwcap.h >>> @@ -8,6 +8,7 @@ >>> #ifndef _ASM_RISCV_HWCAP_H >>> #define _ASM_RISCV_HWCAP_H >>> >>> +#include <asm/errno.h> >>> #include <linux/bits.h> >>> #include <uapi/asm/hwcap.h> >>> >>> @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; >>> enum riscv_isa_ext_id { >>> RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, >>> RISCV_ISA_EXT_SVPBMT, >>> + RISCV_ISA_EXT_ZIHINTPAUSE, >>> RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, >>> }; >>> >>> @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { >>> */ >>> enum riscv_isa_ext_key { >>> RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ >>> + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, >>> RISCV_ISA_EXT_KEY_MAX, >>> }; >>> >>> @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) >>> return RISCV_ISA_EXT_KEY_FPU; >>> case RISCV_ISA_EXT_d: >>> return RISCV_ISA_EXT_KEY_FPU; >>> + case RISCV_ISA_EXT_ZIHINTPAUSE: >>> + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; >>> default: >>> return -EINVAL; >>> } >>> diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h >>> index 134388cbaaa1..1e4f8b4aef79 100644 >>> --- a/arch/riscv/include/asm/vdso/processor.h >>> +++ b/arch/riscv/include/asm/vdso/processor.h >>> @@ -4,15 +4,30 @@ >>> >>> #ifndef __ASSEMBLY__ >>> >>> +#include <linux/jump_label.h> >>> #include <asm/barrier.h> >>> +#include <asm/hwcap.h> >>> >>> static inline void cpu_relax(void) >>> { >>> + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { >>> #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 { >>> + /* >>> + * Reduce instruction retirement. >>> + * This assumes the PC changes. >>> + */ >>> +#ifdef __riscv_zihintpause >>> + __asm__ __volatile__ ("pause"); >>> +#else >>> + /* Encoding of the pause instruction */ >>> + __asm__ __volatile__ (".4byte 0x100000F"); >>> +#endif >>> + } >>> barrier(); >>> } >>> >>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c >>> index fba9e9f46a8c..a123e92b14dd 100644 >>> --- a/arch/riscv/kernel/cpu.c >>> +++ b/arch/riscv/kernel/cpu.c >>> @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), >>> + __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 1b3ec44e25f5..708df2c0bc34 100644 >>> --- a/arch/riscv/kernel/cpufeature.c >>> +++ b/arch/riscv/kernel/cpufeature.c >>> @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) >>> } else { >>> SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); >>> SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); >>> + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); >>> } >>> #undef SET_ISA_EXT_MAP >>> } >> > > > >
On Mon, 20 Jun 2022 13:15:25 PDT (-0700), daolu@rivosinc.com wrote: > 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. > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > Tested-by: Heiko Stuebner <heiko@sntech.de> > Signed-off-by: Dao Lu <daolu@rivosinc.com> > --- > > v1 -> v2: > Remove the usage of static branch, use PAUSE if toolchain supports it > v2 -> v3: > Added the static branch back, cpu_relax() behavior is kept the same for > systems that do not support ZiHintPause > v3 -> v4: > Adopted the newly added unified static keys for extensions > --- > arch/riscv/Makefile | 4 ++++ > arch/riscv/include/asm/hwcap.h | 5 +++++ > arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- > arch/riscv/kernel/cpu.c | 1 + > arch/riscv/kernel/cpufeature.c | 1 + > 5 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 > --- a/arch/riscv/include/asm/hwcap.h > +++ b/arch/riscv/include/asm/hwcap.h > @@ -8,6 +8,7 @@ > #ifndef _ASM_RISCV_HWCAP_H > #define _ASM_RISCV_HWCAP_H > > +#include <asm/errno.h> > #include <linux/bits.h> > #include <uapi/asm/hwcap.h> > > @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; > enum riscv_isa_ext_id { > RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, > RISCV_ISA_EXT_SVPBMT, > + RISCV_ISA_EXT_ZIHINTPAUSE, > RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, > }; > > @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { > */ > enum riscv_isa_ext_key { > RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ > + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, > RISCV_ISA_EXT_KEY_MAX, > }; > > @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) > return RISCV_ISA_EXT_KEY_FPU; > case RISCV_ISA_EXT_d: > return RISCV_ISA_EXT_KEY_FPU; > + case RISCV_ISA_EXT_ZIHINTPAUSE: > + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; > default: > return -EINVAL; > } > diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h > index 134388cbaaa1..1e4f8b4aef79 100644 > --- a/arch/riscv/include/asm/vdso/processor.h > +++ b/arch/riscv/include/asm/vdso/processor.h > @@ -4,15 +4,30 @@ > > #ifndef __ASSEMBLY__ > > +#include <linux/jump_label.h> > #include <asm/barrier.h> > +#include <asm/hwcap.h> > > static inline void cpu_relax(void) > { > + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { > #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 { > + /* > + * Reduce instruction retirement. > + * This assumes the PC changes. > + */ > +#ifdef __riscv_zihintpause > + __asm__ __volatile__ ("pause"); > +#else > + /* Encoding of the pause instruction */ > + __asm__ __volatile__ (".4byte 0x100000F"); > +#endif > + } > barrier(); > } > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > index fba9e9f46a8c..a123e92b14dd 100644 > --- a/arch/riscv/kernel/cpu.c > +++ b/arch/riscv/kernel/cpu.c > @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), > + __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 1b3ec44e25f5..708df2c0bc34 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) > } else { > SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); > SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); > + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); > } > #undef SET_ISA_EXT_MAP > } Thanks, this is on for-next. It needs a sparse patch, which I put in as a link.
On 11/08/2022 16:17, Palmer Dabbelt wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Mon, 20 Jun 2022 13:15:25 PDT (-0700), daolu@rivosinc.com wrote: >> 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. >> >> Reviewed-by: Heiko Stuebner <heiko@sntech.de> >> Tested-by: Heiko Stuebner <heiko@sntech.de> >> Signed-off-by: Dao Lu <daolu@rivosinc.com> >> --- >> >> v1 -> v2: >> Remove the usage of static branch, use PAUSE if toolchain supports it >> v2 -> v3: >> Added the static branch back, cpu_relax() behavior is kept the same for >> systems that do not support ZiHintPause >> v3 -> v4: >> Adopted the newly added unified static keys for extensions >> --- >> arch/riscv/Makefile | 4 ++++ >> arch/riscv/include/asm/hwcap.h | 5 +++++ >> arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- >> arch/riscv/kernel/cpu.c | 1 + >> arch/riscv/kernel/cpufeature.c | 1 + >> 5 files changed, 29 insertions(+), 3 deletions(-) >> >> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile >> index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 >> --- a/arch/riscv/include/asm/hwcap.h >> +++ b/arch/riscv/include/asm/hwcap.h >> @@ -8,6 +8,7 @@ >> #ifndef _ASM_RISCV_HWCAP_H >> #define _ASM_RISCV_HWCAP_H >> >> +#include <asm/errno.h> >> #include <linux/bits.h> >> #include <uapi/asm/hwcap.h> >> >> @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; >> enum riscv_isa_ext_id { >> RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, >> RISCV_ISA_EXT_SVPBMT, >> + RISCV_ISA_EXT_ZIHINTPAUSE, >> RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, >> }; >> >> @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { >> */ >> enum riscv_isa_ext_key { >> RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ >> + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, >> RISCV_ISA_EXT_KEY_MAX, >> }; >> >> @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) >> return RISCV_ISA_EXT_KEY_FPU; >> case RISCV_ISA_EXT_d: >> return RISCV_ISA_EXT_KEY_FPU; >> + case RISCV_ISA_EXT_ZIHINTPAUSE: >> + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; >> default: >> return -EINVAL; >> } >> diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h >> index 134388cbaaa1..1e4f8b4aef79 100644 >> --- a/arch/riscv/include/asm/vdso/processor.h >> +++ b/arch/riscv/include/asm/vdso/processor.h >> @@ -4,15 +4,30 @@ >> >> #ifndef __ASSEMBLY__ >> >> +#include <linux/jump_label.h> >> #include <asm/barrier.h> >> +#include <asm/hwcap.h> >> >> static inline void cpu_relax(void) >> { >> + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { >> #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 { >> + /* >> + * Reduce instruction retirement. >> + * This assumes the PC changes. >> + */ >> +#ifdef __riscv_zihintpause >> + __asm__ __volatile__ ("pause"); >> +#else >> + /* Encoding of the pause instruction */ >> + __asm__ __volatile__ (".4byte 0x100000F"); >> +#endif >> + } >> barrier(); >> } >> >> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c >> index fba9e9f46a8c..a123e92b14dd 100644 >> --- a/arch/riscv/kernel/cpu.c >> +++ b/arch/riscv/kernel/cpu.c >> @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), >> + __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 1b3ec44e25f5..708df2c0bc34 100644 >> --- a/arch/riscv/kernel/cpufeature.c >> +++ b/arch/riscv/kernel/cpufeature.c >> @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) >> } else { >> SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); >> SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); >> + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); >> } >> #undef SET_ISA_EXT_MAP >> } > > Thanks, this is on for-next. It needs a sparse patch, which I put in as a link. This breaks the C=1 build for all toolchains, not just new ones as your sparse patch suggests. I amn't 100% what my CI is running, but I replicated this on my own machine with: sparse --version 0.6.4 (Ubuntu: 0.6.4-2) ---8<--- YACC scripts/dtc/dtc-parser.tab.[ch] HOSTCC scripts/dtc/libfdt/fdt.o HOSTCC scripts/dtc/libfdt/fdt_ro.o HOSTCC scripts/dtc/libfdt/fdt_wip.o UPD include/generated/uapi/linux/version.h HOSTCC scripts/dtc/libfdt/fdt_sw.o UPD include/config/kernel.release HOSTCC scripts/dtc/libfdt/fdt_rw.o HOSTCC scripts/dtc/libfdt/fdt_strerror.o HOSTCC scripts/dtc/libfdt/fdt_empty_tree.o HOSTCC scripts/dtc/libfdt/fdt_addresses.o HOSTCC scripts/dtc/libfdt/fdt_overlay.o HOSTCC scripts/dtc/fdtoverlay.o HOSTCC scripts/dtc/dtc-lexer.lex.o HOSTCC scripts/dtc/dtc-parser.tab.o UPD include/generated/utsrelease.h HOSTLD scripts/dtc/fdtoverlay HOSTLD scripts/dtc/dtc HOSTCC scripts/kallsyms HOSTCC scripts/sorttable HOSTCC scripts/asn1_compiler HOSTCC scripts/selinux/genheaders/genheaders HOSTCC scripts/selinux/mdp/mdp CC scripts/mod/empty.o HOSTCC scripts/mod/mk_elfconfig CC scripts/mod/devicetable-offsets.s CHECK ../scripts/mod/empty.c invalid argument to '-march': '_zihintpause' make[2]: *** [../scripts/Makefile.build:250: scripts/mod/empty.o] Error 1 make[2]: *** Deleting file 'scripts/mod/empty.o' make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/mnt/automation/corp/workspace/ux-test_upstream-next-develop-cd@2/linux/Makefile:1287: prepare0] Error 2 make[1]: Leaving directory '/mnt/automation/corp/workspace/ux-test_upstream-next-develop-cd@2/linux/builddir' make: *** [Makefile:231: __sub-make] Error 2
On 12/08/2022 07:57, Conor Dooley - M52691 wrote: > On 11/08/2022 16:17, Palmer Dabbelt wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On Mon, 20 Jun 2022 13:15:25 PDT (-0700), daolu@rivosinc.com wrote: >>> 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. >>> >>> Reviewed-by: Heiko Stuebner <heiko@sntech.de> >>> Tested-by: Heiko Stuebner <heiko@sntech.de> >>> Signed-off-by: Dao Lu <daolu@rivosinc.com> >>> --- >>> >>> v1 -> v2: >>> Remove the usage of static branch, use PAUSE if toolchain supports it >>> v2 -> v3: >>> Added the static branch back, cpu_relax() behavior is kept the same for >>> systems that do not support ZiHintPause >>> v3 -> v4: >>> Adopted the newly added unified static keys for extensions >>> --- >>> arch/riscv/Makefile | 4 ++++ >>> arch/riscv/include/asm/hwcap.h | 5 +++++ >>> arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- >>> arch/riscv/kernel/cpu.c | 1 + >>> arch/riscv/kernel/cpufeature.c | 1 + >>> 5 files changed, 29 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile >>> index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 >>> --- a/arch/riscv/include/asm/hwcap.h >>> +++ b/arch/riscv/include/asm/hwcap.h >>> @@ -8,6 +8,7 @@ >>> #ifndef _ASM_RISCV_HWCAP_H >>> #define _ASM_RISCV_HWCAP_H >>> >>> +#include <asm/errno.h> >>> #include <linux/bits.h> >>> #include <uapi/asm/hwcap.h> >>> >>> @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; >>> enum riscv_isa_ext_id { >>> RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, >>> RISCV_ISA_EXT_SVPBMT, >>> + RISCV_ISA_EXT_ZIHINTPAUSE, >>> RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, >>> }; >>> >>> @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { >>> */ >>> enum riscv_isa_ext_key { >>> RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ >>> + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, >>> RISCV_ISA_EXT_KEY_MAX, >>> }; >>> >>> @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) >>> return RISCV_ISA_EXT_KEY_FPU; >>> case RISCV_ISA_EXT_d: >>> return RISCV_ISA_EXT_KEY_FPU; >>> + case RISCV_ISA_EXT_ZIHINTPAUSE: >>> + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; >>> default: >>> return -EINVAL; >>> } >>> diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h >>> index 134388cbaaa1..1e4f8b4aef79 100644 >>> --- a/arch/riscv/include/asm/vdso/processor.h >>> +++ b/arch/riscv/include/asm/vdso/processor.h >>> @@ -4,15 +4,30 @@ >>> >>> #ifndef __ASSEMBLY__ >>> >>> +#include <linux/jump_label.h> >>> #include <asm/barrier.h> >>> +#include <asm/hwcap.h> >>> >>> static inline void cpu_relax(void) >>> { >>> + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { >>> #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 { >>> + /* >>> + * Reduce instruction retirement. >>> + * This assumes the PC changes. >>> + */ >>> +#ifdef __riscv_zihintpause >>> + __asm__ __volatile__ ("pause"); >>> +#else >>> + /* Encoding of the pause instruction */ >>> + __asm__ __volatile__ (".4byte 0x100000F"); >>> +#endif >>> + } >>> barrier(); >>> } >>> >>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c >>> index fba9e9f46a8c..a123e92b14dd 100644 >>> --- a/arch/riscv/kernel/cpu.c >>> +++ b/arch/riscv/kernel/cpu.c >>> @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), >>> + __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 1b3ec44e25f5..708df2c0bc34 100644 >>> --- a/arch/riscv/kernel/cpufeature.c >>> +++ b/arch/riscv/kernel/cpufeature.c >>> @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) >>> } else { >>> SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); >>> SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); >>> + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); >>> } >>> #undef SET_ISA_EXT_MAP >>> } >> >> Thanks, this is on for-next. It needs a sparse patch, which I put in as a link. > > This breaks the C=1 build for all toolchains, not just new ones as your sparse > patch suggests. I amn't 100% what my CI is running, but I replicated this on > my own machine with: Argh, I went poking around and my toolchain's binutils etc is newer than I thought. Good for people searching on lore I suppose... Sorry! Conor. > > sparse --version > 0.6.4 (Ubuntu: 0.6.4-2) > > ---8<--- > YACC scripts/dtc/dtc-parser.tab.[ch] > HOSTCC scripts/dtc/libfdt/fdt.o > HOSTCC scripts/dtc/libfdt/fdt_ro.o > HOSTCC scripts/dtc/libfdt/fdt_wip.o > UPD include/generated/uapi/linux/version.h > HOSTCC scripts/dtc/libfdt/fdt_sw.o > UPD include/config/kernel.release > HOSTCC scripts/dtc/libfdt/fdt_rw.o > HOSTCC scripts/dtc/libfdt/fdt_strerror.o > HOSTCC scripts/dtc/libfdt/fdt_empty_tree.o > HOSTCC scripts/dtc/libfdt/fdt_addresses.o > HOSTCC scripts/dtc/libfdt/fdt_overlay.o > HOSTCC scripts/dtc/fdtoverlay.o > HOSTCC scripts/dtc/dtc-lexer.lex.o > HOSTCC scripts/dtc/dtc-parser.tab.o > UPD include/generated/utsrelease.h > HOSTLD scripts/dtc/fdtoverlay > HOSTLD scripts/dtc/dtc > HOSTCC scripts/kallsyms > HOSTCC scripts/sorttable > HOSTCC scripts/asn1_compiler > HOSTCC scripts/selinux/genheaders/genheaders > HOSTCC scripts/selinux/mdp/mdp > CC scripts/mod/empty.o > HOSTCC scripts/mod/mk_elfconfig > CC scripts/mod/devicetable-offsets.s > CHECK ../scripts/mod/empty.c > invalid argument to '-march': '_zihintpause' > > make[2]: *** [../scripts/Makefile.build:250: scripts/mod/empty.o] Error 1 > make[2]: *** Deleting file 'scripts/mod/empty.o' > make[2]: *** Waiting for unfinished jobs.... > make[1]: *** [/mnt/automation/corp/workspace/ux-test_upstream-next-develop-cd@2/linux/Makefile:1287: prepare0] Error 2 > make[1]: Leaving directory '/mnt/automation/corp/workspace/ux-test_upstream-next-develop-cd@2/linux/builddir' > make: *** [Makefile:231: __sub-make] Error 2
On Fri, 12 Aug 2022 00:21:40 PDT (-0700), Conor.Dooley@microchip.com wrote: > On 12/08/2022 07:57, Conor Dooley - M52691 wrote: >> On 11/08/2022 16:17, Palmer Dabbelt wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> On Mon, 20 Jun 2022 13:15:25 PDT (-0700), daolu@rivosinc.com wrote: >>>> 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. >>>> >>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de> >>>> Tested-by: Heiko Stuebner <heiko@sntech.de> >>>> Signed-off-by: Dao Lu <daolu@rivosinc.com> >>>> --- >>>> >>>> v1 -> v2: >>>> Remove the usage of static branch, use PAUSE if toolchain supports it >>>> v2 -> v3: >>>> Added the static branch back, cpu_relax() behavior is kept the same for >>>> systems that do not support ZiHintPause >>>> v3 -> v4: >>>> Adopted the newly added unified static keys for extensions >>>> --- >>>> arch/riscv/Makefile | 4 ++++ >>>> arch/riscv/include/asm/hwcap.h | 5 +++++ >>>> arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- >>>> arch/riscv/kernel/cpu.c | 1 + >>>> arch/riscv/kernel/cpufeature.c | 1 + >>>> 5 files changed, 29 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile >>>> index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 >>>> --- a/arch/riscv/include/asm/hwcap.h >>>> +++ b/arch/riscv/include/asm/hwcap.h >>>> @@ -8,6 +8,7 @@ >>>> #ifndef _ASM_RISCV_HWCAP_H >>>> #define _ASM_RISCV_HWCAP_H >>>> >>>> +#include <asm/errno.h> >>>> #include <linux/bits.h> >>>> #include <uapi/asm/hwcap.h> >>>> >>>> @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; >>>> enum riscv_isa_ext_id { >>>> RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, >>>> RISCV_ISA_EXT_SVPBMT, >>>> + RISCV_ISA_EXT_ZIHINTPAUSE, >>>> RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, >>>> }; >>>> >>>> @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { >>>> */ >>>> enum riscv_isa_ext_key { >>>> RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ >>>> + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, >>>> RISCV_ISA_EXT_KEY_MAX, >>>> }; >>>> >>>> @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) >>>> return RISCV_ISA_EXT_KEY_FPU; >>>> case RISCV_ISA_EXT_d: >>>> return RISCV_ISA_EXT_KEY_FPU; >>>> + case RISCV_ISA_EXT_ZIHINTPAUSE: >>>> + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; >>>> default: >>>> return -EINVAL; >>>> } >>>> diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h >>>> index 134388cbaaa1..1e4f8b4aef79 100644 >>>> --- a/arch/riscv/include/asm/vdso/processor.h >>>> +++ b/arch/riscv/include/asm/vdso/processor.h >>>> @@ -4,15 +4,30 @@ >>>> >>>> #ifndef __ASSEMBLY__ >>>> >>>> +#include <linux/jump_label.h> >>>> #include <asm/barrier.h> >>>> +#include <asm/hwcap.h> >>>> >>>> static inline void cpu_relax(void) >>>> { >>>> + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { >>>> #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 { >>>> + /* >>>> + * Reduce instruction retirement. >>>> + * This assumes the PC changes. >>>> + */ >>>> +#ifdef __riscv_zihintpause >>>> + __asm__ __volatile__ ("pause"); >>>> +#else >>>> + /* Encoding of the pause instruction */ >>>> + __asm__ __volatile__ (".4byte 0x100000F"); >>>> +#endif >>>> + } >>>> barrier(); >>>> } >>>> >>>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c >>>> index fba9e9f46a8c..a123e92b14dd 100644 >>>> --- a/arch/riscv/kernel/cpu.c >>>> +++ b/arch/riscv/kernel/cpu.c >>>> @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), >>>> + __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 1b3ec44e25f5..708df2c0bc34 100644 >>>> --- a/arch/riscv/kernel/cpufeature.c >>>> +++ b/arch/riscv/kernel/cpufeature.c >>>> @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) >>>> } else { >>>> SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); >>>> SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); >>>> + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); >>>> } >>>> #undef SET_ISA_EXT_MAP >>>> } >>> >>> Thanks, this is on for-next. It needs a sparse patch, which I put in as a link. >> >> This breaks the C=1 build for all toolchains, not just new ones as your sparse >> patch suggests. I amn't 100% what my CI is running, but I replicated this on >> my own machine with: > > Argh, I went poking around and my toolchain's binutils etc is newer than I thought. > Good for people searching on lore I suppose... > Sorry! So just to be clear: you're saying this only breaks with new binutils? I ask because Dao is also seeing some crashes, if it's breaking arbitrary builds too then it's a stronger hint to revert it. > Conor. > >> >> sparse --version >> 0.6.4 (Ubuntu: 0.6.4-2) >> >> ---8<--- >> YACC scripts/dtc/dtc-parser.tab.[ch] >> HOSTCC scripts/dtc/libfdt/fdt.o >> HOSTCC scripts/dtc/libfdt/fdt_ro.o >> HOSTCC scripts/dtc/libfdt/fdt_wip.o >> UPD include/generated/uapi/linux/version.h >> HOSTCC scripts/dtc/libfdt/fdt_sw.o >> UPD include/config/kernel.release >> HOSTCC scripts/dtc/libfdt/fdt_rw.o >> HOSTCC scripts/dtc/libfdt/fdt_strerror.o >> HOSTCC scripts/dtc/libfdt/fdt_empty_tree.o >> HOSTCC scripts/dtc/libfdt/fdt_addresses.o >> HOSTCC scripts/dtc/libfdt/fdt_overlay.o >> HOSTCC scripts/dtc/fdtoverlay.o >> HOSTCC scripts/dtc/dtc-lexer.lex.o >> HOSTCC scripts/dtc/dtc-parser.tab.o >> UPD include/generated/utsrelease.h >> HOSTLD scripts/dtc/fdtoverlay >> HOSTLD scripts/dtc/dtc >> HOSTCC scripts/kallsyms >> HOSTCC scripts/sorttable >> HOSTCC scripts/asn1_compiler >> HOSTCC scripts/selinux/genheaders/genheaders >> HOSTCC scripts/selinux/mdp/mdp >> CC scripts/mod/empty.o >> HOSTCC scripts/mod/mk_elfconfig >> CC scripts/mod/devicetable-offsets.s >> CHECK ../scripts/mod/empty.c >> invalid argument to '-march': '_zihintpause' >> >> make[2]: *** [../scripts/Makefile.build:250: scripts/mod/empty.o] Error 1 >> make[2]: *** Deleting file 'scripts/mod/empty.o' >> make[2]: *** Waiting for unfinished jobs.... >> make[1]: *** [/mnt/automation/corp/workspace/ux-test_upstream-next-develop-cd@2/linux/Makefile:1287: prepare0] Error 2 >> make[1]: Leaving directory '/mnt/automation/corp/workspace/ux-test_upstream-next-develop-cd@2/linux/builddir' >> make: *** [Makefile:231: __sub-make] Error 2 >
On 16/08/2022 16:54, Palmer Dabbelt wrote: > On Fri, 12 Aug 2022 00:21:40 PDT (-0700), Conor.Dooley@microchip.com wrote: >> On 12/08/2022 07:57, Conor Dooley - M52691 wrote: >>> On 11/08/2022 16:17, Palmer Dabbelt wrote: >>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>>> >>>> On Mon, 20 Jun 2022 13:15:25 PDT (-0700), daolu@rivosinc.com wrote: >>>>> 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. >>>>> >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de> >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de> >>>>> Signed-off-by: Dao Lu <daolu@rivosinc.com> >>>>> --- >>>>> >>>>> v1 -> v2: >>>>> Remove the usage of static branch, use PAUSE if toolchain supports it >>>>> v2 -> v3: >>>>> Added the static branch back, cpu_relax() behavior is kept the same for >>>>> systems that do not support ZiHintPause >>>>> v3 -> v4: >>>>> Adopted the newly added unified static keys for extensions >>>>> --- >>>>> arch/riscv/Makefile | 4 ++++ >>>>> arch/riscv/include/asm/hwcap.h | 5 +++++ >>>>> arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- >>>>> arch/riscv/kernel/cpu.c | 1 + >>>>> arch/riscv/kernel/cpufeature.c | 1 + >>>>> 5 files changed, 29 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile >>>>> index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 >>>>> --- a/arch/riscv/include/asm/hwcap.h >>>>> +++ b/arch/riscv/include/asm/hwcap.h >>>>> @@ -8,6 +8,7 @@ >>>>> #ifndef _ASM_RISCV_HWCAP_H >>>>> #define _ASM_RISCV_HWCAP_H >>>>> >>>>> +#include <asm/errno.h> >>>>> #include <linux/bits.h> >>>>> #include <uapi/asm/hwcap.h> >>>>> >>>>> @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; >>>>> enum riscv_isa_ext_id { >>>>> RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, >>>>> RISCV_ISA_EXT_SVPBMT, >>>>> + RISCV_ISA_EXT_ZIHINTPAUSE, >>>>> RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, >>>>> }; >>>>> >>>>> @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { >>>>> */ >>>>> enum riscv_isa_ext_key { >>>>> RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ >>>>> + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, >>>>> RISCV_ISA_EXT_KEY_MAX, >>>>> }; >>>>> >>>>> @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) >>>>> return RISCV_ISA_EXT_KEY_FPU; >>>>> case RISCV_ISA_EXT_d: >>>>> return RISCV_ISA_EXT_KEY_FPU; >>>>> + case RISCV_ISA_EXT_ZIHINTPAUSE: >>>>> + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; >>>>> default: >>>>> return -EINVAL; >>>>> } >>>>> diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h >>>>> index 134388cbaaa1..1e4f8b4aef79 100644 >>>>> --- a/arch/riscv/include/asm/vdso/processor.h >>>>> +++ b/arch/riscv/include/asm/vdso/processor.h >>>>> @@ -4,15 +4,30 @@ >>>>> >>>>> #ifndef __ASSEMBLY__ >>>>> >>>>> +#include <linux/jump_label.h> >>>>> #include <asm/barrier.h> >>>>> +#include <asm/hwcap.h> >>>>> >>>>> static inline void cpu_relax(void) >>>>> { >>>>> + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { >>>>> #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 { >>>>> + /* >>>>> + * Reduce instruction retirement. >>>>> + * This assumes the PC changes. >>>>> + */ >>>>> +#ifdef __riscv_zihintpause >>>>> + __asm__ __volatile__ ("pause"); >>>>> +#else >>>>> + /* Encoding of the pause instruction */ >>>>> + __asm__ __volatile__ (".4byte 0x100000F"); >>>>> +#endif >>>>> + } >>>>> barrier(); >>>>> } >>>>> >>>>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c >>>>> index fba9e9f46a8c..a123e92b14dd 100644 >>>>> --- a/arch/riscv/kernel/cpu.c >>>>> +++ b/arch/riscv/kernel/cpu.c >>>>> @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), >>>>> + __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 1b3ec44e25f5..708df2c0bc34 100644 >>>>> --- a/arch/riscv/kernel/cpufeature.c >>>>> +++ b/arch/riscv/kernel/cpufeature.c >>>>> @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) >>>>> } else { >>>>> SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); >>>>> SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); >>>>> + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); >>>>> } >>>>> #undef SET_ISA_EXT_MAP >>>>> } >>>> >>>> Thanks, this is on for-next. It needs a sparse patch, which I put in as a link. >>> >>> This breaks the C=1 build for all toolchains, not just new ones as your sparse >>> patch suggests. I amn't 100% what my CI is running, but I replicated this on >>> my own machine with: >> >> Argh, I went poking around and my toolchain's binutils etc is newer than I thought. >> Good for people searching on lore I suppose... >> Sorry! > > So just to be clear: you're saying this only breaks with new binutils? Yes, sorry - I *thought* the binutils in our CI predated Zihintpause but it doesn't. It (and so would the Zicbom stuff) breaks the builds in our CI as they run with C=1. I manually patched sparse to get that going again & I /suspect/ it may have impacted LKP too. "New" is relative though - it breaks C=1 for anyone running a toolchain from riscv-collab/riscv-gnu-toolchain. I guess it's just that Zicbom is newer so not many people will be on a toolchain that supports that. My GCC doesn't only my clang-15. > I ask because Dao is also seeing some crashes, if it's breaking arbitrary > builds too then it's a stronger hint to revert it. It is breaking module loading on RISC-V in general from what it seems. See: https://lore.kernel.org/linux-riscv/728ecbd5-975a-168e-efab-3c0030be21d5@w6rz.net/ Thanks, Conor. > >> Conor. >> >>> >>> sparse --version >>> 0.6.4 (Ubuntu: 0.6.4-2) >>> >>> ---8<--- >>> YACC scripts/dtc/dtc-parser.tab.[ch] >>> HOSTCC scripts/dtc/libfdt/fdt.o >>> HOSTCC scripts/dtc/libfdt/fdt_ro.o >>> HOSTCC scripts/dtc/libfdt/fdt_wip.o >>> UPD include/generated/uapi/linux/version.h >>> HOSTCC scripts/dtc/libfdt/fdt_sw.o >>> UPD include/config/kernel.release >>> HOSTCC scripts/dtc/libfdt/fdt_rw.o >>> HOSTCC scripts/dtc/libfdt/fdt_strerror.o >>> HOSTCC scripts/dtc/libfdt/fdt_empty_tree.o >>> HOSTCC scripts/dtc/libfdt/fdt_addresses.o >>> HOSTCC scripts/dtc/libfdt/fdt_overlay.o >>> HOSTCC scripts/dtc/fdtoverlay.o >>> HOSTCC scripts/dtc/dtc-lexer.lex.o >>> HOSTCC scripts/dtc/dtc-parser.tab.o >>> UPD include/generated/utsrelease.h >>> HOSTLD scripts/dtc/fdtoverlay >>> HOSTLD scripts/dtc/dtc >>> HOSTCC scripts/kallsyms >>> HOSTCC scripts/sorttable >>> HOSTCC scripts/asn1_compiler >>> HOSTCC scripts/selinux/genheaders/genheaders >>> HOSTCC scripts/selinux/mdp/mdp >>> CC scripts/mod/empty.o >>> HOSTCC scripts/mod/mk_elfconfig >>> CC scripts/mod/devicetable-offsets.s >>> CHECK ../scripts/mod/empty.c >>> invalid argument to '-march': '_zihintpause' >>> >>> make[2]: *** [../scripts/Makefile.build:250: scripts/mod/empty.o] Error 1 >>> make[2]: *** Deleting file 'scripts/mod/empty.o' >>> make[2]: *** Waiting for unfinished jobs.... >>> make[1]: *** [/mnt/automation/corp/workspace/ux-test_upstream-next-develop-cd@2/linux/Makefile:1287: prepare0] Error 2 >>> make[1]: Leaving directory '/mnt/automation/corp/workspace/ux-test_upstream-next-develop-cd@2/linux/builddir' >>> make: *** [Makefile:231: __sub-make] Error 2 >> > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Aug 16, 2022 at 04:04:06PM +0000, Conor.Dooley@microchip.com wrote: > On 16/08/2022 16:54, Palmer Dabbelt wrote: > > On Fri, 12 Aug 2022 00:21:40 PDT (-0700), Conor.Dooley@microchip.com wrote: > >> On 12/08/2022 07:57, Conor Dooley - M52691 wrote: > >>> On 11/08/2022 16:17, Palmer Dabbelt wrote: > >>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >>>> > >>>> On Mon, 20 Jun 2022 13:15:25 PDT (-0700), daolu@rivosinc.com wrote: > >>>>> 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. > >>>>> > >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de> > >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de> > >>>>> Signed-off-by: Dao Lu <daolu@rivosinc.com> > >>>>> --- > >>>>> > >>>>> v1 -> v2: > >>>>> Remove the usage of static branch, use PAUSE if toolchain supports it > >>>>> v2 -> v3: > >>>>> Added the static branch back, cpu_relax() behavior is kept the same for > >>>>> systems that do not support ZiHintPause > >>>>> v3 -> v4: > >>>>> Adopted the newly added unified static keys for extensions > >>>>> --- > >>>>> arch/riscv/Makefile | 4 ++++ > >>>>> arch/riscv/include/asm/hwcap.h | 5 +++++ > >>>>> arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- > >>>>> arch/riscv/kernel/cpu.c | 1 + > >>>>> arch/riscv/kernel/cpufeature.c | 1 + > >>>>> 5 files changed, 29 insertions(+), 3 deletions(-) > >>>>> > >>>>> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > >>>>> index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 > >>>>> --- a/arch/riscv/include/asm/hwcap.h > >>>>> +++ b/arch/riscv/include/asm/hwcap.h > >>>>> @@ -8,6 +8,7 @@ > >>>>> #ifndef _ASM_RISCV_HWCAP_H > >>>>> #define _ASM_RISCV_HWCAP_H > >>>>> > >>>>> +#include <asm/errno.h> > >>>>> #include <linux/bits.h> > >>>>> #include <uapi/asm/hwcap.h> > >>>>> > >>>>> @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; > >>>>> enum riscv_isa_ext_id { > >>>>> RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, > >>>>> RISCV_ISA_EXT_SVPBMT, > >>>>> + RISCV_ISA_EXT_ZIHINTPAUSE, > >>>>> RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, > >>>>> }; > >>>>> > >>>>> @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { > >>>>> */ > >>>>> enum riscv_isa_ext_key { > >>>>> RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ > >>>>> + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, > >>>>> RISCV_ISA_EXT_KEY_MAX, > >>>>> }; > >>>>> > >>>>> @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) > >>>>> return RISCV_ISA_EXT_KEY_FPU; > >>>>> case RISCV_ISA_EXT_d: > >>>>> return RISCV_ISA_EXT_KEY_FPU; > >>>>> + case RISCV_ISA_EXT_ZIHINTPAUSE: > >>>>> + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; > >>>>> default: > >>>>> return -EINVAL; > >>>>> } > >>>>> diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h > >>>>> index 134388cbaaa1..1e4f8b4aef79 100644 > >>>>> --- a/arch/riscv/include/asm/vdso/processor.h > >>>>> +++ b/arch/riscv/include/asm/vdso/processor.h > >>>>> @@ -4,15 +4,30 @@ > >>>>> > >>>>> #ifndef __ASSEMBLY__ > >>>>> > >>>>> +#include <linux/jump_label.h> > >>>>> #include <asm/barrier.h> > >>>>> +#include <asm/hwcap.h> > >>>>> > >>>>> static inline void cpu_relax(void) > >>>>> { > >>>>> + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { > >>>>> #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 { > >>>>> + /* > >>>>> + * Reduce instruction retirement. > >>>>> + * This assumes the PC changes. > >>>>> + */ > >>>>> +#ifdef __riscv_zihintpause > >>>>> + __asm__ __volatile__ ("pause"); > >>>>> +#else > >>>>> + /* Encoding of the pause instruction */ > >>>>> + __asm__ __volatile__ (".4byte 0x100000F"); > >>>>> +#endif > >>>>> + } > >>>>> barrier(); > >>>>> } > >>>>> > >>>>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > >>>>> index fba9e9f46a8c..a123e92b14dd 100644 > >>>>> --- a/arch/riscv/kernel/cpu.c > >>>>> +++ b/arch/riscv/kernel/cpu.c > >>>>> @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), > >>>>> + __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 1b3ec44e25f5..708df2c0bc34 100644 > >>>>> --- a/arch/riscv/kernel/cpufeature.c > >>>>> +++ b/arch/riscv/kernel/cpufeature.c > >>>>> @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) > >>>>> } else { > >>>>> SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); > >>>>> SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); > >>>>> + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); > >>>>> } > >>>>> #undef SET_ISA_EXT_MAP > >>>>> } > >>>> > >>>> Thanks, this is on for-next. It needs a sparse patch, which I put in as a link. > >>> > >>> This breaks the C=1 build for all toolchains, not just new ones as your sparse > >>> patch suggests. I amn't 100% what my CI is running, but I replicated this on > >>> my own machine with: > >> > >> Argh, I went poking around and my toolchain's binutils etc is newer than I thought. > >> Good for people searching on lore I suppose... > >> Sorry! > > > > So just to be clear: you're saying this only breaks with new binutils? > > Yes, sorry - I *thought* the binutils in our CI predated Zihintpause but > it doesn't. It (and so would the Zicbom stuff) breaks the builds in our CI > as they run with C=1. I manually patched sparse to get that going again & I > /suspect/ it may have impacted LKP too. > > "New" is relative though - it breaks C=1 for anyone running a toolchain > from riscv-collab/riscv-gnu-toolchain. > > I guess it's just that Zicbom is newer so not many people will be on a > toolchain that supports that. My GCC doesn't only my clang-15. > > > I ask because Dao is also seeing some crashes, if it's breaking arbitrary > > builds too then it's a stronger hint to revert it. > > It is breaking module loading on RISC-V in general from what it seems. > See: > https://lore.kernel.org/linux-riscv/728ecbd5-975a-168e-efab-3c0030be21d5@w6rz.net/ Hi Conor, I have a patch for this which I'll send in just a second. Thanks, drew > > Thanks, > Conor. > > > > >> Conor. > >> > >>> > >>> sparse --version > >>> 0.6.4 (Ubuntu: 0.6.4-2) > >>> > >>> ---8<--- > >>> YACC scripts/dtc/dtc-parser.tab.[ch] > >>> HOSTCC scripts/dtc/libfdt/fdt.o > >>> HOSTCC scripts/dtc/libfdt/fdt_ro.o > >>> HOSTCC scripts/dtc/libfdt/fdt_wip.o > >>> UPD include/generated/uapi/linux/version.h > >>> HOSTCC scripts/dtc/libfdt/fdt_sw.o > >>> UPD include/config/kernel.release > >>> HOSTCC scripts/dtc/libfdt/fdt_rw.o > >>> HOSTCC scripts/dtc/libfdt/fdt_strerror.o > >>> HOSTCC scripts/dtc/libfdt/fdt_empty_tree.o > >>> HOSTCC scripts/dtc/libfdt/fdt_addresses.o > >>> HOSTCC scripts/dtc/libfdt/fdt_overlay.o > >>> HOSTCC scripts/dtc/fdtoverlay.o > >>> HOSTCC scripts/dtc/dtc-lexer.lex.o > >>> HOSTCC scripts/dtc/dtc-parser.tab.o > >>> UPD include/generated/utsrelease.h > >>> HOSTLD scripts/dtc/fdtoverlay > >>> HOSTLD scripts/dtc/dtc > >>> HOSTCC scripts/kallsyms > >>> HOSTCC scripts/sorttable > >>> HOSTCC scripts/asn1_compiler > >>> HOSTCC scripts/selinux/genheaders/genheaders > >>> HOSTCC scripts/selinux/mdp/mdp > >>> CC scripts/mod/empty.o > >>> HOSTCC scripts/mod/mk_elfconfig > >>> CC scripts/mod/devicetable-offsets.s > >>> CHECK ../scripts/mod/empty.c > >>> invalid argument to '-march': '_zihintpause' > >>> > >>> make[2]: *** [../scripts/Makefile.build:250: scripts/mod/empty.o] Error 1 > >>> make[2]: *** Deleting file 'scripts/mod/empty.o' > >>> make[2]: *** Waiting for unfinished jobs.... > >>> make[1]: *** [/mnt/automation/corp/workspace/ux-test_upstream-next-develop-cd@2/linux/Makefile:1287: prepare0] Error 2 > >>> make[1]: Leaving directory '/mnt/automation/corp/workspace/ux-test_upstream-next-develop-cd@2/linux/builddir' > >>> make: *** [Makefile:231: __sub-make] Error 2 > >> > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 8/16/22 11:04 AM, Conor.Dooley@microchip.com wrote: > On 16/08/2022 16:54, Palmer Dabbelt wrote: >> On Fri, 12 Aug 2022 00:21:40 PDT (-0700), Conor.Dooley@microchip.com wrote: >>> On 12/08/2022 07:57, Conor Dooley - M52691 wrote: >>>> On 11/08/2022 16:17, Palmer Dabbelt wrote: >>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>>>> >>>>> On Mon, 20 Jun 2022 13:15:25 PDT (-0700), daolu@rivosinc.com wrote: >>>>>> 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. >>>>>> >>>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de> >>>>>> Tested-by: Heiko Stuebner <heiko@sntech.de> >>>>>> Signed-off-by: Dao Lu <daolu@rivosinc.com> >>>>>> --- >>>>>> >>>>>> v1 -> v2: >>>>>> Remove the usage of static branch, use PAUSE if toolchain supports it >>>>>> v2 -> v3: >>>>>> Added the static branch back, cpu_relax() behavior is kept the same for >>>>>> systems that do not support ZiHintPause >>>>>> v3 -> v4: >>>>>> Adopted the newly added unified static keys for extensions >>>>>> --- >>>>>> arch/riscv/Makefile | 4 ++++ >>>>>> arch/riscv/include/asm/hwcap.h | 5 +++++ >>>>>> arch/riscv/include/asm/vdso/processor.h | 21 ++++++++++++++++++--- >>>>>> arch/riscv/kernel/cpu.c | 1 + >>>>>> arch/riscv/kernel/cpufeature.c | 1 + >>>>>> 5 files changed, 29 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile >>>>>> index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 >>>>>> --- a/arch/riscv/include/asm/hwcap.h >>>>>> +++ b/arch/riscv/include/asm/hwcap.h >>>>>> @@ -8,6 +8,7 @@ >>>>>> #ifndef _ASM_RISCV_HWCAP_H >>>>>> #define _ASM_RISCV_HWCAP_H >>>>>> >>>>>> +#include <asm/errno.h> >>>>>> #include <linux/bits.h> >>>>>> #include <uapi/asm/hwcap.h> >>>>>> >>>>>> @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; >>>>>> enum riscv_isa_ext_id { >>>>>> RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, >>>>>> RISCV_ISA_EXT_SVPBMT, >>>>>> + RISCV_ISA_EXT_ZIHINTPAUSE, >>>>>> RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, >>>>>> }; >>>>>> >>>>>> @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { >>>>>> */ >>>>>> enum riscv_isa_ext_key { >>>>>> RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ >>>>>> + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, >>>>>> RISCV_ISA_EXT_KEY_MAX, >>>>>> }; >>>>>> >>>>>> @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) >>>>>> return RISCV_ISA_EXT_KEY_FPU; >>>>>> case RISCV_ISA_EXT_d: >>>>>> return RISCV_ISA_EXT_KEY_FPU; >>>>>> + case RISCV_ISA_EXT_ZIHINTPAUSE: >>>>>> + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; >>>>>> default: >>>>>> return -EINVAL; >>>>>> } >>>>>> diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h >>>>>> index 134388cbaaa1..1e4f8b4aef79 100644 >>>>>> --- a/arch/riscv/include/asm/vdso/processor.h >>>>>> +++ b/arch/riscv/include/asm/vdso/processor.h >>>>>> @@ -4,15 +4,30 @@ >>>>>> >>>>>> #ifndef __ASSEMBLY__ >>>>>> >>>>>> +#include <linux/jump_label.h> >>>>>> #include <asm/barrier.h> >>>>>> +#include <asm/hwcap.h> >>>>>> >>>>>> static inline void cpu_relax(void) >>>>>> { >>>>>> + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { >>>>>> #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 { >>>>>> + /* >>>>>> + * Reduce instruction retirement. >>>>>> + * This assumes the PC changes. >>>>>> + */ >>>>>> +#ifdef __riscv_zihintpause >>>>>> + __asm__ __volatile__ ("pause"); >>>>>> +#else >>>>>> + /* Encoding of the pause instruction */ >>>>>> + __asm__ __volatile__ (".4byte 0x100000F"); >>>>>> +#endif >>>>>> + } >>>>>> barrier(); >>>>>> } >>>>>> >>>>>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c >>>>>> index fba9e9f46a8c..a123e92b14dd 100644 >>>>>> --- a/arch/riscv/kernel/cpu.c >>>>>> +++ b/arch/riscv/kernel/cpu.c >>>>>> @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), >>>>>> + __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 1b3ec44e25f5..708df2c0bc34 100644 >>>>>> --- a/arch/riscv/kernel/cpufeature.c >>>>>> +++ b/arch/riscv/kernel/cpufeature.c >>>>>> @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) >>>>>> } else { >>>>>> SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); >>>>>> SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); >>>>>> + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); >>>>>> } >>>>>> #undef SET_ISA_EXT_MAP >>>>>> } >>>>> >>>>> Thanks, this is on for-next. It needs a sparse patch, which I put in as a link. >>>> >>>> This breaks the C=1 build for all toolchains, not just new ones as your sparse >>>> patch suggests. I amn't 100% what my CI is running, but I replicated this on >>>> my own machine with: >>> >>> Argh, I went poking around and my toolchain's binutils etc is newer than I thought. >>> Good for people searching on lore I suppose... >>> Sorry! >> >> So just to be clear: you're saying this only breaks with new binutils? > > Yes, sorry - I *thought* the binutils in our CI predated Zihintpause but > it doesn't. It (and so would the Zicbom stuff) breaks the builds in our CI > as they run with C=1. I manually patched sparse to get that going again & I > /suspect/ it may have impacted LKP too. > > "New" is relative though - it breaks C=1 for anyone running a toolchain > from riscv-collab/riscv-gnu-toolchain. > > I guess it's just that Zicbom is newer so not many people will be on a > toolchain that supports that. My GCC doesn't only my clang-15. > >> I ask because Dao is also seeing some crashes, if it's breaking arbitrary >> builds too then it's a stronger hint to revert it. > > It is breaking module loading on RISC-V in general from what it seems. > See: > https://lore.kernel.org/linux-riscv/728ecbd5-975a-168e-efab-3c0030be21d5@w6rz.net/ This patch also completely breaks the build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y: In file included from <command-line>: ./arch/riscv/include/asm/jump_label.h: In function 'cpu_relax': ././include/linux/compiler_types.h:285:33: warning: 'asm' operand 0 probably does not match constraints 285 | #define asm_volatile_goto(x...) asm goto(x) | ^~~ ./arch/riscv/include/asm/jump_label.h:41:9: note: in expansion of macro 'asm_volatile_goto' 41 | asm_volatile_goto( | ^~~~~~~~~~~~~~~~~ ././include/linux/compiler_types.h:285:33: error: impossible constraint in 'asm' 285 | #define asm_volatile_goto(x...) asm goto(x) | ^~~ ./arch/riscv/include/asm/jump_label.h:41:9: note: in expansion of macro 'asm_volatile_goto' 41 | asm_volatile_goto( | ^~~~~~~~~~~~~~~~~ make[1]: *** [scripts/Makefile.build:249: arch/riscv/kernel/vdso/vgettimeofday.o] Error 1 make: *** [arch/riscv/Makefile:128: vdso_prepare] Error 2 Putting a static branch in such a widely-inlined function seems like a bad idea in general. (A quick measurement of my somewhat-minimal config shows this single static branch as responsible for 40% of the jump table.) I don't think it's even necessary in this case. Why can't we unconditionally run both instructions? If Zihintpause is supported, we trade the nop from the static branch for a div. If it is unsupported, we trade the jump from the static branch for a nop. Regards, Samuel
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index 34cf8a598617..6ddacc6f44b9 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 e48eebdd2631..dc47019a0b38 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -8,6 +8,7 @@ #ifndef _ASM_RISCV_HWCAP_H #define _ASM_RISCV_HWCAP_H +#include <asm/errno.h> #include <linux/bits.h> #include <uapi/asm/hwcap.h> @@ -54,6 +55,7 @@ extern unsigned long elf_hwcap; enum riscv_isa_ext_id { RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, RISCV_ISA_EXT_SVPBMT, + RISCV_ISA_EXT_ZIHINTPAUSE, RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, }; @@ -64,6 +66,7 @@ enum riscv_isa_ext_id { */ enum riscv_isa_ext_key { RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, RISCV_ISA_EXT_KEY_MAX, }; @@ -83,6 +86,8 @@ static __always_inline int riscv_isa_ext2key(int num) return RISCV_ISA_EXT_KEY_FPU; case RISCV_ISA_EXT_d: return RISCV_ISA_EXT_KEY_FPU; + case RISCV_ISA_EXT_ZIHINTPAUSE: + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; default: return -EINVAL; } diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h index 134388cbaaa1..1e4f8b4aef79 100644 --- a/arch/riscv/include/asm/vdso/processor.h +++ b/arch/riscv/include/asm/vdso/processor.h @@ -4,15 +4,30 @@ #ifndef __ASSEMBLY__ +#include <linux/jump_label.h> #include <asm/barrier.h> +#include <asm/hwcap.h> static inline void cpu_relax(void) { + if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) { #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 { + /* + * Reduce instruction retirement. + * This assumes the PC changes. + */ +#ifdef __riscv_zihintpause + __asm__ __volatile__ ("pause"); +#else + /* Encoding of the pause instruction */ + __asm__ __volatile__ (".4byte 0x100000F"); +#endif + } barrier(); } diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index fba9e9f46a8c..a123e92b14dd 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -89,6 +89,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(svpbmt, RISCV_ISA_EXT_SVPBMT), + __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 1b3ec44e25f5..708df2c0bc34 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void) } else { SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); } #undef SET_ISA_EXT_MAP }