Re: [PATCH 1/3] [POC] test implementaion of rt-signals

[Jan Kiszka <jan.kiszka@siemens.com>]

On 01.09.23 14:00, Jan Kiszka wrote:
> On 16.08.23 12:18, Johannes Kirchmair wrote:
>> We implement rt signals to handle exceptions in rt stage.
>>
>> This is done using dovetail specific functions for setting up the signal
>> frame.
>>
>> This can be used to handle fpe exceptions on the fly, like fixing
>> division by zero. An other use case are breakpoints, implemented using the
>> illegal opcode exception. The real time handling of the breakpoints would
>> be handy for conditional breakpoints or also for stopping watchdogs and
>> other tasks in time.
>>
>> Signed-off-by: Johannes Kirchmair <johannes.kirchmair@sigmatek.at>
>> ---
>>  include/cobalt/kernel/ppd.h                   |  3 +
>>  include/cobalt/kernel/thread.h                |  2 +
>>  include/cobalt/signal.h                       |  2 +
>>  include/cobalt/uapi/signal.h                  |  1 +
>>  include/cobalt/uapi/syscall.h                 |  6 ++
>>  kernel/cobalt/arch/x86/Makefile               |  2 +-
>>  .../arch/x86/include/asm/xenomai/thread.h     | 13 ++++
>>  kernel/cobalt/arch/x86/signal_ia32.c          | 75 +++++++++++++++++++
>>  kernel/cobalt/arch/x86/signal_ia64.c          | 37 +++++++++
>>  kernel/cobalt/dovetail/kevents.c              |  5 ++
>>  kernel/cobalt/posix/process.c                 |  3 +-
>>  kernel/cobalt/posix/syscall.c                 | 28 +++++++
>>  kernel/cobalt/posix/syscall32.c               | 16 ++++
>>  kernel/cobalt/thread.c                        | 39 ++++++++++
>>  lib/cobalt/arch/x86/Makefile.am               |  2 +-
>>  lib/cobalt/arch/x86/sigreturn.c               | 36 +++++++++
>>  lib/cobalt/internal.h                         |  2 +
>>  lib/cobalt/signal.c                           | 13 ++++
>>  18 files changed, 282 insertions(+), 3 deletions(-)
>>  create mode 100644 kernel/cobalt/arch/x86/signal_ia32.c
>>  create mode 100644 kernel/cobalt/arch/x86/signal_ia64.c
>>  create mode 100644 lib/cobalt/arch/x86/sigreturn.c
>>
>> diff --git a/include/cobalt/kernel/ppd.h b/include/cobalt/kernel/ppd.h
>> index f0079fe6e..fb2f682da 100644
>> --- a/include/cobalt/kernel/ppd.h
>> +++ b/include/cobalt/kernel/ppd.h
>> @@ -22,6 +22,7 @@
>>  #include <linux/types.h>
>>  #include <linux/atomic.h>
>>  #include <linux/rbtree.h>
>> +#include <linux/signal.h>
>>  #include <cobalt/kernel/heap.h>
>>  
>>  struct cobalt_umm {
>> @@ -32,6 +33,8 @@ struct cobalt_umm {
>>  
>>  struct cobalt_ppd {
>>  	struct cobalt_umm umm;
>> +	void __user *sighand[_NSIG];
>> +	void __user *sigrestorer;
>>  	atomic_t refcnt;
>>  	char *exe_path;
>>  	struct rb_root fds;
>> diff --git a/include/cobalt/kernel/thread.h b/include/cobalt/kernel/thread.h
>> index b79cb8429..33d468419 100644
>> --- a/include/cobalt/kernel/thread.h
>> +++ b/include/cobalt/kernel/thread.h
>> @@ -574,6 +574,8 @@ static inline void xnthread_propagate_schedparam(struct xnthread *curr)
>>  		__xnthread_propagate_schedparam(curr);
>>  }
>>  
>> +int xnthread_handle_rt_signals(unsigned int trapnr, struct pt_regs *regs);
>> +
>>  extern struct xnthread_personality xenomai_personality;
>>  
>>  /** @} */
>> diff --git a/include/cobalt/signal.h b/include/cobalt/signal.h
>> index 62694f93a..3d6540aff 100644
>> --- a/include/cobalt/signal.h
>> +++ b/include/cobalt/signal.h
>> @@ -54,6 +54,8 @@ COBALT_DECL(int, kill(pid_t pid, int sig));
>>  COBALT_DECL(int, sigqueue(pid_t pid, int sig,
>>  			  const union sigval value));
>>  
>> +int cobalt_rt_signal(int sig, void (*handler)(int, siginfo_t *, void *));
>> +
>>  #ifdef __cplusplus
>>  }
>>  #endif
>> diff --git a/include/cobalt/uapi/signal.h b/include/cobalt/uapi/signal.h
>> index 8a7ea15a4..1afb6050a 100644
>> --- a/include/cobalt/uapi/signal.h
>> +++ b/include/cobalt/uapi/signal.h
>> @@ -68,6 +68,7 @@
>>  #define SIGDEBUG_RESCNT_IMBALANCE	7
>>  #define SIGDEBUG_LOCK_BREAK		8
>>  #define SIGDEBUG_MUTEX_SLEEP		9
>> +#define SIGDEBUG_SIGRESTOR		10
>>  
>>  #define COBALT_DELAYMAX			2147483647U
>>  
>> diff --git a/include/cobalt/uapi/syscall.h b/include/cobalt/uapi/syscall.h
>> index 3e65efaab..f14bd8ffe 100644
>> --- a/include/cobalt/uapi/syscall.h
>> +++ b/include/cobalt/uapi/syscall.h
>> @@ -142,6 +142,12 @@
>>  #define sc_cobalt_timerfd_gettime64		119
>>  #define sc_cobalt_pselect64			120
>>  
>> +/*
>> + * Sigmatek specific syscalls
>> + */
>> +#define sc_cobalt_sigreturn			121
>> +#define sc_cobalt_sigaction			122
>> +
>>  #define __NR_COBALT_SYSCALLS			128 /* Power of 2 */
>>  
>>  #endif /* !_COBALT_UAPI_SYSCALL_H */
>> diff --git a/kernel/cobalt/arch/x86/Makefile b/kernel/cobalt/arch/x86/Makefile
>> index 93929b645..e725afbff 100644
>> --- a/kernel/cobalt/arch/x86/Makefile
>> +++ b/kernel/cobalt/arch/x86/Makefile
>> @@ -1,5 +1,5 @@
>>  
>>  obj-$(CONFIG_XENOMAI) += xenomai.o
>> -xenomai-y := machine.o smi.o c1e.o
>> +xenomai-y := machine.o smi.o c1e.o signal_ia32.o signal_ia64.o
>>  
>>  ccflags-y := -I$(srctree)/arch/x86/xenomai/include -I$(srctree)/include/xenomai
>> diff --git a/kernel/cobalt/arch/x86/include/asm/xenomai/thread.h b/kernel/cobalt/arch/x86/include/asm/xenomai/thread.h
>> index 745c32467..4d004680b 100644
>> --- a/kernel/cobalt/arch/x86/include/asm/xenomai/thread.h
>> +++ b/kernel/cobalt/arch/x86/include/asm/xenomai/thread.h
>> @@ -28,5 +28,18 @@
>>  #define xnarch_fault_bp_p(__nr)		((current->ptrace & PT_PTRACED) &&	\
>>  					 ((__nr) == X86_TRAP_DB || (__nr) == X86_TRAP_BP))
>>  #define xnarch_fault_notify(__nr)	(!xnarch_fault_bp_p(__nr))
>> +#define xnarch_fault_code(__regs)		((__regs)->orig_ax)
>> +int xnarch_setup_trap_info(unsigned int vector, struct pt_regs *regs,
>> +			   long errcode, int *sig, struct kernel_siginfo *info);
>> +
>> +int xnarch_setup_rt_frame_ia32(int sig, void *handler, struct kernel_siginfo *si,
>> +			  struct pt_regs *regs, void __user *restorer);
>> +
>> +int xnarch_rt_sigreturn_ia32(struct pt_regs *regs);
>> +
>> +int xnarch_setup_rt_frame_ia64(int sig, void *handler, struct kernel_siginfo *si,
>> +			  struct pt_regs *regs, void __user *restorer);
>> +
>> +int xnarch_rt_sigreturn_ia64(struct pt_regs *regs);
>>  
>>  #endif /* !_COBALT_X86_ASM_THREAD_H */
>> diff --git a/kernel/cobalt/arch/x86/signal_ia32.c b/kernel/cobalt/arch/x86/signal_ia32.c
>> new file mode 100644
>> index 000000000..140016460
>> --- /dev/null
>> +++ b/kernel/cobalt/arch/x86/signal_ia32.c
>> @@ -0,0 +1,75 @@
>> +#include <linux/signal.h>
>> +#include <linux/uaccess.h>
>> +#include <cobalt/kernel/thread.h>
>> +
>> +#include <asm/sigframe.h>
>> +#include <asm/sighandling.h>
>> +#include <asm/fpu/signal.h>
>> +
>> +int xnarch_setup_trap_info(unsigned int vector, struct pt_regs *regs,
>> +			   long errcode, int *sig, struct kernel_siginfo *info)
> 
> Why is this function in file that suggests to handle only 32-bit? It's 
> shared by both 32 and 64 bit.
> 
>> +{
>> +	switch (vector) {
>> +	case 0: /* divide_error */
>> +		*sig = SIGFPE;
>> +		info->si_signo = *sig;
>> +		info->si_errno = 0;
>> +		info->si_code = FPE_INTDIV;
>> +		info->si_addr = (void __user *)regs->ip;
>> +		return 0;
>> +	case 1: /* trap_error */ {
>> +		unsigned long condition;
>> +		get_debugreg(condition, 6);
>> +		set_debugreg(0, 7);
>> +		*sig = SIGTRAP;
>> +		info->si_signo = *sig;
>> +		info->si_errno = errcode;
>> +		info->si_code = get_si_code(condition);
>> +		info->si_addr = (void __user *)regs->ip;
>> +		return 0;
>> +	}
>> +	case 3: /* trap_error */
>> +		*sig = SIGTRAP;
>> +		info->si_signo = *sig;
>> +		info->si_errno = errcode;
>> +		info->si_code = SI_KERNEL;
>> +		info->si_addr = (void __user *)regs->ip;
>> +		return 0;
>> +	case 6: /* invalid_op */
>> +		*sig = SIGILL;
>> +		info->si_signo = *sig;
>> +		info->si_errno = 0;
>> +		info->si_code = ILL_ILLOPN;
>> +		info->si_addr = (void __user *)regs->ip;
>> +		return 0;

I've also added this:

	case 14:
		*sig = SIGSEGV;
		info->si_signo = *sig;
		info->si_errno = errcode;
		info->si_code = 0;
		info->si_addr = 0;
		return 0;

That fixes the related test case.

As you can see, we can neither provide an accurate fault address nor a
proper code because those information are lost along the dovetail
notification path or do not even exist by that time. The address could
probably be forwarded by extending dovetail, the code likely not.

Jan

-- 
Siemens AG, Technology
Linux Expert Center