[PATCH 2/3] arm64: Move do_notify_resume() to entry-common.c

[Liao, Chang]

Hi, Mark

在 2024/2/6 20:38, Mark Rutland 写道:
> Currently do_notify_resume() lives in arch/arm64/kernel/signal.c, but it would
> make more sense for it to live in entry-common.c as it handles more than
> signals, and is coupled with the rest of the return-to-userspace sequence (e.g.
> with unusual DAIF masking that matches the exception return requirements).
> 
> Move do_notify_resume() to entry-common.c.
> 
> There should be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland at arm.com>
> Cc: Catalin Marinas <catalin.marinas at arm.com>
> Cc: James Morse <james.morse at arm.com>
> Cc: Mark Brown <broonie at kernel.org>
> Cc: Will Deacon <will at kernel.org>
> ---
>  arch/arm64/include/asm/exception.h |  2 +-
>  arch/arm64/kernel/entry-common.c   | 32 +++++++++++++++++++++++++++
>  arch/arm64/kernel/signal.c         | 35 ++----------------------------
>  3 files changed, 35 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
> index ad688e157c9be..f296662590c7f 100644
> --- a/arch/arm64/include/asm/exception.h
> +++ b/arch/arm64/include/asm/exception.h
> @@ -74,7 +74,7 @@ void do_el0_fpac(struct pt_regs *regs, unsigned long esr);
>  void do_el1_fpac(struct pt_regs *regs, unsigned long esr);
>  void do_el0_mops(struct pt_regs *regs, unsigned long esr);
>  void do_serror(struct pt_regs *regs, unsigned long esr);
> -void do_notify_resume(struct pt_regs *regs, unsigned long thread_flags);
> +void do_signal(struct pt_regs *regs);
>  
>  void __noreturn panic_bad_stack(struct pt_regs *regs, unsigned long esr, unsigned long far);
>  #endif	/* __ASM_EXCEPTION_H */
> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> index 0fc94207e69a8..3c849ad03bf83 100644
> --- a/arch/arm64/kernel/entry-common.c
> +++ b/arch/arm64/kernel/entry-common.c
> @@ -10,6 +10,7 @@
>  #include <linux/linkage.h>
>  #include <linux/lockdep.h>
>  #include <linux/ptrace.h>
> +#include <linux/resume_user_mode.h>
>  #include <linux/sched.h>
>  #include <linux/sched/debug.h>
>  #include <linux/thread_info.h>
> @@ -126,6 +127,37 @@ static __always_inline void __exit_to_user_mode(void)
>  	lockdep_hardirqs_on(CALLER_ADDR0);
>  }
>  
> +static void do_notify_resume(struct pt_regs *regs, unsigned long thread_flags)
> +{
> +	do {
> +		local_daif_restore(DAIF_PROCCTX);
> +
> +		if (thread_flags & _TIF_NEED_RESCHED)
> +			schedule();
> +
> +		if (thread_flags & _TIF_UPROBE)
> +			uprobe_notify_resume(regs);
> +
> +		if (thread_flags & _TIF_MTE_ASYNC_FAULT) {
> +			clear_thread_flag(TIF_MTE_ASYNC_FAULT);
> +			send_sig_fault(SIGSEGV, SEGV_MTEAERR,
> +				       (void __user *)NULL, current);
> +		}
> +
> +		if (thread_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> +			do_signal(regs);
> +
> +		if (thread_flags & _TIF_NOTIFY_RESUME)
> +			resume_user_mode_work(regs);
> +
> +		if (thread_flags & _TIF_FOREIGN_FPSTATE)
> +			fpsimd_restore_current_state();
> +
> +		local_daif_mask();
> +		thread_flags = read_thread_flags();
> +	} while (thread_flags & _TIF_WORK_MASK);
> +}

What about moving load_daif_restore() and load_daif_mask() outside of the do-while loop?
Invoking them repeatedlly within the loop is redundant, right?

Thanks.

> +
>  static __always_inline void exit_to_user_mode_prepare(struct pt_regs *regs)
>  {
>  	unsigned long flags;
> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> index 50e108741599d..c08e6465e0f49 100644
> --- a/arch/arm64/kernel/signal.c
> +++ b/arch/arm64/kernel/signal.c
> @@ -16,8 +16,8 @@
>  #include <linux/uaccess.h>
>  #include <linux/sizes.h>
>  #include <linux/string.h>
> -#include <linux/resume_user_mode.h>
>  #include <linux/ratelimit.h>
> +#include <linux/rseq.h>
>  #include <linux/syscalls.h>
>  
>  #include <asm/daifflags.h>
> @@ -1207,7 +1207,7 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs)
>   * the kernel can handle, and then we build all the user-level signal handling
>   * stack-frames in one go after that.
>   */
> -static void do_signal(struct pt_regs *regs)
> +void do_signal(struct pt_regs *regs)
>  {
>  	unsigned long continue_addr = 0, restart_addr = 0;
>  	int retval = 0;
> @@ -1278,37 +1278,6 @@ static void do_signal(struct pt_regs *regs)
>  	restore_saved_sigmask();
>  }
>  
> -void do_notify_resume(struct pt_regs *regs, unsigned long thread_flags)
> -{
> -	do {
> -		local_daif_restore(DAIF_PROCCTX);
> -
> -		if (thread_flags & _TIF_NEED_RESCHED)
> -			schedule();
> -
> -		if (thread_flags & _TIF_UPROBE)
> -			uprobe_notify_resume(regs);
> -
> -		if (thread_flags & _TIF_MTE_ASYNC_FAULT) {
> -			clear_thread_flag(TIF_MTE_ASYNC_FAULT);
> -			send_sig_fault(SIGSEGV, SEGV_MTEAERR,
> -				       (void __user *)NULL, current);
> -		}
> -
> -		if (thread_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> -			do_signal(regs);
> -
> -		if (thread_flags & _TIF_NOTIFY_RESUME)
> -			resume_user_mode_work(regs);
> -
> -		if (thread_flags & _TIF_FOREIGN_FPSTATE)
> -			fpsimd_restore_current_state();
> -
> -		local_daif_mask();
> -		thread_flags = read_thread_flags();
> -	} while (thread_flags & _TIF_WORK_MASK);
> -}
> -
>  unsigned long __ro_after_init signal_minsigstksz;
>  
>  /*

-- 
BR
Liao, Chang