2022-05-25 17:00:44

by Dao Lu

Subject: [PATCH v2] arch/riscv: add Zihintpause support

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 <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
Signed-off-by: Dao Lu <[email protected]>
---

v1 -> v2:
Remove the usage of static branch, use PAUSE if toolchain supports it

arch/riscv/Makefile | 4 ++++
arch/riscv/include/asm/hwcap.h | 1 +
arch/riscv/include/asm/vdso/processor.h | 8 +++++++-
arch/riscv/kernel/cpu.c | 1 +
arch/riscv/kernel/cpufeature.c | 2 ++
5 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 7d81102cffd4..900a8fda1a2d 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -56,6 +56,10 @@ riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c
toolchain-need-zicsr-zifencei := $(call cc-option-yn, -march=$(riscv-march-y)_zicsr_zifencei)
riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei

+# Check if the toolchain supports Zihintpause extension
+toolchain-supports-zihintpause := $(call cc-option-yn, -march=$(riscv-march-y)_zihintpause)
+riscv-march-$(toolchain-supports-zihintpause) := $(riscv-march-y)_zihintpause
+
KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
KBUILD_AFLAGS += -march=$(riscv-march-y)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 0734e42f74f2..caa9ee5459b4 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -52,6 +52,7 @@ extern unsigned long elf_hwcap;
*/
enum riscv_isa_ext_id {
RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
+ RISCV_ISA_EXT_ZIHINTPAUSE,
RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
};

diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h
index 134388cbaaa1..4de911a25051 100644
--- a/arch/riscv/include/asm/vdso/processor.h
+++ b/arch/riscv/include/asm/vdso/processor.h
@@ -8,7 +8,13 @@

static inline void cpu_relax(void)
{
-#ifdef __riscv_muldiv
+#ifdef __riscv_zihintpause
+ /*
+ * Reduce instruction retirement.
+ * This assumes the PC changes.
+ */
+ __asm__ __volatile__ ("pause");
+#elif __riscv_muldiv
int dummy;
/* In lieu of a halt instruction, induce a long-latency stall. */
__asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy));
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index ccb617791e56..89e563e9c4cc 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -88,6 +88,7 @@ int riscv_of_parent_hartid(struct device_node *node)
*/
static struct riscv_isa_ext_data isa_ext_arr[] = {
__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
+ __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
};

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 1b2d42d7f589..37ff06682ae6 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -25,6 +25,7 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
__ro_after_init DEFINE_STATIC_KEY_FALSE(cpu_hwcap_fpu);
#endif

+
/**
* riscv_isa_extension_base() - Get base extension word
*
@@ -192,6 +193,7 @@ void __init riscv_fill_hwcap(void)
set_bit(*ext - 'a', this_isa);
} else {
SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
+ SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
}
#undef SET_ISA_EXT_MAP
}
--
2.36.0

2022-06-02 05:30:15

by Palmer Dabbelt

Subject: Re: [PATCH v2] arch/riscv: add Zihintpause support

On Tue, 24 May 2022 14:19:50 PDT (-0700), [email protected] 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 <[email protected]>
> Tested-by: Heiko Stuebner <[email protected]>
> Signed-off-by: Dao Lu <[email protected]>
> ---
>
> v1 -> v2:
> Remove the usage of static branch, use PAUSE if toolchain supports it

Sorry for the back and forth here, but IMO we do want to probe for this
at runtime: sure there's no issue executing the pause (aside from that
PC silliness, but we've all agreed to ignore that) but we still need to
execute the div on platforms that predate Zihintpause otherwise we
regress on cpu_relax() there. IIUC there's still no hardware with
Zihintpause, so regressing on real hardware in favor of an unimplemented
extension is the wrong way to go.

Should be pretty easy to do this with the alternatives mechanism.

>
> arch/riscv/Makefile | 4 ++++
> arch/riscv/include/asm/hwcap.h | 1 +
> arch/riscv/include/asm/vdso/processor.h | 8 +++++++-
> arch/riscv/kernel/cpu.c | 1 +
> arch/riscv/kernel/cpufeature.c | 2 ++
> 5 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index 7d81102cffd4..900a8fda1a2d 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -56,6 +56,10 @@ riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c
> toolchain-need-zicsr-zifencei := $(call cc-option-yn, -march=$(riscv-march-y)_zicsr_zifencei)
> riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei
>
> +# Check if the toolchain supports Zihintpause extension
> +toolchain-supports-zihintpause := $(call cc-option-yn, -march=$(riscv-march-y)_zihintpause)
> +riscv-march-$(toolchain-supports-zihintpause) := $(riscv-march-y)_zihintpause
> +
> KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
> KBUILD_AFLAGS += -march=$(riscv-march-y)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 0734e42f74f2..caa9ee5459b4 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -52,6 +52,7 @@ extern unsigned long elf_hwcap;
> */
> enum riscv_isa_ext_id {
> RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
> + RISCV_ISA_EXT_ZIHINTPAUSE,
> RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
> };
>
> diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h
> index 134388cbaaa1..4de911a25051 100644
> --- a/arch/riscv/include/asm/vdso/processor.h
> +++ b/arch/riscv/include/asm/vdso/processor.h
> @@ -8,7 +8,13 @@
>
> static inline void cpu_relax(void)
> {
> -#ifdef __riscv_muldiv
> +#ifdef __riscv_zihintpause
> + /*
> + * Reduce instruction retirement.
> + * This assumes the PC changes.
> + */
> + __asm__ __volatile__ ("pause");
> +#elif __riscv_muldiv
> int dummy;
> /* In lieu of a halt instruction, induce a long-latency stall. */
> __asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy));
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index ccb617791e56..89e563e9c4cc 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -88,6 +88,7 @@ int riscv_of_parent_hartid(struct device_node *node)
> */
> static struct riscv_isa_ext_data isa_ext_arr[] = {
> __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> + __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> };
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 1b2d42d7f589..37ff06682ae6 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -25,6 +25,7 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
> __ro_after_init DEFINE_STATIC_KEY_FALSE(cpu_hwcap_fpu);
> #endif
>
> +
> /**
> * riscv_isa_extension_base() - Get base extension word
> *
> @@ -192,6 +193,7 @@ void __init riscv_fill_hwcap(void)
> set_bit(*ext - 'a', this_isa);
> } else {
> SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
> + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> }
> #undef SET_ISA_EXT_MAP
> }

2022-06-14 16:43:44

by Heiko Stuebner

Subject: Re: [PATCH v2] arch/riscv: add Zihintpause support

Am Dienstag, 24. Mai 2022, 23:19:50 CEST schrieb Dao Lu:
> 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 <[email protected]>
> Tested-by: Heiko Stuebner <[email protected]>
> Signed-off-by: Dao Lu <[email protected]>
> ---
>
> v1 -> v2:
> Remove the usage of static branch, use PAUSE if toolchain supports it
>
> arch/riscv/Makefile | 4 ++++
> arch/riscv/include/asm/hwcap.h | 1 +
> arch/riscv/include/asm/vdso/processor.h | 8 +++++++-
> arch/riscv/kernel/cpu.c | 1 +
> arch/riscv/kernel/cpufeature.c | 2 ++
> 5 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index 7d81102cffd4..900a8fda1a2d 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -56,6 +56,10 @@ riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c
> toolchain-need-zicsr-zifencei := $(call cc-option-yn, -march=$(riscv-march-y)_zicsr_zifencei)
> riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei
>
> +# Check if the toolchain supports Zihintpause extension
> +toolchain-supports-zihintpause := $(call cc-option-yn, -march=$(riscv-march-y)_zihintpause)
> +riscv-march-$(toolchain-supports-zihintpause) := $(riscv-march-y)_zihintpause
> +
> KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
> KBUILD_AFLAGS += -march=$(riscv-march-y)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 0734e42f74f2..caa9ee5459b4 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -52,6 +52,7 @@ extern unsigned long elf_hwcap;
> */
> enum riscv_isa_ext_id {
> RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,

svpbmt got merged meanwhile, so this patch needs a rebase
onto 5.19-rc.

One more nit below

> + RISCV_ISA_EXT_ZIHINTPAUSE,
> RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
> };
>
> diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h
> index 134388cbaaa1..4de911a25051 100644
> --- a/arch/riscv/include/asm/vdso/processor.h
> +++ b/arch/riscv/include/asm/vdso/processor.h
> @@ -8,7 +8,13 @@
>
> static inline void cpu_relax(void)
> {
> -#ifdef __riscv_muldiv
> +#ifdef __riscv_zihintpause
> + /*
> + * Reduce instruction retirement.
> + * This assumes the PC changes.
> + */
> + __asm__ __volatile__ ("pause");
> +#elif __riscv_muldiv
> int dummy;
> /* In lieu of a halt instruction, induce a long-latency stall. */
> __asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy));
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index ccb617791e56..89e563e9c4cc 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -88,6 +88,7 @@ int riscv_of_parent_hartid(struct device_node *node)
> */
> static struct riscv_isa_ext_data isa_ext_arr[] = {
> __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> + __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> };
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 1b2d42d7f589..37ff06682ae6 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -25,6 +25,7 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
> __ro_after_init DEFINE_STATIC_KEY_FALSE(cpu_hwcap_fpu);
> #endif
>
> +

this is an unrelated change and also is adding an unneeded extra empty line.


Heiko

> /**
> * riscv_isa_extension_base() - Get base extension word
> *
> @@ -192,6 +193,7 @@ void __init riscv_fill_hwcap(void)
> set_bit(*ext - 'a', this_isa);
> } else {
> SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
> + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> }
> #undef SET_ISA_EXT_MAP
> }
>

2022-06-15 17:30:33

by Dao Lu

Subject: Re: [PATCH v2] arch/riscv: add Zihintpause support

Thanks Heiko and Palmer, will address the comments, rebase and send a v3.

On Tue, Jun 14, 2022 at 9:44 AM Heiko Stübner <[email protected]> wrote:
>
> Am Dienstag, 24. Mai 2022, 23:19:50 CEST schrieb Dao Lu:
> > 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 <[email protected]>
> > Tested-by: Heiko Stuebner <[email protected]>
> > Signed-off-by: Dao Lu <[email protected]>
> > ---
> >
> > v1 -> v2:
> > Remove the usage of static branch, use PAUSE if toolchain supports it
> >
> > arch/riscv/Makefile | 4 ++++
> > arch/riscv/include/asm/hwcap.h | 1 +
> > arch/riscv/include/asm/vdso/processor.h | 8 +++++++-
> > arch/riscv/kernel/cpu.c | 1 +
> > arch/riscv/kernel/cpufeature.c | 2 ++
> > 5 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > index 7d81102cffd4..900a8fda1a2d 100644
> > --- a/arch/riscv/Makefile
> > +++ b/arch/riscv/Makefile
> > @@ -56,6 +56,10 @@ riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c
> > toolchain-need-zicsr-zifencei := $(call cc-option-yn, -march=$(riscv-march-y)_zicsr_zifencei)
> > riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei
> >
> > +# Check if the toolchain supports Zihintpause extension
> > +toolchain-supports-zihintpause := $(call cc-option-yn, -march=$(riscv-march-y)_zihintpause)
> > +riscv-march-$(toolchain-supports-zihintpause) := $(riscv-march-y)_zihintpause
> > +
> > KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
> > KBUILD_AFLAGS += -march=$(riscv-march-y)
> >
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 0734e42f74f2..caa9ee5459b4 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -52,6 +52,7 @@ extern unsigned long elf_hwcap;
> > */
> > enum riscv_isa_ext_id {
> > RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
>
> svpbmt got merged meanwhile, so this patch needs a rebase
> onto 5.19-rc.
>
> One more nit below
>
> > + RISCV_ISA_EXT_ZIHINTPAUSE,
> > RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
> > };
> >
> > diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h
> > index 134388cbaaa1..4de911a25051 100644
> > --- a/arch/riscv/include/asm/vdso/processor.h
> > +++ b/arch/riscv/include/asm/vdso/processor.h
> > @@ -8,7 +8,13 @@
> >
> > static inline void cpu_relax(void)
> > {
> > -#ifdef __riscv_muldiv
> > +#ifdef __riscv_zihintpause
> > + /*
> > + * Reduce instruction retirement.
> > + * This assumes the PC changes.
> > + */
> > + __asm__ __volatile__ ("pause");
> > +#elif __riscv_muldiv
> > int dummy;
> > /* In lieu of a halt instruction, induce a long-latency stall. */
> > __asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy));
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index ccb617791e56..89e563e9c4cc 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -88,6 +88,7 @@ int riscv_of_parent_hartid(struct device_node *node)
> > */
> > static struct riscv_isa_ext_data isa_ext_arr[] = {
> > __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > + __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> > __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> > };
> >
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 1b2d42d7f589..37ff06682ae6 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -25,6 +25,7 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
> > __ro_after_init DEFINE_STATIC_KEY_FALSE(cpu_hwcap_fpu);
> > #endif
> >
> > +
>
> this is an unrelated change and also is adding an unneeded extra empty line.
>
>
> Heiko
>
> > /**
> > * riscv_isa_extension_base() - Get base extension word
> > *
> > @@ -192,6 +193,7 @@ void __init riscv_fill_hwcap(void)
> > set_bit(*ext - 'a', this_isa);
> > } else {
> > SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
> > + SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > }
> > #undef SET_ISA_EXT_MAP
> > }
> >
>
>
>
>