Re: [PATCH v6 20/36] nds32: Signal handling support

[Vincent Chen]

Thanks, I got it.

After referring to arm64 and risc-v, we try to refine our code, such as
removing unneeded checking and refining syscall restart flow. We
hope these modifications can enhance the reliability and readability.
However, the following 2 files which you had acked are included in
this modification.
1. arch/nds32/include/asm/nds32.h
(patch: Assembly macros and definitions)
 The definition of  macro tbl and why are removed.
   - Now, we use pt_reg->syscallno instead of 'why' to determine
 whether entering kernel is via syscall or not. Therefore, macro
 'why' is unneeded.

--- a/arch/nds32/include/asm/nds32.h
+++ b/arch/nds32/include/asm/nds32.h
@@ -66,10 +66,6 @@ static inline unsigned long CACHE_LINE_SIZE

 #endif /* __ASSEMBLY__ */

-/* tbl and why is used in ex-scall.S and ex-exit.S */
-#define tbl $r8
-#define why $r8
-
 #define IVB_BASE   PHYS_OFFSET


2. arch/nds32/kernel/ex-scall.S(patch: System calls handling)
a. Define macro tbl
- The marco tbl is used only in this file. So, I move its definition
  from arch/nds32/include/asm/nds32.h to here.
b. Remove 'set why = 0'  when issuing syscall number is invalid
c. Adjust input arguments of syscall_trace_enter

--- a/arch/nds32/kernel/ex-scall.S
+++ b/arch/nds32/kernel/ex-scall.S
...
+#define tbl $r8

 /*
  * $r7 will be writen as syscall nr
- * by retrieving from $ITYPE 'SWID' bitfiled
  */
.macro  get_scno
lwi $r7, [$sp + R15_OFFSET]
@@ -54,7 +49,6 @@ ENTRY(eh_syscall)
get_scno
gie_enable

-ENTRY(eh_syscall_phase_2)
lwi $p0, [tsk+#TSK_TI_FLAGS]

andi$p1, $p0, #_TIF_WORK_SYSCALL_ENTRY
@@ -71,7 +65,6 @@ jmp_systbl:
jr  $p1 ! no return

 _SCNO_EXCEED:
-   moviwhy, 0
ori $r0, $r7, #0
ori$r1, $sp, #0
b   bad_syscall
@@ -81,8 +74,7 @@ _SCNO_EXCEED:
  * context switches, and waiting for our parent to respond.
  */
 __sys_trace:
-   move$r1, $sp
-   move$r0, $r7! trace entry [IP = 0]
+   move$r0, $sp
bal syscall_trace_enter
move$r7, $r0
la  $lp, __sys_trace_return ! return address


If you think these modifications in acked files are not permitted,
we will recover it.

We verify all modifications by LTP 2017 related cases and glibc
2.26 testsuite. We plan to add it in the next version patch and
hope you can give us some comments as before.

Thanks

Vincent



2018-01-24 19:13 GMT+08:00 Arnd Bergmann :
> On Wed, Jan 24, 2018 at 1:56 AM, Vincent Chen  wrote:
>> 2018-01-18 18:30 GMT+08:00 Arnd Bergmann :
>>> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu  wrote:
 From: Greentime Hu 

 This patch adds support for signal handling.

 Signed-off-by: Vincent Chen 
 Signed-off-by: Greentime Hu 
>>>
>>> I never feel qualified enough to properly review signal handling code, so
>>> no Ack from me for this code even though I don't see anything wrong with it.
>>> Hopefully someone else can give an Ack after looking more closely.
>>>
>>
>> Dear Arnd:
>>
>> We'd be glad to improve signal handling code to meet your requirement.
>> Could you
>> tell us which part we need to refine or which implementation is good
>> for us to refer?
>
> No, as I said, the problem is on my side, I just don't understand enough of 
> it.
> I would assume that the arm64 and risc-v implementations are the most
> thoroughly reviewed, but haven't looked at those in enough detail either.
> If your code does something that risc-v doesn't do, try to understand whether
> there should be a difference or not.
>
>   Arnd

Re: [PATCH v6 20/36] nds32: Signal handling support

[Arnd Bergmann]

On Wed, Jan 24, 2018 at 1:56 AM, Vincent Chen  wrote:
> 2018-01-18 18:30 GMT+08:00 Arnd Bergmann :
>> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu  wrote:
>>> From: Greentime Hu 
>>>
>>> This patch adds support for signal handling.
>>>
>>> Signed-off-by: Vincent Chen 
>>> Signed-off-by: Greentime Hu 
>>
>> I never feel qualified enough to properly review signal handling code, so
>> no Ack from me for this code even though I don't see anything wrong with it.
>> Hopefully someone else can give an Ack after looking more closely.
>>
>
> Dear Arnd:
>
> We'd be glad to improve signal handling code to meet your requirement.
> Could you
> tell us which part we need to refine or which implementation is good
> for us to refer?

No, as I said, the problem is on my side, I just don't understand enough of it.
I would assume that the arm64 and risc-v implementations are the most
thoroughly reviewed, but haven't looked at those in enough detail either.
If your code does something that risc-v doesn't do, try to understand whether
there should be a difference or not.

  Arnd

Re: [PATCH v6 20/36] nds32: Signal handling support

[Vincent Chen]

2018-01-18 18:30 GMT+08:00 Arnd Bergmann :
> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu  wrote:
>> From: Greentime Hu 
>>
>> This patch adds support for signal handling.
>>
>> Signed-off-by: Vincent Chen 
>> Signed-off-by: Greentime Hu 
>
> I never feel qualified enough to properly review signal handling code, so
> no Ack from me for this code even though I don't see anything wrong with it.
> Hopefully someone else can give an Ack after looking more closely.
>
>Arnd

Dear Arnd:

We'd be glad to improve signal handling code to meet your requirement.
Could you
tell us which part we need to refine or which implementation is good
for us to refer?

Thanks.

Vincent

Re: [PATCH v6 20/36] nds32: Signal handling support

[Arnd Bergmann]

On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu  wrote:
> From: Greentime Hu 
>
> This patch adds support for signal handling.
>
> Signed-off-by: Vincent Chen 
> Signed-off-by: Greentime Hu 

I never feel qualified enough to properly review signal handling code, so
no Ack from me for this code even though I don't see anything wrong with it.
Hopefully someone else can give an Ack after looking more closely.

   Arnd

[PATCH v6 20/36] nds32: Signal handling support

[Greentime Hu]

From: Greentime Hu 

This patch adds support for signal handling.

Signed-off-by: Vincent Chen 
Signed-off-by: Greentime Hu 
---
 arch/nds32/include/uapi/asm/sigcontext.h |   60 ++
 arch/nds32/kernel/signal.c   |  337 ++
 2 files changed, 397 insertions(+)
 create mode 100644 arch/nds32/include/uapi/asm/sigcontext.h
 create mode 100644 arch/nds32/kernel/signal.c

diff --git a/arch/nds32/include/uapi/asm/sigcontext.h 
b/arch/nds32/include/uapi/asm/sigcontext.h
new file mode 100644
index 000..00567b2
--- /dev/null
+++ b/arch/nds32/include/uapi/asm/sigcontext.h
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2005-2017 Andes Technology Corporation
+
+#ifndef _ASMNDS32_SIGCONTEXT_H
+#define _ASMNDS32_SIGCONTEXT_H
+
+/*
+ * Signal context structure - contains all info to do with the state
+ * before the signal handler was invoked.  Note: only add new entries
+ * to the end of the structure.
+ */
+
+struct zol_struct {
+   unsigned long nds32_lc; /* $LC */
+   unsigned long nds32_le; /* $LE */
+   unsigned long nds32_lb; /* $LB */
+};
+
+struct sigcontext {
+   unsigned long trap_no;
+   unsigned long error_code;
+   unsigned long oldmask;
+   unsigned long nds32_r0;
+   unsigned long nds32_r1;
+   unsigned long nds32_r2;
+   unsigned long nds32_r3;
+   unsigned long nds32_r4;
+   unsigned long nds32_r5;
+   unsigned long nds32_r6;
+   unsigned long nds32_r7;
+   unsigned long nds32_r8;
+   unsigned long nds32_r9;
+   unsigned long nds32_r10;
+   unsigned long nds32_r11;
+   unsigned long nds32_r12;
+   unsigned long nds32_r13;
+   unsigned long nds32_r14;
+   unsigned long nds32_r15;
+   unsigned long nds32_r16;
+   unsigned long nds32_r17;
+   unsigned long nds32_r18;
+   unsigned long nds32_r19;
+   unsigned long nds32_r20;
+   unsigned long nds32_r21;
+   unsigned long nds32_r22;
+   unsigned long nds32_r23;
+   unsigned long nds32_r24;
+   unsigned long nds32_r25;
+   unsigned long nds32_fp; /* $r28 */
+   unsigned long nds32_gp; /* $r29 */
+   unsigned long nds32_lp; /* $r30 */
+   unsigned long nds32_sp; /* $r31 */
+   unsigned long nds32_ipc;
+   unsigned long fault_address;
+   unsigned long used_math_flag;
+   /* FPU Registers */
+   struct zol_struct zol;
+};
+
+#endif
diff --git a/arch/nds32/kernel/signal.c b/arch/nds32/kernel/signal.c
new file mode 100644
index 000..b6313dd
--- /dev/null
+++ b/arch/nds32/kernel/signal.c
@@ -0,0 +1,337 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2005-2017 Andes Technology Corporation
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+struct rt_sigframe {
+   struct siginfo info;
+   struct ucontext uc;
+};
+
+static int restore_sigframe(struct pt_regs *regs,
+   struct rt_sigframe __user * sf)
+{
+   sigset_t set;
+   int err;
+
+   err = __copy_from_user(, >uc.uc_sigmask, sizeof(set));
+   if (err == 0) {
+   set_current_blocked();
+   }
+
+   __get_user_error(regs->uregs[0], >uc.uc_mcontext.nds32_r0, err);
+   __get_user_error(regs->uregs[1], >uc.uc_mcontext.nds32_r1, err);
+   __get_user_error(regs->uregs[2], >uc.uc_mcontext.nds32_r2, err);
+   __get_user_error(regs->uregs[3], >uc.uc_mcontext.nds32_r3, err);
+   __get_user_error(regs->uregs[4], >uc.uc_mcontext.nds32_r4, err);
+   __get_user_error(regs->uregs[5], >uc.uc_mcontext.nds32_r5, err);
+   __get_user_error(regs->uregs[6], >uc.uc_mcontext.nds32_r6, err);
+   __get_user_error(regs->uregs[7], >uc.uc_mcontext.nds32_r7, err);
+   __get_user_error(regs->uregs[8], >uc.uc_mcontext.nds32_r8, err);
+   __get_user_error(regs->uregs[9], >uc.uc_mcontext.nds32_r9, err);
+   __get_user_error(regs->uregs[10], >uc.uc_mcontext.nds32_r10, err);
+   __get_user_error(regs->uregs[11], >uc.uc_mcontext.nds32_r11, err);
+   __get_user_error(regs->uregs[12], >uc.uc_mcontext.nds32_r12, err);
+   __get_user_error(regs->uregs[13], >uc.uc_mcontext.nds32_r13, err);
+   __get_user_error(regs->uregs[14], >uc.uc_mcontext.nds32_r14, err);
+   __get_user_error(regs->uregs[15], >uc.uc_mcontext.nds32_r15, err);
+   __get_user_error(regs->uregs[16], >uc.uc_mcontext.nds32_r16, err);
+   __get_user_error(regs->uregs[17], >uc.uc_mcontext.nds32_r17, err);
+   __get_user_error(regs->uregs[18], >uc.uc_mcontext.nds32_r18, err);
+   __get_user_error(regs->uregs[19], >uc.uc_mcontext.nds32_r19, err);
+   __get_user_error(regs->uregs[20], >uc.uc_mcontext.nds32_r20, err);
+   __get_user_error(regs->uregs[21], >uc.uc_mcontext.nds32_r21, err);
+   __get_user_error(regs->uregs[22],