2024-04-18 11:50:48

by Xu Lu

Subject: [RFC 0/2] riscv: Idle thread using Zawrs extension

This patch series introduces a new implementation of idle thread using
Zawrs extension.

The Zawrs[0] extension introduces two new instructions named WRS.STO and
WRS.NTO in RISC-V. When software registers a reservation set using LR
instruction, a subsequent WRS.STO or WRS.NTO instruction will cause the
hart to stall in a low-power state until a store happens to the
reservation set or an interrupt becomes pending. The difference between
these two instructions is that WRS.STO will terminate stall after an
implementation-defined timeout while WRS.NTO won't.

This patch series implements idle thread using WRS.NTO instruction.
Besides, we found there is no need to send a real IPI to wake up an idle
CPU. Instead, we write IPI information to the reservation set of an idle
CPU to wake it up and let it handle IPI quickly, without going through
tranditional interrupt handling routine.

[0] https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc

Xu Lu (2):
riscv: process: Introduce idle thread using Zawrs extension
riscv: Use Zawrs to accelerate IPI to idle cpu

arch/riscv/Kconfig | 24 +++++++
arch/riscv/include/asm/cpuidle.h | 11 +---
arch/riscv/include/asm/hwcap.h | 1 +
arch/riscv/include/asm/processor.h | 31 +++++++++
arch/riscv/include/asm/smp.h | 14 ++++
arch/riscv/kernel/cpu.c | 5 ++
arch/riscv/kernel/cpufeature.c | 1 +
arch/riscv/kernel/process.c | 102 ++++++++++++++++++++++++++++-
arch/riscv/kernel/smp.c | 39 +++++++----
9 files changed, 205 insertions(+), 23 deletions(-)

--
2.20.1

2024-04-18 11:51:03

by Xu Lu

Subject: [RFC 1/2] riscv: process: Introduce idle thread using Zawrs extension

The Zawrs extension introduces a new instruction WRS.NTO, which will
register a reservation set and causes the hart to temporarily stall
execution in a low-power state until a store occurs to the reservation
set or an interrupt is observed.

This commit implements new version of idle thread for RISC-V via Zawrs
extension.

Signed-off-by: Xu Lu <[email protected]>
Reviewed-by: Hangjing Li <[email protected]>
Reviewed-by: Liang Deng <[email protected]>
Reviewed-by: Wen Chai <[email protected]>
---
arch/riscv/Kconfig | 24 +++++++++++++++++
arch/riscv/include/asm/cpuidle.h | 11 +-------
arch/riscv/include/asm/hwcap.h | 1 +
arch/riscv/include/asm/processor.h | 17 +++++++++++++
arch/riscv/kernel/cpu.c | 5 ++++
arch/riscv/kernel/cpufeature.c | 1 +
arch/riscv/kernel/process.c | 41 +++++++++++++++++++++++++++++-
7 files changed, 89 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index be09c8836d56..a0d344e9803f 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -19,6 +19,7 @@ config RISCV
select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
select ARCH_HAS_BINFMT_FLAT
+ select ARCH_HAS_CPU_FINALIZE_INIT
select ARCH_HAS_CURRENT_STACK_POINTER
select ARCH_HAS_DEBUG_VIRTUAL if MMU
select ARCH_HAS_DEBUG_VM_PGTABLE
@@ -525,6 +526,20 @@ config RISCV_ISA_SVPBMT

If you don't know what to do here, say Y.

+config RISCV_ISA_ZAWRS
+ bool "Zawrs extension support for wait-on-reservation-set instructions"
+ depends on RISCV_ALTERNATIVE
+ default y
+ help
+ Adds support to dynamically detect the presence of the Zawrs
+ extension and enable its usage.
+
+ The Zawrs extension defines a pair of instructions to be used
+ in polling loops that allows a core to enter a low-power state
+ and wait on a store to a memory location.
+
+ If you don't know what to do here, say Y.
+
config TOOLCHAIN_HAS_V
bool
default y
@@ -1075,6 +1090,15 @@ endmenu # "Power management options"

menu "CPU Power Management"

+config RISCV_ZAWRS_IDLE
+ bool "Idle thread using ZAWRS extensions"
+ depends on RISCV_ISA_ZAWRS
+ default y
+ help
+ Adds support to implement idle thread using ZAWRS extension.
+
+ If you don't know what to do here, say Y.
+
source "drivers/cpuidle/Kconfig"

source "drivers/cpufreq/Kconfig"
diff --git a/arch/riscv/include/asm/cpuidle.h b/arch/riscv/include/asm/cpuidle.h
index 71fdc607d4bc..94c9ecb46571 100644
--- a/arch/riscv/include/asm/cpuidle.h
+++ b/arch/riscv/include/asm/cpuidle.h
@@ -10,15 +10,6 @@
#include <asm/barrier.h>
#include <asm/processor.h>

-static inline void cpu_do_idle(void)
-{
- /*
- * Add mb() here to ensure that all
- * IO/MEM accesses are completed prior
- * to entering WFI.
- */
- mb();
- wait_for_interrupt();
-}
+void cpu_do_idle(void);

#endif
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index e17d0078a651..5b358c3cf212 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -81,6 +81,7 @@
#define RISCV_ISA_EXT_ZTSO 72
#define RISCV_ISA_EXT_ZACAS 73
#define RISCV_ISA_EXT_XANDESPMU 74
+#define RISCV_ISA_EXT_ZAWRS 75

#define RISCV_ISA_EXT_XLINUXENVCFG 127

diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index 0faf5f161f1e..1143367de8c6 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -157,6 +157,21 @@ static inline void wait_for_interrupt(void)
__asm__ __volatile__ ("wfi");
}

+static inline void wrs_nto(unsigned long *addr)
+{
+ int val;
+
+ __asm__ __volatile__(
+#ifdef CONFIG_64BIT
+ "lr.d %[p], %[v] \n\t"
+#else
+ "lr.w %[p], %[v] \n\t"
+#endif
+ ".long 0x00d00073 \n\t"
+ : [p] "=&r" (val), [v] "+A" (*addr)
+ : : "memory");
+}
+
extern phys_addr_t dma32_phys_limit;

struct device_node;
@@ -183,6 +198,8 @@ extern int set_unalign_ctl(struct task_struct *tsk, unsigned int val);
#define GET_UNALIGN_CTL(tsk, addr) get_unalign_ctl((tsk), (addr))
#define SET_UNALIGN_CTL(tsk, val) set_unalign_ctl((tsk), (val))

+extern void select_idle_routine(void);
+
#endif /* __ASSEMBLY__ */

#endif /* _ASM_RISCV_PROCESSOR_H */
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index d11d6320fb0d..69cebd41f5f3 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -22,6 +22,11 @@ bool arch_match_cpu_phys_id(int cpu, u64 phys_id)
return phys_id == cpuid_to_hartid_map(cpu);
}

+void __init arch_cpu_finalize_init(void)
+{
+ select_idle_routine();
+}
+
/*
* Returns the hart ID of the given device tree node, or -ENODEV if the node
* isn't an enabled and valid RISC-V hart node.
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 3ed2359eae35..c080e6ca54ba 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -305,6 +305,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
__RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
__RISCV_ISA_EXT_DATA(xandespmu, RISCV_ISA_EXT_XANDESPMU),
+ __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
};

const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index 92922dbd5b5c..9f0f7b888bc1 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -15,6 +15,7 @@
#include <linux/tick.h>
#include <linux/ptrace.h>
#include <linux/uaccess.h>
+#include <linux/static_call.h>

#include <asm/unistd.h>
#include <asm/processor.h>
@@ -37,11 +38,49 @@ EXPORT_SYMBOL(__stack_chk_guard);

extern asmlinkage void ret_from_fork(void);

-void arch_cpu_idle(void)
+static __cpuidle void default_idle(void)
+{
+ /*
+ * Add mb() here to ensure that all
+ * IO/MEM accesses are completed prior
+ * to entering WFI.
+ */
+ mb();
+ wait_for_interrupt();
+}
+
+static __cpuidle void wrs_idle(void)
+{
+ /*
+ * Add mb() here to ensure that all
+ * IO/MEM accesses are completed prior
+ * to entering WRS.NTO.
+ */
+ mb();
+ wrs_nto(&current_thread_info()->flags);
+}
+
+DEFINE_STATIC_CALL_NULL(riscv_idle, default_idle);
+
+void __cpuidle cpu_do_idle(void)
+{
+ static_call(riscv_idle)();
+}
+
+void __cpuidle arch_cpu_idle(void)
{
cpu_do_idle();
}

+void __init select_idle_routine(void)
+{
+ if (IS_ENABLED(CONFIG_RISCV_ZAWRS_IDLE) &&
+ riscv_has_extension_likely(RISCV_ISA_EXT_ZAWRS))
+ static_call_update(riscv_idle, wrs_idle);
+ else
+ static_call_update(riscv_idle, default_idle);
+}
+
int set_unalign_ctl(struct task_struct *tsk, unsigned int val)
{
if (!unaligned_ctl_available())
--
2.20.1

2024-04-18 11:51:23

by Xu Lu

Subject: [RFC 2/2] riscv: Use Zawrs to accelerate IPI to idle cpu

When sending IPI to a cpu which has entered idle state using Zawrs
extension, there is no need to send a physical software interrupt.
Instead, we can write the IPI information to the address reserved by
target cpu, which will wake it from WRS.NTO. Then the target cpu can
handle the IPI directly without falling into traditional interrupt
handling routine.

Signed-off-by: Xu Lu <[email protected]>
---
arch/riscv/include/asm/processor.h | 14 +++++++
arch/riscv/include/asm/smp.h | 14 +++++++
arch/riscv/kernel/process.c | 65 +++++++++++++++++++++++++++++-
arch/riscv/kernel/smp.c | 39 ++++++++++++------
4 files changed, 118 insertions(+), 14 deletions(-)

diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index 1143367de8c6..76091cf2e8be 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -172,6 +172,20 @@ static inline void wrs_nto(unsigned long *addr)
: : "memory");
}

+static inline void wrs_nto_if(int *addr, int val)
+{
+ int prev;
+
+ __asm__ __volatile__(
+ "lr.w %[p], %[a] \n\t"
+ "bne %[p], %[v], 1f \n\t"
+ ".long 0x00d00073 \n\t"
+ "1: \n\t"
+ : [p] "=&r" (prev), [a] "+A" (*addr)
+ : [v] "r" (val)
+ : "memory");
+}
+
extern phys_addr_t dma32_phys_limit;

struct device_node;
diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index 0d555847cde6..2f27fd743092 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -19,6 +19,20 @@ extern unsigned long boot_cpu_hartid;

#include <linux/jump_label.h>

+enum ipi_message_type {
+ IPI_RESCHEDULE,
+ IPI_CALL_FUNC,
+ IPI_CPU_STOP,
+ IPI_CPU_CRASH_STOP,
+ IPI_IRQ_WORK,
+ IPI_TIMER,
+ IPI_MAX
+};
+
+int ipi_virq_base_get(void);
+
+irqreturn_t handle_IPI(int irq, void *data);
+
/*
* Mapping between linux logical cpu index and hartid.
*/
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index 9f0f7b888bc1..7d6bf780d334 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -16,6 +16,7 @@
#include <linux/ptrace.h>
#include <linux/uaccess.h>
#include <linux/static_call.h>
+#include <linux/hardirq.h>

#include <asm/unistd.h>
#include <asm/processor.h>
@@ -27,6 +28,7 @@
#include <asm/cpuidle.h>
#include <asm/vector.h>
#include <asm/cpufeature.h>
+#include <asm/smp.h>

register unsigned long gp_in_global __asm__("gp");

@@ -38,6 +40,8 @@ EXPORT_SYMBOL(__stack_chk_guard);

extern asmlinkage void ret_from_fork(void);

+DEFINE_PER_CPU(atomic_t, idle_ipi_mask);
+
static __cpuidle void default_idle(void)
{
/*
@@ -49,6 +53,16 @@ static __cpuidle void default_idle(void)
wait_for_interrupt();
}

+static __cpuidle void default_idle_enter(void)
+{
+ /* Do nothing */
+}
+
+static __cpuidle void default_idle_exit(void)
+{
+ /* Do nothing */
+}
+
static __cpuidle void wrs_idle(void)
{
/*
@@ -57,10 +71,42 @@ static __cpuidle void wrs_idle(void)
* to entering WRS.NTO.
*/
mb();
+#ifdef CONFIG_SMP
+ wrs_nto_if(&this_cpu_ptr(&idle_ipi_mask)->counter, BIT(IPI_MAX));
+#else
wrs_nto(&current_thread_info()->flags);
+#endif
+}
+
+static __cpuidle void wrs_idle_enter(void)
+{
+#ifdef CONFIG_SMP
+ atomic_set(this_cpu_ptr(&idle_ipi_mask), BIT(IPI_MAX));
+#endif
+}
+
+static __cpuidle void wrs_idle_exit(void)
+{
+#ifdef CONFIG_SMP
+ int pending;
+ unsigned long flags;
+ enum ipi_message_type ipi;
+
+ local_irq_save(flags);
+ pending = atomic_xchg_relaxed(this_cpu_ptr(&idle_ipi_mask), 0);
+ for (ipi = IPI_RESCHEDULE; ipi < IPI_MAX; ipi++)
+ if (pending & BIT(ipi)) {
+ irq_enter();
+ handle_IPI(ipi_virq_base_get() + ipi, NULL);
+ irq_exit();
+ }
+ local_irq_restore(flags);
+#endif
}

DEFINE_STATIC_CALL_NULL(riscv_idle, default_idle);
+DEFINE_STATIC_CALL_NULL(riscv_idle_enter, default_idle_enter);
+DEFINE_STATIC_CALL_NULL(riscv_idle_exit, default_idle_exit);

void __cpuidle cpu_do_idle(void)
{
@@ -72,13 +118,28 @@ void __cpuidle arch_cpu_idle(void)
cpu_do_idle();
}

+void __cpuidle arch_cpu_idle_enter(void)
+{
+ static_call(riscv_idle_enter)();
+}
+
+void __cpuidle arch_cpu_idle_exit(void)
+{
+ static_call(riscv_idle_exit)();
+}
+
void __init select_idle_routine(void)
{
if (IS_ENABLED(CONFIG_RISCV_ZAWRS_IDLE) &&
- riscv_has_extension_likely(RISCV_ISA_EXT_ZAWRS))
+ riscv_has_extension_likely(RISCV_ISA_EXT_ZAWRS)) {
static_call_update(riscv_idle, wrs_idle);
- else
+ static_call_update(riscv_idle_enter, wrs_idle_enter);
+ static_call_update(riscv_idle_exit, wrs_idle_exit);
+ } else {
static_call_update(riscv_idle, default_idle);
+ static_call_update(riscv_idle_enter, default_idle_enter);
+ static_call_update(riscv_idle_exit, default_idle_exit);
+ }
}

int set_unalign_ctl(struct task_struct *tsk, unsigned int val)
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 45dd4035416e..b5416ee41967 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -26,16 +26,6 @@
#include <asm/cacheflush.h>
#include <asm/cpu_ops.h>

-enum ipi_message_type {
- IPI_RESCHEDULE,
- IPI_CALL_FUNC,
- IPI_CPU_STOP,
- IPI_CPU_CRASH_STOP,
- IPI_IRQ_WORK,
- IPI_TIMER,
- IPI_MAX
-};
-
unsigned long __cpuid_to_hartid_map[NR_CPUS] __ro_after_init = {
[0 ... NR_CPUS-1] = INVALID_HARTID
};
@@ -94,14 +84,34 @@ static inline void ipi_cpu_crash_stop(unsigned int cpu, struct pt_regs *regs)
}
#endif

+#if defined(CONFIG_RISCV_ZAWRS_IDLE) && defined(CONFIG_SMP)
+DECLARE_PER_CPU(atomic_t, idle_ipi_mask);
+#endif
+
static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
{
+#if defined(CONFIG_RISCV_ZAWRS_IDLE) && defined(CONFIG_SMP)
+ int cpu, val;
+
+ for_each_cpu(cpu, mask) {
+ val = atomic_fetch_or_relaxed(BIT(op), per_cpu_ptr(&idle_ipi_mask, cpu));
+ if (likely(!(val & BIT(IPI_MAX))))
+ __ipi_send_mask(ipi_desc[op], cpumask_of(cpu));
+ }
+#else
__ipi_send_mask(ipi_desc[op], mask);
+#endif
}

static void send_ipi_single(int cpu, enum ipi_message_type op)
{
- __ipi_send_mask(ipi_desc[op], cpumask_of(cpu));
+#if defined(CONFIG_RISCV_ZAWRS_IDLE) && defined(CONFIG_SMP)
+ int val;
+
+ val = atomic_fetch_or_relaxed(BIT(op), per_cpu_ptr(&idle_ipi_mask, cpu));
+ if (likely(!(val & BIT(IPI_MAX))))
+#endif
+ __ipi_send_mask(ipi_desc[op], cpumask_of(cpu));
}

#ifdef CONFIG_IRQ_WORK
@@ -111,7 +121,7 @@ void arch_irq_work_raise(void)
}
#endif

-static irqreturn_t handle_IPI(int irq, void *data)
+irqreturn_t handle_IPI(int irq, void *data)
{
int ipi = irq - ipi_virq_base;

@@ -332,3 +342,8 @@ void arch_smp_send_reschedule(int cpu)
send_ipi_single(cpu, IPI_RESCHEDULE);
}
EXPORT_SYMBOL_GPL(arch_smp_send_reschedule);
+
+int ipi_virq_base_get(void)
+{
+ return ipi_virq_base;
+}
--
2.20.1

2024-04-18 12:26:30

by Christoph Müllner

Subject: Re: [RFC 0/2] riscv: Idle thread using Zawrs extension

On Thu, Apr 18, 2024 at 1:50 PM Xu Lu <[email protected]> wrote:
>
> This patch series introduces a new implementation of idle thread using
> Zawrs extension.

This overlaps with the following series:
https://lore.kernel.org/all/[email protected]/

BR
Christoph

>
> The Zawrs[0] extension introduces two new instructions named WRS.STO and
> WRS.NTO in RISC-V. When software registers a reservation set using LR
> instruction, a subsequent WRS.STO or WRS.NTO instruction will cause the
> hart to stall in a low-power state until a store happens to the
> reservation set or an interrupt becomes pending. The difference between
> these two instructions is that WRS.STO will terminate stall after an
> implementation-defined timeout while WRS.NTO won't.
>
> This patch series implements idle thread using WRS.NTO instruction.
> Besides, we found there is no need to send a real IPI to wake up an idle
> CPU. Instead, we write IPI information to the reservation set of an idle
> CPU to wake it up and let it handle IPI quickly, without going through
> tranditional interrupt handling routine.
>
> [0] https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
>
> Xu Lu (2):
> riscv: process: Introduce idle thread using Zawrs extension
> riscv: Use Zawrs to accelerate IPI to idle cpu
>
> arch/riscv/Kconfig | 24 +++++++
> arch/riscv/include/asm/cpuidle.h | 11 +---
> arch/riscv/include/asm/hwcap.h | 1 +
> arch/riscv/include/asm/processor.h | 31 +++++++++
> arch/riscv/include/asm/smp.h | 14 ++++
> arch/riscv/kernel/cpu.c | 5 ++
> arch/riscv/kernel/cpufeature.c | 1 +
> arch/riscv/kernel/process.c | 102 ++++++++++++++++++++++++++++-
> arch/riscv/kernel/smp.c | 39 +++++++----
> 9 files changed, 205 insertions(+), 23 deletions(-)
>
> --
> 2.20.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2024-04-18 12:48:55

by Xu Lu

Subject: Re: [External] Re: [RFC 0/2] riscv: Idle thread using Zawrs extension

On Thu, Apr 18, 2024 at 8:26 PM Christoph Müllner
<[email protected]> wrote:
>
> On Thu, Apr 18, 2024 at 1:50 PM Xu Lu <[email protected]> wrote:
> >
> > This patch series introduces a new implementation of idle thread using
> > Zawrs extension.
>
> This overlaps with the following series:
> https://lore.kernel.org/all/20240315134009.580167-7-ajones@ventanamicrocom/

Hi Christoph.
Thanks for your reply!
Actually our patch series is different from this. The work from your
link focuses on providing support for Zawrs and implementing spinlock
using it, while our work focuses on implementing idle thread using
Zawrs and accelerating IPI to idle cpu. Of course, the ISA ZAWRS
config part can be merged. We will refine our code in the next version
to reduce code conflicts.

>
> BR
> Christoph
>
> >
> > The Zawrs[0] extension introduces two new instructions named WRS.STO and
> > WRS.NTO in RISC-V. When software registers a reservation set using LR
> > instruction, a subsequent WRS.STO or WRS.NTO instruction will cause the
> > hart to stall in a low-power state until a store happens to the
> > reservation set or an interrupt becomes pending. The difference between
> > these two instructions is that WRS.STO will terminate stall after an
> > implementation-defined timeout while WRS.NTO won't.
> >
> > This patch series implements idle thread using WRS.NTO instruction.
> > Besides, we found there is no need to send a real IPI to wake up an idle
> > CPU. Instead, we write IPI information to the reservation set of an idle
> > CPU to wake it up and let it handle IPI quickly, without going through
> > tranditional interrupt handling routine.
> >
> > [0] https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> >
> > Xu Lu (2):
> > riscv: process: Introduce idle thread using Zawrs extension
> > riscv: Use Zawrs to accelerate IPI to idle cpu
> >
> > arch/riscv/Kconfig | 24 +++++++
> > arch/riscv/include/asm/cpuidle.h | 11 +---
> > arch/riscv/include/asm/hwcap.h | 1 +
> > arch/riscv/include/asm/processor.h | 31 +++++++++
> > arch/riscv/include/asm/smp.h | 14 ++++
> > arch/riscv/kernel/cpu.c | 5 ++
> > arch/riscv/kernel/cpufeature.c | 1 +
> > arch/riscv/kernel/process.c | 102 ++++++++++++++++++++++++++++-
> > arch/riscv/kernel/smp.c | 39 +++++++----
> > 9 files changed, 205 insertions(+), 23 deletions(-)
> >
> > --
> > 2.20.1
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

2024-04-18 12:56:38

by Christoph Müllner

Subject: Re: [External] Re: [RFC 0/2] riscv: Idle thread using Zawrs extension

On Thu, Apr 18, 2024 at 2:44 PM Xu Lu <[email protected]> wrote:
>
> On Thu, Apr 18, 2024 at 8:26 PM Christoph Müllner
> <[email protected]> wrote:
> >
> > On Thu, Apr 18, 2024 at 1:50 PM Xu Lu <[email protected]> wrote:
> > >
> > > This patch series introduces a new implementation of idle thread using
> > > Zawrs extension.
> >
> > This overlaps with the following series:
> > https://lore.kernel.org/all/[email protected]/
>
> Hi Christoph.
> Thanks for your reply!
> Actually our patch series is different from this. The work from your
> link focuses on providing support for Zawrs and implementing spinlock
> using it, while our work focuses on implementing idle thread using
> Zawrs and accelerating IPI to idle cpu. Of course, the ISA ZAWRS
> config part can be merged. We will refine our code in the next version
> to reduce code conflicts.

Yes, I've seen that this targets another optimization, but the basic
Zawrs support
would be identical to the other patchset (even if it is not).
I would propose that we work on a basic Zawrs support patchset that introduces
the Kconfig, DTS and hwprobe parts (a subset of Andrew's patchset).
Once this is merged, all other optimizations can be built upon it
(spinlocks, idle thread, glibc CPU spinning).
If this proposal is fine for the maintainers/reviewers, then Andrew could resend
these basic-support patches.

BR
Christoph


>
> >
> > BR
> > Christoph
> >
> > >
> > > The Zawrs[0] extension introduces two new instructions named WRS.STO and
> > > WRS.NTO in RISC-V. When software registers a reservation set using LR
> > > instruction, a subsequent WRS.STO or WRS.NTO instruction will cause the
> > > hart to stall in a low-power state until a store happens to the
> > > reservation set or an interrupt becomes pending. The difference between
> > > these two instructions is that WRS.STO will terminate stall after an
> > > implementation-defined timeout while WRS.NTO won't.
> > >
> > > This patch series implements idle thread using WRS.NTO instruction.
> > > Besides, we found there is no need to send a real IPI to wake up an idle
> > > CPU. Instead, we write IPI information to the reservation set of an idle
> > > CPU to wake it up and let it handle IPI quickly, without going through
> > > tranditional interrupt handling routine.
> > >
> > > [0] https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> > >
> > > Xu Lu (2):
> > > riscv: process: Introduce idle thread using Zawrs extension
> > > riscv: Use Zawrs to accelerate IPI to idle cpu
> > >
> > > arch/riscv/Kconfig | 24 +++++++
> > > arch/riscv/include/asm/cpuidle.h | 11 +---
> > > arch/riscv/include/asm/hwcap.h | 1 +
> > > arch/riscv/include/asm/processor.h | 31 +++++++++
> > > arch/riscv/include/asm/smp.h | 14 ++++
> > > arch/riscv/kernel/cpu.c | 5 ++
> > > arch/riscv/kernel/cpufeature.c | 1 +
> > > arch/riscv/kernel/process.c | 102 ++++++++++++++++++++++++++++-
> > > arch/riscv/kernel/smp.c | 39 +++++++----
> > > 9 files changed, 205 insertions(+), 23 deletions(-)
> > >
> > > --
> > > 2.20.1
> > >
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > [email protected]
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv

2024-04-18 13:12:38

by Xu Lu

Subject: Re: [External] Re: [RFC 0/2] riscv: Idle thread using Zawrs extension

On Thu, Apr 18, 2024 at 8:56 PM Christoph Müllner
<[email protected]> wrote:
>
> On Thu, Apr 18, 2024 at 2:44 PM Xu Lu <[email protected]> wrote:
> >
> > On Thu, Apr 18, 2024 at 8:26 PM Christoph Müllner
> > <[email protected]> wrote:
> > >
> > > On Thu, Apr 18, 2024 at 1:50 PM Xu Lu <[email protected]> wrote:
> > > >
> > > > This patch series introduces a new implementation of idle thread using
> > > > Zawrs extension.
> > >
> > > This overlaps with the following series:
> > > https://lore.kernel.org/all/[email protected]/
> >
> > Hi Christoph.
> > Thanks for your reply!
> > Actually our patch series is different from this. The work from your
> > link focuses on providing support for Zawrs and implementing spinlock
> > using it, while our work focuses on implementing idle thread using
> > Zawrs and accelerating IPI to idle cpu. Of course, the ISA ZAWRS
> > config part can be merged. We will refine our code in the next version
> > to reduce code conflicts.
>
> Yes, I've seen that this targets another optimization, but the basic
> Zawrs support
> would be identical to the other patchset (even if it is not).
> I would propose that we work on a basic Zawrs support patchset that introduces
> the Kconfig, DTS and hwprobe parts (a subset of Andrew's patchset).
> Once this is merged, all other optimizations can be built upon it
> (spinlocks, idle thread, glibc CPU spinning).
> If this proposal is fine for the maintainers/reviewers, then Andrew could resend
> these basic-support patches.
>
> BR
> Christoph

Roger that! This does make more sense. We will rebase our code on
Andrew's basic support patches in the next version.

Regards,
Xu Lu

>
>
> >
> > >
> > > BR
> > > Christoph
> > >
> > > >
> > > > The Zawrs[0] extension introduces two new instructions named WRS.STO and
> > > > WRS.NTO in RISC-V. When software registers a reservation set using LR
> > > > instruction, a subsequent WRS.STO or WRS.NTO instruction will cause the
> > > > hart to stall in a low-power state until a store happens to the
> > > > reservation set or an interrupt becomes pending. The difference between
> > > > these two instructions is that WRS.STO will terminate stall after an
> > > > implementation-defined timeout while WRS.NTO won't.
> > > >
> > > > This patch series implements idle thread using WRS.NTO instruction.
> > > > Besides, we found there is no need to send a real IPI to wake up an idle
> > > > CPU. Instead, we write IPI information to the reservation set of an idle
> > > > CPU to wake it up and let it handle IPI quickly, without going through
> > > > tranditional interrupt handling routine.
> > > >
> > > > [0] https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> > > >
> > > > Xu Lu (2):
> > > > riscv: process: Introduce idle thread using Zawrs extension
> > > > riscv: Use Zawrs to accelerate IPI to idle cpu
> > > >
> > > > arch/riscv/Kconfig | 24 +++++++
> > > > arch/riscv/include/asm/cpuidle.h | 11 +---
> > > > arch/riscv/include/asm/hwcap.h | 1 +
> > > > arch/riscv/include/asm/processor.h | 31 +++++++++
> > > > arch/riscv/include/asm/smp.h | 14 ++++
> > > > arch/riscv/kernel/cpu.c | 5 ++
> > > > arch/riscv/kernel/cpufeature.c | 1 +
> > > > arch/riscv/kernel/process.c | 102 ++++++++++++++++++++++++++++-
> > > > arch/riscv/kernel/smp.c | 39 +++++++----
> > > > 9 files changed, 205 insertions(+), 23 deletions(-)
> > > >
> > > > --
> > > > 2.20.1
> > > >
> > > >
> > > > _______________________________________________
> > > > linux-riscv mailing list
> > > > [email protected]
> > > > http://lists.infradead.org/mailman/listinfo/linux-riscv

2024-04-18 14:08:42

by Conor Dooley

Subject: Re: [External] Re: [RFC 0/2] riscv: Idle thread using Zawrs extension

On Thu, Apr 18, 2024 at 09:09:06PM +0800, Xu Lu wrote:
> On Thu, Apr 18, 2024 at 8:56 PM Christoph Müllner
> <[email protected]> wrote:
> >
> > On Thu, Apr 18, 2024 at 2:44 PM Xu Lu <[email protected]> wrote:
> > >
> > > On Thu, Apr 18, 2024 at 8:26 PM Christoph Müllner
> > > <[email protected]> wrote:
> > > >
> > > > On Thu, Apr 18, 2024 at 1:50 PM Xu Lu <[email protected]> wrote:
> > > > >
> > > > > This patch series introduces a new implementation of idle thread using
> > > > > Zawrs extension.
> > > >
> > > > This overlaps with the following series:
> > > > https://lore.kernel.org/all/[email protected]/
> > >
> > > Hi Christoph.
> > > Thanks for your reply!
> > > Actually our patch series is different from this. The work from your
> > > link focuses on providing support for Zawrs and implementing spinlock
> > > using it, while our work focuses on implementing idle thread using
> > > Zawrs and accelerating IPI to idle cpu. Of course, the ISA ZAWRS
> > > config part can be merged. We will refine our code in the next version
> > > to reduce code conflicts.
> >
> > Yes, I've seen that this targets another optimization, but the basic
> > Zawrs support
> > would be identical to the other patchset (even if it is not).
> > I would propose that we work on a basic Zawrs support patchset that introduces
> > the Kconfig, DTS and hwprobe parts (a subset of Andrew's patchset).
> > Once this is merged, all other optimizations can be built upon it
> > (spinlocks, idle thread, glibc CPU spinning).
> > If this proposal is fine for the maintainers/reviewers, then Andrew could resend
> > these basic-support patches.
> >
> > BR
> > Christoph
>
> Roger that! This does make more sense. We will rebase our code on
> Andrew's basic support patches in the next version.

IIRC Drew's working on a new version of the linked series (we were
talking about it yesterday) so hold off for that before doing a rebase
and sending a new version I think.

Thanks,
Conor.

Attachments:

2024-04-18 14:20:05

by Andrew Jones

Subject: Re: [External] Re: [RFC 0/2] riscv: Idle thread using Zawrs extension

On Thu, Apr 18, 2024 at 09:09:06PM +0800, Xu Lu wrote:
> On Thu, Apr 18, 2024 at 8:56 PM Christoph Müllner
> <[email protected]> wrote:
> >
> > On Thu, Apr 18, 2024 at 2:44 PM Xu Lu <[email protected]> wrote:
> > >
> > > On Thu, Apr 18, 2024 at 8:26 PM Christoph Müllner
> > > <[email protected]> wrote:
> > > >
> > > > On Thu, Apr 18, 2024 at 1:50 PM Xu Lu <[email protected]> wrote:
> > > > >
> > > > > This patch series introduces a new implementation of idle thread using
> > > > > Zawrs extension.
> > > >
> > > > This overlaps with the following series:
> > > > https://lore.kernel.org/all/[email protected]/
> > >
> > > Hi Christoph.
> > > Thanks for your reply!
> > > Actually our patch series is different from this. The work from your
> > > link focuses on providing support for Zawrs and implementing spinlock
> > > using it, while our work focuses on implementing idle thread using
> > > Zawrs and accelerating IPI to idle cpu. Of course, the ISA ZAWRS
> > > config part can be merged. We will refine our code in the next version
> > > to reduce code conflicts.
> >
> > Yes, I've seen that this targets another optimization, but the basic
> > Zawrs support
> > would be identical to the other patchset (even if it is not).
> > I would propose that we work on a basic Zawrs support patchset that introduces
> > the Kconfig, DTS and hwprobe parts (a subset of Andrew's patchset).
> > Once this is merged, all other optimizations can be built upon it
> > (spinlocks, idle thread, glibc CPU spinning).
> > If this proposal is fine for the maintainers/reviewers, then Andrew could resend
> > these basic-support patches.
> >
> > BR
> > Christoph
>
> Roger that! This does make more sense. We will rebase our code on
> Andrew's basic support patches in the next version.

And I'm just about to send that next version. I'll send tomorrow morning
if not yet today.

Thanks,
drew



>
> Regards,
> Xu Lu
>
> >
> >
> > >
> > > >
> > > > BR
> > > > Christoph
> > > >
> > > > >
> > > > > The Zawrs[0] extension introduces two new instructions named WRS.STO and
> > > > > WRS.NTO in RISC-V. When software registers a reservation set using LR
> > > > > instruction, a subsequent WRS.STO or WRS.NTO instruction will cause the
> > > > > hart to stall in a low-power state until a store happens to the
> > > > > reservation set or an interrupt becomes pending. The difference between
> > > > > these two instructions is that WRS.STO will terminate stall after an
> > > > > implementation-defined timeout while WRS.NTO won't.
> > > > >
> > > > > This patch series implements idle thread using WRS.NTO instruction.
> > > > > Besides, we found there is no need to send a real IPI to wake up an idle
> > > > > CPU. Instead, we write IPI information to the reservation set of an idle
> > > > > CPU to wake it up and let it handle IPI quickly, without going through
> > > > > tranditional interrupt handling routine.
> > > > >
> > > > > [0] https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> > > > >
> > > > > Xu Lu (2):
> > > > > riscv: process: Introduce idle thread using Zawrs extension
> > > > > riscv: Use Zawrs to accelerate IPI to idle cpu
> > > > >
> > > > > arch/riscv/Kconfig | 24 +++++++
> > > > > arch/riscv/include/asm/cpuidle.h | 11 +---
> > > > > arch/riscv/include/asm/hwcap.h | 1 +
> > > > > arch/riscv/include/asm/processor.h | 31 +++++++++
> > > > > arch/riscv/include/asm/smp.h | 14 ++++
> > > > > arch/riscv/kernel/cpu.c | 5 ++
> > > > > arch/riscv/kernel/cpufeature.c | 1 +
> > > > > arch/riscv/kernel/process.c | 102 ++++++++++++++++++++++++++++-
> > > > > arch/riscv/kernel/smp.c | 39 +++++++----
> > > > > 9 files changed, 205 insertions(+), 23 deletions(-)
> > > > >
> > > > > --
> > > > > 2.20.1
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > linux-riscv mailing list
> > > > > [email protected]
> > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv

2024-04-18 15:06:08

by Conor Dooley

Subject: Re: [RFC 1/2] riscv: process: Introduce idle thread using Zawrs extension

+ Drew,

On Thu, Apr 18, 2024 at 07:49:41PM +0800, Xu Lu wrote:
> The Zawrs extension introduces a new instruction WRS.NTO, which will
> register a reservation set and causes the hart to temporarily stall
> execution in a low-power state until a store occurs to the reservation
> set or an interrupt is observed.
>
> This commit implements new version of idle thread for RISC-V via Zawrs
> extension.
>
> Signed-off-by: Xu Lu <[email protected]>
> Reviewed-by: Hangjing Li <[email protected]>
> Reviewed-by: Liang Deng <[email protected]>
> Reviewed-by: Wen Chai <[email protected]>
> ---
> arch/riscv/Kconfig | 24 +++++++++++++++++
> arch/riscv/include/asm/cpuidle.h | 11 +-------
> arch/riscv/include/asm/hwcap.h | 1 +
> arch/riscv/include/asm/processor.h | 17 +++++++++++++
> arch/riscv/kernel/cpu.c | 5 ++++
> arch/riscv/kernel/cpufeature.c | 1 +
> arch/riscv/kernel/process.c | 41 +++++++++++++++++++++++++++++-
> 7 files changed, 89 insertions(+), 11 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index be09c8836d56..a0d344e9803f 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -19,6 +19,7 @@ config RISCV
> select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
> select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
> select ARCH_HAS_BINFMT_FLAT
> + select ARCH_HAS_CPU_FINALIZE_INIT
> select ARCH_HAS_CURRENT_STACK_POINTER
> select ARCH_HAS_DEBUG_VIRTUAL if MMU
> select ARCH_HAS_DEBUG_VM_PGTABLE
> @@ -525,6 +526,20 @@ config RISCV_ISA_SVPBMT
>
> If you don't know what to do here, say Y.
>
> +config RISCV_ISA_ZAWRS
> + bool "Zawrs extension support for wait-on-reservation-set instructions"
> + depends on RISCV_ALTERNATIVE
> + default y
> + help
> + Adds support to dynamically detect the presence of the Zawrs
> + extension and enable its usage.

Drew, could you, in your update, use the wording:
Add support for enabling optimisations in the kernel when the
Zawrs extension is detected at boot.

There was some confusion recently about what these options were actually
for, because this option doesn't control "dynamic detection" as the
ACPI or DT detection is compiled at all times. I had written a patch for
this wording in other options at the time but had forgotten to properly
send it:
https://lore.kernel.org/linux-riscv/20240418-stable-railway-7cce07e1e440@spud/T/#u

> +
> + The Zawrs extension defines a pair of instructions to be used
> + in polling loops that allows a core to enter a low-power state
> + and wait on a store to a memory location.
> +
> + If you don't know what to do here, say Y.
> +
> config TOOLCHAIN_HAS_V
> bool
> default y
> @@ -1075,6 +1090,15 @@ endmenu # "Power management options"
>
> menu "CPU Power Management"
>
> +config RISCV_ZAWRS_IDLE
> + bool "Idle thread using ZAWRS extensions"
> + depends on RISCV_ISA_ZAWRS
> + default y
> + help
> + Adds support to implement idle thread using ZAWRS extension.
> +
> + If you don't know what to do here, say Y.

I don't think this second option is needed, why would we not always want
to use the Zawrs version of this when it is available? Can we do it
unconditionally when RISCV_ISA_ZAWRS is set and the extension is
detected at runtime?

Cheers,
Conor.

Attachments:

2024-04-18 16:13:27

by Xu Lu

Subject: Re: [External] Re: [RFC 1/2] riscv: process: Introduce idle thread using Zawrs extension

On Thu, Apr 18, 2024 at 11:06 PM Conor Dooley <[email protected]> wrote:
>
> + Drew,
>
> On Thu, Apr 18, 2024 at 07:49:41PM +0800, Xu Lu wrote:
> > The Zawrs extension introduces a new instruction WRS.NTO, which will
> > register a reservation set and causes the hart to temporarily stall
> > execution in a low-power state until a store occurs to the reservation
> > set or an interrupt is observed.
> >
> > This commit implements new version of idle thread for RISC-V via Zawrs
> > extension.
> >
> > Signed-off-by: Xu Lu <[email protected]>
> > Reviewed-by: Hangjing Li <[email protected]>
> > Reviewed-by: Liang Deng <[email protected]>
> > Reviewed-by: Wen Chai <[email protected]>
> > ---
> > arch/riscv/Kconfig | 24 +++++++++++++++++
> > arch/riscv/include/asm/cpuidle.h | 11 +-------
> > arch/riscv/include/asm/hwcap.h | 1 +
> > arch/riscv/include/asm/processor.h | 17 +++++++++++++
> > arch/riscv/kernel/cpu.c | 5 ++++
> > arch/riscv/kernel/cpufeature.c | 1 +
> > arch/riscv/kernel/process.c | 41 +++++++++++++++++++++++++++++-
> > 7 files changed, 89 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index be09c8836d56..a0d344e9803f 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -19,6 +19,7 @@ config RISCV
> > select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
> > select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
> > select ARCH_HAS_BINFMT_FLAT
> > + select ARCH_HAS_CPU_FINALIZE_INIT
> > select ARCH_HAS_CURRENT_STACK_POINTER
> > select ARCH_HAS_DEBUG_VIRTUAL if MMU
> > select ARCH_HAS_DEBUG_VM_PGTABLE
> > @@ -525,6 +526,20 @@ config RISCV_ISA_SVPBMT
> >
> > If you don't know what to do here, say Y.
> >
> > +config RISCV_ISA_ZAWRS
> > + bool "Zawrs extension support for wait-on-reservation-set instructions"
> > + depends on RISCV_ALTERNATIVE
> > + default y
> > + help
> > + Adds support to dynamically detect the presence of the Zawrs
> > + extension and enable its usage.
>
> Drew, could you, in your update, use the wording:
> Add support for enabling optimisations in the kernel when the
> Zawrs extension is detected at boot.
>
> There was some confusion recently about what these options were actually
> for, because this option doesn't control "dynamic detection" as the
> ACPI or DT detection is compiled at all times. I had written a patch for
> this wording in other options at the time but had forgotten to properly
> send it:
> https://lore.kernel.org/linux-riscv/20240418-stable-railway-7cce07e1e440@spud/T/#u
>
> > +
> > + The Zawrs extension defines a pair of instructions to be used
> > + in polling loops that allows a core to enter a low-power state
> > + and wait on a store to a memory location.
> > +
> > + If you don't know what to do here, say Y.
> > +
> > config TOOLCHAIN_HAS_V
> > bool
> > default y
> > @@ -1075,6 +1090,15 @@ endmenu # "Power management options"
> >
> > menu "CPU Power Management"
> >
> > +config RISCV_ZAWRS_IDLE
> > + bool "Idle thread using ZAWRS extensions"
> > + depends on RISCV_ISA_ZAWRS
> > + default y
> > + help
> > + Adds support to implement idle thread using ZAWRS extension.
> > +
> > + If you don't know what to do here, say Y.
>
> I don't think this second option is needed, why would we not always want
> to use the Zawrs version of this when it is available? Can we do it
> unconditionally when RISCV_ISA_ZAWRS is set and the extension is
> detected at runtime?
>
> Cheers,
> Conor.

Indeed, we can always choose WRS.NTO when entering idle.

This config is introduced for the second commit in this patch series.
In the second commit, we detect whether the target cpu is idle when
sending IPI and write IPI info to the reserve set of idle cpu so as to
avoid sending a physical IPI. Besides, the target idle cpu need not to
go through traditional interrupt handling routine. However, if all
cpus are busy and hardly enter idle, this commit may introduce
performance overhead of extra instructions when sending IPI. Thus we
introduce this config just in case.

Regards,
Xu Lu

>
>

2024-04-18 19:11:13

by Andrew Jones

Subject: Re: [RFC 1/2] riscv: process: Introduce idle thread using Zawrs extension

On Thu, Apr 18, 2024 at 04:05:55PM +0100, Conor Dooley wrote:
> + Drew,
>
> On Thu, Apr 18, 2024 at 07:49:41PM +0800, Xu Lu wrote:
> > The Zawrs extension introduces a new instruction WRS.NTO, which will
> > register a reservation set and causes the hart to temporarily stall
> > execution in a low-power state until a store occurs to the reservation
> > set or an interrupt is observed.
> >
> > This commit implements new version of idle thread for RISC-V via Zawrs
> > extension.
> >
> > Signed-off-by: Xu Lu <[email protected]>
> > Reviewed-by: Hangjing Li <[email protected]>
> > Reviewed-by: Liang Deng <[email protected]>
> > Reviewed-by: Wen Chai <[email protected]>
> > ---
> > arch/riscv/Kconfig | 24 +++++++++++++++++
> > arch/riscv/include/asm/cpuidle.h | 11 +-------
> > arch/riscv/include/asm/hwcap.h | 1 +
> > arch/riscv/include/asm/processor.h | 17 +++++++++++++
> > arch/riscv/kernel/cpu.c | 5 ++++
> > arch/riscv/kernel/cpufeature.c | 1 +
> > arch/riscv/kernel/process.c | 41 +++++++++++++++++++++++++++++-
> > 7 files changed, 89 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index be09c8836d56..a0d344e9803f 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -19,6 +19,7 @@ config RISCV
> > select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
> > select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
> > select ARCH_HAS_BINFMT_FLAT
> > + select ARCH_HAS_CPU_FINALIZE_INIT
> > select ARCH_HAS_CURRENT_STACK_POINTER
> > select ARCH_HAS_DEBUG_VIRTUAL if MMU
> > select ARCH_HAS_DEBUG_VM_PGTABLE
> > @@ -525,6 +526,20 @@ config RISCV_ISA_SVPBMT
> >
> > If you don't know what to do here, say Y.
> >
> > +config RISCV_ISA_ZAWRS
> > + bool "Zawrs extension support for wait-on-reservation-set instructions"
> > + depends on RISCV_ALTERNATIVE
> > + default y
> > + help
> > + Adds support to dynamically detect the presence of the Zawrs
> > + extension and enable its usage.
>
> Drew, could you, in your update, use the wording:
> Add support for enabling optimisations in the kernel when the
> Zawrs extension is detected at boot.

How about

The Zawrs extension defines a pair of instructions to be used in
polling loops which allow a hart to enter a low-power state or to
trap to the hypervisor while waiting on a store to a memory location.
Enable the use of these instructions when the Zawrs extension is
detected at boot.

Thanks,
drew

2024-04-18 22:00:55

by Samuel Holland

Subject: Re: [RFC 1/2] riscv: process: Introduce idle thread using Zawrs extension

Hi Drew,

On 2024-04-18 2:10 PM, Andrew Jones wrote:
> On Thu, Apr 18, 2024 at 04:05:55PM +0100, Conor Dooley wrote:
>> + Drew,
>>
>> On Thu, Apr 18, 2024 at 07:49:41PM +0800, Xu Lu wrote:
>>> The Zawrs extension introduces a new instruction WRS.NTO, which will
>>> register a reservation set and causes the hart to temporarily stall
>>> execution in a low-power state until a store occurs to the reservation
>>> set or an interrupt is observed.
>>>
>>> This commit implements new version of idle thread for RISC-V via Zawrs
>>> extension.
>>>
>>> Signed-off-by: Xu Lu <[email protected]>
>>> Reviewed-by: Hangjing Li <[email protected]>
>>> Reviewed-by: Liang Deng <[email protected]>
>>> Reviewed-by: Wen Chai <[email protected]>
>>> ---
>>> arch/riscv/Kconfig | 24 +++++++++++++++++
>>> arch/riscv/include/asm/cpuidle.h | 11 +-------
>>> arch/riscv/include/asm/hwcap.h | 1 +
>>> arch/riscv/include/asm/processor.h | 17 +++++++++++++
>>> arch/riscv/kernel/cpu.c | 5 ++++
>>> arch/riscv/kernel/cpufeature.c | 1 +
>>> arch/riscv/kernel/process.c | 41 +++++++++++++++++++++++++++++-
>>> 7 files changed, 89 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>>> index be09c8836d56..a0d344e9803f 100644
>>> --- a/arch/riscv/Kconfig
>>> +++ b/arch/riscv/Kconfig
>>> @@ -19,6 +19,7 @@ config RISCV
>>> select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
>>> select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
>>> select ARCH_HAS_BINFMT_FLAT
>>> + select ARCH_HAS_CPU_FINALIZE_INIT
>>> select ARCH_HAS_CURRENT_STACK_POINTER
>>> select ARCH_HAS_DEBUG_VIRTUAL if MMU
>>> select ARCH_HAS_DEBUG_VM_PGTABLE
>>> @@ -525,6 +526,20 @@ config RISCV_ISA_SVPBMT
>>>
>>> If you don't know what to do here, say Y.
>>>
>>> +config RISCV_ISA_ZAWRS
>>> + bool "Zawrs extension support for wait-on-reservation-set instructions"
>>> + depends on RISCV_ALTERNATIVE
>>> + default y
>>> + help
>>> + Adds support to dynamically detect the presence of the Zawrs
>>> + extension and enable its usage.
>>
>> Drew, could you, in your update, use the wording:
>> Add support for enabling optimisations in the kernel when the
>> Zawrs extension is detected at boot.
>
> How about
>
> The Zawrs extension defines a pair of instructions to be used in
> polling loops which allow a hart to enter a low-power state or to
> trap to the hypervisor while waiting on a store to a memory location.
> Enable the use of these instructions when the Zawrs extension is

^ in the kernel

I believe "in the kernel" was an important part of the clarification that these
Kconfig options do not affect whether userspace can use these instructions.

Regards,
Samuel

> detected at boot.
>
> Thanks,
> drew
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2024-04-18 22:10:09

by Conor Dooley

Subject: Re: [RFC 1/2] riscv: process: Introduce idle thread using Zawrs extension

On Thu, Apr 18, 2024 at 05:00:42PM -0500, Samuel Holland wrote:
> Hi Drew,
>
> On 2024-04-18 2:10 PM, Andrew Jones wrote:
> > On Thu, Apr 18, 2024 at 04:05:55PM +0100, Conor Dooley wrote:
> >> + Drew,
> >>
> >> On Thu, Apr 18, 2024 at 07:49:41PM +0800, Xu Lu wrote:
> >>> The Zawrs extension introduces a new instruction WRS.NTO, which will
> >>> register a reservation set and causes the hart to temporarily stall
> >>> execution in a low-power state until a store occurs to the reservation
> >>> set or an interrupt is observed.
> >>>
> >>> This commit implements new version of idle thread for RISC-V via Zawrs
> >>> extension.
> >>>
> >>> Signed-off-by: Xu Lu <[email protected]>
> >>> Reviewed-by: Hangjing Li <[email protected]>
> >>> Reviewed-by: Liang Deng <[email protected]>
> >>> Reviewed-by: Wen Chai <[email protected]>
> >>> ---
> >>> arch/riscv/Kconfig | 24 +++++++++++++++++
> >>> arch/riscv/include/asm/cpuidle.h | 11 +-------
> >>> arch/riscv/include/asm/hwcap.h | 1 +
> >>> arch/riscv/include/asm/processor.h | 17 +++++++++++++
> >>> arch/riscv/kernel/cpu.c | 5 ++++
> >>> arch/riscv/kernel/cpufeature.c | 1 +
> >>> arch/riscv/kernel/process.c | 41 +++++++++++++++++++++++++++++-
> >>> 7 files changed, 89 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> >>> index be09c8836d56..a0d344e9803f 100644
> >>> --- a/arch/riscv/Kconfig
> >>> +++ b/arch/riscv/Kconfig
> >>> @@ -19,6 +19,7 @@ config RISCV
> >>> select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
> >>> select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
> >>> select ARCH_HAS_BINFMT_FLAT
> >>> + select ARCH_HAS_CPU_FINALIZE_INIT
> >>> select ARCH_HAS_CURRENT_STACK_POINTER
> >>> select ARCH_HAS_DEBUG_VIRTUAL if MMU
> >>> select ARCH_HAS_DEBUG_VM_PGTABLE
> >>> @@ -525,6 +526,20 @@ config RISCV_ISA_SVPBMT
> >>>
> >>> If you don't know what to do here, say Y.
> >>>
> >>> +config RISCV_ISA_ZAWRS
> >>> + bool "Zawrs extension support for wait-on-reservation-set instructions"
> >>> + depends on RISCV_ALTERNATIVE
> >>> + default y
> >>> + help
> >>> + Adds support to dynamically detect the presence of the Zawrs
> >>> + extension and enable its usage.
> >>
> >> Drew, could you, in your update, use the wording:
> >> Add support for enabling optimisations in the kernel when the
> >> Zawrs extension is detected at boot.
> >
> > How about

Probably should have said, this was just a replacement for the first
paragraph, not the entire text.

> >
> > The Zawrs extension defines a pair of instructions to be used in
> > polling loops which allow a hart to enter a low-power state or to
> > trap to the hypervisor while waiting on a store to a memory location.
> > Enable the use of these instructions when the Zawrs extension is
>
> ^ in the kernel
>
> I believe "in the kernel" was an important part of the clarification that these
> Kconfig options do not affect whether userspace can use these instructions.

Meant to reply earlier but forgot. Samuel's correct, it is indeed the
key bit I wanted, I just suggest what's above to match what was in the
patch I had sent earlier today. Don't really care all that much if it
is a match nor not, but I do care about the help text actually
describing /who/ gets to use the extension when the option is enabled.

Thanks,
Conor.

Attachments:

2024-04-22 08:22:45

by Conor Dooley

Subject: Re: [External] Re: [RFC 1/2] riscv: process: Introduce idle thread using Zawrs extension

On Fri, Apr 19, 2024 at 12:14:47AM +0800, Xu Lu wrote:
> On Thu, Apr 18, 2024 at 11:06 PM Conor Dooley <[email protected]> wrote:
> > On Thu, Apr 18, 2024 at 07:49:41PM +0800, Xu Lu wrote:

> > > + The Zawrs extension defines a pair of instructions to be used
> > > + in polling loops that allows a core to enter a low-power state
> > > + and wait on a store to a memory location.
> > > +
> > > + If you don't know what to do here, say Y.
> > > +
> > > config TOOLCHAIN_HAS_V
> > > bool
> > > default y
> > > @@ -1075,6 +1090,15 @@ endmenu # "Power management options"
> > >
> > > menu "CPU Power Management"
> > >
> > > +config RISCV_ZAWRS_IDLE
> > > + bool "Idle thread using ZAWRS extensions"
> > > + depends on RISCV_ISA_ZAWRS
> > > + default y
> > > + help
> > > + Adds support to implement idle thread using ZAWRS extension.
> > > +
> > > + If you don't know what to do here, say Y.
> >
> > I don't think this second option is needed, why would we not always want
> > to use the Zawrs version of this when it is available? Can we do it
> > unconditionally when RISCV_ISA_ZAWRS is set and the extension is
> > detected at runtime?
> >
> > Cheers,
> > Conor.
>
> Indeed, we can always choose WRS.NTO when entering idle.
>
> This config is introduced for the second commit in this patch series.
> In the second commit, we detect whether the target cpu is idle when
> sending IPI and write IPI info to the reserve set of idle cpu so as to
> avoid sending a physical IPI. Besides, the target idle cpu need not to
> go through traditional interrupt handling routine. However, if all
> cpus are busy and hardly enter idle, this commit may introduce
> performance overhead of extra instructions when sending IPI. Thus we
> introduce this config just in case.

Could you add the downsides into the help text of the config option so
that people can understand why to enable/disable the option?

Thanks,
Conor.

Attachments: