[PATCH RFC 0/2] s390: keep syscall number / return value in separate pt_regs members

[PATCH RFC 0/2] s390: keep syscall number / return value in separate pt_regs members

[Sven Schnelle]

After a recent discussion on the ML about syscall semantics wrt. pre-initializing
the return value to -ENOSYS and the special handling s390 has, I made two patches to
clean that up. See the commit messages for more details.

The second patch is just a cleanup and could be dropped if not wanted.

Sven Schnelle (2):
  s390/syscall: Keep syscall return in extra ptregs member
  s390/syscall: Keep syscall number in extra ptregs member

 arch/s390/include/asm/ptrace.h  |  9 ++++++---
 arch/s390/include/asm/syscall.h | 13 +++++++------
 arch/s390/kernel/ptrace.c       |  2 +-
 arch/s390/kernel/signal.c       | 12 +++++-------
 arch/s390/kernel/syscall.c      | 26 +++++++-------------------
 5 files changed, 26 insertions(+), 36 deletions(-)

-- 
2.53.0

[PATCH 1/2] s390/syscall: Keep syscall return in extra ptregs member

[Sven Schnelle]

On s390, both syscall number and return value share ptregs::gprs[2].
Before executing the syscall, gpr[2] contains the syscall number,
and after execution it gets the return value assigned.

This is problematic with tracing: When either seccomp or ptrace set
a return value and the syscall number to -1 to skip the syscall, the
return value will be overwritten. If the return value is positive,
it's impossible to determine that the syscall should be skipped.

During conversion to generic entry a PIF_SYSCALL_RET_SET flag was
introduced which is set when syscall_set_return_value() is executed.

Peter Zijlstra proposed in [1] to introduce a new member in ptregs
which keeps the return value and copy this to ptregs::gprs[2] before
exit to userspace instead.

To avoid increasing ptregs size add it to the union which contains
int_parm_long and struct tpi_info as these members are not used during
syscalls.

[1] https://lore.kernel.org/all/20260703105718.GO751831@noisy.programming.kicks-ass.net/

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
 arch/s390/include/asm/ptrace.h  |  9 ++++++---
 arch/s390/include/asm/syscall.h |  5 ++---
 arch/s390/kernel/syscall.c      | 15 +++------------
 3 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
index 495e310c3d6d..f39b349da9a7 100644
--- a/arch/s390/include/asm/ptrace.h
+++ b/arch/s390/include/asm/ptrace.h
@@ -15,9 +15,8 @@
 
 #define PIF_SYSCALL			0	/* inside a system call */
 #define PIF_PSW_ADDR_ADJUSTED		1	/* psw address has been adjusted */
-#define PIF_SYSCALL_RET_SET		2	/* return value was set via ptrace */
-#define PIF_GUEST_FAULT			3	/* indicates program check in sie64a */
-#define PIF_FTRACE_FULL_REGS		4	/* all register contents valid (ftrace) */
+#define PIF_GUEST_FAULT			2	/* indicates program check in sie64a */
+#define PIF_FTRACE_FULL_REGS		3	/* all register contents valid (ftrace) */
 
 #define _PIF_SYSCALL			BIT(PIF_SYSCALL)
 #define _PIF_ADDR_PSW_ADJUSTED		BIT(PIF_PSW_ADDR_ADJUSTED)
@@ -130,6 +129,10 @@ struct pt_regs {
 			unsigned int int_parm;
 			unsigned long int_parm_long;
 		};
+		struct {
+			unsigned long __unused;
+			unsigned long syscall_ret;
+		};
 		struct tpi_info tpi_info;
 	};
 	unsigned long flags;
diff --git a/arch/s390/include/asm/syscall.h b/arch/s390/include/asm/syscall.h
index 4271e4169f45..22b6ed20a9ae 100644
--- a/arch/s390/include/asm/syscall.h
+++ b/arch/s390/include/asm/syscall.h
@@ -52,15 +52,14 @@ static inline long syscall_get_error(struct task_struct *task,
 static inline long syscall_get_return_value(struct task_struct *task,
 					    struct pt_regs *regs)
 {
-	return regs->gprs[2];
+	return regs->syscall_ret;
 }
 
 static inline void syscall_set_return_value(struct task_struct *task,
 					    struct pt_regs *regs,
 					    int error, long val)
 {
-	set_pt_regs_flag(regs, PIF_SYSCALL_RET_SET);
-	regs->gprs[2] = error ? error : val;
+	regs->syscall_ret = error ? error : val;
 }
 
 static inline void syscall_get_arguments(struct task_struct *task,
diff --git a/arch/s390/kernel/syscall.c b/arch/s390/kernel/syscall.c
index 75d5a3cab14e..ce244dceec6d 100644
--- a/arch/s390/kernel/syscall.c
+++ b/arch/s390/kernel/syscall.c
@@ -117,25 +117,16 @@ void noinstr __do_syscall(struct pt_regs *regs, int per_trap)
 		regs->int_code |= nr;
 	}
 	regs->gprs[2] = nr;
+	regs->syscall_ret = -ENOSYS;
 	if (nr == __NR_restart_syscall && !(current->restart_block.arch_data & 1)) {
 		regs->psw.addr = current->restart_block.arch_data;
 		current->restart_block.arch_data = 1;
 	}
 	nr = syscall_enter_from_user_mode_work(regs, nr);
-	/*
-	 * In the s390 ptrace ABI, both the syscall number and the return value
-	 * use gpr2. However, userspace puts the syscall number either in the
-	 * svc instruction itself, or uses gpr1. To make at least skipping syscalls
-	 * work, the ptrace code sets PIF_SYSCALL_RET_SET, which is checked here
-	 * and if set, the syscall will be skipped.
-	 */
-	if (unlikely(test_and_clear_pt_regs_flag(regs, PIF_SYSCALL_RET_SET)))
-		goto out;
-	regs->gprs[2] = -ENOSYS;
 	if (likely(nr < NR_syscalls)) {
 		nr = array_index_nospec(nr, NR_syscalls);
-		regs->gprs[2] = sys_call_table[nr](regs);
+		regs->syscall_ret = sys_call_table[nr](regs);
 	}
-out:
+	regs->gprs[2] = regs->syscall_ret;
 	syscall_exit_to_user_mode(regs);
 }
-- 
2.53.0

[PATCH 2/2] s390/syscall: Keep syscall number in extra ptregs member

[Sven Schnelle]

Move the syscall number from ptregs::int_code to another union member
called syscall_nr. This simplifies the code a bit.

Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
 arch/s390/include/asm/ptrace.h  |  2 +-
 arch/s390/include/asm/syscall.h |  8 +++++---
 arch/s390/kernel/ptrace.c       |  2 +-
 arch/s390/kernel/signal.c       | 12 +++++-------
 arch/s390/kernel/syscall.c      | 11 ++++-------
 5 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
index f39b349da9a7..2edc85ddcba8 100644
--- a/arch/s390/include/asm/ptrace.h
+++ b/arch/s390/include/asm/ptrace.h
@@ -130,7 +130,7 @@ struct pt_regs {
 			unsigned long int_parm_long;
 		};
 		struct {
-			unsigned long __unused;
+			unsigned long syscall_nr;
 			unsigned long syscall_ret;
 		};
 		struct tpi_info tpi_info;
diff --git a/arch/s390/include/asm/syscall.h b/arch/s390/include/asm/syscall.h
index 22b6ed20a9ae..abb0d4348912 100644
--- a/arch/s390/include/asm/syscall.h
+++ b/arch/s390/include/asm/syscall.h
@@ -14,13 +14,15 @@
 #include <linux/err.h>
 #include <asm/ptrace.h>
 
+#define SVC_LEN 2
 extern const sys_call_ptr_t sys_call_table[];
 
 static inline long syscall_get_nr(struct task_struct *task,
 				  struct pt_regs *regs)
 {
-	return test_pt_regs_flag(regs, PIF_SYSCALL) ?
-		(regs->int_code & 0xffff) : -1;
+	if (!test_pt_regs_flag(regs, PIF_SYSCALL))
+		return -1;
+	return regs->syscall_nr;
 }
 
 static inline void syscall_set_nr(struct task_struct *task,
@@ -32,7 +34,7 @@ static inline void syscall_set_nr(struct task_struct *task,
 	 * the target task is stopped for tracing on entering syscall, so
 	 * there is no need to have the same check syscall_get_nr() has.
 	 */
-	regs->int_code = (regs->int_code & ~0xffff) | (nr & 0xffff);
+	regs->syscall_nr = nr;
 }
 
 static inline void syscall_rollback(struct task_struct *task,
diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
index 125ca4c4e30c..00e825d01504 100644
--- a/arch/s390/kernel/ptrace.c
+++ b/arch/s390/kernel/ptrace.c
@@ -349,7 +349,7 @@ static int __poke_user(struct task_struct *child, addr_t addr, addr_t data)
 			addr == offsetof(struct user, regs.gprs[2])) {
 			struct pt_regs *regs = task_pt_regs(child);
 
-			regs->int_code = 0x20000 | (data & 0xffff);
+			regs->syscall_nr = data;
 		}
 		*(addr_t *)((addr_t) &regs->psw + addr) = data;
 	} else if (addr < offsetof(struct user, regs.orig_gpr2)) {
diff --git a/arch/s390/kernel/signal.c b/arch/s390/kernel/signal.c
index 4874de5edea0..c3c69294ff33 100644
--- a/arch/s390/kernel/signal.c
+++ b/arch/s390/kernel/signal.c
@@ -447,12 +447,12 @@ void arch_do_signal_or_restart(struct pt_regs *regs)
 	 * call information.
 	 */
 	current->thread.system_call =
-		test_pt_regs_flag(regs, PIF_SYSCALL) ? regs->int_code : 0;
+		test_pt_regs_flag(regs, PIF_SYSCALL) ? regs->syscall_nr : 0;
 
 	if (get_signal(&ksig)) {
 		/* Whee!  Actually deliver the signal.  */
 		if (current->thread.system_call) {
-			regs->int_code = current->thread.system_call;
+			regs->syscall_nr = current->thread.system_call;
 			/* Check for system call restarting. */
 			switch (regs->gprs[2]) {
 			case -ERESTART_RESTARTBLOCK:
@@ -467,9 +467,7 @@ void arch_do_signal_or_restart(struct pt_regs *regs)
 				fallthrough;
 			case -ERESTARTNOINTR:
 				regs->gprs[2] = regs->orig_gpr2;
-				regs->psw.addr =
-					__rewind_psw(regs->psw,
-						     regs->int_code >> 16);
+				regs->psw.addr = __rewind_psw(regs->psw, SVC_LEN);
 				break;
 			}
 		}
@@ -484,7 +482,7 @@ void arch_do_signal_or_restart(struct pt_regs *regs)
 	/* No handlers present - check for system call restart */
 	clear_pt_regs_flag(regs, PIF_SYSCALL);
 	if (current->thread.system_call) {
-		regs->int_code = current->thread.system_call;
+		regs->syscall_nr = current->thread.system_call;
 		switch (regs->gprs[2]) {
 		case -ERESTART_RESTARTBLOCK:
 			/* Restart with sys_restart_syscall */
@@ -498,7 +496,7 @@ void arch_do_signal_or_restart(struct pt_regs *regs)
 		case -ERESTARTSYS:
 		case -ERESTARTNOINTR:
 			regs->gprs[2] = regs->orig_gpr2;
-			regs->psw.addr = __rewind_psw(regs->psw, regs->int_code >> 16);
+			regs->psw.addr = __rewind_psw(regs->psw, SVC_LEN);
 			if (test_thread_flag(TIF_SINGLE_STEP))
 				clear_thread_flag(TIF_PER_TRAP);
 			break;
diff --git a/arch/s390/kernel/syscall.c b/arch/s390/kernel/syscall.c
index ce244dceec6d..62e283ffef37 100644
--- a/arch/s390/kernel/syscall.c
+++ b/arch/s390/kernel/syscall.c
@@ -100,7 +100,10 @@ void noinstr __do_syscall(struct pt_regs *regs, int per_trap)
 	enter_from_user_mode(regs);
 	add_random_kstack_offset();
 	regs->psw = get_lowcore()->svc_old_psw;
-	regs->int_code = get_lowcore()->svc_int_code;
+	nr = get_lowcore()->svc_int_code & 0xffff;
+	if (likely(!nr))
+		nr = regs->gprs[1];
+	regs->syscall_nr = nr;
 	update_timer_sys();
 	if (cpu_has_bear())
 		current->thread.last_break = regs->last_break;
@@ -110,12 +113,6 @@ void noinstr __do_syscall(struct pt_regs *regs, int per_trap)
 		set_thread_flag(TIF_PER_TRAP);
 	regs->flags = 0;
 	set_pt_regs_flag(regs, PIF_SYSCALL);
-	nr = regs->int_code & 0xffff;
-	if (likely(!nr)) {
-		nr = regs->gprs[1] & 0xffff;
-		regs->int_code &= ~0xffffUL;
-		regs->int_code |= nr;
-	}
 	regs->gprs[2] = nr;
 	regs->syscall_ret = -ENOSYS;
 	if (nr == __NR_restart_syscall && !(current->restart_block.arch_data & 1)) {
-- 
2.53.0

Re: [PATCH 2/2] s390/syscall: Keep syscall number in extra ptregs member

[Heiko Carstens]

On Wed, Jul 15, 2026 at 03:38:30PM +0200, Sven Schnelle wrote:
> Move the syscall number from ptregs::int_code to another union member
> called syscall_nr. This simplifies the code a bit.
> 
> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> ---
>  arch/s390/include/asm/ptrace.h  |  2 +-
>  arch/s390/include/asm/syscall.h |  8 +++++---
>  arch/s390/kernel/ptrace.c       |  2 +-
>  arch/s390/kernel/signal.c       | 12 +++++-------
>  arch/s390/kernel/syscall.c      | 11 ++++-------
>  5 files changed, 16 insertions(+), 19 deletions(-)

...

> diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
> index 125ca4c4e30c..00e825d01504 100644
> --- a/arch/s390/kernel/ptrace.c
> +++ b/arch/s390/kernel/ptrace.c
> @@ -349,7 +349,7 @@ static int __poke_user(struct task_struct *child, addr_t addr, addr_t data)
>  			addr == offsetof(struct user, regs.gprs[2])) {
>  			struct pt_regs *regs = task_pt_regs(child);
>  
> -			regs->int_code = 0x20000 | (data & 0xffff);
> +			regs->syscall_nr = data;

This breaks peek_user(). So handling for that needs to be added too.

> -				regs->psw.addr =
> -					__rewind_psw(regs->psw,
> -						     regs->int_code >> 16);
> +				regs->psw.addr = __rewind_psw(regs->psw, SVC_LEN);

Why? This is unrelated to what you want to achieve, and it is broken
if the svc instruction is executed via exrl. Plus, let's not use
hardcoded instruction lengths if the CPU provides you the correct
length anyway.

Besides that it _seems_ to be ok. :)