From: Jan Kiszka <jan.kiszka@siemens.com>
To: Johannes Kirchmair <johannes.kirchmair@sigmatek.at>,
xenomai@lists.linux.dev
Subject: Re: [PATCH 1/3] [POC] test implementaion of rt-signals
Date: Sat, 9 Sep 2023 17:05:42 +0530 [thread overview]
Message-ID: <ec99faa4-c3e5-46ac-9f71-5fc45172964e@siemens.com> (raw)
In-Reply-To: <20230908105043.1355310-1-johannes.kirchmair@sigmatek.at>
On 08.09.23 12:50, 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/syscall.h | 2 +
> kernel/cobalt/arch/x86/Makefile | 2 +-
> .../arch/x86/include/asm/xenomai/thread.h | 3 ++
> kernel/cobalt/arch/x86/signal.c | 51 +++++++++++++++++++
> kernel/cobalt/dovetail/kevents.c | 3 ++
> kernel/cobalt/posix/syscall.c | 32 ++++++++++++
> kernel/cobalt/thread.c | 38 ++++++++++++++
> kernel/cobalt/trace/cobalt-posix.h | 4 +-
> lib/cobalt/arch/x86/Makefile.am | 2 +-
> lib/cobalt/arch/x86/sigreturn.c | 36 +++++++++++++
> lib/cobalt/internal.h | 2 +
> lib/cobalt/signal.c | 13 +++++
> 15 files changed, 192 insertions(+), 3 deletions(-)
> create mode 100644 kernel/cobalt/arch/x86/signal.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 3e65efaab..3913cc610 100644
> --- a/include/cobalt/uapi/syscall.h
> +++ b/include/cobalt/uapi/syscall.h
> @@ -141,6 +141,8 @@
> #define sc_cobalt_timerfd_settime64 118
> #define sc_cobalt_timerfd_gettime64 119
> #define sc_cobalt_pselect64 120
> +#define sc_cobalt_sigreturn 121
> +#define sc_cobalt_sigaction 122
>
> #define __NR_COBALT_SYSCALLS 128 /* Power of 2 */
>
> diff --git a/kernel/cobalt/arch/x86/Makefile b/kernel/cobalt/arch/x86/Makefile
> index 93929b645..e3d8eb476 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.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..70f3df2f1 100644
> --- a/kernel/cobalt/arch/x86/include/asm/xenomai/thread.h
> +++ b/kernel/cobalt/arch/x86/include/asm/xenomai/thread.h
> @@ -28,5 +28,8 @@
> #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);
>
> #endif /* !_COBALT_X86_ASM_THREAD_H */
> diff --git a/kernel/cobalt/arch/x86/signal.c b/kernel/cobalt/arch/x86/signal.c
> new file mode 100644
> index 000000000..2e8fdc571
> --- /dev/null
> +++ b/kernel/cobalt/arch/x86/signal.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#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>
> +#include <asm/trapnr.h>
> +
> +int xnarch_setup_trap_info(unsigned int vector, struct pt_regs *regs,
> + long errcode, int *sig, struct kernel_siginfo *info)
> +{
> + switch (vector) {
> + case X86_TRAP_DE: /* 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 X86_TRAP_UD: /* 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 X86_TRAP_MF:
> + *sig = SIGFPE;
> +
> + info->si_signo = *sig;
> + info->si_errno = 0;
> + info->si_code = 0;
> + info->si_addr = (void __user *)regs->ip;
> + return 0;
> + case X86_TRAP_PF:
> + *sig = SIGSEGV;
> + info->si_signo = *sig;
> + info->si_errno = errcode;
> + info->si_code = 0;
> + info->si_addr = 0;
> + return 0;
Indention got damaged.
> + default:
> + break;
> + }
> +
> + return -ENOSYS;
> +}
> +
> diff --git a/kernel/cobalt/dovetail/kevents.c b/kernel/cobalt/dovetail/kevents.c
> index 4da4f51b7..09eb12acb 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,
> diff --git a/kernel/cobalt/posix/syscall.c b/kernel/cobalt/posix/syscall.c
> index 46c4998e4..d570bd595 100644
> --- a/kernel/cobalt/posix/syscall.c
> +++ b/kernel/cobalt/posix/syscall.c
> @@ -277,6 +277,38 @@ static COBALT_SYSCALL(serialdbg, current,
> return 0;
> }
>
> +static COBALT_SYSCALL(sigreturn, current, (void))
> +{
> + struct pt_regs *regs = task_pt_regs(current);
> + int ret;
> +
> + ret = dovetail_restore_rt_signal_frame(regs);
> + if (ret < 0)
> + goto badframe;
> +
> + return __xn_reg_rval(regs);
> +
> +badframe:
> + xnthread_signal(xnthread_current(), SIGSEGV, 0);
> + return -1;
> +}
> +
> +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;
Checked if we can do better during binding or thread mapping but,
indeed, none of those calls provide a convenient channel to carry this
information.
But then let's avoid the "optimization" and simply always deliver this
from userspace (what you already do) and always assign here in kernel space.
> +
> + return 0;
> +}
> +
> static void stringify_feature_set(unsigned long fset, char *buf, int size)
> {
> unsigned long feature;
> diff --git a/kernel/cobalt/thread.c b/kernel/cobalt/thread.c
> index 41804b24f..ae8fa5c88 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,42 @@ int xnthread_killall(int grace, int mask)
> }
> EXPORT_SYMBOL_GPL(xnthread_killall);
>
> +int xnthread_handle_rt_signals(unsigned int trapnr, struct pt_regs *regs)
> +{
> + unsigned int code = xnarch_fault_code(regs);
> + unsigned int vector = trapnr;
> + struct cobalt_ppd *sys_ppd;
> + struct kernel_siginfo si;
> + struct ksignal ksig;
> + int sig, ret = 0;
> +
> + code = xnarch_fault_code(regs);
Duplicate assignment. But actually I see no value in asking the arch
first for this fault code, only to deliver it to an arch-specific call,
and only to it. Just let xnarch_setup_trap_info retrieve the fault code
itself.
> + 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/kernel/cobalt/trace/cobalt-posix.h b/kernel/cobalt/trace/cobalt-posix.h
> index 47dc77e1c..8eda3bb27 100644
> --- a/kernel/cobalt/trace/cobalt-posix.h
> +++ b/kernel/cobalt/trace/cobalt-posix.h
> @@ -173,7 +173,9 @@
> __cobalt_symbolic_syscall(timer_gettime64), \
> __cobalt_symbolic_syscall(timerfd_settime64), \
> __cobalt_symbolic_syscall(timerfd_gettime64), \
> - __cobalt_symbolic_syscall(pselect64))
> + __cobalt_symbolic_syscall(pselect64), \
> + __cobalt_symbolic_syscall(sigreturn), \
> + __cobalt_symbolic_syscall(sigaction))
>
> DECLARE_EVENT_CLASS(cobalt_syscall_entry,
> TP_PROTO(unsigned int nr),
> 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..df961469e
> --- /dev/null
> +++ b/lib/cobalt/arch/x86/sigreturn.c
> @@ -0,0 +1,36 @@
> +#include <cobalt/uapi/syscall.h>
> +#include "internal.h"
> +
> +extern void cobalt_sigreturn (void) asm ("__cobalt_sigreturn") __attribute__ ((visibility ("hidden")));
> +
> +#define TO_STR(x) #x
> +
> +#ifdef __x86_64__
> +#define build_restorer(syscall_bit, syscall) \
> + asm(".text\n" \
> + " .align 16\n" \
> + "__cobalt_sigreturn:\n" \
> + " movq $ " TO_STR(syscall_bit) ", %rax\n" \
> + " orq $ " TO_STR(syscall) ", %rax\n" \
> + " syscall")
> +#endif
> +
> +#ifdef __i386__
> +#define build_restorer(syscall_bit, syscall) \
> + asm(".text\n" \
> + " .align 16\n" \
> + "__cobalt_sigreturn:\n" \
> + " movl $ " TO_STR(syscall_bit) ", %eax\n" \
> + " orl $ " TO_STR(syscall) ", %eax\n" \
> + " int $0x80")
> +#endif
I still need to play with those two in order to see if we can do it more
elegantly, ie. without assembly.
> +
> +/*
> + * __COBALT_SYSCALL_BIT | sc_cobalt_sigreturn
> + */
> +build_restorer(__COBALT_SYSCALL_BIT, sc_cobalt_sigreturn);
> +
> +void *cobalt_get_restorer(void)
> +{
> + return &cobalt_sigreturn;
> +}
Not sure if we need that getter, definitely not as non-inline function
unless you make cobalt_sigreturn static.
> 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;
> +}
Thanks,
Jan
--
Siemens AG, Technology
Linux Expert Center
next prev parent reply other threads:[~2023-09-09 11:35 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-08 10:50 [PATCH 1/3] [POC] test implementaion of rt-signals Johannes Kirchmair
2023-09-08 10:50 ` [PATCH 2/3] [POC] Add rt_signal test Johannes Kirchmair
2023-09-08 10:50 ` [PATCH 3/3] [POC] add a tool to measure rt_signal latency Johannes Kirchmair
2023-09-08 10:54 ` [PATCH 1/3] [POC] test implementaion of rt-signals Johannes Kirchmair
2023-09-09 11:35 ` Jan Kiszka [this message]
-- strict thread matches above, loose matches on Subject: below --
2023-08-16 10:18 Johannes Kirchmair
2023-08-16 11:24 ` Florian Bezdeka
2023-08-16 11:36 ` Jan Kiszka
2023-08-16 11:59 ` Johannes Kirchmair
2023-09-07 10:48 ` Johannes Kirchmair
2023-09-11 8:41 ` Florian Bezdeka
2023-09-01 12:00 ` Jan Kiszka
2023-09-01 13:38 ` Jan Kiszka
2023-09-04 6:55 ` Johannes Kirchmair
2023-09-07 13:39 ` Jan Kiszka
2023-09-07 13:58 ` Johannes Kirchmair
2023-09-01 13:51 ` Jan Kiszka
2023-09-01 14:11 ` Jan Kiszka
2023-09-04 7:04 ` Johannes Kirchmair
2024-03-05 15:54 ` Richard Weinberger
2024-03-05 17:05 ` Jan Kiszka
2024-03-05 17:14 ` Richard Weinberger
2023-05-09 13:13 Johannes Kirchmair
2023-05-09 13:17 ` Johannes Kirchmair
2023-05-12 17:38 ` Jan Kiszka
2023-05-15 6:50 ` Johannes Kirchmair
2023-05-15 10:38 ` Jan Kiszka
2023-05-16 6:46 ` Johannes Kirchmair
2023-05-16 6:52 ` Jan Kiszka
2023-08-09 9:50 ` Schaffner, Tobias
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ec99faa4-c3e5-46ac-9f71-5fc45172964e@siemens.com \
--to=jan.kiszka@siemens.com \
--cc=johannes.kirchmair@sigmatek.at \
--cc=xenomai@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).