[PATCH] add dovtail signal setup function

[PATCH] add dovtail signal setup function

[Johannes Kirchmair]

add an interface to dovetail for signal frame setup and restoring.

This is useful for implementing rt signals within Xenomai.

The interface makes it possible to use the setup use functions provided
by Linux.

This is just a proof of concept using a very naive approach using most
of the code provided by Linux to setup signals.
The patch should be discussed, especially considering if using the
Linux functions is viable in a rt context.

Also to discuss would be the interface itself,
for convenience I used the kernels struct ksignal for testing. But maybe
we should maintain our own struct, that is not that feature rich.
---
 arch/x86/include/asm/sighandling.h |  3 ++
 arch/x86/kernel/signal_32.c        |  2 +-
 arch/x86/kernel/signal_64.c        |  2 +-
 include/linux/dovetail.h           |  4 +++
 kernel/dovetail.c                  | 57 ++++++++++++++++++++++++++++++
 5 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/sighandling.h b/arch/x86/include/asm/sighandling.h
index e770c4fc47f4..ed6b7fcd7250 100644
--- a/arch/x86/include/asm/sighandling.h
+++ b/arch/x86/include/asm/sighandling.h
@@ -24,4 +24,7 @@ int ia32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
 int x64_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
 int x32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
 
+bool ia32_restore_sigcontext(struct pt_regs *regs, struct sigcontext_32 __user *usc);
+bool restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *usc, unsigned long uc_flags);
+
 #endif /* _ASM_X86_SIGHANDLING_H */
diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
index 9027fc088f97..dc8d60378e75 100644
--- a/arch/x86/kernel/signal_32.c
+++ b/arch/x86/kernel/signal_32.c
@@ -74,7 +74,7 @@ static inline void reload_segments(struct sigcontext_32 *sc)
 /*
  * Do a signal return; undo the signal stack.
  */
-static bool ia32_restore_sigcontext(struct pt_regs *regs,
+bool ia32_restore_sigcontext(struct pt_regs *regs,
 				    struct sigcontext_32 __user *usc)
 {
 	struct sigcontext_32 sc;
diff --git a/arch/x86/kernel/signal_64.c b/arch/x86/kernel/signal_64.c
index 13a1e6083837..d24845ee6a8d 100644
--- a/arch/x86/kernel/signal_64.c
+++ b/arch/x86/kernel/signal_64.c
@@ -47,7 +47,7 @@ static void force_valid_ss(struct pt_regs *regs)
 		regs->ss = __USER_DS;
 }
 
-static bool restore_sigcontext(struct pt_regs *regs,
+bool restore_sigcontext(struct pt_regs *regs,
 			       struct sigcontext __user *usc,
 			       unsigned long uc_flags)
 {
diff --git a/include/linux/dovetail.h b/include/linux/dovetail.h
index 70b78fd55d5e..3384437a4c74 100644
--- a/include/linux/dovetail.h
+++ b/include/linux/dovetail.h
@@ -150,6 +150,10 @@ void oob_trampoline(void);
 
 void arch_inband_task_init(struct task_struct *p);
 
+int dovetail_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs);
+
+int dovetail_restore_rt_signal_frame(struct pt_regs *regs);
+
 int dovetail_start(void);
 
 void dovetail_stop(void);
diff --git a/kernel/dovetail.c b/kernel/dovetail.c
index 79fd75918b37..2a61bf1dbc23 100644
--- a/kernel/dovetail.c
+++ b/kernel/dovetail.c
@@ -11,6 +11,9 @@
 #include <asm/syscall.h>
 #include <uapi/asm-generic/dovetail.h>
 
+#include <asm/sighandling.h>
+#include <asm/sigframe.h>
+
 static bool dovetail_enabled;
 
 void __weak arch_inband_task_init(struct task_struct *p)
@@ -447,3 +450,57 @@ void dovetail_stop(void)
 	smp_wmb();
 }
 EXPORT_SYMBOL_GPL(dovetail_stop);
+
+int dovetail_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs)
+{
+	int ret;
+	if (regs->cs == __USER_CS) {
+	        ret = x64_setup_rt_frame(ksig, regs);
+	} else if (regs->cs == __USER32_CS) {
+	        ksig->ka.sa.sa_flags |= SA_IA32_ABI;
+	        ret = ia32_setup_rt_frame(ksig, regs);
+	} else {
+		ret = 1;
+	}
+
+	return ret;
+}
+
+static int dovetail_restore_64_rt_signal_frame(struct pt_regs *regs)
+{
+	struct rt_sigframe __user *frame;
+	unsigned long uc_flags;
+
+	frame = (struct rt_sigframe __user *)(regs->sp - sizeof(long));
+	if (!access_ok(frame, sizeof(*frame)))
+		return -1;
+	if (__get_user(uc_flags, &frame->uc.uc_flags))
+		return -1;
+
+	if (!restore_sigcontext(regs, &frame->uc.uc_mcontext, uc_flags))
+		return -1;
+
+	return regs->ax;
+}
+
+static int dovetail_restore_32_rt_signal_frame(struct pt_regs *regs)
+{
+	struct rt_sigframe_ia32 __user *frame = (struct rt_sigframe_ia32 __user *)(regs->sp - 4);
+
+	if (!access_ok(frame, sizeof(*frame)))
+	        return -1;
+
+	if (ia32_restore_sigcontext(regs, &frame->uc.uc_mcontext) != true)
+	        return -1;
+
+	return regs->ax;
+}
+
+int dovetail_restore_rt_signal_frame(struct pt_regs *regs)
+{
+	if (regs->cs == __USER_CS)
+		return dovetail_restore_64_rt_signal_frame(regs);
+	else
+		return dovetail_restore_32_rt_signal_frame(regs);
+}
+
-- 
2.25.1

Re: [PATCH] add dovtail signal setup function

[Jan Kiszka]

On 09.05.23 15:11, Johannes Kirchmair wrote:
> add an interface to dovetail for signal frame setup and restoring.
> 
> This is useful for implementing rt signals within Xenomai.
> 
> The interface makes it possible to use the setup use functions provided
> by Linux.
> 
> This is just a proof of concept using a very naive approach using most
> of the code provided by Linux to setup signals.
> The patch should be discussed, especially considering if using the
> Linux functions is viable in a rt context.

I didn't find issues in using the Linux services so far. But the code
needs restructuring to move arch-specific bits into the respective arch
code.

There is also the issue with the return value that Tobias found. I think
the dovetail helpers do not need to return the return register, rather
only 0 on success or a negative error code.

> 
> Also to discuss would be the interface itself,
> for convenience I used the kernels struct ksignal for testing. But maybe
> we should maintain our own struct, that is not that feature rich.

Before inventing something new, we would really have to understand that
this is better. Also, some of the functions you reused expect ksignal -
another reason to keep it.

> ---
>  arch/x86/include/asm/sighandling.h |  3 ++
>  arch/x86/kernel/signal_32.c        |  2 +-
>  arch/x86/kernel/signal_64.c        |  2 +-
>  include/linux/dovetail.h           |  4 +++
>  kernel/dovetail.c                  | 57 ++++++++++++++++++++++++++++++
>  5 files changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/sighandling.h b/arch/x86/include/asm/sighandling.h
> index e770c4fc47f4..ed6b7fcd7250 100644
> --- a/arch/x86/include/asm/sighandling.h
> +++ b/arch/x86/include/asm/sighandling.h
> @@ -24,4 +24,7 @@ int ia32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
>  int x64_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
>  int x32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
>  
> +bool ia32_restore_sigcontext(struct pt_regs *regs, struct sigcontext_32 __user *usc);
> +bool restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *usc, unsigned long uc_flags);
> +
>  #endif /* _ASM_X86_SIGHANDLING_H */
> diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
> index 9027fc088f97..dc8d60378e75 100644
> --- a/arch/x86/kernel/signal_32.c
> +++ b/arch/x86/kernel/signal_32.c
> @@ -74,7 +74,7 @@ static inline void reload_segments(struct sigcontext_32 *sc)
>  /*
>   * Do a signal return; undo the signal stack.
>   */
> -static bool ia32_restore_sigcontext(struct pt_regs *regs,
> +bool ia32_restore_sigcontext(struct pt_regs *regs,
>  				    struct sigcontext_32 __user *usc)
>  {
>  	struct sigcontext_32 sc;
> diff --git a/arch/x86/kernel/signal_64.c b/arch/x86/kernel/signal_64.c
> index 13a1e6083837..d24845ee6a8d 100644
> --- a/arch/x86/kernel/signal_64.c
> +++ b/arch/x86/kernel/signal_64.c
> @@ -47,7 +47,7 @@ static void force_valid_ss(struct pt_regs *regs)
>  		regs->ss = __USER_DS;
>  }
>  
> -static bool restore_sigcontext(struct pt_regs *regs,
> +bool restore_sigcontext(struct pt_regs *regs,
>  			       struct sigcontext __user *usc,
>  			       unsigned long uc_flags)
>  {
> diff --git a/include/linux/dovetail.h b/include/linux/dovetail.h
> index 70b78fd55d5e..3384437a4c74 100644
> --- a/include/linux/dovetail.h
> +++ b/include/linux/dovetail.h
> @@ -150,6 +150,10 @@ void oob_trampoline(void);
>  
>  void arch_inband_task_init(struct task_struct *p);
>  
> +int dovetail_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs);
> +
> +int dovetail_restore_rt_signal_frame(struct pt_regs *regs);
> +

I think, this makes a reasonable interface. Its goal would be to
create/restore a Linux-compatible signal frame and hide all those
details from the RT core.

>  int dovetail_start(void);
>  
>  void dovetail_stop(void);
> diff --git a/kernel/dovetail.c b/kernel/dovetail.c
> index 79fd75918b37..2a61bf1dbc23 100644
> --- a/kernel/dovetail.c
> +++ b/kernel/dovetail.c
> @@ -11,6 +11,9 @@
>  #include <asm/syscall.h>
>  #include <uapi/asm-generic/dovetail.h>
>  
> +#include <asm/sighandling.h>
> +#include <asm/sigframe.h>
> +
>  static bool dovetail_enabled;
>  
>  void __weak arch_inband_task_init(struct task_struct *p)
> @@ -447,3 +450,57 @@ void dovetail_stop(void)
>  	smp_wmb();
>  }
>  EXPORT_SYMBOL_GPL(dovetail_stop);
> +
> +int dovetail_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs)
> +{
> +	int ret;
> +	if (regs->cs == __USER_CS) {
> +	        ret = x64_setup_rt_frame(ksig, regs);
> +	} else if (regs->cs == __USER32_CS) {
> +	        ksig->ka.sa.sa_flags |= SA_IA32_ABI;
> +	        ret = ia32_setup_rt_frame(ksig, regs);
> +	} else {
> +		ret = 1;
> +	}
> +
> +	return ret;
> +}
> +
> +static int dovetail_restore_64_rt_signal_frame(struct pt_regs *regs)
> +{
> +	struct rt_sigframe __user *frame;
> +	unsigned long uc_flags;
> +
> +	frame = (struct rt_sigframe __user *)(regs->sp - sizeof(long));
> +	if (!access_ok(frame, sizeof(*frame)))
> +		return -1;
> +	if (__get_user(uc_flags, &frame->uc.uc_flags))
> +		return -1;
> +
> +	if (!restore_sigcontext(regs, &frame->uc.uc_mcontext, uc_flags))
> +		return -1;
> +
> +	return regs->ax;
> +}
> +
> +static int dovetail_restore_32_rt_signal_frame(struct pt_regs *regs)
> +{
> +	struct rt_sigframe_ia32 __user *frame = (struct rt_sigframe_ia32 __user *)(regs->sp - 4);
> +
> +	if (!access_ok(frame, sizeof(*frame)))
> +	        return -1;
> +
> +	if (ia32_restore_sigcontext(regs, &frame->uc.uc_mcontext) != true)
> +	        return -1;
> +
> +	return regs->ax;
> +}
> +
> +int dovetail_restore_rt_signal_frame(struct pt_regs *regs)
> +{
> +	if (regs->cs == __USER_CS)
> +		return dovetail_restore_64_rt_signal_frame(regs);
> +	else
> +		return dovetail_restore_32_rt_signal_frame(regs);
> +}
> +

These need to go into x86. Not sure if there would be anything generic
remaining, likely not.

And then we need code for arm and arm64.

Jan

-- 
Siemens AG, Technology
Linux Expert Center

Re: [PATCH] add dovtail signal setup function

[Florian Bezdeka]

On Wed, 2023-08-16 at 12:18 +0200, Johannes Kirchmair wrote:
> +int dovetail_restore_64_rt_signal_frame(struct pt_regs *regs)
> +{
> +	struct rt_sigframe __user *frame;
> +	unsigned long uc_flags;
> +
> +	frame = (struct rt_sigframe __user *)(regs->sp - sizeof(long));
> +	if (!access_ok(frame, sizeof(*frame)))
> +		return -1;
> +	if (__get_user(uc_flags, &frame->uc.uc_flags))
> +		return -1;
> +
> +	if (!restore_sigcontext(regs, &frame->uc.uc_mcontext, uc_flags))
> +		return -1;
> +
> +	return 0;
> +}
> +

Just a drive-by comment, didn't check all the details yet:

Instead of returning -1 the underlying error (= return the return value
of the function calls) should be reported and/or use proper error
"codes" such as -EINVAL or -EFAULT.

Florian

Re: [PATCH] add dovtail signal setup function

[Jan Kiszka]

On 16.08.23 12:18, Johannes Kirchmair wrote:
> add an interface to dovetail for signal frame setup and restoring.
> 

Subject should clarify that this is x86-only. Normally:

x86: dovetail: ...

But then you are touching generic headers as well. I think we need to
look into arm and arm64 first to decide about the patch structure.

> This is useful for implementing rt signals within Xenomai.
> 
> The interface makes it possible to use the setup use functions provided
> by Linux.
> 
> This is just a proof of concept using a very naive approach using most
> of the code provided by Linux to setup signals.
> The patch should be discussed, especially considering if using the
> Linux functions is viable in a rt context.
> 
> Also to discuss would be the interface itself,
> for convenience I used the kernels struct ksignal for testing. But maybe
> we should maintain our own struct, that is not that feature rich.

Signed-off missing.

> ---
>  arch/x86/include/asm/sighandling.h |  6 ++++++
>  arch/x86/kernel/signal.c           | 22 ++++++++++++++++++++++
>  arch/x86/kernel/signal_32.c        | 13 +++++++++++++
>  arch/x86/kernel/signal_64.c        | 17 +++++++++++++++++
>  include/linux/dovetail.h           |  4 ++++
>  kernel/dovetail.c                  | 14 ++++++++++++++
>  6 files changed, 76 insertions(+)
> 
> diff --git a/arch/x86/include/asm/sighandling.h b/arch/x86/include/asm/sighandling.h
> index e770c4fc47f4..35ffb1c52173 100644
> --- a/arch/x86/include/asm/sighandling.h
> +++ b/arch/x86/include/asm/sighandling.h
> @@ -24,4 +24,10 @@ int ia32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
>  int x64_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
>  int x32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
>  
> +int dovetail_restore_32_rt_signal_frame(struct pt_regs *regs);
> +int dovetail_restore_64_rt_signal_frame(struct pt_regs *regs);
> +
> +int dovetail_arch_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs);
> +int dovetail_arch_restore_rt_signal_frame(struct pt_regs *regs);
> +
>  #endif /* _ASM_X86_SIGHANDLING_H */
> diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
> index cfeec3ee877e..d060931403dc 100644
> --- a/arch/x86/kernel/signal.c
> +++ b/arch/x86/kernel/signal.c
> @@ -353,6 +353,28 @@ void signal_fault(struct pt_regs *regs, void __user *frame, char *where)
>  	force_sig(SIGSEGV);
>  }
>  
> +int dovetail_arch_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs)

Overlong line - try checkpatch.pl.

> +{
> +	int ret;

newline

> +	if (regs->cs == __USER_CS) {
> +	        ret = x64_setup_rt_frame(ksig, regs);
> +	} else if (regs->cs == __USER32_CS) {
> +	        ksig->ka.sa.sa_flags |= SA_IA32_ABI;
> +	        ret = ia32_setup_rt_frame(ksig, regs);
> +	} else {
> +		ret = 1;

What does "1" mean? Is that used by *_setup_rt_frame as well to signal
errors? Is that case possible at all, ie. does kernel code check for the
same condition?

> +	}
> +	return ret;
> +}
> +
> +int dovetail_arch_restore_rt_signal_frame(struct pt_regs *regs)
> +{
> +	if (regs->cs == __USER_CS)
> +		return dovetail_restore_64_rt_signal_frame(regs);
> +	else
> +		return dovetail_restore_32_rt_signal_frame(regs);
> +}
> +
>  #ifdef CONFIG_DYNAMIC_SIGFRAME
>  #ifdef CONFIG_STRICT_SIGALTSTACK_SIZE
>  static bool strict_sigaltstack_size __ro_after_init = true;
> diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
> index 9027fc088f97..a99318a70755 100644
> --- a/arch/x86/kernel/signal_32.c
> +++ b/arch/x86/kernel/signal_32.c
> @@ -145,6 +145,19 @@ SYSCALL32_DEFINE0(sigreturn)
>  	return 0;
>  }
>  
> +int dovetail_restore_32_rt_signal_frame(struct pt_regs *regs)
> +{
> +	struct rt_sigframe_ia32 __user *frame = (struct rt_sigframe_ia32 __user *)(regs->sp - 4);
> +
> +	if (!access_ok(frame, sizeof(*frame)))
> +	        return -1;
> +
> +	if (ia32_restore_sigcontext(regs, &frame->uc.uc_mcontext) != true)
> +	        return -1;
> +
> +	return 0;
> +}
> +
>  SYSCALL32_DEFINE0(rt_sigreturn)
>  {
>  	struct pt_regs *regs = current_pt_regs();
> diff --git a/arch/x86/kernel/signal_64.c b/arch/x86/kernel/signal_64.c
> index 13a1e6083837..67b52bcc5b5e 100644
> --- a/arch/x86/kernel/signal_64.c
> +++ b/arch/x86/kernel/signal_64.c
> @@ -237,6 +237,23 @@ int x64_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs)
>  	return -EFAULT;
>  }
>  
> +int dovetail_restore_64_rt_signal_frame(struct pt_regs *regs)
> +{
> +	struct rt_sigframe __user *frame;
> +	unsigned long uc_flags;
> +
> +	frame = (struct rt_sigframe __user *)(regs->sp - sizeof(long));
> +	if (!access_ok(frame, sizeof(*frame)))
> +		return -1;
> +	if (__get_user(uc_flags, &frame->uc.uc_flags))
> +		return -1;
> +
> +	if (!restore_sigcontext(regs, &frame->uc.uc_mcontext, uc_flags))
> +		return -1;
> +
> +	return 0;
> +}
> +
>  /*
>   * Do a signal return; undo the signal stack.
>   */
> diff --git a/include/linux/dovetail.h b/include/linux/dovetail.h
> index 70b78fd55d5e..3384437a4c74 100644
> --- a/include/linux/dovetail.h
> +++ b/include/linux/dovetail.h
> @@ -150,6 +150,10 @@ void oob_trampoline(void);
>  
>  void arch_inband_task_init(struct task_struct *p);
>  
> +int dovetail_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs);
> +
> +int dovetail_restore_rt_signal_frame(struct pt_regs *regs);
> +
>  int dovetail_start(void);
>  
>  void dovetail_stop(void);
> diff --git a/kernel/dovetail.c b/kernel/dovetail.c
> index 79fd75918b37..de220e90950d 100644
> --- a/kernel/dovetail.c
> +++ b/kernel/dovetail.c
> @@ -11,6 +11,9 @@
>  #include <asm/syscall.h>
>  #include <uapi/asm-generic/dovetail.h>
>  
> +#include <asm/sighandling.h>
> +#include <asm/sigframe.h>
> +
>  static bool dovetail_enabled;
>  
>  void __weak arch_inband_task_init(struct task_struct *p)
> @@ -447,3 +450,14 @@ void dovetail_stop(void)
>  	smp_wmb();
>  }
>  EXPORT_SYMBOL_GPL(dovetail_stop);
> +
> +int dovetail_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs)
> +{
> +	return dovetail_arch_setup_rt_signal_frame(ksig, regs);
> +}
> +
> +int dovetail_restore_rt_signal_frame(struct pt_regs *regs)
> +{
> +	return dovetail_arch_restore_rt_signal_frame(regs);
> +}
> +

Those could become static inlines as well. Or simply let the archs
implement them, like the kernel does as well.

Jan

-- 
Siemens AG, Technology
Linux Expert Center

RE: [PATCH] add dovtail signal setup function

[Johannes Kirchmair]

Hey Jan and Tobias,

this is a new version of the patches I have for rt exception handling in Xenomai.
I changed stuff primarily to fix the bug Tobias found.

@Tobias  thank you again for finding the problem with my last patch series!

I also move some stuff around to have arch specific stuff in  arch/x86.

Freundliche Grüße
Johannes

> -----Original Message-----
> From: Johannes Kirchmair <johannes.kirchmair@sigmatek.at>
> Sent: Mittwoch, 16. August 2023 12:19
> To: xenomai@lists.linux.dev
> Cc: Johannes Kirchmair <johannes.kirchmair@sigmatek.at>
> Subject: [PATCH] add dovtail signal setup function
> 
> add an interface to dovetail for signal frame setup and restoring.
> 
> This is useful for implementing rt signals within Xenomai.
> 
> The interface makes it possible to use the setup use functions provided
> by Linux.
> 
> This is just a proof of concept using a very naive approach using most
> of the code provided by Linux to setup signals.
> The patch should be discussed, especially considering if using the
> Linux functions is viable in a rt context.
> 
> Also to discuss would be the interface itself,
> for convenience I used the kernels struct ksignal for testing. But maybe
> we should maintain our own struct, that is not that feature rich.
> ---
>  arch/x86/include/asm/sighandling.h |  6 ++++++
>  arch/x86/kernel/signal.c           | 22 ++++++++++++++++++++++
>  arch/x86/kernel/signal_32.c        | 13 +++++++++++++
>  arch/x86/kernel/signal_64.c        | 17 +++++++++++++++++
>  include/linux/dovetail.h           |  4 ++++
>  kernel/dovetail.c                  | 14 ++++++++++++++
>  6 files changed, 76 insertions(+)
> 
> diff --git a/arch/x86/include/asm/sighandling.h
> b/arch/x86/include/asm/sighandling.h
> index e770c4fc47f4..35ffb1c52173 100644
> --- a/arch/x86/include/asm/sighandling.h
> +++ b/arch/x86/include/asm/sighandling.h
> @@ -24,4 +24,10 @@ int ia32_setup_rt_frame(struct ksignal *ksig, struct pt_regs
> *regs);
>  int x64_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
>  int x32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
> 
> +int dovetail_restore_32_rt_signal_frame(struct pt_regs *regs);
> +int dovetail_restore_64_rt_signal_frame(struct pt_regs *regs);
> +
> +int dovetail_arch_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs
> *regs);
> +int dovetail_arch_restore_rt_signal_frame(struct pt_regs *regs);
> +
>  #endif /* _ASM_X86_SIGHANDLING_H */
> diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
> index cfeec3ee877e..d060931403dc 100644
> --- a/arch/x86/kernel/signal.c
> +++ b/arch/x86/kernel/signal.c
> @@ -353,6 +353,28 @@ void signal_fault(struct pt_regs *regs, void __user
> *frame, char *where)
>  	force_sig(SIGSEGV);
>  }
> 
> +int dovetail_arch_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs
> *regs)
> +{
> +	int ret;
> +	if (regs->cs == __USER_CS) {
> +	        ret = x64_setup_rt_frame(ksig, regs);
> +	} else if (regs->cs == __USER32_CS) {
> +	        ksig->ka.sa.sa_flags |= SA_IA32_ABI;
> +	        ret = ia32_setup_rt_frame(ksig, regs);
> +	} else {
> +		ret = 1;
> +	}
> +	return ret;
> +}
> +
> +int dovetail_arch_restore_rt_signal_frame(struct pt_regs *regs)
> +{
> +	if (regs->cs == __USER_CS)
> +		return dovetail_restore_64_rt_signal_frame(regs);
> +	else
> +		return dovetail_restore_32_rt_signal_frame(regs);
> +}
> +
>  #ifdef CONFIG_DYNAMIC_SIGFRAME
>  #ifdef CONFIG_STRICT_SIGALTSTACK_SIZE
>  static bool strict_sigaltstack_size __ro_after_init = true;
> diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
> index 9027fc088f97..a99318a70755 100644
> --- a/arch/x86/kernel/signal_32.c
> +++ b/arch/x86/kernel/signal_32.c
> @@ -145,6 +145,19 @@ SYSCALL32_DEFINE0(sigreturn)
>  	return 0;
>  }
> 
> +int dovetail_restore_32_rt_signal_frame(struct pt_regs *regs)
> +{
> +	struct rt_sigframe_ia32 __user *frame = (struct rt_sigframe_ia32 __user
> *)(regs->sp - 4);
> +
> +	if (!access_ok(frame, sizeof(*frame)))
> +	        return -1;
> +
> +	if (ia32_restore_sigcontext(regs, &frame->uc.uc_mcontext) != true)
> +	        return -1;
> +
> +	return 0;
> +}
> +
>  SYSCALL32_DEFINE0(rt_sigreturn)
>  {
>  	struct pt_regs *regs = current_pt_regs();
> diff --git a/arch/x86/kernel/signal_64.c b/arch/x86/kernel/signal_64.c
> index 13a1e6083837..67b52bcc5b5e 100644
> --- a/arch/x86/kernel/signal_64.c
> +++ b/arch/x86/kernel/signal_64.c
> @@ -237,6 +237,23 @@ int x64_setup_rt_frame(struct ksignal *ksig, struct
> pt_regs *regs)
>  	return -EFAULT;
>  }
> 
> +int dovetail_restore_64_rt_signal_frame(struct pt_regs *regs)
> +{
> +	struct rt_sigframe __user *frame;
> +	unsigned long uc_flags;
> +
> +	frame = (struct rt_sigframe __user *)(regs->sp - sizeof(long));
> +	if (!access_ok(frame, sizeof(*frame)))
> +		return -1;
> +	if (__get_user(uc_flags, &frame->uc.uc_flags))
> +		return -1;
> +
> +	if (!restore_sigcontext(regs, &frame->uc.uc_mcontext, uc_flags))
> +		return -1;
> +
> +	return 0;
> +}
> +
>  /*
>   * Do a signal return; undo the signal stack.
>   */
> diff --git a/include/linux/dovetail.h b/include/linux/dovetail.h
> index 70b78fd55d5e..3384437a4c74 100644
> --- a/include/linux/dovetail.h
> +++ b/include/linux/dovetail.h
> @@ -150,6 +150,10 @@ void oob_trampoline(void);
> 
>  void arch_inband_task_init(struct task_struct *p);
> 
> +int dovetail_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs);
> +
> +int dovetail_restore_rt_signal_frame(struct pt_regs *regs);
> +
>  int dovetail_start(void);
> 
>  void dovetail_stop(void);
> diff --git a/kernel/dovetail.c b/kernel/dovetail.c
> index 79fd75918b37..de220e90950d 100644
> --- a/kernel/dovetail.c
> +++ b/kernel/dovetail.c
> @@ -11,6 +11,9 @@
>  #include <asm/syscall.h>
>  #include <uapi/asm-generic/dovetail.h>
> 
> +#include <asm/sighandling.h>
> +#include <asm/sigframe.h>
> +
>  static bool dovetail_enabled;
> 
>  void __weak arch_inband_task_init(struct task_struct *p)
> @@ -447,3 +450,14 @@ void dovetail_stop(void)
>  	smp_wmb();
>  }
>  EXPORT_SYMBOL_GPL(dovetail_stop);
> +
> +int dovetail_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs)
> +{
> +	return dovetail_arch_setup_rt_signal_frame(ksig, regs);
> +}
> +
> +int dovetail_restore_rt_signal_frame(struct pt_regs *regs)
> +{
> +	return dovetail_arch_restore_rt_signal_frame(regs);
> +}
> +
> --
> 2.25.1

[PATCH] add dovtail signal setup function

[Johannes Kirchmair]

add an interface to dovetail for signal frame setup and restoring.

This is useful for implementing rt signals within Xenomai.

The interface makes it possible to use the setup use functions provided
by Linux.

This is just a proof of concept using a very naive approach using most
of the code provided by Linux to setup signals.
The patch should be discussed, especially considering if using the
Linux functions is viable in a rt context.

Also to discuss would be the interface itself,
for convenience I used the kernels struct ksignal for testing. But maybe
we should maintain our own struct, that is not that feature rich.
---
 arch/x86/include/asm/sighandling.h |  6 ++++++
 arch/x86/kernel/signal.c           | 22 ++++++++++++++++++++++
 arch/x86/kernel/signal_32.c        | 13 +++++++++++++
 arch/x86/kernel/signal_64.c        | 17 +++++++++++++++++
 include/linux/dovetail.h           |  4 ++++
 kernel/dovetail.c                  | 14 ++++++++++++++
 6 files changed, 76 insertions(+)

diff --git a/arch/x86/include/asm/sighandling.h b/arch/x86/include/asm/sighandling.h
index e770c4fc47f4..35ffb1c52173 100644
--- a/arch/x86/include/asm/sighandling.h
+++ b/arch/x86/include/asm/sighandling.h
@@ -24,4 +24,10 @@ int ia32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
 int x64_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
 int x32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
 
+int dovetail_restore_32_rt_signal_frame(struct pt_regs *regs);
+int dovetail_restore_64_rt_signal_frame(struct pt_regs *regs);
+
+int dovetail_arch_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs);
+int dovetail_arch_restore_rt_signal_frame(struct pt_regs *regs);
+
 #endif /* _ASM_X86_SIGHANDLING_H */
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index cfeec3ee877e..d060931403dc 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -353,6 +353,28 @@ void signal_fault(struct pt_regs *regs, void __user *frame, char *where)
 	force_sig(SIGSEGV);
 }
 
+int dovetail_arch_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs)
+{
+	int ret;
+	if (regs->cs == __USER_CS) {
+	        ret = x64_setup_rt_frame(ksig, regs);
+	} else if (regs->cs == __USER32_CS) {
+	        ksig->ka.sa.sa_flags |= SA_IA32_ABI;
+	        ret = ia32_setup_rt_frame(ksig, regs);
+	} else {
+		ret = 1;
+	}
+	return ret;
+}
+
+int dovetail_arch_restore_rt_signal_frame(struct pt_regs *regs)
+{
+	if (regs->cs == __USER_CS)
+		return dovetail_restore_64_rt_signal_frame(regs);
+	else
+		return dovetail_restore_32_rt_signal_frame(regs);
+}
+
 #ifdef CONFIG_DYNAMIC_SIGFRAME
 #ifdef CONFIG_STRICT_SIGALTSTACK_SIZE
 static bool strict_sigaltstack_size __ro_after_init = true;
diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
index 9027fc088f97..a99318a70755 100644
--- a/arch/x86/kernel/signal_32.c
+++ b/arch/x86/kernel/signal_32.c
@@ -145,6 +145,19 @@ SYSCALL32_DEFINE0(sigreturn)
 	return 0;
 }
 
+int dovetail_restore_32_rt_signal_frame(struct pt_regs *regs)
+{
+	struct rt_sigframe_ia32 __user *frame = (struct rt_sigframe_ia32 __user *)(regs->sp - 4);
+
+	if (!access_ok(frame, sizeof(*frame)))
+	        return -1;
+
+	if (ia32_restore_sigcontext(regs, &frame->uc.uc_mcontext) != true)
+	        return -1;
+
+	return 0;
+}
+
 SYSCALL32_DEFINE0(rt_sigreturn)
 {
 	struct pt_regs *regs = current_pt_regs();
diff --git a/arch/x86/kernel/signal_64.c b/arch/x86/kernel/signal_64.c
index 13a1e6083837..67b52bcc5b5e 100644
--- a/arch/x86/kernel/signal_64.c
+++ b/arch/x86/kernel/signal_64.c
@@ -237,6 +237,23 @@ int x64_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs)
 	return -EFAULT;
 }
 
+int dovetail_restore_64_rt_signal_frame(struct pt_regs *regs)
+{
+	struct rt_sigframe __user *frame;
+	unsigned long uc_flags;
+
+	frame = (struct rt_sigframe __user *)(regs->sp - sizeof(long));
+	if (!access_ok(frame, sizeof(*frame)))
+		return -1;
+	if (__get_user(uc_flags, &frame->uc.uc_flags))
+		return -1;
+
+	if (!restore_sigcontext(regs, &frame->uc.uc_mcontext, uc_flags))
+		return -1;
+
+	return 0;
+}
+
 /*
  * Do a signal return; undo the signal stack.
  */
diff --git a/include/linux/dovetail.h b/include/linux/dovetail.h
index 70b78fd55d5e..3384437a4c74 100644
--- a/include/linux/dovetail.h
+++ b/include/linux/dovetail.h
@@ -150,6 +150,10 @@ void oob_trampoline(void);
 
 void arch_inband_task_init(struct task_struct *p);
 
+int dovetail_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs);
+
+int dovetail_restore_rt_signal_frame(struct pt_regs *regs);
+
 int dovetail_start(void);
 
 void dovetail_stop(void);
diff --git a/kernel/dovetail.c b/kernel/dovetail.c
index 79fd75918b37..de220e90950d 100644
--- a/kernel/dovetail.c
+++ b/kernel/dovetail.c
@@ -11,6 +11,9 @@
 #include <asm/syscall.h>
 #include <uapi/asm-generic/dovetail.h>
 
+#include <asm/sighandling.h>
+#include <asm/sigframe.h>
+
 static bool dovetail_enabled;
 
 void __weak arch_inband_task_init(struct task_struct *p)
@@ -447,3 +450,14 @@ void dovetail_stop(void)
 	smp_wmb();
 }
 EXPORT_SYMBOL_GPL(dovetail_stop);
+
+int dovetail_setup_rt_signal_frame(struct ksignal *ksig, struct pt_regs *regs)
+{
+	return dovetail_arch_setup_rt_signal_frame(ksig, regs);
+}
+
+int dovetail_restore_rt_signal_frame(struct pt_regs *regs)
+{
+	return dovetail_arch_restore_rt_signal_frame(regs);
+}
+
-- 
2.25.1