Re: [RFC PATCH 2/2] Revert "csky: Add support for restartable sequence"

[Guo Ren <guoren@kernel.org>]

 Hi Mathieu,

Sorry for forgetting to CC you in the last patch, and that patch has
been merged into master which has the problem of syscall restart.

I still want to keep rseq feature for csky, and implement the
RSEQ_SKIP_FASTPATH for self-test, it that okay?

diff --git a/tools/testing/selftests/rseq/param_test.c
b/tools/testing/selftests/rseq/param_test.c
index 699ad5f93c34..1e67b212ad98 100644
--- a/tools/testing/selftests/rseq/param_test.c
+++ b/tools/testing/selftests/rseq/param_test.c
@@ -208,6 +208,8 @@ unsigned int yield_mod_cnt, nr_abort;
        "bnez " INJECT_ASM_REG ", 222b\n\t" \
        "333:\n\t"

+#elif defined(__csky__)
+#define RSEQ_SKIP_FASTPATH
 #else
 #error unsupported target
 #endif

diff --git a/tools/testing/selftests/rseq/rseq.h
b/tools/testing/selftests/rseq/rseq.h
index 3f63eb362b92..c3dce298b36c 100644
--- a/tools/testing/selftests/rseq/rseq.h
+++ b/tools/testing/selftests/rseq/rseq.h
@@ -79,6 +79,8 @@ extern int __rseq_handled;
 #include <rseq-mips.h>
 #elif defined(__s390__)
 #include <rseq-s390.h>
+#elif defined(__csky__)
+#include <rseq-csky.h>
 #else
 #error unsupported target
 #endif

diff --git a/tools/testing/selftests/rseq/rseq-csky.h
b/tools/testing/selftests/rseq/rseq-csky.h
new file mode 100644
index 000000000000..ecad40e9aeda
--- /dev/null
+++ b/tools/testing/selftests/rseq/rseq-csky.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: LGPL-2.1 OR MIT */
+
+#if __ORDER_LITTLE_ENDIAN__ == 1234
+#define RSEQ_SIG       0xc1e6643f  /* mtcr    r6, cr<31, 15> */
+#else
+#error "Currently, RSEQ only supports Little-Endian version"
+#endif
+
+/*
+ * bar.brwarws: ordering barrier for all load/store instructions
+ *              before/after
+ *
+ * |31|30 26|25 21|20 16|15  10|9   5|4           0|
+ *  1  10000 00000 00000 100001        00001 0 bw br aw ar
+ *
+ * b: before
+ * a: after
+ * r: read
+ * w: write
+ *
+ * Here are all combinations:
+ *
+ * bar.brw
+ * bar.br
+ * bar.bw
+ * bar.arw
+ * bar.ar
+ * bar.aw
+ * bar.brwarw
+ * bar.brarw
+ * bar.bwarw
+ * bar.brwar
+ * bar.brwaw
+ * bar.brar
+ * bar.bwaw
+ */
+#define __bar_brw()    asm volatile (".long 0x842cc000\n":::"memory")
+#define __bar_br()     asm volatile (".long 0x8424c000\n":::"memory")
+#define __bar_bw()     asm volatile (".long 0x8428c000\n":::"memory")
+#define __bar_arw()    asm volatile (".long 0x8423c000\n":::"memory")
+#define __bar_ar()     asm volatile (".long 0x8421c000\n":::"memory")
+#define __bar_aw()     asm volatile (".long 0x8422c000\n":::"memory")
+#define __bar_brwarw() asm volatile (".long 0x842fc000\n":::"memory")
+#define __bar_brarw()  asm volatile (".long 0x8427c000\n":::"memory")
+#define __bar_bwarw()  asm volatile (".long 0x842bc000\n":::"memory")
+#define __bar_brwar()  asm volatile (".long 0x842dc000\n":::"memory")
+#define __bar_brwaw()  asm volatile (".long 0x842ec000\n":::"memory")
+#define __bar_brar()   asm volatile (".long 0x8425c000\n":::"memory")
+#define __bar_brar()   asm volatile (".long 0x8425c000\n":::"memory")
+#define __bar_bwaw()   asm volatile (".long 0x842ac000\n":::"memory")
+
+#define rseq_smp_mb()  __bar_brwarw()
+#define rseq_smp_rmb() __bar_brar()
+#define rseq_smp_wmb() __bar_bwaw()
+
+#define rseq_smp_load_acquire(p)                                       \
+__extension__ ({                                                       \
+       __typeof(*p) ____p1 = RSEQ_READ_ONCE(*p);                       \
+       __bar_brarw();                                                  \
+       ____p1;                                                         \
+})
+
+#define rseq_smp_acquire__after_ctrl_dep()     rseq_smp_rmb()
+
+#define rseq_smp_store_release(p, v)                                   \
+do {                                                                   \
+       __bar_brwaw();                                                  \
+       RSEQ_WRITE_ONCE(*p, v);                                         \
+} while (0)
+
+#include "rseq-skip.h"

On Sat, Jul 24, 2021 at 12:16 AM Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> This reverts commit 9866d141a0977ace974400bf1f793dfc163409ce.
>
> The csky rseq support has been merged without ever notifying the rseq
> maintainers, and without any of the required asssembler glue in the rseq
> selftests, which means it is entirely untested.
>
> It is also derived from a non-upstream riscv patch which has known bugs.
I noticed that in [1]:

As Al Viro pointed out on IRC, the rseq_signal_deliver() should go after syscall
restart handling, similarly to what is done on every other supported
architecture.

[1] https://lore.kernel.org/linux-riscv/1257037909.25426.1626705790861.JavaMail.zimbra@efficios.com/

I still want to fixup it, instead of revert it.

>
> The assembly part of this revert should be carefully reviewed by the
> architecture maintainer because it touches code which has changed since
> the merge of the reverted patch.
>
> The rseq selftests assembly glue should be introduced at the same time
> as the architecture rseq support. Without the presence of any test, I
> recommend reverting rseq support from csky for now.
>
> Link: https://lore.kernel.org/lkml/1257037909.25426.1626705790861.JavaMail.zimbra@efficios.com/
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Guo Ren <guoren@kernel.org>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: linux-csky@vger.kernel.org
> ---
>  arch/csky/Kconfig         | 1 -
>  arch/csky/kernel/entry.S  | 4 ----
>  arch/csky/kernel/signal.c | 3 ---
>  3 files changed, 8 deletions(-)
>
> diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
> index 2716f6395ba7..c9655f09e591 100644
> --- a/arch/csky/Kconfig
> +++ b/arch/csky/Kconfig
> @@ -66,7 +66,6 @@ config CSKY
>         select HAVE_PERF_USER_STACK_DUMP
>         select HAVE_DMA_CONTIGUOUS
>         select HAVE_REGS_AND_STACK_ACCESS_API
> -       select HAVE_RSEQ
>         select HAVE_STACKPROTECTOR
>         select HAVE_SYSCALL_TRACEPOINTS
>         select MAY_HAVE_SPARSE_IRQ
> diff --git a/arch/csky/kernel/entry.S b/arch/csky/kernel/entry.S
> index d89afe3b24cc..cc2a7e84c8e5 100644
> --- a/arch/csky/kernel/entry.S
> +++ b/arch/csky/kernel/entry.S
> @@ -50,10 +50,6 @@ ENTRY(csky_systemcall)
>         SAVE_ALL TRAP0_SIZE
>         zero_fp
>         context_tracking
> -#ifdef CONFIG_RSEQ_DEBUG
> -       mov     a0, sp
> -       jbsr    rseq_syscall
> -#endif
>         psrset  ee, ie
>
>         lrw     r9, __NR_syscalls
> diff --git a/arch/csky/kernel/signal.c b/arch/csky/kernel/signal.c
> index 312f046d452d..3cf08732b142 100644
> --- a/arch/csky/kernel/signal.c
> +++ b/arch/csky/kernel/signal.c
> @@ -175,8 +175,6 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs)
>         sigset_t *oldset = sigmask_to_save();
>         int ret;
>
> -       rseq_signal_deliver(ksig, regs);
> -
>         /* Are we from a system call? */
>         if (in_syscall(regs)) {
>                 /* Avoid additional syscall restarting via ret_from_exception */
> @@ -262,6 +260,5 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
>
>         if (thread_info_flags & _TIF_NOTIFY_RESUME) {
>                 tracehook_notify_resume(regs);
> -               rseq_handle_notify_resume(NULL, regs);
>         }
>  }
> --
> 2.20.1
>


--
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/