Differ from generic entry, due to historical reasons, ARM64 need to
save/restore during syscall entry/exit because ARM64 use a scratch
register (ip(r12) on AArch32, x7 on AArch64) to denote syscall entry/exit.

In preparation for moving arm64 over to the generic entry code,
add arch_ptrace_report_syscall_entry/exit() as the default
ptrace_report_syscall_entry/exit() implementation. This allows
arm64 to implement the architecture specific version.

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>
---
 kernel/entry/syscall-common.c | 43 +++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c
index 66e6ba7fa80c..27310e611567 100644
--- a/kernel/entry/syscall-common.c
+++ b/kernel/entry/syscall-common.c
@@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
 	}
 }
 
+/**
+ * arch_ptrace_report_syscall_entry - Architecture specific
+ *				      ptrace_report_syscall_entry().
+ *
+ * Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry().
+ * Defaults to ptrace_report_syscall_entry.
+ *
+ * The main purpose is to support arch-specific ptrace_report_syscall_entry()
+ * implementation.
+ */
+static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs);
+
+#ifndef arch_ptrace_report_syscall_entry
+static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs)
+{
+	return ptrace_report_syscall_entry(regs);
+}
+#endif
+
 long syscall_trace_enter(struct pt_regs *regs, long syscall,
 				unsigned long work)
 {
@@ -34,7 +53,7 @@ long syscall_trace_enter(struct pt_regs *regs, long syscall,
 
 	/* Handle ptrace */
 	if (work & (SYSCALL_WORK_SYSCALL_TRACE | SYSCALL_WORK_SYSCALL_EMU)) {
-		ret = ptrace_report_syscall_entry(regs);
+		ret = arch_ptrace_report_syscall_entry(regs);
 		if (ret || (work & SYSCALL_WORK_SYSCALL_EMU))
 			return -1L;
 	}
@@ -84,6 +103,26 @@ static inline bool report_single_step(unsigned long work)
 	return work & SYSCALL_WORK_SYSCALL_EXIT_TRAP;
 }
 
+/**
+ * arch_ptrace_report_syscall_exit - Architecture specific
+ *				     ptrace_report_syscall_exit.
+ *
+ * Invoked from syscall_exit_work() to wrap ptrace_report_syscall_exit().
+ *
+ * The main purpose is to support arch-specific ptrace_report_syscall_exit
+ * implementation.
+ */
+static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs,
+							    int step);
+
+#ifndef arch_ptrace_report_syscall_exit
+static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs,
+							    int step)
+{
+	ptrace_report_syscall_exit(regs, step);
+}
+#endif
+
 void syscall_exit_work(struct pt_regs *regs, unsigned long work)
 {
 	bool step;
@@ -108,5 +147,5 @@ void syscall_exit_work(struct pt_regs *regs, unsigned long work)
 
 	step = report_single_step(work);
 	if (step || work & SYSCALL_WORK_SYSCALL_TRACE)
-		ptrace_report_syscall_exit(regs, step);
+		arch_ptrace_report_syscall_exit(regs, step);
 }
-- 
2.34.1

On 17/11/2025 14:30, Jinjie Ruan wrote:
> Differ from generic entry, due to historical reasons, ARM64 need to
> save/restore during syscall entry/exit because ARM64 use a scratch
> register (ip(r12) on AArch32, x7 on AArch64) to denote syscall entry/exit.
>
> In preparation for moving arm64 over to the generic entry code,
> add arch_ptrace_report_syscall_entry/exit() as the default
> ptrace_report_syscall_entry/exit() implementation. This allows
> arm64 to implement the architecture specific version.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Suggested-by: Kevin Brodsky <kevin.brodsky@arm.com>

I don't think I suggested this patch. I see that I suggested renaming
some functions on v3, but I don't think that justifies a Suggested-by tag.

> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
>  kernel/entry/syscall-common.c | 43 +++++++++++++++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c
> index 66e6ba7fa80c..27310e611567 100644
> --- a/kernel/entry/syscall-common.c
> +++ b/kernel/entry/syscall-common.c
> @@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
>  	}
>  }
>  
> +/**
> + * arch_ptrace_report_syscall_entry - Architecture specific
> + *				      ptrace_report_syscall_entry().
> + *
> + * Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry().
> + * Defaults to ptrace_report_syscall_entry.
> + *
> + * The main purpose is to support arch-specific ptrace_report_syscall_entry()
> + * implementation.
> + */
> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs);
> +
> +#ifndef arch_ptrace_report_syscall_entry
> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs)
> +{
> +	return ptrace_report_syscall_entry(regs);

I saw that Thomas suggested this approach on v4, and it makes sense to
me, but I find the naming surprising. If an architecture does need extra
handling, then the generic function should never be called from generic
code. So it seems to me that the more logical change would be:

* Rename: ptrace_report_syscall_entry -> __ptrace_report_syscall_entry
* Introduce ptrace_report_syscall_entry(), defaults to
__ptrace_report_syscall_entry()

All this would be done in <linux/ptrace.h>, where it clearly belongs.
The __ prefix makes it clear that the generic function is not the main
interface. Even better, no need to change any caller with that approach.

- Kevin

> +}
> +#endif
> +
>  long syscall_trace_enter(struct pt_regs *regs, long syscall,
>  				unsigned long work)
>  {
> @@ -34,7 +53,7 @@ long syscall_trace_enter(struct pt_regs *regs, long syscall,
>  
>  	/* Handle ptrace */
>  	if (work & (SYSCALL_WORK_SYSCALL_TRACE | SYSCALL_WORK_SYSCALL_EMU)) {
> -		ret = ptrace_report_syscall_entry(regs);
> +		ret = arch_ptrace_report_syscall_entry(regs);
>  		if (ret || (work & SYSCALL_WORK_SYSCALL_EMU))
>  			return -1L;
>  	}
> @@ -84,6 +103,26 @@ static inline bool report_single_step(unsigned long work)
>  	return work & SYSCALL_WORK_SYSCALL_EXIT_TRAP;
>  }
>  
> +/**
> + * arch_ptrace_report_syscall_exit - Architecture specific
> + *				     ptrace_report_syscall_exit.
> + *
> + * Invoked from syscall_exit_work() to wrap ptrace_report_syscall_exit().
> + *
> + * The main purpose is to support arch-specific ptrace_report_syscall_exit
> + * implementation.
> + */
> +static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs,
> +							    int step);
> +
> +#ifndef arch_ptrace_report_syscall_exit
> +static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs,
> +							    int step)
> +{
> +	ptrace_report_syscall_exit(regs, step);
> +}
> +#endif
> +
>  void syscall_exit_work(struct pt_regs *regs, unsigned long work)
>  {
>  	bool step;
> @@ -108,5 +147,5 @@ void syscall_exit_work(struct pt_regs *regs, unsigned long work)
>  
>  	step = report_single_step(work);
>  	if (step || work & SYSCALL_WORK_SYSCALL_TRACE)
> -		ptrace_report_syscall_exit(regs, step);
> +		arch_ptrace_report_syscall_exit(regs, step);
>  }

On 2025/11/19 1:13, Kevin Brodsky wrote:
> On 17/11/2025 14:30, Jinjie Ruan wrote:
>> Differ from generic entry, due to historical reasons, ARM64 need to
>> save/restore during syscall entry/exit because ARM64 use a scratch
>> register (ip(r12) on AArch32, x7 on AArch64) to denote syscall entry/exit.
>>
>> In preparation for moving arm64 over to the generic entry code,
>> add arch_ptrace_report_syscall_entry/exit() as the default
>> ptrace_report_syscall_entry/exit() implementation. This allows
>> arm64 to implement the architecture specific version.
>>
>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>> Suggested-by: Kevin Brodsky <kevin.brodsky@arm.com>
> 
> I don't think I suggested this patch. I see that I suggested renaming
> some functions on v3, but I don't think that justifies a Suggested-by tag.
> 
>> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>>  kernel/entry/syscall-common.c | 43 +++++++++++++++++++++++++++++++++--
>>  1 file changed, 41 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c
>> index 66e6ba7fa80c..27310e611567 100644
>> --- a/kernel/entry/syscall-common.c
>> +++ b/kernel/entry/syscall-common.c
>> @@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
>>  	}
>>  }
>>  
>> +/**
>> + * arch_ptrace_report_syscall_entry - Architecture specific
>> + *				      ptrace_report_syscall_entry().
>> + *
>> + * Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry().
>> + * Defaults to ptrace_report_syscall_entry.
>> + *
>> + * The main purpose is to support arch-specific ptrace_report_syscall_entry()
>> + * implementation.
>> + */
>> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs);
>> +
>> +#ifndef arch_ptrace_report_syscall_entry
>> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs)
>> +{
>> +	return ptrace_report_syscall_entry(regs);
> 
> I saw that Thomas suggested this approach on v4, and it makes sense to
> me, but I find the naming surprising. If an architecture does need extra
> handling, then the generic function should never be called from generic
> code. So it seems to me that the more logical change would be:
> 
> * Rename: ptrace_report_syscall_entry -> __ptrace_report_syscall_entry
> * Introduce ptrace_report_syscall_entry(), defaults to
> __ptrace_report_syscall_entry()

If ptrace_report_syscall_entry() is defined in linux/ptrace.h, and an
architecture also needs to redefine this function, but the
architecture's own <asm/entry-common.h> must include <linux/ptrace.h>,
the function will end up being defined twice and cause a "duplicate
definition" compile error.

> 
> All this would be done in <linux/ptrace.h>, where it clearly belongs.
> The __ prefix makes it clear that the generic function is not the main
> interface. Even better, no need to change any caller with that approach.
> 
> - Kevin
> 
>> +}
>> +#endif
>> +
>>  long syscall_trace_enter(struct pt_regs *regs, long syscall,
>>  				unsigned long work)
>>  {
>> @@ -34,7 +53,7 @@ long syscall_trace_enter(struct pt_regs *regs, long syscall,
>>  
>>  	/* Handle ptrace */
>>  	if (work & (SYSCALL_WORK_SYSCALL_TRACE | SYSCALL_WORK_SYSCALL_EMU)) {
>> -		ret = ptrace_report_syscall_entry(regs);
>> +		ret = arch_ptrace_report_syscall_entry(regs);
>>  		if (ret || (work & SYSCALL_WORK_SYSCALL_EMU))
>>  			return -1L;
>>  	}
>> @@ -84,6 +103,26 @@ static inline bool report_single_step(unsigned long work)
>>  	return work & SYSCALL_WORK_SYSCALL_EXIT_TRAP;
>>  }
>>  
>> +/**
>> + * arch_ptrace_report_syscall_exit - Architecture specific
>> + *				     ptrace_report_syscall_exit.
>> + *
>> + * Invoked from syscall_exit_work() to wrap ptrace_report_syscall_exit().
>> + *
>> + * The main purpose is to support arch-specific ptrace_report_syscall_exit
>> + * implementation.
>> + */
>> +static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs,
>> +							    int step);
>> +
>> +#ifndef arch_ptrace_report_syscall_exit
>> +static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs,
>> +							    int step)
>> +{
>> +	ptrace_report_syscall_exit(regs, step);
>> +}
>> +#endif
>> +
>>  void syscall_exit_work(struct pt_regs *regs, unsigned long work)
>>  {
>>  	bool step;
>> @@ -108,5 +147,5 @@ void syscall_exit_work(struct pt_regs *regs, unsigned long work)
>>  
>>  	step = report_single_step(work);
>>  	if (step || work & SYSCALL_WORK_SYSCALL_TRACE)
>> -		ptrace_report_syscall_exit(regs, step);
>> +		arch_ptrace_report_syscall_exit(regs, step);
>>  }
>

On 24/11/2025 10:34, Jinjie Ruan wrote:
>
> On 2025/11/19 1:13, Kevin Brodsky wrote:
>> On 17/11/2025 14:30, Jinjie Ruan wrote:
>>> [...]
>>>
>>> diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c
>>> index 66e6ba7fa80c..27310e611567 100644
>>> --- a/kernel/entry/syscall-common.c
>>> +++ b/kernel/entry/syscall-common.c
>>> @@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
>>>  	}
>>>  }
>>>  
>>> +/**
>>> + * arch_ptrace_report_syscall_entry - Architecture specific
>>> + *				      ptrace_report_syscall_entry().
>>> + *
>>> + * Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry().
>>> + * Defaults to ptrace_report_syscall_entry.
>>> + *
>>> + * The main purpose is to support arch-specific ptrace_report_syscall_entry()
>>> + * implementation.
>>> + */
>>> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs);
>>> +
>>> +#ifndef arch_ptrace_report_syscall_entry
>>> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs)
>>> +{
>>> +	return ptrace_report_syscall_entry(regs);
>> I saw that Thomas suggested this approach on v4, and it makes sense to
>> me, but I find the naming surprising. If an architecture does need extra
>> handling, then the generic function should never be called from generic
>> code. So it seems to me that the more logical change would be:
>>
>> * Rename: ptrace_report_syscall_entry -> __ptrace_report_syscall_entry
>> * Introduce ptrace_report_syscall_entry(), defaults to
>> __ptrace_report_syscall_entry()
> If ptrace_report_syscall_entry() is defined in linux/ptrace.h, and an
> architecture also needs to redefine this function, but the
> architecture's own <asm/entry-common.h> must include <linux/ptrace.h>,
> the function will end up being defined twice and cause a "duplicate
> definition" compile error.

There's plenty of arch-defined functions in <linux/ptrace.h> already.
__ptrace_report_syscall_entry() should be defined inside an #ifndef and
architectures can define their own implementation in <asm/ptrace.h>,
like force_successful_syscall_return() for instance.

- Kevin

>> All this would be done in <linux/ptrace.h>, where it clearly belongs.
>> The __ prefix makes it clear that the generic function is not the main
>> interface. Even better, no need to change any caller with that approach.
>>
>> - Kevin
>>
>>> [...]

On 2025/11/24 23:23, Kevin Brodsky wrote:
> On 24/11/2025 10:34, Jinjie Ruan wrote:
>>
>> On 2025/11/19 1:13, Kevin Brodsky wrote:
>>> On 17/11/2025 14:30, Jinjie Ruan wrote:
>>>> [...]
>>>>
>>>> diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c
>>>> index 66e6ba7fa80c..27310e611567 100644
>>>> --- a/kernel/entry/syscall-common.c
>>>> +++ b/kernel/entry/syscall-common.c
>>>> @@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
>>>>  	}
>>>>  }
>>>>  
>>>> +/**
>>>> + * arch_ptrace_report_syscall_entry - Architecture specific
>>>> + *				      ptrace_report_syscall_entry().
>>>> + *
>>>> + * Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry().
>>>> + * Defaults to ptrace_report_syscall_entry.
>>>> + *
>>>> + * The main purpose is to support arch-specific ptrace_report_syscall_entry()
>>>> + * implementation.
>>>> + */
>>>> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs);
>>>> +
>>>> +#ifndef arch_ptrace_report_syscall_entry
>>>> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs)
>>>> +{
>>>> +	return ptrace_report_syscall_entry(regs);
>>> I saw that Thomas suggested this approach on v4, and it makes sense to
>>> me, but I find the naming surprising. If an architecture does need extra
>>> handling, then the generic function should never be called from generic
>>> code. So it seems to me that the more logical change would be:
>>>
>>> * Rename: ptrace_report_syscall_entry -> __ptrace_report_syscall_entry
>>> * Introduce ptrace_report_syscall_entry(), defaults to
>>> __ptrace_report_syscall_entry()
>> If ptrace_report_syscall_entry() is defined in linux/ptrace.h, and an
>> architecture also needs to redefine this function, but the
>> architecture's own <asm/entry-common.h> must include <linux/ptrace.h>,
>> the function will end up being defined twice and cause a "duplicate
>> definition" compile error.
> 
> There's plenty of arch-defined functions in <linux/ptrace.h> already.
> __ptrace_report_syscall_entry() should be defined inside an #ifndef and
> architectures can define their own implementation in <asm/ptrace.h>,
> like force_successful_syscall_return() for instance.

Shared functions like ptrace_report_syscall() are all defined in
<linux/ptrace.h>.
When we want to override __ptrace_report_syscall_entry() in
<asm/ptrace.h> we still have to include <linux/ptrace.h> again,then the
redefine problem occurs again.

What we actually need to reuse is ptrace_report_syscall_entry() (or
__ptrace_report_syscall_entry()).

The arch version need to reuse and wrap ptrace_report_syscall_entry(),
because for instance arm64 needs to perform additional operations before
and after this step. Therefore, I believe the current implementation is
appropriate.

> 
> - Kevin
> 
>>> All this would be done in <linux/ptrace.h>, where it clearly belongs.
>>> The __ prefix makes it clear that the generic function is not the main
>>> interface. Even better, no need to change any caller with that approach.
>>>
>>> - Kevin
>>>
>>>> [...]
>

On 25/11/2025 03:43, Jinjie Ruan wrote:
>
> On 2025/11/24 23:23, Kevin Brodsky wrote:
>> On 24/11/2025 10:34, Jinjie Ruan wrote:
>>> On 2025/11/19 1:13, Kevin Brodsky wrote:
>>>> On 17/11/2025 14:30, Jinjie Ruan wrote:
>>>>> [...]
>>>>>
>>>>> diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c
>>>>> index 66e6ba7fa80c..27310e611567 100644
>>>>> --- a/kernel/entry/syscall-common.c
>>>>> +++ b/kernel/entry/syscall-common.c
>>>>> @@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
>>>>>  	}
>>>>>  }
>>>>>  
>>>>> +/**
>>>>> + * arch_ptrace_report_syscall_entry - Architecture specific
>>>>> + *				      ptrace_report_syscall_entry().
>>>>> + *
>>>>> + * Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry().
>>>>> + * Defaults to ptrace_report_syscall_entry.
>>>>> + *
>>>>> + * The main purpose is to support arch-specific ptrace_report_syscall_entry()
>>>>> + * implementation.
>>>>> + */
>>>>> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs);
>>>>> +
>>>>> +#ifndef arch_ptrace_report_syscall_entry
>>>>> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs)
>>>>> +{
>>>>> +	return ptrace_report_syscall_entry(regs);
>>>> I saw that Thomas suggested this approach on v4, and it makes sense to
>>>> me, but I find the naming surprising. If an architecture does need extra
>>>> handling, then the generic function should never be called from generic
>>>> code. So it seems to me that the more logical change would be:
>>>>
>>>> * Rename: ptrace_report_syscall_entry -> __ptrace_report_syscall_entry
>>>> * Introduce ptrace_report_syscall_entry(), defaults to
>>>> __ptrace_report_syscall_entry()
>>> If ptrace_report_syscall_entry() is defined in linux/ptrace.h, and an
>>> architecture also needs to redefine this function, but the
>>> architecture's own <asm/entry-common.h> must include <linux/ptrace.h>,
>>> the function will end up being defined twice and cause a "duplicate
>>> definition" compile error.
>> There's plenty of arch-defined functions in <linux/ptrace.h> already.
>> __ptrace_report_syscall_entry() should be defined inside an #ifndef and
>> architectures can define their own implementation in <asm/ptrace.h>,
>> like force_successful_syscall_return() for instance.
> Shared functions like ptrace_report_syscall() are all defined in
> <linux/ptrace.h>.
> When we want to override __ptrace_report_syscall_entry() in
> <asm/ptrace.h> we still have to include <linux/ptrace.h> again,then the
> redefine problem occurs again.
>
> What we actually need to reuse is ptrace_report_syscall_entry() (or
> __ptrace_report_syscall_entry()).

You're right, this is yet another of those circular definition problems...

> The arch version need to reuse and wrap ptrace_report_syscall_entry(),
> because for instance arm64 needs to perform additional operations before
> and after this step. Therefore, I believe the current implementation is
> appropriate.

I'm still not fond of arch_X() wrapping X() as this is unusual, but I
don't have a better idea so let's stick to that. It also makes sense to
have this done in syscall-common.c rather than a header considering the
risk of circular dependency.

- Kevin