Add some syscall arch functions to support arm64 to use generic syscall
code, which do not affect existing architectures that use generic entry:

 - arch_pre/post_report_syscall_entry/exit().

Also make syscall_exit_work() not static and move report_single_step() to
thread_info.h, which can be used by arm64 later.

Suggested-by: Mark Rutland <mark.rutland@arm.com>
Suggested-by: Kevin Brodsky <kevin.brodsky@arm.com>
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 include/linux/entry-common.h  |   1 +
 include/linux/thread_info.h   |  13 +++++
 kernel/entry/syscall-common.c | 100 ++++++++++++++++++++++++++++++----
 3 files changed, 103 insertions(+), 11 deletions(-)

diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
index 1ae3143d4b12..39a2d41af05e 100644
--- a/include/linux/entry-common.h
+++ b/include/linux/entry-common.h
@@ -178,4 +178,5 @@ void syscall_exit_to_user_mode_work(struct pt_regs *regs);
  */
 void syscall_exit_to_user_mode(struct pt_regs *regs);
 
+void syscall_exit_work(struct pt_regs *regs, unsigned long work);
 #endif
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 9ea0b28068f4..062de9666ef3 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -55,6 +55,19 @@ enum syscall_work_bit {
 #define SYSCALL_WORK_SYSCALL_AUDIT	BIT(SYSCALL_WORK_BIT_SYSCALL_AUDIT)
 #define SYSCALL_WORK_SYSCALL_USER_DISPATCH BIT(SYSCALL_WORK_BIT_SYSCALL_USER_DISPATCH)
 #define SYSCALL_WORK_SYSCALL_EXIT_TRAP	BIT(SYSCALL_WORK_BIT_SYSCALL_EXIT_TRAP)
+
+/*
+ * If SYSCALL_EMU is set, then the only reason to report is when
+ * SINGLESTEP is set (i.e. PTRACE_SYSEMU_SINGLESTEP).  This syscall
+ * instruction has been already reported in syscall_enter_from_user_mode().
+ */
+static inline bool report_single_step(unsigned long work)
+{
+	if (work & SYSCALL_WORK_SYSCALL_EMU)
+		return false;
+
+	return work & SYSCALL_WORK_SYSCALL_EXIT_TRAP;
+}
 #endif
 
 #include <asm/thread_info.h>
diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c
index 0eb036986ad4..73f87d09e04e 100644
--- a/kernel/entry/syscall-common.c
+++ b/kernel/entry/syscall-common.c
@@ -17,6 +17,49 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
 	}
 }
 
+/**
+ * arch_pre_report_syscall_entry - Architecture specific work before
+ *				   report_syscall_entry().
+ *
+ * Invoked from syscall_trace_enter() to prepare for ptrace_report_syscall_entry().
+ * Defaults to NOP.
+ *
+ * The main purpose is for saving a general purpose register clobbered
+ * in the tracee.
+ */
+static inline unsigned long arch_pre_report_syscall_entry(struct pt_regs *regs);
+
+#ifndef arch_pre_report_syscall_entry
+static inline unsigned long arch_pre_report_syscall_entry(struct pt_regs *regs)
+{
+	return 0;
+}
+#endif
+
+/**
+ * arch_post_report_syscall_entry - Architecture specific work after
+ *			            report_syscall_entry().
+ *
+ * Invoked from syscall_trace_enter() after calling ptrace_report_syscall_entry().
+ * Defaults to NOP.
+ *
+ * The main purpose is for restoring a general purpose register clobbered
+ * in the trace saved in arch_pre_report_syscall_entry(), also it can
+ * do something arch-specific according to the return value of
+ * ptrace_report_syscall_entry().
+ */
+static inline void arch_post_report_syscall_entry(struct pt_regs *regs,
+						  unsigned long saved_reg,
+						  long ret);
+
+#ifndef arch_post_report_syscall_entry
+static inline void arch_post_report_syscall_entry(struct pt_regs *regs,
+						  unsigned long saved_reg,
+						  long ret)
+{
+}
+#endif
+
 long syscall_trace_enter(struct pt_regs *regs, long syscall,
 				unsigned long work)
 {
@@ -34,7 +77,9 @@ long syscall_trace_enter(struct pt_regs *regs, long syscall,
 
 	/* Handle ptrace */
 	if (work & (SYSCALL_WORK_SYSCALL_TRACE | SYSCALL_WORK_SYSCALL_EMU)) {
+		unsigned long saved_reg = arch_pre_report_syscall_entry(regs);
 		ret = ptrace_report_syscall_entry(regs);
+		arch_post_report_syscall_entry(regs, saved_reg, ret);
 		if (ret || (work & SYSCALL_WORK_SYSCALL_EMU))
 			return -1L;
 	}
@@ -71,20 +116,50 @@ noinstr void syscall_enter_from_user_mode_prepare(struct pt_regs *regs)
 	instrumentation_end();
 }
 
-/*
- * If SYSCALL_EMU is set, then the only reason to report is when
- * SINGLESTEP is set (i.e. PTRACE_SYSEMU_SINGLESTEP).  This syscall
- * instruction has been already reported in syscall_enter_from_user_mode().
+/**
+ * arch_pre_report_syscall_exit - Architecture specific work before
+ *				  report_syscall_exit().
+ *
+ * Invoked from syscall_exit_work() to prepare for ptrace_report_syscall_exit().
+ * Defaults to NOP.
+ *
+ * The main purpose is for saving a general purpose register clobbered
+ * in the trace.
  */
-static inline bool report_single_step(unsigned long work)
-{
-	if (work & SYSCALL_WORK_SYSCALL_EMU)
-		return false;
+static inline unsigned long arch_pre_report_syscall_exit(struct pt_regs *regs,
+							     unsigned long work);
 
-	return work & SYSCALL_WORK_SYSCALL_EXIT_TRAP;
+#ifndef arch_pre_report_syscall_exit
+static inline unsigned long arch_pre_report_syscall_exit(struct pt_regs *regs,
+							     unsigned long work)
+{
+	return 0;
 }
+#endif
+
+/**
+ * arch_post_report_syscall_exit - Architecture specific work after
+ *			           report_syscall_exit().
+ *
+ * Invoked from syscall_exit_work() after calling ptrace_report_syscall_exit().
+ * Defaults to NOP.
+ *
+ * The main purpose is for restoring a general purpose register clobbered
+ * in the trace saved in arch_pre_report_syscall_exit().
+ */
+static inline void arch_post_report_syscall_exit(struct pt_regs *regs,
+						 unsigned long saved_reg,
+						 unsigned long work);
+
+#ifndef arch_post_report_syscall_exit
+static inline void arch_post_report_syscall_exit(struct pt_regs *regs,
+						 unsigned long saved_reg,
+						 unsigned long work)
+{
+}
+#endif
 
-static void syscall_exit_work(struct pt_regs *regs, unsigned long work)
+void syscall_exit_work(struct pt_regs *regs, unsigned long work)
 {
 	bool step;
 
@@ -107,8 +182,11 @@ static void syscall_exit_work(struct pt_regs *regs, unsigned long work)
 		trace_sys_exit(regs, syscall_get_return_value(current, regs));
 
 	step = report_single_step(work);
-	if (step || work & SYSCALL_WORK_SYSCALL_TRACE)
+	if (step || work & SYSCALL_WORK_SYSCALL_TRACE) {
+		unsigned long saved_reg = arch_pre_report_syscall_exit(regs, work);
 		ptrace_report_syscall_exit(regs, step);
+		arch_post_report_syscall_exit(regs, saved_reg, work);
+	}
 }
 
 /*
-- 
2.34.1

On Fri, Oct 25 2024 at 18:06, Jinjie Ruan wrote:

$Subject: Can you please make this simply:

    entry: Add arch_pre/post_report_syscall_entry/exit()

> Add some syscall arch functions to support arm64 to use generic syscall
> code, which do not affect existing architectures that use generic entry:
>
>  - arch_pre/post_report_syscall_entry/exit().

> Also make syscall_exit_work() not static and move report_single_step() to
> thread_info.h, which can be used by arm64 later.

This does way too many things which have nothing to do with the subject
line.

>  long syscall_trace_enter(struct pt_regs *regs, long syscall,
>  				unsigned long work)
>  {
> @@ -34,7 +77,9 @@ long syscall_trace_enter(struct pt_regs *regs, long syscall,
>  
>  	/* Handle ptrace */
>  	if (work & (SYSCALL_WORK_SYSCALL_TRACE | SYSCALL_WORK_SYSCALL_EMU)) {
> +		unsigned long saved_reg = arch_pre_report_syscall_entry(regs);

Lacks a new line between declaration and code.

>  		ret = ptrace_report_syscall_entry(regs);
> +		arch_post_report_syscall_entry(regs, saved_reg, ret);

Though I'm not sure whether these pre/post hooks buy anything. It's
probably simpler to do:

-  		ret = ptrace_report_syscall_entry(regs);
+		ret = arch_ptrace_report_syscall_entry(regs);

And have the default implementation as

        return ptrace_report_syscall_entry(regs);

and let ARM64 implement it's magic around it in the architecture
header. The actual ptrace_report_syscall_entry() is simple enough to be
in both places. That reduces the inflation of architecture specific
helpers and keeps the code tidy.

Thanks,

        tglx