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

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

On 09.05.23 15:17, Johannes Kirchmair wrote:
> Hello Jan and Tobias,
> 
> I just dropped the patches on the mailing list.
> For the dovetail functions I took a rather naive approach, using most of the frame setup code provided by Linux.
> I just tested it for 32bit and 64bit x86 Applications and it seems to work. 
> 
> I also added a tool rt_signal_hist that runs in a cycle and measures the time an exception handling takes.
> It is afterwards put into an txt file that reassembles an histogram of the timings.
> 
> Unfortunately as a 32bit application the rt_signal_hist tool does not run to completion, printing the following:
> " Xenomai/cobalt: sleeping while holding mutex
> CPU time limit exceeded"
> 
> Maybe the naive approach is not so reliable ;-/
> Hope I find the time the look into it. 
> 

I looked into the x86 signalframe helpers before and found some traces
of locking that make them incompatible - if I rember correctly. After
that, I dropped the idea of just removing the "static".

> Sorry for not putting more effort into this so fare.
> 
> The next most important step in my opinion would be to define a clean interface into dovetail for the signal frame setup and restoring.
> Actually that's the task I consider one of the harder and I am not so sure how to approach this.
> 

Exactly, and we need to involve Philippe now soon. Let me medidate over
some interface options as well. Will pick up if you don't find the time
soon.

Jan

> Best regards
> Johannes
> 
>> -----Original Message-----
>> From: Johannes Kirchmair <johannes.kirchmair@sigmatek.at>
>> Sent: Dienstag, 9. Mai 2023 15:14
>> To: xenomai@lists.linux.dev
>> Cc: Johannes Kirchmair <johannes.kirchmair@sigmatek.at>
>> Subject: [PATCH 1/3] [POC] test implementaion of rt-signals
>>
>> 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/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          | 26 +++++++
>>  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               | 32 ++++++++
>>  lib/cobalt/internal.h                         |  2 +
>>  lib/cobalt/signal.c                           | 13 ++++
>>  17 files changed, 266 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/syscall.h b/include/cobalt/uapi/syscall.h
>> index 9646a0d97..6307fae7f 100644
>> --- a/include/cobalt/uapi/syscall.h
>> +++ b/include/cobalt/uapi/syscall.h
>> @@ -137,6 +137,12 @@
>>  #define sc_cobalt_recvmmsg64			114
>>  #define sc_cobalt_cond_wait_prologue64		115
>>
>> +/*
>> + * Sigmatek specific syscalls
>> + */
>> +#define sc_cobalt_sigreturn			120
>> +#define sc_cobalt_sigaction			121
>> +
>>  #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..e78c8c4d4
>> --- /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)
>> +{
>> +	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;
>> +	case 16: { /* coprocessor_error */
>> +		*sig = SIGFPE;
>> +
>> +		info->si_signo = *sig;
>> +		info->si_errno = 0;
>> +		info->si_code = 0;
>> +		info->si_addr = (void __user *)regs->ip;
>> +		return 0;
>> +	}
>> +	default:
>> +		break;
>> +	}
>> +
>> +	return -ENOSYS;
>> +}
>> +
>> +int xnarch_rt_sigreturn_ia32(struct pt_regs *regs)
>> +{
>> +	int ret;
>> +
>> +	ret = dovetail_restore_rt_signal_frame(regs);
>> +	if (ret < 0)
>> +		goto badframe;
>> +
>> +	return regs->ax;
>> +
>> +badframe:
>> +	xnthread_call_mayday(xnthread_current(), SIGKILL);
>> +	return -1;
>> +}
>> +
>> diff --git a/kernel/cobalt/arch/x86/signal_ia64.c
>> b/kernel/cobalt/arch/x86/signal_ia64.c
>> new file mode 100644
>> index 000000000..a7044db40
>> --- /dev/null
>> +++ b/kernel/cobalt/arch/x86/signal_ia64.c
>> @@ -0,0 +1,26 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + *  Copyright (C) 1991, 1992  Linus Torvalds
>> + *  Copyright (C) 2000, 2001, 2002 Andi Kleen SuSE Labs
>> + *
>> + *  1997-11-28  Modified for POSIX.1b signals by Richard Henderson
>> + *  2000-06-20  Pentium III FXSR, SSE support by Gareth Hughes
>> + *  2000-2002   x86-64 support by Andi Kleen
>> + */
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> +
>> +#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_rt_sigreturn_ia64(struct pt_regs *regs)
>> +{
>> +	return dovetail_restore_rt_signal_frame(regs);
>> +}
>> +
>> diff --git a/kernel/cobalt/dovetail/kevents.c b/kernel/cobalt/dovetail/kevents.c
>> index 4da4f51b7..61417717b 100644
>> --- a/kernel/cobalt/dovetail/kevents.c
>> +++ b/kernel/cobalt/dovetail/kevents.c
>> @@ -57,6 +57,9 @@ void handle_oob_trap_entry(unsigned int trapnr, struct
>> pt_regs *regs)
>>  		xnsched_run();
>>  	}
>>
>> +	if (xnthread_handle_rt_signals(trapnr, regs) == 0)
>> +		return;
>> +
>>  	/*
>>  	 * If we experienced a trap on behalf of a shadow thread
>>  	 * running in primary mode, move it to the Linux domain,
>> @@ -88,6 +91,8 @@ void handle_oob_trap_entry(unsigned int trapnr, struct
>> pt_regs *regs)
>>  		xnstat_counter_inc(&thread->stat.pf);
>>
>>  	xnthread_relax(xnarch_fault_notify(trapnr), SIGDEBUG_MIGRATE_FAULT);
>> +
>> +	return;
>>  }
>>
>>  static inline int handle_setaffinity_event(struct dovetail_migration_data *d)
>> diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
>> index 1abc86f37..2069129cb 100644
>> --- a/kernel/cobalt/posix/process.c
>> +++ b/kernel/cobalt/posix/process.c
>> @@ -738,9 +738,10 @@ void cobalt_unregister_debugged_thread(struct
>> xnthread *thread)
>>  		cobalt_resume_debugged_process(process);
>>  }
>>
>> +#ifdef CONFIG_SMP
>> +
>>  int cobalt_handle_setaffinity_event(struct task_struct *task)
>>  {
>> -#ifdef CONFIG_SMP
>>  	struct xnthread *thread;
>>  	spl_t s;
>>
>> diff --git a/kernel/cobalt/posix/syscall.c b/kernel/cobalt/posix/syscall.c
>> index 46c4998e4..b4bd4c587 100644
>> --- a/kernel/cobalt/posix/syscall.c
>> +++ b/kernel/cobalt/posix/syscall.c
>> @@ -277,6 +277,34 @@ static COBALT_SYSCALL(serialdbg, current,
>>  	return 0;
>>  }
>>
>> +static COBALT_SYSCALL(sigreturn, current, (void))
>> +{
>> +	struct pt_regs *regs = task_pt_regs(current);
>> +
>> +	if (regs->cs == __USER_CS)
>> +		xnarch_rt_sigreturn_ia64(regs);
>> +	if (regs->cs == __USER32_CS)
>> +		xnarch_rt_sigreturn_ia32(regs);
>> +
>> +	return __xn_reg_rval(regs);
>> +}
>> +
>> +static COBALT_SYSCALL(sigaction, current, (int sig, void __user *handler,
>> +		      void __user *restorer))
>> +{
>> +	struct cobalt_ppd *sys_ppd = cobalt_ppd_get(0);
>> +
>> +	if (sig < 0 || sig >= _NSIG)
>> +		return -EINVAL;
>> +
>> +	sys_ppd->sighand[sig] = handler;
>> +
>> +	if (!sys_ppd->sigrestorer)
>> +		sys_ppd->sigrestorer = restorer;
>> +
>> +	return 0;
>> +}
>> +
>>  static void stringify_feature_set(unsigned long fset, char *buf, int size)
>>  {
>>  	unsigned long feature;
>> diff --git a/kernel/cobalt/posix/syscall32.c b/kernel/cobalt/posix/syscall32.c
>> index b65a0a760..f8b21bfb2 100644
>> --- a/kernel/cobalt/posix/syscall32.c
>> +++ b/kernel/cobalt/posix/syscall32.c
>> @@ -774,6 +774,22 @@ COBALT_SYSCALL32emu(sigqueue, conforming,
>>  	return ret ?: __cobalt_sigqueue(pid, sig, &val);
>>  }
>>
>> +COBALT_SYSCALL32emu(sigaction, current,
>> +		    (int sig, void __user *handler, void __user *restorer))
>> +{
>> +	struct cobalt_ppd *sys_ppd = cobalt_ppd_get(0);
>> +
>> +	if (sig < 0 || sig >= _NSIG)
>> +		return -EINVAL;
>> +
>> +	sys_ppd->sighand[sig] = handler;
>> +
>> +	if (!sys_ppd->sigrestorer)
>> +		sys_ppd->sigrestorer = restorer;
>> +
>> +	return 0;
>> +}
>> +
>>  COBALT_SYSCALL32emu(monitor_wait, nonrestartable,
>>  		    (struct cobalt_monitor_shadow __user *u_mon,
>>  		     int event, const struct old_timespec32 __user *u_ts,
>> diff --git a/kernel/cobalt/thread.c b/kernel/cobalt/thread.c
>> index 41804b24f..71f97c481 100644
>> --- a/kernel/cobalt/thread.c
>> +++ b/kernel/cobalt/thread.c
>> @@ -25,6 +25,7 @@
>>  #include <linux/signal.h>
>>  #include <linux/pid.h>
>>  #include <linux/sched.h>
>> +#include <asm/sighandling.h>
>>  #include <uapi/linux/sched/types.h>
>>  #include <cobalt/kernel/sched.h>
>>  #include <cobalt/kernel/timer.h>
>> @@ -43,6 +44,7 @@
>>  #include <pipeline/inband_work.h>
>>  #include <pipeline/sched.h>
>>  #include <trace/events/cobalt-core.h>
>> +#include "posix/process.h"
>>  #include "debug.h"
>>
>>  static DECLARE_WAIT_QUEUE_HEAD(join_all);
>> @@ -2520,6 +2522,43 @@ int xnthread_killall(int grace, int mask)
>>  }
>>  EXPORT_SYMBOL_GPL(xnthread_killall);
>>
>> +int xnthread_handle_rt_signals(unsigned int trapnr, struct pt_regs *regs)
>> +{
>> +	struct ksignal ksig;
>> +
>> +	unsigned int vector = trapnr;
>> +	unsigned int code = xnarch_fault_code(regs);
>> +	struct cobalt_ppd *sys_ppd;
>> +	int sig, ret = 0;
>> +	struct kernel_siginfo si;
>> +
>> +	code = xnarch_fault_code(regs);
>> +	ret = xnarch_setup_trap_info(vector, regs, code, &sig, &si);
>> +	if (ret || sig == 0)
>> +		return 1;
>> +
>> +	sys_ppd = cobalt_ppd_get(0);
>> +	if (sig >= _NSIG ||
>> +	    sys_ppd->sighand[sig] == NULL ||
>> +	    sys_ppd->sighand[sig] == SIG_DFL)
>> +		return 1;
>> +
>> +	if (sys_ppd->sigrestorer == NULL)
>> +		return 1;
>> +
>> +	ksig.sig = sig;
>> +	memcpy(&ksig.info, &si, sizeof(si));
>> +	ksig.ka.sa.sa_flags = SA_SIGINFO | SA_RESTORER;
>> +	ksig.ka.sa.sa_restorer = sys_ppd->sigrestorer;
>> +	ksig.ka.sa.sa_handler = sys_ppd->sighand[sig];
>> +
>> +	ret = dovetail_setup_rt_signal_frame(&ksig, regs);
>> +	if (ret)
>> +		return 1;
>> +
>> +	return 0;
>> +}
>> +
>>  /* Xenomai's generic personality. */
>>  struct xnthread_personality xenomai_personality = {
>>  	.name = "core",
>> diff --git a/lib/cobalt/arch/x86/Makefile.am b/lib/cobalt/arch/x86/Makefile.am
>> index a5095be3d..14f5eff97 100644
>> --- a/lib/cobalt/arch/x86/Makefile.am
>> +++ b/lib/cobalt/arch/x86/Makefile.am
>> @@ -2,7 +2,7 @@ noinst_LTLIBRARIES = libarch.la
>>
>>  libarch_la_LDFLAGS = @XENO_LIB_LDFLAGS@
>>
>> -libarch_la_SOURCES = features.c
>> +libarch_la_SOURCES = features.c sigreturn.c
>>
>>  libarch_la_CPPFLAGS =			\
>>  	@XENO_COBALT_CFLAGS@ 		\
>> diff --git a/lib/cobalt/arch/x86/sigreturn.c b/lib/cobalt/arch/x86/sigreturn.c
>> new file mode 100644
>> index 000000000..76292e145
>> --- /dev/null
>> +++ b/lib/cobalt/arch/x86/sigreturn.c
>> @@ -0,0 +1,32 @@
>> +#include <cobalt/uapi/syscall.h>
>> +#include "internal.h"
>> +
>> +extern void cobalt_sigreturn (void) asm ("__cobalt_sigreturn") __attribute__
>> ((visibility ("hidden")));
>> +
>> +#ifdef __x86_64__
>> +#define build_restorer(syscall_number)                                         \
>> +	asm(".text\n"                                                          \
>> +	    "    .align 16\n"                                                  \
>> +	    "__cobalt_sigreturn:\n"                                            \
>> +	    "    movq $ " #syscall_number ", %rax\n"                           \
>> +	    "    syscall")
>> +#endif
>> +
>> +#ifdef __i386__
>> +#define build_restorer(syscall_number)                                         \
>> +	asm(".text\n"                                                          \
>> +	    "    .align 16\n"                                                  \
>> +	    "__cobalt_sigreturn:\n"                                            \
>> +	    "    movl $ " #syscall_number ", %eax\n"                           \
>> +	    "    int  $0x80")
>> +#endif
>> +
>> +/*
>> + * __COBALT_SYSCALL_BIT | sc_cobalt_sigreturn
>> + */
>> +build_restorer(0x10000078);
>> +
>> +void *cobalt_get_restorer(void)
>> +{
>> +	return &cobalt_sigreturn;
>> +}
>> diff --git a/lib/cobalt/internal.h b/lib/cobalt/internal.h
>> index acb3989f1..4782d154a 100644
>> --- a/lib/cobalt/internal.h
>> +++ b/lib/cobalt/internal.h
>> @@ -132,4 +132,6 @@ static inline bool cobalt_features_available(unsigned int
>> feat_mask)
>>  	return (cobalt_features & feat_mask) == feat_mask;
>>  }
>>
>> +extern void *cobalt_get_restorer(void);
>> +
>>  #endif /* _LIB_COBALT_INTERNAL_H */
>> diff --git a/lib/cobalt/signal.c b/lib/cobalt/signal.c
>> index 40d315ebb..af174d570 100644
>> --- a/lib/cobalt/signal.c
>> +++ b/lib/cobalt/signal.c
>> @@ -126,3 +126,16 @@ COBALT_IMPL(int, sigqueue, (pid_t pid, int sig, const
>> union sigval value))
>>
>>  	return 0;
>>  }
>> +
>> +int cobalt_rt_signal(int sig, void (*handler)(int, siginfo_t *, void *))
>> +{
>> +	int ret;
>> +
>> +	ret = XENOMAI_SYSCALL3(sc_cobalt_sigaction, sig, handler,
>> cobalt_get_restorer());
>> +	if (ret) {
>> +		errno = -ret;
>> +		return -1;
>> +	}
>> +
>> +	return 0;
>> +}
>> --
>> 2.25.1
> 

-- 
Siemens AG, Technology
Competence Center Embedded Linux