Re: [PATCH v8 11/12] arm64: entry: Convert to generic entry

[Jinjie Ruan <ruanjinjie@huawei.com>]

On 2025/11/28 21:32, Kevin Brodsky wrote:
> On 28/11/2025 04:34, Jinjie Ruan wrote:
>>
>> On 2025/11/27 21:31, Kevin Brodsky wrote:
>>> On 26/11/2025 08:14, Jinjie Ruan wrote:
>>>> Currently, x86, Riscv, Loongarch use the generic entry which makes
>>>> maintainers' work easier and codes more elegant. arm64 has already
>>>> switched to the generic IRQ entry, so completely convert arm64 to use
>>>> the generic entry infrastructure from kernel/entry/*.
>>>>
>>>> The changes are below:
>>>>  - Remove TIF_SYSCALL_* flag, _TIF_WORK_MASK, _TIF_SYSCALL_WORK,
>>> _TIF_WORK_MASK is now removed in patch 1.
>>>
>>>>    and remove has_syscall_work(), as _TIF_SYSCALL_WORK is equal with
>>>>    SYSCALL_WORK_ENTER.
>>>>
>>>> [...]
>>>>
>>>> +static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs,
>>>> +							    int step)
>>>> +{
>>>> +	unsigned long saved_reg;
>>>> +	int regno;
>>>> +
>>>> +	saved_reg = ptrace_save_reg(regs, PTRACE_SYSCALL_EXIT, &regno);
>>>> +	if (!step) {
>>> A difference I noticed here is that the generic report_single_step()
>>> always returns false if SYSCALL_EMU is set. I don't know if the
>>> combination of SYSCALL_EMU and SINGLESTEP is meaningful, but if it is
>>> then I think that's a behaviour change.
>> commit 64eb35f701f0 ("ptrace: Migrate TIF_SYSCALL_EMU to use
>> SYSCALL_WORK flag") has changed the following code:
>>
>> Therefore, the original logic returns false in these cases for
>> report_single_step() :
>>
>> - Only _TIF_SYSCALL_EMU is set.
>>
>> - Both _TIF_SINGLESTEP and _TIF_SYSCALL_EMU are set.
>>
>> - Neither TIF_SINGLESTEP nor _TIF_SYSCALL_EMU is set;
>>
>>
>>     #define SYSEMU_STEP    (_TIF_SINGLESTEP | _TIF_SYSCALL_EMU)
>>
>>     static inline bool report_single_step(unsigned long ti_work)
>>     {
>>        return (ti_work & SYSEMU_STEP) == _TIF_SINGLESTEP;
>>     }
> 
> The code did look different before this commit, but AFAICT it was
> functionally equivalent w.r.t. SYSEMU / SINGLESTEP.
> 
>> I think the "returns false if SYSCALL_EMU is set" behaviour is correct
>> according to the Man's Manual， both PTRACE_SYSEMU and
>> PTRACE_SYSEMU_SINGLESTEP need to report the syscal only once on syscall
>> entry.
>>
>>     “For PTRACE_SYSEMU, continue and stop on entry to the next
>>       system call, which will not be executed.  See the
>>       documentation on syscall-stops below.  For
>>       PTRACE_SYSEMU_SINGLESTEP, do the same but also singlestep
>>       if not a system call. “
> 
> That seems sensible (based on my very limited understanding of SYSEMU),
> nevertheless it is not what arm64 currently does AFAIU. To follow the



> same logic as the rest, this change should be made in a separate patch.

Right, and the man page description seems to match the comments of the
report_single_step() function.

"74 /*
 75  * If SYSCALL_EMU is set, then the only reason to report is when
 76  * SINGLESTEP is set (i.e. PTRACE_SYSEMU_SINGLESTEP).  This syscall
 77  * instruction has been already reported in
syscall_enter_from_user_mode().
 78  */
"

> 
>> Link：https://man7.org/linux/man-pages/man2/ptrace.2.html
>>
>>>> [...]
>>>>
>>>>  #define TIF_UPROBE		5	/* uprobe breakpoint or singlestep */
>>>>  #define TIF_MTE_ASYNC_FAULT	6	/* MTE Asynchronous Tag Check Fault */
>>>>  #define TIF_NOTIFY_SIGNAL	7	/* signal notifications exist */
>>>> -#define TIF_SYSCALL_TRACE	8	/* syscall trace active */
>>>> -#define TIF_SYSCALL_AUDIT	9	/* syscall auditing */
>>>> -#define TIF_SYSCALL_TRACEPOINT	10	/* syscall tracepoint for ftrace */
>>>> -#define TIF_SECCOMP		11	/* syscall secure computing */
>>>> -#define TIF_SYSCALL_EMU		12	/* syscall emulation active */
>>>> +#define TIF_SECCOMP		11      /* syscall secure computing */
>>>> +#define TIF_SYSCALL_EMU		12      /* syscall emulation active */
>>> These seem to have reappeared in v8 for some reason?
>> v8 add "ARCH_SYSCALL_WORK_EXIT" to be defined as "SECCOMP | SYSCALL_EMU"
>>   to keep the arm64 behaviour unchanged as mentioned in v7.
> 
> Ah then that is where the issue is, I missed that: surely switching to
> generic entry means that we are using SYSCALL_WORK_BIT_* rather than
> TIF_* for all these flags?

I think they may be the same thing as you mentioned in v7，neither
SYSCALL_WORK_EXIT nor report_single_step() excluded SYSCALL_EMU, maybe
we should clarify them for arm64 together in a separate patch.

1、"The generic report_single_step() always returns false if SYSCALL_EMU
is set."

2、"
     > -void syscall_exit_to_user_mode_prepare(struct pt_regs *regs)
     > -{
     > -	unsigned long flags = read_thread_flags();
     > -
     > -	rseq_syscall(regs);
     > -
     > -	if (has_syscall_work(flags) || flags & _TIF_SINGLESTEP)

     I believe switching to the generic function introduces a change
     here: syscall_exit_work() is only called if a flag in
SYSCALL_WORK_EXIT is set, and this set does not include SYSCALL_EMU and
    SECCOMP. Practically this means that audit_syscall_exit() will no
    longer be called if only SECCOMP and/or SYSCALL_EMU is set.

   It doesn't feel like a major behaviour change, but it should be
   pointed out."

> 
> - Kevin
>