1. Patchew
  2. linux
  3. [PATCH v3 0/3] arm64: entry: Convert to generic entry
  4. View patch

[PATCH v3 1/3] entry: Add some arch funcs to support arm64 to use generic entry

Jinjie Ruan posted 3 patches 1 year, 5 months ago
There is a newer version of this series
[PATCH v3 1/3] entry: Add some arch funcs to support arm64 to use generic entry
Posted by Jinjie Ruan 1 year, 5 months ago 
Add some arch functions to support arm64 to use generic entry, which do not
affect existing architectures that use generic entry:

 - arch_prepare/post_report_syscall_entry/exit().

 - arch_enter_from_kernel_mode(), arch_exit_to_kernel_mode_prepare().

 - arch_irqentry_exit_need_resched() to support architecture-related
   need_resched() check logic.

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

x86 and Riscv compilation test ok after this patch.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
---
v3:
- Make the arch funcs not use __weak as Thomas suggested.
- Make arch_forget_syscall() folded in arch_post_report_syscall_entry().
- __always_inline -> inline.
- Move report_single_step() to thread_info.h for arm64
- Add Suggested-by.
- Update the commit message.

v2:
- Fix a bug that not call arch_post_report_syscall_entry() in
  syscall_trace_enter() if ptrace_report_syscall_entry() return not zero.
- Update the commit message.
---
 include/linux/entry-common.h | 90 ++++++++++++++++++++++++++++++++++++
 include/linux/thread_info.h  | 13 ++++++
 kernel/entry/common.c        | 37 +++++++--------
 3 files changed, 122 insertions(+), 18 deletions(-)

diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
index b0fb775a600d..2aea23ca9d66 100644
--- a/include/linux/entry-common.h
+++ b/include/linux/entry-common.h
@@ -290,6 +290,94 @@ static __always_inline void arch_exit_to_user_mode(void);
 static __always_inline void arch_exit_to_user_mode(void) { }
 #endif
 
+/**
+ * arch_enter_from_kernel_mode - Architecture specific check work.
+ */
+static inline void arch_enter_from_kernel_mode(struct pt_regs *regs);
+
+#ifndef arch_enter_from_kernel_mode
+static inline void arch_enter_from_kernel_mode(struct pt_regs *regs) { }
+#endif
+
+/**
+ * arch_exit_to_kernel_mode_prepare - Architecture specific final work before
+ *				      exit to kernel mode.
+ */
+static inline void arch_exit_to_kernel_mode_prepare(struct pt_regs *regs);
+
+#ifndef arch_exit_to_kernel_mode_prepare
+static inline void arch_exit_to_kernel_mode_prepare(struct pt_regs *regs) { }
+#endif
+
+/**
+ * arch_prepare_report_syscall_entry - Architecture specific work before
+ *			               report_syscall_entry().
+ */
+static inline unsigned long arch_prepare_report_syscall_entry(struct pt_regs *regs);
+
+#ifndef arch_prepare_report_syscall_entry
+static inline unsigned long arch_prepare_report_syscall_entry(struct pt_regs *regs)
+{
+	return 0;
+}
+#endif
+
+/**
+ * arch_post_report_syscall_entry - Architecture specific work after
+ *			            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
+
+/**
+ * arch_prepare_report_syscall_exit - Architecture specific work before
+ *			              report_syscall_exit().
+ */
+static inline unsigned long arch_prepare_report_syscall_exit(struct pt_regs *regs,
+							     unsigned long work);
+
+#ifndef arch_prepare_report_syscall_exit
+static inline unsigned long arch_prepare_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().
+ */
+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
+
+/**
+ * arch_irqentry_exit_need_resched - Architecture specific need resched function
+ */
+static inline bool arch_irqentry_exit_need_resched(void);
+
+#ifndef arch_irqentry_exit_need_resched
+static inline bool arch_irqentry_exit_need_resched(void) { return true; }
+#endif
+
 /**
  * arch_do_signal_or_restart -  Architecture specific signal delivery function
  * @regs:	Pointer to currents pt_regs
@@ -552,4 +640,6 @@ irqentry_state_t noinstr irqentry_nmi_enter(struct pt_regs *regs);
  */
 void noinstr irqentry_nmi_exit(struct pt_regs *regs, irqentry_state_t irq_state);
 
+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/common.c b/kernel/entry/common.c
index 90843cc38588..cd76391ffcb9 100644
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -28,6 +28,7 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
 long syscall_trace_enter(struct pt_regs *regs, long syscall,
 				unsigned long work)
 {
+	unsigned long saved_reg;
 	long ret = 0;
 
 	/*
@@ -42,8 +43,10 @@ long syscall_trace_enter(struct pt_regs *regs, long syscall,
 
 	/* Handle ptrace */
 	if (work & (SYSCALL_WORK_SYSCALL_TRACE | SYSCALL_WORK_SYSCALL_EMU)) {
+		saved_reg = arch_prepare_report_syscall_entry(regs);
 		ret = ptrace_report_syscall_entry(regs);
-		if (ret || (work & SYSCALL_WORK_SYSCALL_EMU))
+		arch_post_report_syscall_entry(regs, saved_reg, ret);
+		if (ret || work & SYSCALL_WORK_SYSCALL_EMU)
 			return -1L;
 	}
 
@@ -133,21 +136,9 @@ __always_inline unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
 	return ti_work;
 }
 
-/*
- * 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;
-}
-
-static void syscall_exit_work(struct pt_regs *regs, unsigned long work)
+void syscall_exit_work(struct pt_regs *regs, unsigned long work)
 {
+	unsigned long saved_reg;
 	bool step;
 
 	/*
@@ -169,8 +160,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) {
+		saved_reg = arch_prepare_report_syscall_exit(regs, work);
 		ptrace_report_syscall_exit(regs, step);
+		arch_post_report_syscall_exit(regs, saved_reg, work);
+	}
 }
 
 /*
@@ -244,6 +238,8 @@ noinstr irqentry_state_t irqentry_enter(struct pt_regs *regs)
 		return ret;
 	}
 
+	arch_enter_from_kernel_mode(regs);
+
 	/*
 	 * If this entry hit the idle task invoke ct_irq_enter() whether
 	 * RCU is watching or not.
@@ -307,7 +303,7 @@ void raw_irqentry_exit_cond_resched(void)
 		rcu_irq_exit_check_preempt();
 		if (IS_ENABLED(CONFIG_DEBUG_ENTRY))
 			WARN_ON_ONCE(!on_thread_stack());
-		if (need_resched())
+		if (need_resched() && arch_irqentry_exit_need_resched())
 			preempt_schedule_irq();
 	}
 }
@@ -332,7 +328,12 @@ noinstr void irqentry_exit(struct pt_regs *regs, irqentry_state_t state)
 	/* Check whether this returns to user mode */
 	if (user_mode(regs)) {
 		irqentry_exit_to_user_mode(regs);
-	} else if (!regs_irqs_disabled(regs)) {
+		return;
+	}
+
+	arch_exit_to_kernel_mode_prepare(regs);
+
+	if (!regs_irqs_disabled(regs)) {
 		/*
 		 * If RCU was not watching on entry this needs to be done
 		 * carefully and needs the same ordering of lockdep/tracing
-- 
2.34.1
Re: [PATCH v3 1/3] entry: Add some arch funcs to support arm64 to use generic entry
Posted by Kevin Brodsky 1 year, 4 months ago 
On 29/06/2024 10:55, Jinjie Ruan wrote:
> Add some arch functions to support arm64 to use generic entry, which do not
> affect existing architectures that use generic entry:
>
>  - arch_prepare/post_report_syscall_entry/exit().
>
>  - arch_enter_from_kernel_mode(), arch_exit_to_kernel_mode_prepare().
>
>  - arch_irqentry_exit_need_resched() to support architecture-related
>    need_resched() check logic.
>
> Also make syscall_exit_work() not static and move report_single_step() to
> thread_info.h, which can be used by arm64 later.
>
> x86 and Riscv compilation test ok after this patch.
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> v3:
> - Make the arch funcs not use __weak as Thomas suggested.
> - Make arch_forget_syscall() folded in arch_post_report_syscall_entry().
> - __always_inline -> inline.
> - Move report_single_step() to thread_info.h for arm64
> - Add Suggested-by.
> - Update the commit message.
>
> v2:
> - Fix a bug that not call arch_post_report_syscall_entry() in
>   syscall_trace_enter() if ptrace_report_syscall_entry() return not zero.
> - Update the commit message.
> ---
>  include/linux/entry-common.h | 90 ++++++++++++++++++++++++++++++++++++
>  include/linux/thread_info.h  | 13 ++++++
>  kernel/entry/common.c        | 37 +++++++--------
>  3 files changed, 122 insertions(+), 18 deletions(-)
>
> diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
> index b0fb775a600d..2aea23ca9d66 100644
> --- a/include/linux/entry-common.h
> +++ b/include/linux/entry-common.h
> @@ -290,6 +290,94 @@ static __always_inline void arch_exit_to_user_mode(void);
>  static __always_inline void arch_exit_to_user_mode(void) { }
>  #endif
>  
> +/**
> + * arch_enter_from_kernel_mode - Architecture specific check work.

Maybe those comments could be expanded to be closer to the existing
ones, like arch_enter_from_user_mode()? It would help if they were more
specific as to where they are called (especially the *report_syscall*
ones) and how they are expected to be used.

> + */
> +static inline void arch_enter_from_kernel_mode(struct pt_regs *regs);
> +
> +#ifndef arch_enter_from_kernel_mode
> +static inline void arch_enter_from_kernel_mode(struct pt_regs *regs) { }
> +#endif
> +
> +/**
> + * arch_exit_to_kernel_mode_prepare - Architecture specific final work before
> + *				      exit to kernel mode.
> + */
> +static inline void arch_exit_to_kernel_mode_prepare(struct pt_regs *regs);

Any reason to suffix this function with "prepare"? Just
arch_exit_to_kernel_mode() seems appropriate (symmetric with
arch_enter_from_kernel_mode()).

> +
> +#ifndef arch_exit_to_kernel_mode_prepare
> +static inline void arch_exit_to_kernel_mode_prepare(struct pt_regs *regs) { }
> +#endif
> +
> +/**
> + * arch_prepare_report_syscall_entry - Architecture specific work before
> + *			               report_syscall_entry().
> + */
> +static inline unsigned long arch_prepare_report_syscall_entry(struct pt_regs *regs);

The most common naming patterns for such arch helper pairs seems to be
pre/post, so maybe arch_pre_report_syscall_entry()?

> +
> +#ifndef arch_prepare_report_syscall_entry
> +static inline unsigned long arch_prepare_report_syscall_entry(struct pt_regs *regs)
> +{
> +	return 0;
> +}
> +#endif
> +
> +/**
> + * arch_post_report_syscall_entry - Architecture specific work after
> + *			            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
> +
> +/**
> + * arch_prepare_report_syscall_exit - Architecture specific work before
> + *			              report_syscall_exit().
> + */
> +static inline unsigned long arch_prepare_report_syscall_exit(struct pt_regs *regs,
> +							     unsigned long work);
> +
> +#ifndef arch_prepare_report_syscall_exit
> +static inline unsigned long arch_prepare_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().
> + */
> +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
> +
> +/**
> + * arch_irqentry_exit_need_resched - Architecture specific need resched function
> + */
> +static inline bool arch_irqentry_exit_need_resched(void);
> +
> +#ifndef arch_irqentry_exit_need_resched
> +static inline bool arch_irqentry_exit_need_resched(void) { return true; }
> +#endif

Nit: the existing arch_* hooks seem to be declared close to the function
they are called from (e.g. arch_enter_from_user_mode() just before
enter_from_user_mode()), maybe we could do the same with those new
hooks, where possible.

> +
>  /**
>   * arch_do_signal_or_restart -  Architecture specific signal delivery function
>   * @regs:	Pointer to currents pt_regs
> @@ -552,4 +640,6 @@ irqentry_state_t noinstr irqentry_nmi_enter(struct pt_regs *regs);
>   */
>  void noinstr irqentry_nmi_exit(struct pt_regs *regs, irqentry_state_t irq_state);
>  
> +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/common.c b/kernel/entry/common.c
> index 90843cc38588..cd76391ffcb9 100644
> --- a/kernel/entry/common.c
> +++ b/kernel/entry/common.c
> @@ -28,6 +28,7 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
>  long syscall_trace_enter(struct pt_regs *regs, long syscall,
>  				unsigned long work)
>  {
> +	unsigned long saved_reg;

Nit: could be declared inside the if block.

Kevin

>  	long ret = 0;
>  
>  	/*
> @@ -42,8 +43,10 @@ long syscall_trace_enter(struct pt_regs *regs, long syscall,
>  
>  	/* Handle ptrace */
>  	if (work & (SYSCALL_WORK_SYSCALL_TRACE | SYSCALL_WORK_SYSCALL_EMU)) {
> +		saved_reg = arch_prepare_report_syscall_entry(regs);
>  		ret = ptrace_report_syscall_entry(regs);
> -		if (ret || (work & SYSCALL_WORK_SYSCALL_EMU))
> +		arch_post_report_syscall_entry(regs, saved_reg, ret);
> +		if (ret || work & SYSCALL_WORK_SYSCALL_EMU)
>  			return -1L;
>  	}
>  
> [...]
Re: [PATCH v3 1/3] entry: Add some arch funcs to support arm64 to use generic entry
Posted by Jinjie Ruan 1 year, 3 months ago 

On 2024/8/20 19:41, Kevin Brodsky wrote:
> On 29/06/2024 10:55, Jinjie Ruan wrote:
>> Add some arch functions to support arm64 to use generic entry, which do not
>> affect existing architectures that use generic entry:
>>
>>  - arch_prepare/post_report_syscall_entry/exit().
>>
>>  - arch_enter_from_kernel_mode(), arch_exit_to_kernel_mode_prepare().
>>
>>  - arch_irqentry_exit_need_resched() to support architecture-related
>>    need_resched() check logic.
>>
>> Also make syscall_exit_work() not static and move report_single_step() to
>> thread_info.h, which can be used by arm64 later.
>>
>> x86 and Riscv compilation test ok after this patch.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
>> ---
>> v3:
>> - Make the arch funcs not use __weak as Thomas suggested.
>> - Make arch_forget_syscall() folded in arch_post_report_syscall_entry().
>> - __always_inline -> inline.
>> - Move report_single_step() to thread_info.h for arm64
>> - Add Suggested-by.
>> - Update the commit message.
>>
>> v2:
>> - Fix a bug that not call arch_post_report_syscall_entry() in
>>   syscall_trace_enter() if ptrace_report_syscall_entry() return not zero.
>> - Update the commit message.
>> ---
>>  include/linux/entry-common.h | 90 ++++++++++++++++++++++++++++++++++++
>>  include/linux/thread_info.h  | 13 ++++++
>>  kernel/entry/common.c        | 37 +++++++--------
>>  3 files changed, 122 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
>> index b0fb775a600d..2aea23ca9d66 100644
>> --- a/include/linux/entry-common.h
>> +++ b/include/linux/entry-common.h
>> @@ -290,6 +290,94 @@ static __always_inline void arch_exit_to_user_mode(void);
>>  static __always_inline void arch_exit_to_user_mode(void) { }
>>  #endif
>>  
>> +/**
>> + * arch_enter_from_kernel_mode - Architecture specific check work.
> 
> Maybe those comments could be expanded to be closer to the existing
> ones, like arch_enter_from_user_mode()? It would help if they were more
> specific as to where they are called (especially the *report_syscall*
> ones) and how they are expected to be used.

You are right! It needed to be expanded.

> 
>> + */
>> +static inline void arch_enter_from_kernel_mode(struct pt_regs *regs);
>> +
>> +#ifndef arch_enter_from_kernel_mode
>> +static inline void arch_enter_from_kernel_mode(struct pt_regs *regs) { }
>> +#endif
>> +
>> +/**
>> + * arch_exit_to_kernel_mode_prepare - Architecture specific final work before
>> + *				      exit to kernel mode.
>> + */
>> +static inline void arch_exit_to_kernel_mode_prepare(struct pt_regs *regs);
> 
> Any reason to suffix this function with "prepare"? Just
> arch_exit_to_kernel_mode() seems appropriate (symmetric with
> arch_enter_from_kernel_mode()).

prepare means it is the first function before all other exit_to_kernel
operation in irqentry_exit(), but as the order problem, it can be
adjusted to the last to aligh with the older arm64 version.

> 
>> +
>> +#ifndef arch_exit_to_kernel_mode_prepare
>> +static inline void arch_exit_to_kernel_mode_prepare(struct pt_regs *regs) { }
>> +#endif
>> +
>> +/**
>> + * arch_prepare_report_syscall_entry - Architecture specific work before
>> + *			               report_syscall_entry().
>> + */
>> +static inline unsigned long arch_prepare_report_syscall_entry(struct pt_regs *regs);
> 
> The most common naming patterns for such arch helper pairs seems to be
> pre/post, so maybe arch_pre_report_syscall_entry()?

Right!

> 
>> +
>> +#ifndef arch_prepare_report_syscall_entry
>> +static inline unsigned long arch_prepare_report_syscall_entry(struct pt_regs *regs)
>> +{
>> +	return 0;
>> +}
>> +#endif
>> +
>> +/**
>> + * arch_post_report_syscall_entry - Architecture specific work after
>> + *			            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
>> +
>> +/**
>> + * arch_prepare_report_syscall_exit - Architecture specific work before
>> + *			              report_syscall_exit().
>> + */
>> +static inline unsigned long arch_prepare_report_syscall_exit(struct pt_regs *regs,
>> +							     unsigned long work);
>> +
>> +#ifndef arch_prepare_report_syscall_exit
>> +static inline unsigned long arch_prepare_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().
>> + */
>> +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
>> +
>> +/**
>> + * arch_irqentry_exit_need_resched - Architecture specific need resched function
>> + */
>> +static inline bool arch_irqentry_exit_need_resched(void);
>> +
>> +#ifndef arch_irqentry_exit_need_resched
>> +static inline bool arch_irqentry_exit_need_resched(void) { return true; }
>> +#endif
> 
> Nit: the existing arch_* hooks seem to be declared close to the function
> they are called from (e.g. arch_enter_from_user_mode() just before
> enter_from_user_mode()), maybe we could do the same with those new
> hooks, where possible.

Yes, do the same with those new hooks will be nice.

> 
>> +
>>  /**
>>   * arch_do_signal_or_restart -  Architecture specific signal delivery function
>>   * @regs:	Pointer to currents pt_regs
>> @@ -552,4 +640,6 @@ irqentry_state_t noinstr irqentry_nmi_enter(struct pt_regs *regs);
>>   */
>>  void noinstr irqentry_nmi_exit(struct pt_regs *regs, irqentry_state_t irq_state);
>>  
>> +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/common.c b/kernel/entry/common.c
>> index 90843cc38588..cd76391ffcb9 100644
>> --- a/kernel/entry/common.c
>> +++ b/kernel/entry/common.c
>> @@ -28,6 +28,7 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
>>  long syscall_trace_enter(struct pt_regs *regs, long syscall,
>>  				unsigned long work)
>>  {
>> +	unsigned long saved_reg;
> 
> Nit: could be declared inside the if block.

Right!

> 
> Kevin
> 
>>  	long ret = 0;
>>  
>>  	/*
>> @@ -42,8 +43,10 @@ long syscall_trace_enter(struct pt_regs *regs, long syscall,
>>  
>>  	/* Handle ptrace */
>>  	if (work & (SYSCALL_WORK_SYSCALL_TRACE | SYSCALL_WORK_SYSCALL_EMU)) {
>> +		saved_reg = arch_prepare_report_syscall_entry(regs);
>>  		ret = ptrace_report_syscall_entry(regs);
>> -		if (ret || (work & SYSCALL_WORK_SYSCALL_EMU))
>> +		arch_post_report_syscall_entry(regs, saved_reg, ret);
>> +		if (ret || work & SYSCALL_WORK_SYSCALL_EMU)
>>  			return -1L;
>>  	}
>>  
>> [...]
>
Re: [PATCH v3 1/3] entry: Add some arch funcs to support arm64 to use generic entry
Posted by Kevin Brodsky 1 year, 3 months ago 
On 22/08/2024 14:36, Jinjie Ruan wrote:
>>> +/**
>>> + * arch_exit_to_kernel_mode_prepare - Architecture specific final work before
>>> + *				      exit to kernel mode.
>>> + */
>>> +static inline void arch_exit_to_kernel_mode_prepare(struct pt_regs *regs);
>> Any reason to suffix this function with "prepare"? Just
>> arch_exit_to_kernel_mode() seems appropriate (symmetric with
>> arch_enter_from_kernel_mode()).
> prepare means it is the first function before all other exit_to_kernel
> operation in irqentry_exit(), but as the order problem, it can be
> adjusted to the last to aligh with the older arm64 version.

I understand the rationale, but I don't think this aligns very well with
the other hooks - they are generally called after the functions they are
called from, without suggesting where they are called (beginning/end of
the function). In particular we already have
arch_exit_to_user_mode_prepare(), which is named so because it is called
from exit_to_user_mode_prepare(). If we use "prepare" as a suffix here,
I'm concerned we're confusing rather than clarifying things.

Kevin

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.