[PATCH 00/23] powerpc: Syscall wrapper and register clearing

[PATCH 00/23] powerpc: Syscall wrapper and register clearing

[Rohan McLure]

V4 available here:

Link: https://lore.kernel.org/all/20220824020548.62625-1-rmclure@linux.ibm.com/

Implement a syscall wrapper, causing arguments to handlers to be passed
via a struct pt_regs on the stack. The syscall wrapper is implemented
for all platforms other than the Cell processor, from which SPUs expect
the ability to directly call syscall handler symbols with the regular
in-register calling convention.

Adopting syscall wrappers requires redefinition of architecture-specific
syscalls and compatibility syscalls to use the SYSCALL_DEFINE and
COMPAT_SYSCALL_DEFINE macros, as well as removal of direct-references to
the emitted syscall-handler symbols from within the kernel. This work
lead to the following modernisations of powerpc's syscall handlers:

 - Replace syscall 82 semantics with sys_old_select and remove
   ppc_select handler, which features direct call to both sys_old_select
   and sys_select.
 - Use a generic fallocate compatibility syscall

Replace asm implementation of syscall table with C implementation for
more compile-time checks.

Many compatibility syscalls are candidates to be removed in favour of
generically defined handlers, but exhibit different parameter orderings
and numberings due to 32-bit ABI support for 64-bit parameters. The
parameter reorderings are however consistent with arm. A future patch
series will serve to modernise syscalls by providing generic
implementations featuring these reorderings.

The design of this syscall is very similar to the s390, x86 and arm64
implementations. See also Commit 4378a7d4be30 (arm64: implement syscall wrappers).
The motivation for this change is that it allows for the clearing of
register state when entering the kernel via through interrupt handlers
on 64-bit servers. This serves to reduce the influence of values in
registers carried over from the interrupted process, e.g. syscall
parameters from user space, or user state at the site of a pagefault.
All values in registers are saved and zeroized at the entry to an
interrupt handler and restored afterward. While this may sound like a
heavy-weight mitigation, many gprs are already saved and restored on
handling of an interrupt, and the mmap_bench benchmark on Power 9 guest,
repeatedly invoking the pagefault handler suggests at most ~0.8%
regression in performance. Realistic workloads are not constantly
producing interrupts, and so this does not indicate realistic slowdown.

Using wrapped syscalls yields to a performance improvement of ~5.6% on
the null_syscall benchmark on pseries guests, by removing the need for
system_call_exception to allocate its own stack frame. This amortises
the additional costs of saving and restoring non-volatile registers
(register clearing is cheap on super scalar platforms), and so the
final mitigation actually yields a net performance improvement of ~0.6%
on the null_syscall benchmark.

The clearing of general purpose registers on interrupts other than
syscalls is enabled by default only on Book3E 64-bit systems (where the
mitigation is inexpensive), but available to other 64-bit systems via
the INTERRUPT_SANITIZE_REGISTERS Kconfig option. This mitigation is
optional, as the speculation influence of interrupts is likely less than
that of syscalls.

Patch Changelog:

 - Format orig_r3 handling as its own patch rather than just a revert.
 - Provide asm-generic BE implementation of long-long munging syscall
   compatiblity arguments.
 - Syscall #82 now refers to generic sys_old_select or
   comptat_sys_old_select.
 - Drop 'inline' on static helper functions for mmap, personality.
 - Remove arch-specific sys fallocate implementation that was meant to
   have been removed in V2.
 - Remove references to syscall wrapper until it is introduced.
 - Rearrange patch series so the last five patches are syscall wrapper >
   syscall register clears > interrupt register clears.
 - Whether non-syscall interrupts should clear registers is now
   configurable by INTERRUPT_SANITIZE_REGISTERS.

Rohan McLure (23):
  powerpc: Remove asmlinkage from syscall handler definitions
  powerpc: Save caller r3 prior to system_call_exception
  powerpc: Add ZEROIZE_GPRS macros for register clears
  powerpc/64s: Use {ZEROIZE,SAVE,REST}_GPRS macros in sc, scv 0 handlers
  powerpc/32: Clarify interrupt restores with REST_GPR macro in
    entry_32.S
  powerpc/64e: Clarify register saves and clears with
    {SAVE,ZEROIZE}_GPRS
  powerpc/64s: Fix comment on interrupt handler prologue
  powerpc: Fix fallocate and fadvise64_64 compat parameter combination
  asm-generic: compat: Support BE for long long args in 32-bit ABIs
  powerpc: Use generic fallocate compatibility syscall
  powerpc/32: Remove powerpc select specialisation
  powerpc: Remove direct call to personality syscall handler
  powerpc: Remove direct call to mmap2 syscall handlers
  powerpc: Provide do_ppc64_personality helper
  powerpc: Adopt SYSCALL_DEFINE for arch-specific syscall handlers
  powerpc: Include all arch-specific syscall prototypes
  powerpc: Enable compile-time check for syscall handlers
  powerpc: Use common syscall handler type
  powerpc: Provide syscall wrapper
  powerpc/64s: Clear/restore caller gprs in syscall interrupt/return
  powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig
  powerpc/64s: Clear gprs on interrupt routine entry in Book3S
  powerpc/64e: Clear gprs on interrupt routine entry on Book3E

 arch/powerpc/Kconfig                         |  10 ++
 arch/powerpc/include/asm/interrupt.h         |   3 +-
 arch/powerpc/include/asm/ppc_asm.h           |  22 +++
 arch/powerpc/include/asm/syscall.h           |  11 +-
 arch/powerpc/include/asm/syscall_wrapper.h   |  84 ++++++++++
 arch/powerpc/include/asm/syscalls.h          | 148 +++++++++++++----
 .../ppc32.h => include/asm/syscalls_32.h}    |   0
 arch/powerpc/include/asm/unistd.h            |   1 +
 arch/powerpc/kernel/entry_32.S               |  40 ++---
 arch/powerpc/kernel/exceptions-64e.S         |  35 ++--
 arch/powerpc/kernel/exceptions-64s.S         |  41 ++++-
 arch/powerpc/kernel/interrupt_64.S           |  92 +++++-----
 arch/powerpc/kernel/signal_32.c              |   2 +-
 arch/powerpc/kernel/sys_ppc32.c              |  66 +++-----
 arch/powerpc/kernel/syscall.c                |  32 ++--
 arch/powerpc/kernel/syscalls.c               |  61 ++++---
 arch/powerpc/kernel/syscalls/syscall.tbl     |  24 +--
 arch/powerpc/kernel/{systbl.S => systbl.c}   |  30 ++--
 arch/powerpc/kernel/vdso.c                   |   6 +-
 arch/powerpc/perf/callchain_32.c             |   2 +-
 arch/powerpc/platforms/cell/spu_callbacks.c  |   6 +-
 include/asm-generic/compat.h                 |   9 +-
 .../arch/powerpc/entry/syscalls/syscall.tbl  |  24 +--
 23 files changed, 491 insertions(+), 258 deletions(-)
 create mode 100644 arch/powerpc/include/asm/syscall_wrapper.h
 rename arch/powerpc/{kernel/ppc32.h => include/asm/syscalls_32.h} (100%)
 rename arch/powerpc/kernel/{systbl.S => systbl.c} (55%)

-- 
2.34.1

[PATCH 01/23] powerpc: Remove asmlinkage from syscall handler definitions

[Rohan McLure]

The asmlinkage macro has no special meaning in powerpc, and prior to
this patch is used sporadically on some syscall handler definitions. On
architectures that do not define asmlinkage, it resolves to extern "C"
for C++ compilers and a nop otherwise. The current invocations of
asmlinkage provide far from complete support for C++ toolchains, and so
the macro serves no purpose in powerpc.

Remove all invocations of asmlinkage in arch/powerpc. These incidentally
only occur in syscall definitions and prototypes.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
---
V2 -> V3: new patch
---
 arch/powerpc/include/asm/syscalls.h | 16 ++++++++--------
 arch/powerpc/kernel/sys_ppc32.c     |  8 ++++----
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index a2b13e55254f..21c2faaa2957 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -10,14 +10,14 @@
 
 struct rtas_args;
 
-asmlinkage long sys_mmap(unsigned long addr, size_t len,
-		unsigned long prot, unsigned long flags,
-		unsigned long fd, off_t offset);
-asmlinkage long sys_mmap2(unsigned long addr, size_t len,
-		unsigned long prot, unsigned long flags,
-		unsigned long fd, unsigned long pgoff);
-asmlinkage long ppc64_personality(unsigned long personality);
-asmlinkage long sys_rtas(struct rtas_args __user *uargs);
+long sys_mmap(unsigned long addr, size_t len,
+	      unsigned long prot, unsigned long flags,
+	      unsigned long fd, off_t offset);
+long sys_mmap2(unsigned long addr, size_t len,
+	       unsigned long prot, unsigned long flags,
+	       unsigned long fd, unsigned long pgoff);
+long ppc64_personality(unsigned long personality);
+long sys_rtas(struct rtas_args __user *uargs);
 int ppc_select(int n, fd_set __user *inp, fd_set __user *outp,
 	       fd_set __user *exp, struct __kernel_old_timeval __user *tvp);
 long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
diff --git a/arch/powerpc/kernel/sys_ppc32.c b/arch/powerpc/kernel/sys_ppc32.c
index 16ff0399a257..f4edcc9489fb 100644
--- a/arch/powerpc/kernel/sys_ppc32.c
+++ b/arch/powerpc/kernel/sys_ppc32.c
@@ -85,20 +85,20 @@ compat_ssize_t compat_sys_readahead(int fd, u32 r4, u32 offset1, u32 offset2, u3
 	return ksys_readahead(fd, merge_64(offset1, offset2), count);
 }
 
-asmlinkage int compat_sys_truncate64(const char __user * path, u32 reg4,
+int compat_sys_truncate64(const char __user * path, u32 reg4,
 				unsigned long len1, unsigned long len2)
 {
 	return ksys_truncate(path, merge_64(len1, len2));
 }
 
-asmlinkage long compat_sys_fallocate(int fd, int mode, u32 offset1, u32 offset2,
+long compat_sys_fallocate(int fd, int mode, u32 offset1, u32 offset2,
 				     u32 len1, u32 len2)
 {
 	return ksys_fallocate(fd, mode, ((loff_t)offset1 << 32) | offset2,
 			     merge_64(len1, len2));
 }
 
-asmlinkage int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long len1,
+int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long len1,
 				 unsigned long len2)
 {
 	return ksys_ftruncate(fd, merge_64(len1, len2));
@@ -111,7 +111,7 @@ long ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
 				 advice);
 }
 
-asmlinkage long compat_sys_sync_file_range2(int fd, unsigned int flags,
+long compat_sys_sync_file_range2(int fd, unsigned int flags,
 				   unsigned offset1, unsigned offset2,
 				   unsigned nbytes1, unsigned nbytes2)
 {
-- 
2.34.1

[PATCH 02/23] powerpc: Save caller r3 prior to system_call_exception

[Rohan McLure]

This reverts commit 8875f47b7681 ("powerpc/syscall: Save r3 in regs->orig_r3
").

Save caller's original r3 state to the kernel stackframe before entering
system_call_exception. This allows for user registers to be cleared by
the time system_call_exception is entered, reducing the influence of
user registers on speculation within the kernel.

Prior to this commit, orig_r3 was saved at the beginning of
system_call_exception. Instead, save orig_r3 while the user value is
still live in r3.

Also replicate this early save in 32-bit. A similar save was removed in
commit 6f76a01173cc ("powerpc/syscall: implement system call entry/exit
logic in C for PPC32") when 32-bit adopted system_call_exception. Revert
its removal of orig_r3 saves.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
V2 -> V3: New commit.
V4 -> V5: New commit message, as we do more than just revert 8875f47b7681.
---
 arch/powerpc/kernel/entry_32.S     | 1 +
 arch/powerpc/kernel/interrupt_64.S | 2 ++
 arch/powerpc/kernel/syscall.c      | 1 -
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 1d599df6f169..44dfce9a60c5 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -101,6 +101,7 @@ __kuep_unlock:
 
 	.globl	transfer_to_syscall
 transfer_to_syscall:
+	stw	r3, ORIG_GPR3(r1)
 	stw	r11, GPR1(r1)
 	stw	r11, 0(r1)
 	mflr	r12
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index ce25b28cf418..71d2d9497283 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -91,6 +91,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name)
 	li	r11,\trapnr
 	std	r11,_TRAP(r1)
 	std	r12,_CCR(r1)
+	std	r3,ORIG_GPR3(r1)
 	addi	r10,r1,STACK_FRAME_OVERHEAD
 	ld	r11,exception_marker@toc(r2)
 	std	r11,-16(r10)		/* "regshere" marker */
@@ -275,6 +276,7 @@ END_BTB_FLUSH_SECTION
 	std	r10,_LINK(r1)
 	std	r11,_TRAP(r1)
 	std	r12,_CCR(r1)
+	std	r3,ORIG_GPR3(r1)
 	addi	r10,r1,STACK_FRAME_OVERHEAD
 	ld	r11,exception_marker@toc(r2)
 	std	r11,-16(r10)		/* "regshere" marker */
diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
index 81ace9e8b72b..64102a64fd84 100644
--- a/arch/powerpc/kernel/syscall.c
+++ b/arch/powerpc/kernel/syscall.c
@@ -25,7 +25,6 @@ notrace long system_call_exception(long r3, long r4, long r5,
 	kuap_lock();
 
 	add_random_kstack_offset();
-	regs->orig_gpr3 = r3;
 
 	if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
 		BUG_ON(irq_soft_mask_return() != IRQS_ALL_DISABLED);
-- 
2.34.1

[PATCH 03/23] powerpc: Add ZEROIZE_GPRS macros for register clears

[Rohan McLure]

Provide register zeroing macros, following the same convention as
existing register stack save/restore macros, to be used in later
change to concisely zero a sequence of consecutive gprs.

The resulting macros are called ZEROIZE_GPRS and ZEROIZE_NVGPRS, keeping
with the naming of the accompanying restore and save macros, and usage
of zeroize to describe this operation elsewhere in the kernel.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
V1 -> V2: Change 'ZERO' usage in naming to 'NULLIFY', a more obvious verb
V2 -> V3: Change 'NULLIFY' usage in naming to 'ZEROIZE', which has
precedent in kernel and explicitly specifies that we are zeroing.
V3 -> V4: Update commit message to use zeroize.
V4 -> V5: The reason for the patch is to add zeroize macros. Move that
to first paragraph in patch description.
---
 arch/powerpc/include/asm/ppc_asm.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index 83c02f5a7f2a..b95689ada59c 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -33,6 +33,20 @@
 	.endr
 .endm
 
+/*
+ * This expands to a sequence of register clears for regs start to end
+ * inclusive, of the form:
+ *
+ *   li rN, 0
+ */
+.macro ZEROIZE_REGS start, end
+	.Lreg=\start
+	.rept (\end - \start + 1)
+	li	.Lreg, 0
+	.Lreg=.Lreg+1
+	.endr
+.endm
+
 /*
  * Macros for storing registers into and loading registers from
  * exception frames.
@@ -49,6 +63,14 @@
 #define REST_NVGPRS(base)		REST_GPRS(13, 31, base)
 #endif
 
+#define	ZEROIZE_GPRS(start, end)	ZEROIZE_REGS start, end
+#ifdef __powerpc64__
+#define	ZEROIZE_NVGPRS()		ZEROIZE_GPRS(14, 31)
+#else
+#define	ZEROIZE_NVGPRS()		ZEROIZE_GPRS(13, 31)
+#endif
+#define	ZEROIZE_GPR(n)			ZEROIZE_GPRS(n, n)
+
 #define SAVE_GPR(n, base)		SAVE_GPRS(n, n, base)
 #define REST_GPR(n, base)		REST_GPRS(n, n, base)
 
-- 
2.34.1

[PATCH 04/23] powerpc/64s: Use {ZEROIZE,SAVE,REST}_GPRS macros in sc, scv 0 handlers

[Rohan McLure]

Use the convenience macros for saving/clearing/restoring gprs in keeping
with syscall calling conventions. The plural variants of these macros
can store a range of registers for concision.

This works well when the user gpr value we are hoping to save is still
live. In the syscall interrupt handlers, user register state is
sometimes juggled between registers. Hold-off from issuing the SAVE_GPR
macro for applicable neighbouring lines to highlight the delicate
register save logic.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
V1 -> V2: Update summary
V2 -> V3: Update summary regarding exclusions for the SAVE_GPR marco.
Acknowledge new name for ZEROIZE_GPR{,S} macros.
V4 -> V5: Move to beginning of series
---
 arch/powerpc/kernel/interrupt_64.S | 43 ++++++----------------------
 1 file changed, 9 insertions(+), 34 deletions(-)

diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 71d2d9497283..7d92a7a54727 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -71,12 +71,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name)
 	mfcr	r12
 	li	r11,0
 	/* Can we avoid saving r3-r8 in common case? */
-	std	r3,GPR3(r1)
-	std	r4,GPR4(r1)
-	std	r5,GPR5(r1)
-	std	r6,GPR6(r1)
-	std	r7,GPR7(r1)
-	std	r8,GPR8(r1)
+	SAVE_GPRS(3, 8, r1)
 	/* Zero r9-r12, this should only be required when restoring all GPRs */
 	std	r11,GPR9(r1)
 	std	r11,GPR10(r1)
@@ -149,17 +144,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	/* Could zero these as per ABI, but we may consider a stricter ABI
 	 * which preserves these if libc implementations can benefit, so
 	 * restore them for now until further measurement is done. */
-	ld	r0,GPR0(r1)
-	ld	r4,GPR4(r1)
-	ld	r5,GPR5(r1)
-	ld	r6,GPR6(r1)
-	ld	r7,GPR7(r1)
-	ld	r8,GPR8(r1)
+	REST_GPR(0, r1)
+	REST_GPRS(4, 8, r1)
 	/* Zero volatile regs that may contain sensitive kernel data */
-	li	r9,0
-	li	r10,0
-	li	r11,0
-	li	r12,0
+	ZEROIZE_GPRS(9, 12)
 	mtspr	SPRN_XER,r0
 
 	/*
@@ -182,7 +170,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	ld	r5,_XER(r1)
 
 	REST_NVGPRS(r1)
-	ld	r0,GPR0(r1)
+	REST_GPR(0, r1)
 	mtcr	r2
 	mtctr	r3
 	mtlr	r4
@@ -250,12 +238,7 @@ END_BTB_FLUSH_SECTION
 	mfcr	r12
 	li	r11,0
 	/* Can we avoid saving r3-r8 in common case? */
-	std	r3,GPR3(r1)
-	std	r4,GPR4(r1)
-	std	r5,GPR5(r1)
-	std	r6,GPR6(r1)
-	std	r7,GPR7(r1)
-	std	r8,GPR8(r1)
+	SAVE_GPRS(3, 8, r1)
 	/* Zero r9-r12, this should only be required when restoring all GPRs */
 	std	r11,GPR9(r1)
 	std	r11,GPR10(r1)
@@ -345,16 +328,8 @@ END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
 	cmpdi	r3,0
 	bne	.Lsyscall_restore_regs
 	/* Zero volatile regs that may contain sensitive kernel data */
-	li	r0,0
-	li	r4,0
-	li	r5,0
-	li	r6,0
-	li	r7,0
-	li	r8,0
-	li	r9,0
-	li	r10,0
-	li	r11,0
-	li	r12,0
+	ZEROIZE_GPR(0)
+	ZEROIZE_GPRS(4, 12)
 	mtctr	r0
 	mtspr	SPRN_XER,r0
 .Lsyscall_restore_regs_cont:
@@ -380,7 +355,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	REST_NVGPRS(r1)
 	mtctr	r3
 	mtspr	SPRN_XER,r4
-	ld	r0,GPR0(r1)
+	REST_GPR(0, r1)
 	REST_GPRS(4, 12, r1)
 	b	.Lsyscall_restore_regs_cont
 .Lsyscall_rst_end:
-- 
2.34.1

[PATCH 05/23] powerpc/32: Clarify interrupt restores with REST_GPR macro in entry_32.S

[Rohan McLure]

Restoring the register state of the interrupted thread involves issuing
a large number of predictable loads to the kernel stack frame. Issue the
REST_GPR{,S} macros to clearly signal when this is happening, and bunch
together restores at the end of the interrupt handler where the saved
value is not consumed earlier in the handler code.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Reported-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
V2 -> V3: New patch.
V3 -> V4: Minimise restores in the unrecoverable window between
restoring SRR0/1 and return from interrupt.
---
 arch/powerpc/kernel/entry_32.S | 33 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 44dfce9a60c5..e4b694cebc44 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -68,7 +68,7 @@ prepare_transfer_to_handler:
 	lwz	r9,_MSR(r11)		/* if sleeping, clear MSR.EE */
 	rlwinm	r9,r9,0,~MSR_EE
 	lwz	r12,_LINK(r11)		/* and return to address in LR */
-	lwz	r2, GPR2(r11)
+	REST_GPR(2, r11)
 	b	fast_exception_return
 _ASM_NOKPROBE_SYMBOL(prepare_transfer_to_handler)
 #endif /* CONFIG_PPC_BOOK3S_32 || CONFIG_E500 */
@@ -144,7 +144,7 @@ ret_from_syscall:
 	lwz	r7,_NIP(r1)
 	lwz	r8,_MSR(r1)
 	cmpwi	r3,0
-	lwz	r3,GPR3(r1)
+	REST_GPR(3, r1)
 syscall_exit_finish:
 	mtspr	SPRN_SRR0,r7
 	mtspr	SPRN_SRR1,r8
@@ -152,8 +152,8 @@ syscall_exit_finish:
 	bne	3f
 	mtcr	r5
 
-1:	lwz	r2,GPR2(r1)
-	lwz	r1,GPR1(r1)
+1:	REST_GPR(2, r1)
+	REST_GPR(1, r1)
 	rfi
 #ifdef CONFIG_40x
 	b .	/* Prevent prefetch past rfi */
@@ -165,10 +165,8 @@ syscall_exit_finish:
 	REST_NVGPRS(r1)
 	mtctr	r4
 	mtxer	r5
-	lwz	r0,GPR0(r1)
-	lwz	r3,GPR3(r1)
-	REST_GPRS(4, 11, r1)
-	lwz	r12,GPR12(r1)
+	REST_GPR(0, r1)
+	REST_GPRS(3, 12, r1)
 	b	1b
 
 #ifdef CONFIG_44x
@@ -260,9 +258,8 @@ fast_exception_return:
 	beq	3f			/* if not, we've got problems */
 #endif
 
-2:	REST_GPRS(3, 6, r11)
-	lwz	r10,_CCR(r11)
-	REST_GPRS(1, 2, r11)
+2:	lwz	r10,_CCR(r11)
+	REST_GPRS(1, 6, r11)
 	mtcr	r10
 	lwz	r10,_LINK(r11)
 	mtlr	r10
@@ -277,7 +274,7 @@ fast_exception_return:
 	mtspr	SPRN_SRR0,r12
 	REST_GPR(9, r11)
 	REST_GPR(12, r11)
-	lwz	r11,GPR11(r11)
+	REST_GPR(11, r11)
 	rfi
 #ifdef CONFIG_40x
 	b .	/* Prevent prefetch past rfi */
@@ -454,9 +451,8 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return)
 	lwz	r3,_MSR(r1);						\
 	andi.	r3,r3,MSR_PR;						\
 	bne	interrupt_return;					\
-	lwz	r0,GPR0(r1);						\
-	lwz	r2,GPR2(r1);						\
-	REST_GPRS(3, 8, r1);						\
+	REST_GPR(0, r1);						\
+	REST_GPRS(2, 8, r1);						\
 	lwz	r10,_XER(r1);						\
 	lwz	r11,_CTR(r1);						\
 	mtspr	SPRN_XER,r10;						\
@@ -475,11 +471,8 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return)
 	lwz	r12,_MSR(r1);						\
 	mtspr	exc_lvl_srr0,r11;					\
 	mtspr	exc_lvl_srr1,r12;					\
-	lwz	r9,GPR9(r1);						\
-	lwz	r12,GPR12(r1);						\
-	lwz	r10,GPR10(r1);						\
-	lwz	r11,GPR11(r1);						\
-	lwz	r1,GPR1(r1);						\
+	REST_GPRS(9, 12, r1);						\
+	REST_GPR(1, r1);						\
 	exc_lvl_rfi;							\
 	b	.;		/* prevent prefetch past exc_lvl_rfi */
 
-- 
2.34.1

Re: [PATCH 05/23] powerpc/32: Clarify interrupt restores with REST_GPR macro in entry_32.S

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> Restoring the register state of the interrupted thread involves issuing
> a large number of predictable loads to the kernel stack frame. Issue the
> REST_GPR{,S} macros to clearly signal when this is happening, and bunch
> together restores at the end of the interrupt handler where the saved
> value is not consumed earlier in the handler code.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> Reported-by: Christophe Leroy <christophe.leroy@csgroup.eu>

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> ---
> V2 -> V3: New patch.
> V3 -> V4: Minimise restores in the unrecoverable window between
> restoring SRR0/1 and return from interrupt.
> ---
>  arch/powerpc/kernel/entry_32.S | 33 +++++++++++++-------------------
>  1 file changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 44dfce9a60c5..e4b694cebc44 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -68,7 +68,7 @@ prepare_transfer_to_handler:
>  	lwz	r9,_MSR(r11)		/* if sleeping, clear MSR.EE */
>  	rlwinm	r9,r9,0,~MSR_EE
>  	lwz	r12,_LINK(r11)		/* and return to address in LR */
> -	lwz	r2, GPR2(r11)
> +	REST_GPR(2, r11)
>  	b	fast_exception_return
>  _ASM_NOKPROBE_SYMBOL(prepare_transfer_to_handler)
>  #endif /* CONFIG_PPC_BOOK3S_32 || CONFIG_E500 */
> @@ -144,7 +144,7 @@ ret_from_syscall:
>  	lwz	r7,_NIP(r1)
>  	lwz	r8,_MSR(r1)
>  	cmpwi	r3,0
> -	lwz	r3,GPR3(r1)
> +	REST_GPR(3, r1)
>  syscall_exit_finish:
>  	mtspr	SPRN_SRR0,r7
>  	mtspr	SPRN_SRR1,r8
> @@ -152,8 +152,8 @@ syscall_exit_finish:
>  	bne	3f
>  	mtcr	r5
>  
> -1:	lwz	r2,GPR2(r1)
> -	lwz	r1,GPR1(r1)
> +1:	REST_GPR(2, r1)
> +	REST_GPR(1, r1)
>  	rfi
>  #ifdef CONFIG_40x
>  	b .	/* Prevent prefetch past rfi */
> @@ -165,10 +165,8 @@ syscall_exit_finish:
>  	REST_NVGPRS(r1)
>  	mtctr	r4
>  	mtxer	r5
> -	lwz	r0,GPR0(r1)
> -	lwz	r3,GPR3(r1)
> -	REST_GPRS(4, 11, r1)
> -	lwz	r12,GPR12(r1)
> +	REST_GPR(0, r1)
> +	REST_GPRS(3, 12, r1)
>  	b	1b
>  
>  #ifdef CONFIG_44x
> @@ -260,9 +258,8 @@ fast_exception_return:
>  	beq	3f			/* if not, we've got problems */
>  #endif
>  
> -2:	REST_GPRS(3, 6, r11)
> -	lwz	r10,_CCR(r11)
> -	REST_GPRS(1, 2, r11)
> +2:	lwz	r10,_CCR(r11)
> +	REST_GPRS(1, 6, r11)
>  	mtcr	r10
>  	lwz	r10,_LINK(r11)
>  	mtlr	r10
> @@ -277,7 +274,7 @@ fast_exception_return:
>  	mtspr	SPRN_SRR0,r12
>  	REST_GPR(9, r11)
>  	REST_GPR(12, r11)
> -	lwz	r11,GPR11(r11)
> +	REST_GPR(11, r11)
>  	rfi
>  #ifdef CONFIG_40x
>  	b .	/* Prevent prefetch past rfi */
> @@ -454,9 +451,8 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return)
>  	lwz	r3,_MSR(r1);						\
>  	andi.	r3,r3,MSR_PR;						\
>  	bne	interrupt_return;					\
> -	lwz	r0,GPR0(r1);						\
> -	lwz	r2,GPR2(r1);						\
> -	REST_GPRS(3, 8, r1);						\
> +	REST_GPR(0, r1);						\
> +	REST_GPRS(2, 8, r1);						\
>  	lwz	r10,_XER(r1);						\
>  	lwz	r11,_CTR(r1);						\
>  	mtspr	SPRN_XER,r10;						\
> @@ -475,11 +471,8 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return)
>  	lwz	r12,_MSR(r1);						\
>  	mtspr	exc_lvl_srr0,r11;					\
>  	mtspr	exc_lvl_srr1,r12;					\
> -	lwz	r9,GPR9(r1);						\
> -	lwz	r12,GPR12(r1);						\
> -	lwz	r10,GPR10(r1);						\
> -	lwz	r11,GPR11(r1);						\
> -	lwz	r1,GPR1(r1);						\
> +	REST_GPRS(9, 12, r1);						\
> +	REST_GPR(1, r1);						\
>  	exc_lvl_rfi;							\
>  	b	.;		/* prevent prefetch past exc_lvl_rfi */
>  
> -- 
> 2.34.1

[PATCH 06/23] powerpc/64e: Clarify register saves and clears with {SAVE,ZEROIZE}_GPRS

[Rohan McLure]

The common interrupt handler prologue macro and the bad_stack
trampolines include consecutive sequences of register saves, and some
register clears. Neaten such instances by expanding use of the SAVE_GPRS
macro and employing the ZEROIZE_GPR macro when appropriate.

Also simplify an invocation of SAVE_GPRS targetting all non-volatile
registers to SAVE_NVGPRS.

Signed-off-by: Rohan Mclure <rmclure@linux.ibm.com>
Reported-by: Nicholas Piggin <npiggin@gmail.com>
---
V3 -> V4: New commit.
---
 arch/powerpc/kernel/exceptions-64e.S | 27 +++++++++++---------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 67dc4e3179a0..48c640ca425d 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -216,17 +216,14 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
 	mtlr	r10
 	mtcr	r11
 
-	ld	r10,GPR10(r1)
-	ld	r11,GPR11(r1)
-	ld	r12,GPR12(r1)
+	REST_GPRS(10, 12, r1)
 	mtspr	\scratch,r0
 
 	std	r10,\paca_ex+EX_R10(r13);
 	std	r11,\paca_ex+EX_R11(r13);
 	ld	r10,_NIP(r1)
 	ld	r11,_MSR(r1)
-	ld	r0,GPR0(r1)
-	ld	r1,GPR1(r1)
+	REST_GPRS(0, 1, r1)
 	mtspr	\srr0,r10
 	mtspr	\srr1,r11
 	ld	r10,\paca_ex+EX_R10(r13)
@@ -372,16 +369,15 @@ ret_from_mc_except:
 /* Core exception code for all exceptions except TLB misses. */
 #define EXCEPTION_COMMON_LVL(n, scratch, excf)				    \
 exc_##n##_common:							    \
-	std	r0,GPR0(r1);		/* save r0 in stackframe */	    \
-	std	r2,GPR2(r1);		/* save r2 in stackframe */	    \
-	SAVE_GPRS(3, 9, r1);		/* save r3 - r9 in stackframe */    \
+	SAVE_GPR(0, r1);		/* save r0 in stackframe */	    \
+	SAVE_GPRS(2, 9, r1);		/* save r2 - r9 in stackframe */    \
 	std	r10,_NIP(r1);		/* save SRR0 to stackframe */	    \
 	std	r11,_MSR(r1);		/* save SRR1 to stackframe */	    \
 	beq	2f;			/* if from kernel mode */	    \
 2:	ld	r3,excf+EX_R10(r13);	/* get back r10 */		    \
 	ld	r4,excf+EX_R11(r13);	/* get back r11 */		    \
 	mfspr	r5,scratch;		/* get back r13 */		    \
-	std	r12,GPR12(r1);		/* save r12 in stackframe */	    \
+	SAVE_GPR(12, r1);		/* save r12 in stackframe */	    \
 	ld	r2,PACATOC(r13);	/* get kernel TOC into r2 */	    \
 	mflr	r6;			/* save LR in stackframe */	    \
 	mfctr	r7;			/* save CTR in stackframe */	    \
@@ -390,7 +386,7 @@ exc_##n##_common:							    \
 	lwz	r10,excf+EX_CR(r13);	/* load orig CR back from PACA	*/  \
 	lbz	r11,PACAIRQSOFTMASK(r13); /* get current IRQ softe */	    \
 	ld	r12,exception_marker@toc(r2);				    \
-	li	r0,0;							    \
+	ZEROIZE_GPR(0);							    \
 	std	r3,GPR10(r1);		/* save r10 to stackframe */	    \
 	std	r4,GPR11(r1);		/* save r11 to stackframe */	    \
 	std	r5,GPR13(r1);		/* save it to stackframe */	    \
@@ -1056,15 +1052,14 @@ bad_stack_book3e:
 	mfspr	r11,SPRN_ESR
 	std	r10,_DEAR(r1)
 	std	r11,_ESR(r1)
-	std	r0,GPR0(r1);		/* save r0 in stackframe */	    \
-	std	r2,GPR2(r1);		/* save r2 in stackframe */	    \
-	SAVE_GPRS(3, 9, r1);		/* save r3 - r9 in stackframe */    \
+	SAVE_GPR(0, r1);		/* save r0 in stackframe */	    \
+	SAVE_GPRS(2, 9, r1);		/* save r2 - r9 in stackframe */    \
 	ld	r3,PACA_EXGEN+EX_R10(r13);/* get back r10 */		    \
 	ld	r4,PACA_EXGEN+EX_R11(r13);/* get back r11 */		    \
 	mfspr	r5,SPRN_SPRG_GEN_SCRATCH;/* get back r13 XXX can be wrong */ \
 	std	r3,GPR10(r1);		/* save r10 to stackframe */	    \
 	std	r4,GPR11(r1);		/* save r11 to stackframe */	    \
-	std	r12,GPR12(r1);		/* save r12 in stackframe */	    \
+	SAVE_GPR(12, r1);		/* save r12 in stackframe */	    \
 	std	r5,GPR13(r1);		/* save it to stackframe */	    \
 	mflr	r10
 	mfctr	r11
@@ -1072,12 +1067,12 @@ bad_stack_book3e:
 	std	r10,_LINK(r1)
 	std	r11,_CTR(r1)
 	std	r12,_XER(r1)
-	SAVE_GPRS(14, 31, r1)
+	SAVE_NVGPRS(r1)
 	lhz	r12,PACA_TRAP_SAVE(r13)
 	std	r12,_TRAP(r1)
 	addi	r11,r1,INT_FRAME_SIZE
 	std	r11,0(r1)
-	li	r12,0
+	ZEROIZE_GPR(12)
 	std	r12,0(r11)
 	ld	r2,PACATOC(r13)
 1:	addi	r3,r1,STACK_FRAME_OVERHEAD
-- 
2.34.1

Re: [PATCH 06/23] powerpc/64e: Clarify register saves and clears with {SAVE,ZEROIZE}_GPRS

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> The common interrupt handler prologue macro and the bad_stack
> trampolines include consecutive sequences of register saves, and some
> register clears. Neaten such instances by expanding use of the SAVE_GPRS
> macro and employing the ZEROIZE_GPR macro when appropriate.
>
> Also simplify an invocation of SAVE_GPRS targetting all non-volatile
> registers to SAVE_NVGPRS.
>
> Signed-off-by: Rohan Mclure <rmclure@linux.ibm.com>
> Reported-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> V3 -> V4: New commit.
> ---
>  arch/powerpc/kernel/exceptions-64e.S | 27 +++++++++++---------------
>  1 file changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
> index 67dc4e3179a0..48c640ca425d 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -216,17 +216,14 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
>  	mtlr	r10
>  	mtcr	r11
>  
> -	ld	r10,GPR10(r1)
> -	ld	r11,GPR11(r1)
> -	ld	r12,GPR12(r1)
> +	REST_GPRS(10, 12, r1)
>  	mtspr	\scratch,r0
>  
>  	std	r10,\paca_ex+EX_R10(r13);
>  	std	r11,\paca_ex+EX_R11(r13);
>  	ld	r10,_NIP(r1)
>  	ld	r11,_MSR(r1)
> -	ld	r0,GPR0(r1)
> -	ld	r1,GPR1(r1)
> +	REST_GPRS(0, 1, r1)

I'm hesitant to have a multi reg restore that also clobbers the base
register. It works if we always restore in order, and there might never
be a good reason to change that, but it's possibly a bit easier to miss
when you're looking at the code.

Otherwise,

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

>  	mtspr	\srr0,r10
>  	mtspr	\srr1,r11
>  	ld	r10,\paca_ex+EX_R10(r13)
> @@ -372,16 +369,15 @@ ret_from_mc_except:
>  /* Core exception code for all exceptions except TLB misses. */
>  #define EXCEPTION_COMMON_LVL(n, scratch, excf)				    \
>  exc_##n##_common:							    \
> -	std	r0,GPR0(r1);		/* save r0 in stackframe */	    \
> -	std	r2,GPR2(r1);		/* save r2 in stackframe */	    \
> -	SAVE_GPRS(3, 9, r1);		/* save r3 - r9 in stackframe */    \
> +	SAVE_GPR(0, r1);		/* save r0 in stackframe */	    \
> +	SAVE_GPRS(2, 9, r1);		/* save r2 - r9 in stackframe */    \
>  	std	r10,_NIP(r1);		/* save SRR0 to stackframe */	    \
>  	std	r11,_MSR(r1);		/* save SRR1 to stackframe */	    \
>  	beq	2f;			/* if from kernel mode */	    \
>  2:	ld	r3,excf+EX_R10(r13);	/* get back r10 */		    \
>  	ld	r4,excf+EX_R11(r13);	/* get back r11 */		    \
>  	mfspr	r5,scratch;		/* get back r13 */		    \
> -	std	r12,GPR12(r1);		/* save r12 in stackframe */	    \
> +	SAVE_GPR(12, r1);		/* save r12 in stackframe */	    \
>  	ld	r2,PACATOC(r13);	/* get kernel TOC into r2 */	    \
>  	mflr	r6;			/* save LR in stackframe */	    \
>  	mfctr	r7;			/* save CTR in stackframe */	    \
> @@ -390,7 +386,7 @@ exc_##n##_common:							    \
>  	lwz	r10,excf+EX_CR(r13);	/* load orig CR back from PACA	*/  \
>  	lbz	r11,PACAIRQSOFTMASK(r13); /* get current IRQ softe */	    \
>  	ld	r12,exception_marker@toc(r2);				    \
> -	li	r0,0;							    \
> +	ZEROIZE_GPR(0);							    \
>  	std	r3,GPR10(r1);		/* save r10 to stackframe */	    \
>  	std	r4,GPR11(r1);		/* save r11 to stackframe */	    \
>  	std	r5,GPR13(r1);		/* save it to stackframe */	    \
> @@ -1056,15 +1052,14 @@ bad_stack_book3e:
>  	mfspr	r11,SPRN_ESR
>  	std	r10,_DEAR(r1)
>  	std	r11,_ESR(r1)
> -	std	r0,GPR0(r1);		/* save r0 in stackframe */	    \
> -	std	r2,GPR2(r1);		/* save r2 in stackframe */	    \
> -	SAVE_GPRS(3, 9, r1);		/* save r3 - r9 in stackframe */    \
> +	SAVE_GPR(0, r1);		/* save r0 in stackframe */	    \
> +	SAVE_GPRS(2, 9, r1);		/* save r2 - r9 in stackframe */    \
>  	ld	r3,PACA_EXGEN+EX_R10(r13);/* get back r10 */		    \
>  	ld	r4,PACA_EXGEN+EX_R11(r13);/* get back r11 */		    \
>  	mfspr	r5,SPRN_SPRG_GEN_SCRATCH;/* get back r13 XXX can be wrong */ \
>  	std	r3,GPR10(r1);		/* save r10 to stackframe */	    \
>  	std	r4,GPR11(r1);		/* save r11 to stackframe */	    \
> -	std	r12,GPR12(r1);		/* save r12 in stackframe */	    \
> +	SAVE_GPR(12, r1);		/* save r12 in stackframe */	    \
>  	std	r5,GPR13(r1);		/* save it to stackframe */	    \
>  	mflr	r10
>  	mfctr	r11
> @@ -1072,12 +1067,12 @@ bad_stack_book3e:
>  	std	r10,_LINK(r1)
>  	std	r11,_CTR(r1)
>  	std	r12,_XER(r1)
> -	SAVE_GPRS(14, 31, r1)
> +	SAVE_NVGPRS(r1)
>  	lhz	r12,PACA_TRAP_SAVE(r13)
>  	std	r12,_TRAP(r1)
>  	addi	r11,r1,INT_FRAME_SIZE
>  	std	r11,0(r1)
> -	li	r12,0
> +	ZEROIZE_GPR(12)
>  	std	r12,0(r11)
>  	ld	r2,PACATOC(r13)
>  1:	addi	r3,r1,STACK_FRAME_OVERHEAD
> -- 
> 2.34.1

[PATCH 07/23] powerpc/64s: Fix comment on interrupt handler prologue

[Rohan McLure]

Interrupt handlers on 64s systems will often need to save register state
from the interrupted process to make space for loading special purpose
registers or for internal state.

Fix a comment documenting a common code path macro in the beginning of
interrupt handlers where r10 is saved to the PACA to afford space for
the value of the CFAR. Comment is currently written as if r10-r12 are
saved to PACA, but in fact only r10 is saved, with r11-r12 saved much
later. The distance in code between these saves has grown over the many
revisions of this macro. Fix this by signalling with a comment where
r11-r12 are saved to the PACA.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Reported-by: Nicholas Piggin <npiggin@gmail.com>
---
V1 -> V2: Given its own commit
V2 -> V3: Annotate r11-r12 save locations with comment.
---
 arch/powerpc/kernel/exceptions-64s.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 3d0dc133a9ae..a3b51441b039 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -281,7 +281,7 @@ BEGIN_FTR_SECTION
 	mfspr	r9,SPRN_PPR
 END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	HMT_MEDIUM
-	std	r10,IAREA+EX_R10(r13)		/* save r10 - r12 */
+	std	r10,IAREA+EX_R10(r13)		/* save r10 */
 	.if ICFAR
 BEGIN_FTR_SECTION
 	mfspr	r10,SPRN_CFAR
@@ -321,7 +321,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
 	mfctr	r10
 	std	r10,IAREA+EX_CTR(r13)
 	mfcr	r9
-	std	r11,IAREA+EX_R11(r13)
+	std	r11,IAREA+EX_R11(r13)		/* save r11 - r12 */
 	std	r12,IAREA+EX_R12(r13)
 
 	/*
-- 
2.34.1

Re: [PATCH 07/23] powerpc/64s: Fix comment on interrupt handler prologue

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> Interrupt handlers on 64s systems will often need to save register state
> from the interrupted process to make space for loading special purpose
> registers or for internal state.
>
> Fix a comment documenting a common code path macro in the beginning of
> interrupt handlers where r10 is saved to the PACA to afford space for
> the value of the CFAR. Comment is currently written as if r10-r12 are
> saved to PACA, but in fact only r10 is saved, with r11-r12 saved much
> later. The distance in code between these saves has grown over the many
> revisions of this macro. Fix this by signalling with a comment where
> r11-r12 are saved to the PACA.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> Reported-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> ---
> V1 -> V2: Given its own commit
> V2 -> V3: Annotate r11-r12 save locations with comment.
> ---
>  arch/powerpc/kernel/exceptions-64s.S | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 3d0dc133a9ae..a3b51441b039 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -281,7 +281,7 @@ BEGIN_FTR_SECTION
>  	mfspr	r9,SPRN_PPR
>  END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  	HMT_MEDIUM
> -	std	r10,IAREA+EX_R10(r13)		/* save r10 - r12 */
> +	std	r10,IAREA+EX_R10(r13)		/* save r10 */
>  	.if ICFAR
>  BEGIN_FTR_SECTION
>  	mfspr	r10,SPRN_CFAR
> @@ -321,7 +321,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
>  	mfctr	r10
>  	std	r10,IAREA+EX_CTR(r13)
>  	mfcr	r9
> -	std	r11,IAREA+EX_R11(r13)
> +	std	r11,IAREA+EX_R11(r13)		/* save r11 - r12 */
>  	std	r12,IAREA+EX_R12(r13)
>  
>  	/*
> -- 
> 2.34.1

[PATCH 08/23] powerpc: Fix fallocate and fadvise64_64 compat parameter combination

[Rohan McLure]

As reported[1] by Arnd, the arch-specific fadvise64_64 and fallocate
compatibility handlers assume parameters are passed with 32-bit
big-endian ABI. This affects the assignment of odd-even parameter pairs
to the high or low words of a 64-bit syscall parameter.

Fix fadvise64_64 fallocate compat handlers to correctly swap upper/lower
32 bits conditioned on endianness.

A future patch will replace the arch-specific compat fallocate with an
asm-generic implementation. This patch is intended for ease of
back-port.

[1]: https://lore.kernel.org/all/be29926f-226e-48dc-871a-e29a54e80583@www.fastmail.com/

Fixes: 57f48b4b74e7 ("powerpc/compat_sys: swap hi/lo parts of 64-bit syscall args in LE mode")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
V4 -> V5: New patch.
---
 arch/powerpc/include/asm/syscalls.h | 12 ++++++++++++
 arch/powerpc/kernel/sys_ppc32.c     | 14 +-------------
 arch/powerpc/kernel/syscalls.c      |  4 ++--
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index 21c2faaa2957..16b668515d15 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -8,6 +8,18 @@
 #include <linux/types.h>
 #include <linux/compat.h>
 
+/*
+ * long long munging:
+ * The 32 bit ABI passes long longs in an odd even register pair.
+ * High and low parts are swapped depending on endian mode,
+ * so define a macro (similar to mips linux32) to handle that.
+ */
+#ifdef __LITTLE_ENDIAN__
+#define merge_64(low, high) ((u64)high << 32) | low
+#else
+#define merge_64(high, low) ((u64)high << 32) | low
+#endif
+
 struct rtas_args;
 
 long sys_mmap(unsigned long addr, size_t len,
diff --git a/arch/powerpc/kernel/sys_ppc32.c b/arch/powerpc/kernel/sys_ppc32.c
index f4edcc9489fb..ba363328da2b 100644
--- a/arch/powerpc/kernel/sys_ppc32.c
+++ b/arch/powerpc/kernel/sys_ppc32.c
@@ -56,18 +56,6 @@ unsigned long compat_sys_mmap2(unsigned long addr, size_t len,
 	return sys_mmap(addr, len, prot, flags, fd, pgoff << 12);
 }
 
-/* 
- * long long munging:
- * The 32 bit ABI passes long longs in an odd even register pair.
- * High and low parts are swapped depending on endian mode,
- * so define a macro (similar to mips linux32) to handle that.
- */
-#ifdef __LITTLE_ENDIAN__
-#define merge_64(low, high) ((u64)high << 32) | low
-#else
-#define merge_64(high, low) ((u64)high << 32) | low
-#endif
-
 compat_ssize_t compat_sys_pread64(unsigned int fd, char __user *ubuf, compat_size_t count,
 			     u32 reg6, u32 pos1, u32 pos2)
 {
@@ -94,7 +82,7 @@ int compat_sys_truncate64(const char __user * path, u32 reg4,
 long compat_sys_fallocate(int fd, int mode, u32 offset1, u32 offset2,
 				     u32 len1, u32 len2)
 {
-	return ksys_fallocate(fd, mode, ((loff_t)offset1 << 32) | offset2,
+	return ksys_fallocate(fd, mode, merge_64(offset1, offset2),
 			     merge_64(len1, len2));
 }
 
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index fc999140bc27..abc3fbb3c490 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -98,8 +98,8 @@ long ppc64_personality(unsigned long personality)
 long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
 		      u32 len_high, u32 len_low)
 {
-	return ksys_fadvise64_64(fd, (u64)offset_high << 32 | offset_low,
-				 (u64)len_high << 32 | len_low, advice);
+	return ksys_fadvise64_64(fd, merge_64(offset_high, offset_low),
+				 merge_64(len_high, len_low), advice);
 }
 
 SYSCALL_DEFINE0(switch_endian)
-- 
2.34.1

Re: [PATCH 08/23] powerpc: Fix fallocate and fadvise64_64 compat parameter combination

[Arnd Bergmann]

On Fri, Sep 16, 2022, at 7:32 AM, Rohan McLure wrote:
> As reported[1] by Arnd, the arch-specific fadvise64_64 and fallocate
> compatibility handlers assume parameters are passed with 32-bit
> big-endian ABI. This affects the assignment of odd-even parameter pairs
> to the high or low words of a 64-bit syscall parameter.
>
> Fix fadvise64_64 fallocate compat handlers to correctly swap upper/lower
> 32 bits conditioned on endianness.
>
> A future patch will replace the arch-specific compat fallocate with an
> asm-generic implementation. This patch is intended for ease of
> back-port.
>
> [1]: 
> https://lore.kernel.org/all/be29926f-226e-48dc-871a-e29a54e80583@www.fastmail.com/
>
> Fixes: 57f48b4b74e7 ("powerpc/compat_sys: swap hi/lo parts of 64-bit 
> syscall args in LE mode")
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>

Looks good, thanks!

Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Cc: stable@vger.kernel.org # v5.12+

Re: [PATCH 08/23] powerpc: Fix fallocate and fadvise64_64 compat parameter combination

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> As reported[1] by Arnd, the arch-specific fadvise64_64 and fallocate
> compatibility handlers assume parameters are passed with 32-bit
> big-endian ABI. This affects the assignment of odd-even parameter pairs
> to the high or low words of a 64-bit syscall parameter.
>
> Fix fadvise64_64 fallocate compat handlers to correctly swap upper/lower
> 32 bits conditioned on endianness.
>
> A future patch will replace the arch-specific compat fallocate with an
> asm-generic implementation. This patch is intended for ease of
> back-port.
>
> [1]: https://lore.kernel.org/all/be29926f-226e-48dc-871a-e29a54e80583@www.fastmail.com/
>
> Fixes: 57f48b4b74e7 ("powerpc/compat_sys: swap hi/lo parts of 64-bit syscall args in LE mode")
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>

I'm still not sure if ppc32 is usable with LE kernel, but in any case
the patch is good I think.

Reviewed-by: Nicholas Piggin <npiggin@gmai.com>

> ---
> V4 -> V5: New patch.
> ---
>  arch/powerpc/include/asm/syscalls.h | 12 ++++++++++++
>  arch/powerpc/kernel/sys_ppc32.c     | 14 +-------------
>  arch/powerpc/kernel/syscalls.c      |  4 ++--
>  3 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
> index 21c2faaa2957..16b668515d15 100644
> --- a/arch/powerpc/include/asm/syscalls.h
> +++ b/arch/powerpc/include/asm/syscalls.h
> @@ -8,6 +8,18 @@
>  #include <linux/types.h>
>  #include <linux/compat.h>
>  
> +/*
> + * long long munging:
> + * The 32 bit ABI passes long longs in an odd even register pair.
> + * High and low parts are swapped depending on endian mode,
> + * so define a macro (similar to mips linux32) to handle that.
> + */
> +#ifdef __LITTLE_ENDIAN__
> +#define merge_64(low, high) ((u64)high << 32) | low
> +#else
> +#define merge_64(high, low) ((u64)high << 32) | low
> +#endif
> +
>  struct rtas_args;
>  
>  long sys_mmap(unsigned long addr, size_t len,
> diff --git a/arch/powerpc/kernel/sys_ppc32.c b/arch/powerpc/kernel/sys_ppc32.c
> index f4edcc9489fb..ba363328da2b 100644
> --- a/arch/powerpc/kernel/sys_ppc32.c
> +++ b/arch/powerpc/kernel/sys_ppc32.c
> @@ -56,18 +56,6 @@ unsigned long compat_sys_mmap2(unsigned long addr, size_t len,
>  	return sys_mmap(addr, len, prot, flags, fd, pgoff << 12);
>  }
>  
> -/* 
> - * long long munging:
> - * The 32 bit ABI passes long longs in an odd even register pair.
> - * High and low parts are swapped depending on endian mode,
> - * so define a macro (similar to mips linux32) to handle that.
> - */
> -#ifdef __LITTLE_ENDIAN__
> -#define merge_64(low, high) ((u64)high << 32) | low
> -#else
> -#define merge_64(high, low) ((u64)high << 32) | low
> -#endif
> -
>  compat_ssize_t compat_sys_pread64(unsigned int fd, char __user *ubuf, compat_size_t count,
>  			     u32 reg6, u32 pos1, u32 pos2)
>  {
> @@ -94,7 +82,7 @@ int compat_sys_truncate64(const char __user * path, u32 reg4,
>  long compat_sys_fallocate(int fd, int mode, u32 offset1, u32 offset2,
>  				     u32 len1, u32 len2)
>  {
> -	return ksys_fallocate(fd, mode, ((loff_t)offset1 << 32) | offset2,
> +	return ksys_fallocate(fd, mode, merge_64(offset1, offset2),
>  			     merge_64(len1, len2));
>  }
>  
> diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
> index fc999140bc27..abc3fbb3c490 100644
> --- a/arch/powerpc/kernel/syscalls.c
> +++ b/arch/powerpc/kernel/syscalls.c
> @@ -98,8 +98,8 @@ long ppc64_personality(unsigned long personality)
>  long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
>  		      u32 len_high, u32 len_low)
>  {
> -	return ksys_fadvise64_64(fd, (u64)offset_high << 32 | offset_low,
> -				 (u64)len_high << 32 | len_low, advice);
> +	return ksys_fadvise64_64(fd, merge_64(offset_high, offset_low),
> +				 merge_64(len_high, len_low), advice);
>  }
>  
>  SYSCALL_DEFINE0(switch_endian)
> -- 
> 2.34.1

[PATCH 09/23] asm-generic: compat: Support BE for long long args in 32-bit ABIs

[Rohan McLure]

32-bit ABIs support passing 64-bit integers by registers via argument
translation. Commit 59c10c52f573 ("riscv: compat: syscall: Add
compat_sys_call_table implementation") implements the compat_arg_u64
macro for efficiently defining little endian compatibility syscalls.

Architectures supporting big endianness may benefit from reciprocal
argument translation, but are welcome also to implement their own.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
V4 -> V5: New patch.
---
 include/asm-generic/compat.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/compat.h b/include/asm-generic/compat.h
index d06308a2a7a8..aeb257ad3d1a 100644
--- a/include/asm-generic/compat.h
+++ b/include/asm-generic/compat.h
@@ -14,12 +14,17 @@
 #define COMPAT_OFF_T_MAX	0x7fffffff
 #endif
 
-#if !defined(compat_arg_u64) && !defined(CONFIG_CPU_BIG_ENDIAN)
+#ifndef compat_arg_u64
+#ifdef CONFIG_CPU_BIG_ENDIAN
 #define compat_arg_u64(name)		u32  name##_lo, u32  name##_hi
 #define compat_arg_u64_dual(name)	u32, name##_lo, u32, name##_hi
+#else
+#define compat_arg_u64(name)		u32  name##_hi, u32  name##_lo
+#define compat_arg_u64_dual(name)	u32, name##_hi, u32, name##_lo
+#endif
 #define compat_arg_u64_glue(name)	(((u64)name##_lo & 0xffffffffUL) | \
 					 ((u64)name##_hi << 32))
-#endif
+#endif /* compat_arg_u64 */
 
 /* These types are common across all compat ABIs */
 typedef u32 compat_size_t;
-- 
2.34.1

Re: [PATCH 09/23] asm-generic: compat: Support BE for long long args in 32-bit ABIs

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> 32-bit ABIs support passing 64-bit integers by registers via argument
> translation. Commit 59c10c52f573 ("riscv: compat: syscall: Add
> compat_sys_call_table implementation") implements the compat_arg_u64
> macro for efficiently defining little endian compatibility syscalls.
>
> Architectures supporting big endianness may benefit from reciprocal
> argument translation, but are welcome also to implement their own.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>

Better put Arnd and linux-arch on cc...

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> ---
> V4 -> V5: New patch.
> ---
>  include/asm-generic/compat.h | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/asm-generic/compat.h b/include/asm-generic/compat.h
> index d06308a2a7a8..aeb257ad3d1a 100644
> --- a/include/asm-generic/compat.h
> +++ b/include/asm-generic/compat.h
> @@ -14,12 +14,17 @@
>  #define COMPAT_OFF_T_MAX	0x7fffffff
>  #endif
>  
> -#if !defined(compat_arg_u64) && !defined(CONFIG_CPU_BIG_ENDIAN)
> +#ifndef compat_arg_u64
> +#ifdef CONFIG_CPU_BIG_ENDIAN
>  #define compat_arg_u64(name)		u32  name##_lo, u32  name##_hi
>  #define compat_arg_u64_dual(name)	u32, name##_lo, u32, name##_hi
> +#else
> +#define compat_arg_u64(name)		u32  name##_hi, u32  name##_lo
> +#define compat_arg_u64_dual(name)	u32, name##_hi, u32, name##_lo
> +#endif
>  #define compat_arg_u64_glue(name)	(((u64)name##_lo & 0xffffffffUL) | \
>  					 ((u64)name##_hi << 32))
> -#endif
> +#endif /* compat_arg_u64 */
>  
>  /* These types are common across all compat ABIs */
>  typedef u32 compat_size_t;
> -- 
> 2.34.1

Re: [PATCH 09/23] asm-generic: compat: Support BE for long long args in 32-bit ABIs

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> 32-bit ABIs support passing 64-bit integers by registers via argument
> translation. Commit 59c10c52f573 ("riscv: compat: syscall: Add
> compat_sys_call_table implementation") implements the compat_arg_u64
> macro for efficiently defining little endian compatibility syscalls.
>
> Architectures supporting big endianness may benefit from reciprocal
> argument translation, but are welcome also to implement their own.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>

Better put Arnd and linux-arch on cc...

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> ---
> V4 -> V5: New patch.
> ---
>  include/asm-generic/compat.h | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/asm-generic/compat.h b/include/asm-generic/compat.h
> index d06308a2a7a8..aeb257ad3d1a 100644
> --- a/include/asm-generic/compat.h
> +++ b/include/asm-generic/compat.h
> @@ -14,12 +14,17 @@
>  #define COMPAT_OFF_T_MAX	0x7fffffff
>  #endif
>  
> -#if !defined(compat_arg_u64) && !defined(CONFIG_CPU_BIG_ENDIAN)
> +#ifndef compat_arg_u64
> +#ifdef CONFIG_CPU_BIG_ENDIAN
>  #define compat_arg_u64(name)		u32  name##_lo, u32  name##_hi
>  #define compat_arg_u64_dual(name)	u32, name##_lo, u32, name##_hi
> +#else
> +#define compat_arg_u64(name)		u32  name##_hi, u32  name##_lo
> +#define compat_arg_u64_dual(name)	u32, name##_hi, u32, name##_lo
> +#endif
>  #define compat_arg_u64_glue(name)	(((u64)name##_lo & 0xffffffffUL) | \
>  					 ((u64)name##_hi << 32))
> -#endif
> +#endif /* compat_arg_u64 */
>  
>  /* These types are common across all compat ABIs */
>  typedef u32 compat_size_t;
> -- 
> 2.34.1

Re: [PATCH 09/23] asm-generic: compat: Support BE for long long args in 32-bit ABIs

[Arnd Bergmann]

On Fri, Sep 16, 2022, at 7:32 AM, Rohan McLure wrote:
> 32-bit ABIs support passing 64-bit integers by registers via argument
> translation. Commit 59c10c52f573 ("riscv: compat: syscall: Add
> compat_sys_call_table implementation") implements the compat_arg_u64
> macro for efficiently defining little endian compatibility syscalls.
>
> Architectures supporting big endianness may benefit from reciprocal
> argument translation, but are welcome also to implement their own.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

[PATCH 10/23] powerpc: Use generic fallocate compatibility syscall

[Rohan McLure]

The powerpc fallocate compat syscall handler is identical to the
generic implementation provided by commit 59c10c52f573f ("riscv:
compat: syscall: Add compat_sys_call_table implementation"), and as
such can be removed in favour of the generic implementation.

A future patch series will replace more architecture-defined syscall
handlers with generic implementations, dependent on introducing generic
implementations that are compatible with powerpc and arm's parameter
reorderings.

Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
V1 -> V2: Remove arch-specific fallocate handler.
V2 -> V3: Remove generic fallocate prototype. Move to beginning of
series.
V4 -> V5: Remove implementation as well which I somehow failed to do.
Replace local BE compat_arg_u64 with generic.
---
 arch/powerpc/include/asm/syscalls.h | 2 --
 arch/powerpc/include/asm/unistd.h   | 1 +
 arch/powerpc/kernel/sys_ppc32.c     | 7 -------
 3 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index 16b668515d15..960b3871db72 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -51,8 +51,6 @@ compat_ssize_t compat_sys_readahead(int fd, u32 r4, u32 offset1, u32 offset2, u3
 int compat_sys_truncate64(const char __user *path, u32 reg4,
 			  unsigned long len1, unsigned long len2);
 
-long compat_sys_fallocate(int fd, int mode, u32 offset1, u32 offset2, u32 len1, u32 len2);
-
 int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long len1,
 			   unsigned long len2);
 
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index b1129b4ef57d..659a996c75aa 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -45,6 +45,7 @@
 #define __ARCH_WANT_SYS_UTIME
 #define __ARCH_WANT_SYS_NEWFSTATAT
 #define __ARCH_WANT_COMPAT_STAT
+#define __ARCH_WANT_COMPAT_FALLOCATE
 #define __ARCH_WANT_COMPAT_SYS_SENDFILE
 #endif
 #define __ARCH_WANT_SYS_FORK
diff --git a/arch/powerpc/kernel/sys_ppc32.c b/arch/powerpc/kernel/sys_ppc32.c
index ba363328da2b..d961634976d8 100644
--- a/arch/powerpc/kernel/sys_ppc32.c
+++ b/arch/powerpc/kernel/sys_ppc32.c
@@ -79,13 +79,6 @@ int compat_sys_truncate64(const char __user * path, u32 reg4,
 	return ksys_truncate(path, merge_64(len1, len2));
 }
 
-long compat_sys_fallocate(int fd, int mode, u32 offset1, u32 offset2,
-				     u32 len1, u32 len2)
-{
-	return ksys_fallocate(fd, mode, merge_64(offset1, offset2),
-			     merge_64(len1, len2));
-}
-
 int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long len1,
 				 unsigned long len2)
 {
-- 
2.34.1

Re: [PATCH 10/23] powerpc: Use generic fallocate compatibility syscall

[Arnd Bergmann]

On Fri, Sep 16, 2022, at 7:32 AM, Rohan McLure wrote:
> The powerpc fallocate compat syscall handler is identical to the
> generic implementation provided by commit 59c10c52f573f ("riscv:
> compat: syscall: Add compat_sys_call_table implementation"), and as
> such can be removed in favour of the generic implementation.
>
> A future patch series will replace more architecture-defined syscall
> handlers with generic implementations, dependent on introducing generic
> implementations that are compatible with powerpc and arm's parameter
> reorderings.
>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> V1 -> V2: Remove arch-specific fallocate handler.
> V2 -> V3: Remove generic fallocate prototype. Move to beginning of
> series.
> V4 -> V5: Remove implementation as well which I somehow failed to do.
> Replace local BE compat_arg_u64 with generic.

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

[PATCH 11/23] powerpc/32: Remove powerpc select specialisation

[Rohan McLure]

Syscall #82 has been implemented for 32-bit platforms in a unique way on
powerpc systems. This hack will in effect guess whether the caller is
expecting new select semantics or old select semantics. It does so via a
guess, based off the first parameter. In new select, this parameter
represents the length of a user-memory array of file descriptors, and in
old select this is a pointer to an arguments structure.

The heuristic simply interprets sufficiently large values of its first
parameter as being a call to old select. The following is a discussion
on how this syscall should be handled.

Link: https://lore.kernel.org/lkml/13737de5-0eb7-e881-9af0-163b0d29a1a0@csgroup.eu/

As discussed in this thread, the existence of such a hack suggests that for
whatever powerpc binaries may predate glibc, it is most likely that they
would have taken use of the old select semantics. x86 and arm64 both
implement this syscall with oldselect semantics.

Remove the powerpc implementation, and update syscall.tbl to refer to emit
a reference to sys_old_select and compat_sys_old_select
for 32-bit binaries, in keeping with how other architectures support
syscall #82.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
V1 -> V2: Remove arch-specific select handler
V2 -> V3: Remove ppc_old_select prototype in <asm/syscalls.h>. Move to
earlier in patch series
V4 -> V5: Use compat_sys_old_select on 64-bit systems.
---
 arch/powerpc/include/asm/syscalls.h           |  2 --
 arch/powerpc/kernel/syscalls.c                | 17 -----------------
 arch/powerpc/kernel/syscalls/syscall.tbl      |  2 +-
 .../arch/powerpc/entry/syscalls/syscall.tbl   |  2 +-
 4 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index 960b3871db72..20cbd29b1228 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -30,8 +30,6 @@ long sys_mmap2(unsigned long addr, size_t len,
 	       unsigned long fd, unsigned long pgoff);
 long ppc64_personality(unsigned long personality);
 long sys_rtas(struct rtas_args __user *uargs);
-int ppc_select(int n, fd_set __user *inp, fd_set __user *outp,
-	       fd_set __user *exp, struct __kernel_old_timeval __user *tvp);
 long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
 		      u32 len_high, u32 len_low);
 
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index abc3fbb3c490..34e1ae88e15b 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -63,23 +63,6 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
 	return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);
 }
 
-#ifdef CONFIG_PPC32
-/*
- * Due to some executables calling the wrong select we sometimes
- * get wrong args.  This determines how the args are being passed
- * (a single ptr to them all args passed) then calls
- * sys_select() with the appropriate args. -- Cort
- */
-int
-ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct __kernel_old_timeval __user *tvp)
-{
-	if ((unsigned long)n >= 4096)
-		return sys_old_select((void __user *)n);
-
-	return sys_select(n, inp, outp, exp, tvp);
-}
-#endif
-
 #ifdef CONFIG_PPC64
 long ppc64_personality(unsigned long personality)
 {
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index 2600b4237292..64f27cbbdd2c 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -110,7 +110,7 @@
 79	common	settimeofday			sys_settimeofday		compat_sys_settimeofday
 80	common	getgroups			sys_getgroups
 81	common	setgroups			sys_setgroups
-82	32	select				ppc_select			sys_ni_syscall
+82	32	select				sys_old_select			compat_sys_old_select
 82	64	select				sys_ni_syscall
 82	spu	select				sys_ni_syscall
 83	common	symlink				sys_symlink
diff --git a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
index 2600b4237292..64f27cbbdd2c 100644
--- a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
+++ b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
@@ -110,7 +110,7 @@
 79	common	settimeofday			sys_settimeofday		compat_sys_settimeofday
 80	common	getgroups			sys_getgroups
 81	common	setgroups			sys_setgroups
-82	32	select				ppc_select			sys_ni_syscall
+82	32	select				sys_old_select			compat_sys_old_select
 82	64	select				sys_ni_syscall
 82	spu	select				sys_ni_syscall
 83	common	symlink				sys_symlink
-- 
2.34.1

[PATCH 12/23] powerpc: Remove direct call to personality syscall handler

[Rohan McLure]

Syscall handlers should not be invoked internally by their symbol names,
as these symbols defined by the architecture-defined SYSCALL_DEFINE
macro. Fortunately, in the case of ppc64_personality, its call to
sys_personality can be replaced with an invocation to the
equivalent ksys_personality inline helper in <linux/syscalls.h>.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
V1 -> V2: Use inline helper to deduplicate bodies in compat/regular
implementations.
V3 -> V4: Move to be applied before syscall wrapper.
---
 arch/powerpc/kernel/syscalls.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index 34e1ae88e15b..a04c97faa21a 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -71,7 +71,7 @@ long ppc64_personality(unsigned long personality)
 	if (personality(current->personality) == PER_LINUX32
 	    && personality(personality) == PER_LINUX)
 		personality = (personality & ~PER_MASK) | PER_LINUX32;
-	ret = sys_personality(personality);
+	ret = ksys_personality(personality);
 	if (personality(ret) == PER_LINUX32)
 		ret = (ret & ~PER_MASK) | PER_LINUX;
 	return ret;
-- 
2.34.1

[PATCH 13/23] powerpc: Remove direct call to mmap2 syscall handlers

[Rohan McLure]

Syscall handlers should not be invoked internally by their symbol names,
as these symbols defined by the architecture-defined SYSCALL_DEFINE
macro. Move the compatibility syscall definition for mmap2 to
syscalls.c, so that all mmap implementations can share a helper function.

Remove 'inline' on static mmap helper.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
V1 -> V2: Move mmap2 compat implementation to asm/kernel/syscalls.c.
V3 -> V4: Move to be applied before syscall wrapper introduced.
V4 -> V5: Remove 'inline' in helper.
---
 arch/powerpc/kernel/sys_ppc32.c |  9 ---------
 arch/powerpc/kernel/syscalls.c  | 17 ++++++++++++++---
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/sys_ppc32.c b/arch/powerpc/kernel/sys_ppc32.c
index d961634976d8..776ae7565fc5 100644
--- a/arch/powerpc/kernel/sys_ppc32.c
+++ b/arch/powerpc/kernel/sys_ppc32.c
@@ -25,7 +25,6 @@
 #include <linux/poll.h>
 #include <linux/personality.h>
 #include <linux/stat.h>
-#include <linux/mman.h>
 #include <linux/in.h>
 #include <linux/syscalls.h>
 #include <linux/unistd.h>
@@ -48,14 +47,6 @@
 #include <asm/syscalls.h>
 #include <asm/switch_to.h>
 
-unsigned long compat_sys_mmap2(unsigned long addr, size_t len,
-			  unsigned long prot, unsigned long flags,
-			  unsigned long fd, unsigned long pgoff)
-{
-	/* This should remain 12 even if PAGE_SIZE changes */
-	return sys_mmap(addr, len, prot, flags, fd, pgoff << 12);
-}
-
 compat_ssize_t compat_sys_pread64(unsigned int fd, char __user *ubuf, compat_size_t count,
 			     u32 reg6, u32 pos1, u32 pos2)
 {
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index a04c97faa21a..9830957498b0 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -36,9 +36,9 @@
 #include <asm/time.h>
 #include <asm/unistd.h>
 
-static inline long do_mmap2(unsigned long addr, size_t len,
-			unsigned long prot, unsigned long flags,
-			unsigned long fd, unsigned long off, int shift)
+static long do_mmap2(unsigned long addr, size_t len,
+		     unsigned long prot, unsigned long flags,
+		     unsigned long fd, unsigned long off, int shift)
 {
 	if (!arch_validate_prot(prot, addr))
 		return -EINVAL;
@@ -56,6 +56,17 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, size_t, len,
 	return do_mmap2(addr, len, prot, flags, fd, pgoff, PAGE_SHIFT-12);
 }
 
+#ifdef CONFIG_COMPAT
+COMPAT_SYSCALL_DEFINE6(mmap2,
+		       unsigned long, addr, size_t, len,
+		       unsigned long, prot, unsigned long, flags,
+		       unsigned long, fd, unsigned long, pgoff)
+{
+	/* This should remain 12 even if PAGE_SIZE changes */
+	return do_mmap2(addr, len, prot, flags, fd, pgoff << 12, PAGE_SHIFT-12);
+}
+#endif
+
 SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
 		unsigned long, prot, unsigned long, flags,
 		unsigned long, fd, off_t, offset)
-- 
2.34.1

[PATCH 14/23] powerpc: Provide do_ppc64_personality helper

[Rohan McLure]

Avoid duplication in future patch that will define the ppc64_personality
syscall handler in terms of the SYSCALL_DEFINE and COMPAT_SYSCALL_DEFINE
macros, by extracting the common body of ppc64_personality into a helper
function.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
V2 -> V3: New commit.
V4 -> V5: Remove 'inline'.
---
 arch/powerpc/kernel/syscalls.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index 9830957498b0..135a0b9108d5 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -75,7 +75,7 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
 }
 
 #ifdef CONFIG_PPC64
-long ppc64_personality(unsigned long personality)
+static long do_ppc64_personality(unsigned long personality)
 {
 	long ret;
 
@@ -87,6 +87,10 @@ long ppc64_personality(unsigned long personality)
 		ret = (ret & ~PER_MASK) | PER_LINUX;
 	return ret;
 }
+long ppc64_personality(unsigned long personality)
+{
+	return do_ppc64_personality(personality);
+}
 #endif
 
 long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
-- 
2.34.1

[PATCH 15/23] powerpc: Adopt SYSCALL_DEFINE for arch-specific syscall handlers

[Rohan McLure]

Arch-specific implementations of syscall handlers are currently used
over generic implementations for the following reasons:

1. Semantics unique to powerpc
2. Compatibility syscalls require 'argument padding' to comply with
   64-bit argument convention in ELF32 abi.
3. Parameter types or order is different in other architectures.

These syscall handlers have been defined prior to this patch series
without invoking the SYSCALL_DEFINE or COMPAT_SYSCALL_DEFINE macros with
custom input and output types. We remove every such direct definition in
favour of the aforementioned macros.

Also update syscalls.tbl in order to refer to the symbol names generated
by each of these macros. Since ppc64_personality can be called by both
64 bit and 32 bit binaries through compatibility, we must generate both
both compat_sys_ and sys_ symbols for this handler.

As an aside:
A number of architectures including arm and powerpc agree on an
alternative argument order and numbering for most of these arch-specific
handlers. A future patch series may allow for asm/unistd.h to signal
through its defines that a generic implementation of these syscall
handlers with the correct calling convention be emitted, through the
__ARCH_WANT_COMPAT_SYS_... convention.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
V1 -> V2: All syscall handlers wrapped by this macro.
V2 -> V3: Move creation of do_ppc64_personality helper to prior patch.
V3 -> V4: Fix parenthesis alignment. Don't emit sys_*** symbols.
V4 -> V5: Use 'aside' in the asm-generic rant in commit message.
---
 arch/powerpc/include/asm/syscalls.h          | 10 ++---
 arch/powerpc/kernel/sys_ppc32.c              | 38 +++++++++++-------
 arch/powerpc/kernel/syscalls.c               | 17 ++++++--
 arch/powerpc/kernel/syscalls/syscall.tbl     | 22 +++++-----
 .../arch/powerpc/entry/syscalls/syscall.tbl  | 22 +++++-----
 5 files changed, 64 insertions(+), 45 deletions(-)

diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index 20cbd29b1228..525d2aa0c8ca 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -28,10 +28,10 @@ long sys_mmap(unsigned long addr, size_t len,
 long sys_mmap2(unsigned long addr, size_t len,
 	       unsigned long prot, unsigned long flags,
 	       unsigned long fd, unsigned long pgoff);
-long ppc64_personality(unsigned long personality);
+long sys_ppc64_personality(unsigned long personality);
 long sys_rtas(struct rtas_args __user *uargs);
-long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
-		      u32 len_high, u32 len_low);
+long sys_ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
+			  u32 len_high, u32 len_low);
 
 #ifdef CONFIG_COMPAT
 unsigned long compat_sys_mmap2(unsigned long addr, size_t len,
@@ -52,8 +52,8 @@ int compat_sys_truncate64(const char __user *path, u32 reg4,
 int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long len1,
 			   unsigned long len2);
 
-long ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
-		     size_t len, int advice);
+long compat_sys_ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
+				size_t len, int advice);
 
 long compat_sys_sync_file_range2(int fd, unsigned int flags,
 				 unsigned int offset1, unsigned int offset2,
diff --git a/arch/powerpc/kernel/sys_ppc32.c b/arch/powerpc/kernel/sys_ppc32.c
index 776ae7565fc5..dcc3c9fd4cfd 100644
--- a/arch/powerpc/kernel/sys_ppc32.c
+++ b/arch/powerpc/kernel/sys_ppc32.c
@@ -47,45 +47,55 @@
 #include <asm/syscalls.h>
 #include <asm/switch_to.h>
 
-compat_ssize_t compat_sys_pread64(unsigned int fd, char __user *ubuf, compat_size_t count,
-			     u32 reg6, u32 pos1, u32 pos2)
+COMPAT_SYSCALL_DEFINE6(ppc_pread64,
+		       unsigned int, fd,
+		       char __user *, ubuf, compat_size_t, count,
+		       u32, reg6, u32, pos1, u32, pos2)
 {
 	return ksys_pread64(fd, ubuf, count, merge_64(pos1, pos2));
 }
 
-compat_ssize_t compat_sys_pwrite64(unsigned int fd, const char __user *ubuf, compat_size_t count,
-			      u32 reg6, u32 pos1, u32 pos2)
+COMPAT_SYSCALL_DEFINE6(ppc_pwrite64,
+		       unsigned int, fd,
+		       const char __user *, ubuf, compat_size_t, count,
+		       u32, reg6, u32, pos1, u32, pos2)
 {
 	return ksys_pwrite64(fd, ubuf, count, merge_64(pos1, pos2));
 }
 
-compat_ssize_t compat_sys_readahead(int fd, u32 r4, u32 offset1, u32 offset2, u32 count)
+COMPAT_SYSCALL_DEFINE5(ppc_readahead,
+		       int, fd, u32, r4,
+		       u32, offset1, u32, offset2, u32, count)
 {
 	return ksys_readahead(fd, merge_64(offset1, offset2), count);
 }
 
-int compat_sys_truncate64(const char __user * path, u32 reg4,
-				unsigned long len1, unsigned long len2)
+COMPAT_SYSCALL_DEFINE4(ppc_truncate64,
+		       const char __user *, path, u32, reg4,
+		       unsigned long, len1, unsigned long, len2)
 {
 	return ksys_truncate(path, merge_64(len1, len2));
 }
 
-int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long len1,
-				 unsigned long len2)
+COMPAT_SYSCALL_DEFINE4(ppc_ftruncate64,
+		       unsigned int, fd, u32, reg4,
+		       unsigned long, len1, unsigned long, len2)
 {
 	return ksys_ftruncate(fd, merge_64(len1, len2));
 }
 
-long ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
-		     size_t len, int advice)
+COMPAT_SYSCALL_DEFINE6(ppc32_fadvise64,
+		       int, fd, u32, unused, u32, offset1, u32, offset2,
+		       size_t, len, int, advice)
 {
 	return ksys_fadvise64_64(fd, merge_64(offset1, offset2), len,
 				 advice);
 }
 
-long compat_sys_sync_file_range2(int fd, unsigned int flags,
-				   unsigned offset1, unsigned offset2,
-				   unsigned nbytes1, unsigned nbytes2)
+COMPAT_SYSCALL_DEFINE6(ppc_sync_file_range2,
+		       int, fd, unsigned int, flags,
+		       unsigned int, offset1, unsigned int, offset2,
+		       unsigned int, nbytes1, unsigned int, nbytes2)
 {
 	loff_t offset = merge_64(offset1, offset2);
 	loff_t nbytes = merge_64(nbytes1, nbytes2);
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index 135a0b9108d5..3a35c690150b 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -87,14 +87,23 @@ static long do_ppc64_personality(unsigned long personality)
 		ret = (ret & ~PER_MASK) | PER_LINUX;
 	return ret;
 }
-long ppc64_personality(unsigned long personality)
+
+SYSCALL_DEFINE1(ppc64_personality, unsigned long, personality)
 {
 	return do_ppc64_personality(personality);
 }
-#endif
 
-long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
-		      u32 len_high, u32 len_low)
+#ifdef CONFIG_COMPAT
+COMPAT_SYSCALL_DEFINE1(ppc64_personality, unsigned long, personality)
+{
+	return do_ppc64_personality(personality);
+}
+#endif /* CONFIG_COMPAT */
+#endif /* CONFIG_PPC64 */
+
+SYSCALL_DEFINE6(ppc_fadvise64_64,
+		int, fd, int, advice, u32, offset_high, u32, offset_low,
+		u32, len_high, u32, len_low)
 {
 	return ksys_fadvise64_64(fd, merge_64(offset_high, offset_low),
 				 merge_64(len_high, len_low), advice);
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index 64f27cbbdd2c..2bca64f96164 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -178,9 +178,9 @@
 133	common	fchdir				sys_fchdir
 134	common	bdflush				sys_ni_syscall
 135	common	sysfs				sys_sysfs
-136	32	personality			sys_personality			ppc64_personality
-136	64	personality			ppc64_personality
-136	spu	personality			ppc64_personality
+136	32	personality			sys_personality			compat_sys_ppc64_personality
+136	64	personality			sys_ppc64_personality
+136	spu	personality			sys_ppc64_personality
 137	common	afs_syscall			sys_ni_syscall
 138	common	setfsuid			sys_setfsuid
 139	common	setfsgid			sys_setfsgid
@@ -228,8 +228,8 @@
 176	64	rt_sigtimedwait			sys_rt_sigtimedwait
 177	nospu 	rt_sigqueueinfo			sys_rt_sigqueueinfo		compat_sys_rt_sigqueueinfo
 178	nospu 	rt_sigsuspend			sys_rt_sigsuspend		compat_sys_rt_sigsuspend
-179	common	pread64				sys_pread64			compat_sys_pread64
-180	common	pwrite64			sys_pwrite64			compat_sys_pwrite64
+179	common	pread64				sys_pread64			compat_sys_ppc_pread64
+180	common	pwrite64			sys_pwrite64			compat_sys_ppc_pwrite64
 181	common	chown				sys_chown
 182	common	getcwd				sys_getcwd
 183	common	capget				sys_capget
@@ -242,10 +242,10 @@
 188	common 	putpmsg				sys_ni_syscall
 189	nospu	vfork				sys_vfork
 190	common	ugetrlimit			sys_getrlimit			compat_sys_getrlimit
-191	common	readahead			sys_readahead			compat_sys_readahead
+191	common	readahead			sys_readahead			compat_sys_ppc_readahead
 192	32	mmap2				sys_mmap2			compat_sys_mmap2
-193	32	truncate64			sys_truncate64			compat_sys_truncate64
-194	32	ftruncate64			sys_ftruncate64			compat_sys_ftruncate64
+193	32	truncate64			sys_truncate64			compat_sys_ppc_truncate64
+194	32	ftruncate64			sys_ftruncate64			compat_sys_ppc_ftruncate64
 195	32	stat64				sys_stat64
 196	32	lstat64				sys_lstat64
 197	32	fstat64				sys_fstat64
@@ -288,7 +288,7 @@
 230	common	io_submit			sys_io_submit			compat_sys_io_submit
 231	common	io_cancel			sys_io_cancel
 232	nospu	set_tid_address			sys_set_tid_address
-233	common	fadvise64			sys_fadvise64			ppc32_fadvise64
+233	common	fadvise64			sys_fadvise64			compat_sys_ppc32_fadvise64
 234	nospu	exit_group			sys_exit_group
 235	nospu	lookup_dcookie			sys_lookup_dcookie		compat_sys_lookup_dcookie
 236	common	epoll_create			sys_epoll_create
@@ -323,7 +323,7 @@
 251	spu	utimes				sys_utimes
 252	common	statfs64			sys_statfs64			compat_sys_statfs64
 253	common	fstatfs64			sys_fstatfs64			compat_sys_fstatfs64
-254	32	fadvise64_64			ppc_fadvise64_64
+254	32	fadvise64_64			sys_ppc_fadvise64_64
 254	spu	fadvise64_64			sys_ni_syscall
 255	common	rtas				sys_rtas
 256	32	sys_debug_setcontext		sys_debug_setcontext		sys_ni_syscall
@@ -390,7 +390,7 @@
 305	common	signalfd			sys_signalfd			compat_sys_signalfd
 306	common	timerfd_create			sys_timerfd_create
 307	common	eventfd				sys_eventfd
-308	common	sync_file_range2		sys_sync_file_range2		compat_sys_sync_file_range2
+308	common	sync_file_range2		sys_sync_file_range2		compat_sys_ppc_sync_file_range2
 309	nospu	fallocate			sys_fallocate			compat_sys_fallocate
 310	nospu	subpage_prot			sys_subpage_prot
 311	32	timerfd_settime			sys_timerfd_settime32
diff --git a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
index 64f27cbbdd2c..2bca64f96164 100644
--- a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
+++ b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
@@ -178,9 +178,9 @@
 133	common	fchdir				sys_fchdir
 134	common	bdflush				sys_ni_syscall
 135	common	sysfs				sys_sysfs
-136	32	personality			sys_personality			ppc64_personality
-136	64	personality			ppc64_personality
-136	spu	personality			ppc64_personality
+136	32	personality			sys_personality			compat_sys_ppc64_personality
+136	64	personality			sys_ppc64_personality
+136	spu	personality			sys_ppc64_personality
 137	common	afs_syscall			sys_ni_syscall
 138	common	setfsuid			sys_setfsuid
 139	common	setfsgid			sys_setfsgid
@@ -228,8 +228,8 @@
 176	64	rt_sigtimedwait			sys_rt_sigtimedwait
 177	nospu 	rt_sigqueueinfo			sys_rt_sigqueueinfo		compat_sys_rt_sigqueueinfo
 178	nospu 	rt_sigsuspend			sys_rt_sigsuspend		compat_sys_rt_sigsuspend
-179	common	pread64				sys_pread64			compat_sys_pread64
-180	common	pwrite64			sys_pwrite64			compat_sys_pwrite64
+179	common	pread64				sys_pread64			compat_sys_ppc_pread64
+180	common	pwrite64			sys_pwrite64			compat_sys_ppc_pwrite64
 181	common	chown				sys_chown
 182	common	getcwd				sys_getcwd
 183	common	capget				sys_capget
@@ -242,10 +242,10 @@
 188	common 	putpmsg				sys_ni_syscall
 189	nospu	vfork				sys_vfork
 190	common	ugetrlimit			sys_getrlimit			compat_sys_getrlimit
-191	common	readahead			sys_readahead			compat_sys_readahead
+191	common	readahead			sys_readahead			compat_sys_ppc_readahead
 192	32	mmap2				sys_mmap2			compat_sys_mmap2
-193	32	truncate64			sys_truncate64			compat_sys_truncate64
-194	32	ftruncate64			sys_ftruncate64			compat_sys_ftruncate64
+193	32	truncate64			sys_truncate64			compat_sys_ppc_truncate64
+194	32	ftruncate64			sys_ftruncate64			compat_sys_ppc_ftruncate64
 195	32	stat64				sys_stat64
 196	32	lstat64				sys_lstat64
 197	32	fstat64				sys_fstat64
@@ -288,7 +288,7 @@
 230	common	io_submit			sys_io_submit			compat_sys_io_submit
 231	common	io_cancel			sys_io_cancel
 232	nospu	set_tid_address			sys_set_tid_address
-233	common	fadvise64			sys_fadvise64			ppc32_fadvise64
+233	common	fadvise64			sys_fadvise64			compat_sys_ppc32_fadvise64
 234	nospu	exit_group			sys_exit_group
 235	nospu	lookup_dcookie			sys_lookup_dcookie		compat_sys_lookup_dcookie
 236	common	epoll_create			sys_epoll_create
@@ -323,7 +323,7 @@
 251	spu	utimes				sys_utimes
 252	common	statfs64			sys_statfs64			compat_sys_statfs64
 253	common	fstatfs64			sys_fstatfs64			compat_sys_fstatfs64
-254	32	fadvise64_64			ppc_fadvise64_64
+254	32	fadvise64_64			sys_ppc_fadvise64_64
 254	spu	fadvise64_64			sys_ni_syscall
 255	common	rtas				sys_rtas
 256	32	sys_debug_setcontext		sys_debug_setcontext		sys_ni_syscall
@@ -390,7 +390,7 @@
 305	common	signalfd			sys_signalfd			compat_sys_signalfd
 306	common	timerfd_create			sys_timerfd_create
 307	common	eventfd				sys_eventfd
-308	common	sync_file_range2		sys_sync_file_range2		compat_sys_sync_file_range2
+308	common	sync_file_range2		sys_sync_file_range2		compat_sys_ppc_sync_file_range2
 309	nospu	fallocate			sys_fallocate			compat_sys_fallocate
 310	nospu	subpage_prot			sys_subpage_prot
 311	32	timerfd_settime			sys_timerfd_settime32
-- 
2.34.1

Re: [PATCH 15/23] powerpc: Adopt SYSCALL_DEFINE for arch-specific syscall handlers

[Rohan McLure]

> On 16 Sep 2022, at 3:32 pm, Rohan McLure <rmclure@linux.ibm.com> wrote:
> 
> Arch-specific implementations of syscall handlers are currently used
> over generic implementations for the following reasons:
> 
> 1. Semantics unique to powerpc
> 2. Compatibility syscalls require 'argument padding' to comply with
>   64-bit argument convention in ELF32 abi.
> 3. Parameter types or order is different in other architectures.
> 
> These syscall handlers have been defined prior to this patch series
> without invoking the SYSCALL_DEFINE or COMPAT_SYSCALL_DEFINE macros with
> custom input and output types. We remove every such direct definition in
> favour of the aforementioned macros.
> 
> Also update syscalls.tbl in order to refer to the symbol names generated
> by each of these macros. Since ppc64_personality can be called by both
> 64 bit and 32 bit binaries through compatibility, we must generate both
> both compat_sys_ and sys_ symbols for this handler.
> 
> As an aside:
> A number of architectures including arm and powerpc agree on an
> alternative argument order and numbering for most of these arch-specific
> handlers. A future patch series may allow for asm/unistd.h to signal
> through its defines that a generic implementation of these syscall
> handlers with the correct calling convention be emitted, through the
> __ARCH_WANT_COMPAT_SYS_... convention.
> 
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> V1 -> V2: All syscall handlers wrapped by this macro.
> V2 -> V3: Move creation of do_ppc64_personality helper to prior patch.
> V3 -> V4: Fix parenthesis alignment. Don't emit sys_*** symbols.
> V4 -> V5: Use 'aside' in the asm-generic rant in commit message.
> ---
> arch/powerpc/include/asm/syscalls.h          | 10 ++---
> arch/powerpc/kernel/sys_ppc32.c              | 38 +++++++++++-------
> arch/powerpc/kernel/syscalls.c               | 17 ++++++--
> arch/powerpc/kernel/syscalls/syscall.tbl     | 22 +++++-----
> .../arch/powerpc/entry/syscalls/syscall.tbl  | 22 +++++-----
> 5 files changed, 64 insertions(+), 45 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
> index 20cbd29b1228..525d2aa0c8ca 100644
> --- a/arch/powerpc/include/asm/syscalls.h
> +++ b/arch/powerpc/include/asm/syscalls.h
> @@ -28,10 +28,10 @@ long sys_mmap(unsigned long addr, size_t len,
> long sys_mmap2(unsigned long addr, size_t len,
> 	       unsigned long prot, unsigned long flags,
> 	       unsigned long fd, unsigned long pgoff);
> -long ppc64_personality(unsigned long personality);
> +long sys_ppc64_personality(unsigned long personality);
> long sys_rtas(struct rtas_args __user *uargs);
> -long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
> -		      u32 len_high, u32 len_low);
> +long sys_ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
> +			  u32 len_high, u32 len_low);
> 
> #ifdef CONFIG_COMPAT
> unsigned long compat_sys_mmap2(unsigned long addr, size_t len,
> @@ -52,8 +52,8 @@ int compat_sys_truncate64(const char __user *path, u32 reg4,
> int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long len1,
> 			   unsigned long len2);
> 
> -long ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
> -		     size_t len, int advice);
> +long compat_sys_ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
> +				size_t len, int advice);
> 
> long compat_sys_sync_file_range2(int fd, unsigned int flags,
> 				 unsigned int offset1, unsigned int offset2,
> diff --git a/arch/powerpc/kernel/sys_ppc32.c b/arch/powerpc/kernel/sys_ppc32.c
> index 776ae7565fc5..dcc3c9fd4cfd 100644
> --- a/arch/powerpc/kernel/sys_ppc32.c
> +++ b/arch/powerpc/kernel/sys_ppc32.c
> @@ -47,45 +47,55 @@
> #include <asm/syscalls.h>
> #include <asm/switch_to.h>
> 
> -compat_ssize_t compat_sys_pread64(unsigned int fd, char __user *ubuf, compat_size_t count,
> -			     u32 reg6, u32 pos1, u32 pos2)
> +COMPAT_SYSCALL_DEFINE6(ppc_pread64,
> +		       unsigned int, fd,
> +		       char __user *, ubuf, compat_size_t, count,
> +		       u32, reg6, u32, pos1, u32, pos2)
> {
> 	return ksys_pread64(fd, ubuf, count, merge_64(pos1, pos2));
> }
> 
> -compat_ssize_t compat_sys_pwrite64(unsigned int fd, const char __user *ubuf, compat_size_t count,
> -			      u32 reg6, u32 pos1, u32 pos2)
> +COMPAT_SYSCALL_DEFINE6(ppc_pwrite64,
> +		       unsigned int, fd,
> +		       const char __user *, ubuf, compat_size_t, count,
> +		       u32, reg6, u32, pos1, u32, pos2)
> {
> 	return ksys_pwrite64(fd, ubuf, count, merge_64(pos1, pos2));
> }
> 
> -compat_ssize_t compat_sys_readahead(int fd, u32 r4, u32 offset1, u32 offset2, u32 count)
> +COMPAT_SYSCALL_DEFINE5(ppc_readahead,
> +		       int, fd, u32, r4,
> +		       u32, offset1, u32, offset2, u32, count)
> {
> 	return ksys_readahead(fd, merge_64(offset1, offset2), count);
> }
> 
> -int compat_sys_truncate64(const char __user * path, u32 reg4,
> -				unsigned long len1, unsigned long len2)
> +COMPAT_SYSCALL_DEFINE4(ppc_truncate64,
> +		       const char __user *, path, u32, reg4,
> +		       unsigned long, len1, unsigned long, len2)
> {
> 	return ksys_truncate(path, merge_64(len1, len2));
> }
> 
> -int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long len1,
> -				 unsigned long len2)
> +COMPAT_SYSCALL_DEFINE4(ppc_ftruncate64,
> +		       unsigned int, fd, u32, reg4,
> +		       unsigned long, len1, unsigned long, len2)
> {
> 	return ksys_ftruncate(fd, merge_64(len1, len2));
> }
> 
> -long ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
> -		     size_t len, int advice)
> +COMPAT_SYSCALL_DEFINE6(ppc32_fadvise64,
> +		       int, fd, u32, unused, u32, offset1, u32, offset2,
> +		       size_t, len, int, advice)
> {
> 	return ksys_fadvise64_64(fd, merge_64(offset1, offset2), len,
> 				 advice);
> }
> 
> -long compat_sys_sync_file_range2(int fd, unsigned int flags,
> -				   unsigned offset1, unsigned offset2,
> -				   unsigned nbytes1, unsigned nbytes2)
> +COMPAT_SYSCALL_DEFINE6(ppc_sync_file_range2,
> +		       int, fd, unsigned int, flags,
> +		       unsigned int, offset1, unsigned int, offset2,
> +		       unsigned int, nbytes1, unsigned int, nbytes2)
> {
> 	loff_t offset = merge_64(offset1, offset2);
> 	loff_t nbytes = merge_64(nbytes1, nbytes2);
> diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
> index 135a0b9108d5..3a35c690150b 100644
> --- a/arch/powerpc/kernel/syscalls.c
> +++ b/arch/powerpc/kernel/syscalls.c
> @@ -87,14 +87,23 @@ static long do_ppc64_personality(unsigned long personality)
> 		ret = (ret & ~PER_MASK) | PER_LINUX;
> 	return ret;
> }
> -long ppc64_personality(unsigned long personality)
> +
> +SYSCALL_DEFINE1(ppc64_personality, unsigned long, personality)
> {
> 	return do_ppc64_personality(personality);
> }
> -#endif
> 
> -long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
> -		      u32 len_high, u32 len_low)
> +#ifdef CONFIG_COMPAT
> +COMPAT_SYSCALL_DEFINE1(ppc64_personality, unsigned long, personality)
> +{
> +	return do_ppc64_personality(personality);
> +}
> +#endif /* CONFIG_COMPAT */
> +#endif /* CONFIG_PPC64 */
> +
> +SYSCALL_DEFINE6(ppc_fadvise64_64,
> +		int, fd, int, advice, u32, offset_high, u32, offset_low,
> +		u32, len_high, u32, len_low)
> {
> 	return ksys_fadvise64_64(fd, merge_64(offset_high, offset_low),
> 				 merge_64(len_high, len_low), advice);
> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
> index 64f27cbbdd2c..2bca64f96164 100644
> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> @@ -178,9 +178,9 @@
> 133	common	fchdir				sys_fchdir
> 134	common	bdflush				sys_ni_syscall
> 135	common	sysfs				sys_sysfs
> -136	32	personality			sys_personality			ppc64_personality
> -136	64	personality			ppc64_personality
> -136	spu	personality			ppc64_personality
> +136	32	personality			sys_personality			compat_sys_ppc64_personality
> +136	64	personality			sys_ppc64_personality
> +136	spu	personality			sys_ppc64_personality
> 137	common	afs_syscall			sys_ni_syscall
> 138	common	setfsuid			sys_setfsuid
> 139	common	setfsgid			sys_setfsgid
> @@ -228,8 +228,8 @@
> 176	64	rt_sigtimedwait			sys_rt_sigtimedwait
> 177	nospu 	rt_sigqueueinfo			sys_rt_sigqueueinfo		compat_sys_rt_sigqueueinfo
> 178	nospu 	rt_sigsuspend			sys_rt_sigsuspend		compat_sys_rt_sigsuspend
> -179	common	pread64				sys_pread64			compat_sys_pread64
> -180	common	pwrite64			sys_pwrite64			compat_sys_pwrite64
> +179	common	pread64				sys_pread64			compat_sys_ppc_pread64
> +180	common	pwrite64			sys_pwrite64			compat_sys_ppc_pwrite64
> 181	common	chown				sys_chown
> 182	common	getcwd				sys_getcwd
> 183	common	capget				sys_capget
> @@ -242,10 +242,10 @@
> 188	common 	putpmsg				sys_ni_syscall
> 189	nospu	vfork				sys_vfork
> 190	common	ugetrlimit			sys_getrlimit			compat_sys_getrlimit
> -191	common	readahead			sys_readahead			compat_sys_readahead
> +191	common	readahead			sys_readahead			compat_sys_ppc_readahead
> 192	32	mmap2				sys_mmap2			compat_sys_mmap2
> -193	32	truncate64			sys_truncate64			compat_sys_truncate64
> -194	32	ftruncate64			sys_ftruncate64			compat_sys_ftruncate64
> +193	32	truncate64			sys_truncate64			compat_sys_ppc_truncate64
> +194	32	ftruncate64			sys_ftruncate64			compat_sys_ppc_ftruncate64
> 195	32	stat64				sys_stat64
> 196	32	lstat64				sys_lstat64
> 197	32	fstat64				sys_fstat64
> @@ -288,7 +288,7 @@
> 230	common	io_submit			sys_io_submit			compat_sys_io_submit
> 231	common	io_cancel			sys_io_cancel
> 232	nospu	set_tid_address			sys_set_tid_address
> -233	common	fadvise64			sys_fadvise64			ppc32_fadvise64
> +233	common	fadvise64			sys_fadvise64			compat_sys_ppc32_fadvise64
> 234	nospu	exit_group			sys_exit_group
> 235	nospu	lookup_dcookie			sys_lookup_dcookie		compat_sys_lookup_dcookie
> 236	common	epoll_create			sys_epoll_create
> @@ -323,7 +323,7 @@
> 251	spu	utimes				sys_utimes
> 252	common	statfs64			sys_statfs64			compat_sys_statfs64
> 253	common	fstatfs64			sys_fstatfs64			compat_sys_fstatfs64
> -254	32	fadvise64_64			ppc_fadvise64_64
> +254	32	fadvise64_64			sys_ppc_fadvise64_64
> 254	spu	fadvise64_64			sys_ni_syscall
> 255	common	rtas				sys_rtas
> 256	32	sys_debug_setcontext		sys_debug_setcontext		sys_ni_syscall
> @@ -390,7 +390,7 @@
> 305	common	signalfd			sys_signalfd			compat_sys_signalfd
> 306	common	timerfd_create			sys_timerfd_create
> 307	common	eventfd				sys_eventfd
> -308	common	sync_file_range2		sys_sync_file_range2		compat_sys_sync_file_range2
> +308	common	sync_file_range2		sys_sync_file_range2		compat_sys_ppc_sync_file_range2
> 309	nospu	fallocate			sys_fallocate			compat_sys_fallocate
> 310	nospu	subpage_prot			sys_subpage_prot
> 311	32	timerfd_settime			sys_timerfd_settime32
> diff --git a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
> index 64f27cbbdd2c..2bca64f96164 100644
> --- a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
> +++ b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
> @@ -178,9 +178,9 @@
> 133	common	fchdir				sys_fchdir
> 134	common	bdflush				sys_ni_syscall
> 135	common	sysfs				sys_sysfs
> -136	32	personality			sys_personality			ppc64_personality
> -136	64	personality			ppc64_personality
> -136	spu	personality			ppc64_personality
> +136	32	personality			sys_personality			compat_sys_ppc64_personality
> +136	64	personality			sys_ppc64_personality
> +136	spu	personality			sys_ppc64_personality
> 137	common	afs_syscall			sys_ni_syscall
> 138	common	setfsuid			sys_setfsuid
> 139	common	setfsgid			sys_setfsgid
> @@ -228,8 +228,8 @@
> 176	64	rt_sigtimedwait			sys_rt_sigtimedwait
> 177	nospu 	rt_sigqueueinfo			sys_rt_sigqueueinfo		compat_sys_rt_sigqueueinfo
> 178	nospu 	rt_sigsuspend			sys_rt_sigsuspend		compat_sys_rt_sigsuspend
> -179	common	pread64				sys_pread64			compat_sys_pread64
> -180	common	pwrite64			sys_pwrite64			compat_sys_pwrite64
> +179	common	pread64				sys_pread64			compat_sys_ppc_pread64
> +180	common	pwrite64			sys_pwrite64			compat_sys_ppc_pwrite64
> 181	common	chown				sys_chown
> 182	common	getcwd				sys_getcwd
> 183	common	capget				sys_capget
> @@ -242,10 +242,10 @@
> 188	common 	putpmsg				sys_ni_syscall
> 189	nospu	vfork				sys_vfork
> 190	common	ugetrlimit			sys_getrlimit			compat_sys_getrlimit
> -191	common	readahead			sys_readahead			compat_sys_readahead
> +191	common	readahead			sys_readahead			compat_sys_ppc_readahead
> 192	32	mmap2				sys_mmap2			compat_sys_mmap2
> -193	32	truncate64			sys_truncate64			compat_sys_truncate64
> -194	32	ftruncate64			sys_ftruncate64			compat_sys_ftruncate64
> +193	32	truncate64			sys_truncate64			compat_sys_ppc_truncate64
> +194	32	ftruncate64			sys_ftruncate64			compat_sys_ppc_ftruncate64
> 195	32	stat64				sys_stat64
> 196	32	lstat64				sys_lstat64
> 197	32	fstat64				sys_fstat64
> @@ -288,7 +288,7 @@
> 230	common	io_submit			sys_io_submit			compat_sys_io_submit
> 231	common	io_cancel			sys_io_cancel
> 232	nospu	set_tid_address			sys_set_tid_address
> -233	common	fadvise64			sys_fadvise64			ppc32_fadvise64
> +233	common	fadvise64			sys_fadvise64			compat_sys_ppc32_fadvise64
> 234	nospu	exit_group			sys_exit_group
> 235	nospu	lookup_dcookie			sys_lookup_dcookie		compat_sys_lookup_dcookie
> 236	common	epoll_create			sys_epoll_create
> @@ -323,7 +323,7 @@
> 251	spu	utimes				sys_utimes
> 252	common	statfs64			sys_statfs64			compat_sys_statfs64
> 253	common	fstatfs64			sys_fstatfs64			compat_sys_fstatfs64
> -254	32	fadvise64_64			ppc_fadvise64_64
> +254	32	fadvise64_64			sys_ppc_fadvise64_64
> 254	spu	fadvise64_64			sys_ni_syscall
> 255	common	rtas				sys_rtas
> 256	32	sys_debug_setcontext		sys_debug_setcontext		sys_ni_syscall
> @@ -390,7 +390,7 @@
> 305	common	signalfd			sys_signalfd			compat_sys_signalfd
> 306	common	timerfd_create			sys_timerfd_create
> 307	common	eventfd				sys_eventfd
> -308	common	sync_file_range2		sys_sync_file_range2		compat_sys_sync_file_range2
> +308	common	sync_file_range2		sys_sync_file_range2		compat_sys_ppc_sync_file_range2
> 309	nospu	fallocate			sys_fallocate			compat_sys_fallocate
> 310	nospu	subpage_prot			sys_subpage_prot
> 311	32	timerfd_settime			sys_timerfd_settime32
> -- 
> 2.34.1
> 

Apologies. This is in fact V5. I can resend if anyone needs me to.

Rohan

Re: [PATCH 15/23] powerpc: Adopt SYSCALL_DEFINE for arch-specific syscall handlers

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> Arch-specific implementations of syscall handlers are currently used
> over generic implementations for the following reasons:
>
> 1. Semantics unique to powerpc
> 2. Compatibility syscalls require 'argument padding' to comply with
>    64-bit argument convention in ELF32 abi.
> 3. Parameter types or order is different in other architectures.
>
> These syscall handlers have been defined prior to this patch series
> without invoking the SYSCALL_DEFINE or COMPAT_SYSCALL_DEFINE macros with
> custom input and output types. We remove every such direct definition in
> favour of the aforementioned macros.
>
> Also update syscalls.tbl in order to refer to the symbol names generated
> by each of these macros. Since ppc64_personality can be called by both
> 64 bit and 32 bit binaries through compatibility, we must generate both
> both compat_sys_ and sys_ symbols for this handler.
>
> As an aside:
> A number of architectures including arm and powerpc agree on an
> alternative argument order and numbering for most of these arch-specific
> handlers. A future patch series may allow for asm/unistd.h to signal
> through its defines that a generic implementation of these syscall
> handlers with the correct calling convention be emitted, through the
> __ARCH_WANT_COMPAT_SYS_... convention.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>

Looks good

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> ---
> V1 -> V2: All syscall handlers wrapped by this macro.
> V2 -> V3: Move creation of do_ppc64_personality helper to prior patch.
> V3 -> V4: Fix parenthesis alignment. Don't emit sys_*** symbols.
> V4 -> V5: Use 'aside' in the asm-generic rant in commit message.
> ---
>  arch/powerpc/include/asm/syscalls.h          | 10 ++---
>  arch/powerpc/kernel/sys_ppc32.c              | 38 +++++++++++-------
>  arch/powerpc/kernel/syscalls.c               | 17 ++++++--
>  arch/powerpc/kernel/syscalls/syscall.tbl     | 22 +++++-----
>  .../arch/powerpc/entry/syscalls/syscall.tbl  | 22 +++++-----
>  5 files changed, 64 insertions(+), 45 deletions(-)
>

[PATCH 16/23] powerpc: Include all arch-specific syscall prototypes

[Rohan McLure]

Forward declare all syscall handler prototypes where a generic prototype
is not provided in either linux/syscalls.h or linux/compat.h in
asm/syscalls.h. This is required for compile-time type-checking for
syscall handlers, which is implemented later in this series.

32-bit compatibility syscall handlers are expressed in terms of types in
ppc32.h. Expose this header globally.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
V1 -> V2: Explicitly include prototypes.
V2 -> V3: Remove extraneous #include <asm/compat.h> and ppc_fallocate
prototype. Rename header.
V4 -> V5: Clean. Elaborate comment on long long munging. Remove
prototype hiding conditional on SYSCALL_WRAPPER.
---
 arch/powerpc/include/asm/syscalls.h          | 97 ++++++++++++++----
 .../ppc32.h => include/asm/syscalls_32.h}    |  0
 arch/powerpc/kernel/signal_32.c              |  2 +-
 arch/powerpc/perf/callchain_32.c             |  2 +-
 4 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index 525d2aa0c8ca..5d106acf7906 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -8,6 +8,14 @@
 #include <linux/types.h>
 #include <linux/compat.h>
 
+#ifdef CONFIG_PPC64
+#include <asm/syscalls_32.h>
+#endif
+#include <asm/unistd.h>
+#include <asm/ucontext.h>
+
+struct rtas_args;
+
 /*
  * long long munging:
  * The 32 bit ABI passes long longs in an odd even register pair.
@@ -20,44 +28,89 @@
 #define merge_64(high, low) ((u64)high << 32) | low
 #endif
 
-struct rtas_args;
+long sys_ni_syscall(void);
+
+/*
+ * PowerPC architecture-specific syscalls
+ */
+
+long sys_rtas(struct rtas_args __user *uargs);
+
+#ifdef CONFIG_PPC64
+long sys_ppc64_personality(unsigned long personality);
+#ifdef CONFIG_COMPAT
+long compat_sys_ppc64_personality(unsigned long personality);
+#endif /* CONFIG_COMPAT */
+#endif /* CONFIG_PPC64 */
 
+long sys_swapcontext(struct ucontext __user *old_ctx,
+		     struct ucontext __user *new_ctx, long ctx_size);
 long sys_mmap(unsigned long addr, size_t len,
 	      unsigned long prot, unsigned long flags,
 	      unsigned long fd, off_t offset);
 long sys_mmap2(unsigned long addr, size_t len,
 	       unsigned long prot, unsigned long flags,
 	       unsigned long fd, unsigned long pgoff);
-long sys_ppc64_personality(unsigned long personality);
-long sys_rtas(struct rtas_args __user *uargs);
-long sys_ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
-			  u32 len_high, u32 len_low);
+long sys_switch_endian(void);
 
-#ifdef CONFIG_COMPAT
-unsigned long compat_sys_mmap2(unsigned long addr, size_t len,
-			       unsigned long prot, unsigned long flags,
-			       unsigned long fd, unsigned long pgoff);
-
-compat_ssize_t compat_sys_pread64(unsigned int fd, char __user *ubuf, compat_size_t count,
-				  u32 reg6, u32 pos1, u32 pos2);
+#ifdef CONFIG_PPC32
+long sys_sigreturn(void);
+long sys_debug_setcontext(struct ucontext __user *ctx, int ndbg,
+			  struct sig_dbg_op __user *dbg);
+#endif
 
-compat_ssize_t compat_sys_pwrite64(unsigned int fd, const char __user *ubuf, compat_size_t count,
-				   u32 reg6, u32 pos1, u32 pos2);
+long sys_rt_sigreturn(void);
 
-compat_ssize_t compat_sys_readahead(int fd, u32 r4, u32 offset1, u32 offset2, u32 count);
+long sys_subpage_prot(unsigned long addr,
+		      unsigned long len, u32 __user *map);
 
-int compat_sys_truncate64(const char __user *path, u32 reg4,
-			  unsigned long len1, unsigned long len2);
+#ifdef CONFIG_COMPAT
+long compat_sys_swapcontext(struct ucontext32 __user *old_ctx,
+			    struct ucontext32 __user *new_ctx,
+			    int ctx_size);
+long compat_sys_old_getrlimit(unsigned int resource,
+			      struct compat_rlimit __user *rlim);
+long compat_sys_sigreturn(void);
+long compat_sys_rt_sigreturn(void);
+#endif /* CONFIG_COMPAT */
 
-int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long len1,
-			   unsigned long len2);
+/*
+ * Architecture specific signatures required by long long munging:
+ * The 32 bit ABI passes long longs in an odd even register pair.
+ * The following signatures provide a machine long parameter for
+ * each register that will be supplied. The implementation is
+ * responsible for combining parameter pairs.
+ */
 
+#ifdef CONFIG_COMPAT
+long compat_sys_mmap2(unsigned long addr, size_t len,
+		      unsigned long prot, unsigned long flags,
+		      unsigned long fd, unsigned long pgoff);
+long compat_sys_ppc_pread64(unsigned int fd,
+			    char __user *ubuf, compat_size_t count,
+			    u32 reg6, u32 pos1, u32 pos2);
+long compat_sys_ppc_pwrite64(unsigned int fd,
+			     const char __user *ubuf, compat_size_t count,
+			     u32 reg6, u32 pos1, u32 pos2);
+long compat_sys_ppc_readahead(int fd, u32 r4,
+			      u32 offset1, u32 offset2, u32 count);
+long compat_sys_ppc_truncate64(const char __user *path, u32 reg4,
+			       unsigned long len1, unsigned long len2);
+long compat_sys_ppc_ftruncate64(unsigned int fd, u32 reg4,
+				unsigned long len1, unsigned long len2);
 long compat_sys_ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
 				size_t len, int advice);
+long compat_sys_ppc_sync_file_range2(int fd, unsigned int flags,
+				     unsigned int offset1,
+				     unsigned int offset2,
+				     unsigned int nbytes1,
+				     unsigned int nbytes2);
+#endif /* CONFIG_COMPAT */
 
-long compat_sys_sync_file_range2(int fd, unsigned int flags,
-				 unsigned int offset1, unsigned int offset2,
-				 unsigned int nbytes1, unsigned int nbytes2);
+#if defined(CONFIG_PPC32) || defined(CONFIG_COMPAT)
+long sys_ppc_fadvise64_64(int fd, int advice,
+			  u32 offset_high, u32 offset_low,
+			  u32 len_high, u32 len_low);
 #endif
 
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/kernel/ppc32.h b/arch/powerpc/include/asm/syscalls_32.h
similarity index 100%
rename from arch/powerpc/kernel/ppc32.h
rename to arch/powerpc/include/asm/syscalls_32.h
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 157a7403e3eb..c114c7f25645 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -43,7 +43,7 @@
 #include <asm/tm.h>
 #include <asm/asm-prototypes.h>
 #ifdef CONFIG_PPC64
-#include "ppc32.h"
+#include <asm/syscalls_32.h>
 #include <asm/unistd.h>
 #else
 #include <asm/ucontext.h>
diff --git a/arch/powerpc/perf/callchain_32.c b/arch/powerpc/perf/callchain_32.c
index b83c47b7947f..ea8cfe3806dc 100644
--- a/arch/powerpc/perf/callchain_32.c
+++ b/arch/powerpc/perf/callchain_32.c
@@ -19,7 +19,7 @@
 #include "callchain.h"
 
 #ifdef CONFIG_PPC64
-#include "../kernel/ppc32.h"
+#include <asm/syscalls_32.h>
 #else  /* CONFIG_PPC64 */
 
 #define __SIGNAL_FRAMESIZE32	__SIGNAL_FRAMESIZE
-- 
2.34.1

Re: [PATCH 16/23] powerpc: Include all arch-specific syscall prototypes

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> Forward declare all syscall handler prototypes where a generic prototype
> is not provided in either linux/syscalls.h or linux/compat.h in
> asm/syscalls.h. This is required for compile-time type-checking for
> syscall handlers, which is implemented later in this series.
>
> 32-bit compatibility syscall handlers are expressed in terms of types in
> ppc32.h. Expose this header globally.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>

I didn't go through all the code moving carefully but I guess that
would blow up with the compile time checking so it must be right :)

Acked-by: Nicholas Piggin <npiggin@gmail.com>

> ---
> V1 -> V2: Explicitly include prototypes.
> V2 -> V3: Remove extraneous #include <asm/compat.h> and ppc_fallocate
> prototype. Rename header.
> V4 -> V5: Clean. Elaborate comment on long long munging. Remove
> prototype hiding conditional on SYSCALL_WRAPPER.
> ---
>  arch/powerpc/include/asm/syscalls.h          | 97 ++++++++++++++----
>  .../ppc32.h => include/asm/syscalls_32.h}    |  0
>  arch/powerpc/kernel/signal_32.c              |  2 +-
>  arch/powerpc/perf/callchain_32.c             |  2 +-
>  4 files changed, 77 insertions(+), 24 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
> index 525d2aa0c8ca..5d106acf7906 100644
> --- a/arch/powerpc/include/asm/syscalls.h
> +++ b/arch/powerpc/include/asm/syscalls.h
> @@ -8,6 +8,14 @@
>  #include <linux/types.h>
>  #include <linux/compat.h>
>  
> +#ifdef CONFIG_PPC64
> +#include <asm/syscalls_32.h>
> +#endif
> +#include <asm/unistd.h>
> +#include <asm/ucontext.h>
> +
> +struct rtas_args;
> +
>  /*
>   * long long munging:
>   * The 32 bit ABI passes long longs in an odd even register pair.
> @@ -20,44 +28,89 @@
>  #define merge_64(high, low) ((u64)high << 32) | low
>  #endif
>  
> -struct rtas_args;
> +long sys_ni_syscall(void);
> +
> +/*
> + * PowerPC architecture-specific syscalls
> + */
> +
> +long sys_rtas(struct rtas_args __user *uargs);
> +
> +#ifdef CONFIG_PPC64
> +long sys_ppc64_personality(unsigned long personality);
> +#ifdef CONFIG_COMPAT
> +long compat_sys_ppc64_personality(unsigned long personality);
> +#endif /* CONFIG_COMPAT */
> +#endif /* CONFIG_PPC64 */
>  
> +long sys_swapcontext(struct ucontext __user *old_ctx,
> +		     struct ucontext __user *new_ctx, long ctx_size);
>  long sys_mmap(unsigned long addr, size_t len,
>  	      unsigned long prot, unsigned long flags,
>  	      unsigned long fd, off_t offset);
>  long sys_mmap2(unsigned long addr, size_t len,
>  	       unsigned long prot, unsigned long flags,
>  	       unsigned long fd, unsigned long pgoff);
> -long sys_ppc64_personality(unsigned long personality);
> -long sys_rtas(struct rtas_args __user *uargs);
> -long sys_ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
> -			  u32 len_high, u32 len_low);
> +long sys_switch_endian(void);
>  
> -#ifdef CONFIG_COMPAT
> -unsigned long compat_sys_mmap2(unsigned long addr, size_t len,
> -			       unsigned long prot, unsigned long flags,
> -			       unsigned long fd, unsigned long pgoff);
> -
> -compat_ssize_t compat_sys_pread64(unsigned int fd, char __user *ubuf, compat_size_t count,
> -				  u32 reg6, u32 pos1, u32 pos2);
> +#ifdef CONFIG_PPC32
> +long sys_sigreturn(void);
> +long sys_debug_setcontext(struct ucontext __user *ctx, int ndbg,
> +			  struct sig_dbg_op __user *dbg);
> +#endif
>  
> -compat_ssize_t compat_sys_pwrite64(unsigned int fd, const char __user *ubuf, compat_size_t count,
> -				   u32 reg6, u32 pos1, u32 pos2);
> +long sys_rt_sigreturn(void);
>  
> -compat_ssize_t compat_sys_readahead(int fd, u32 r4, u32 offset1, u32 offset2, u32 count);
> +long sys_subpage_prot(unsigned long addr,
> +		      unsigned long len, u32 __user *map);
>  
> -int compat_sys_truncate64(const char __user *path, u32 reg4,
> -			  unsigned long len1, unsigned long len2);
> +#ifdef CONFIG_COMPAT
> +long compat_sys_swapcontext(struct ucontext32 __user *old_ctx,
> +			    struct ucontext32 __user *new_ctx,
> +			    int ctx_size);
> +long compat_sys_old_getrlimit(unsigned int resource,
> +			      struct compat_rlimit __user *rlim);
> +long compat_sys_sigreturn(void);
> +long compat_sys_rt_sigreturn(void);
> +#endif /* CONFIG_COMPAT */
>  
> -int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long len1,
> -			   unsigned long len2);
> +/*
> + * Architecture specific signatures required by long long munging:
> + * The 32 bit ABI passes long longs in an odd even register pair.
> + * The following signatures provide a machine long parameter for
> + * each register that will be supplied. The implementation is
> + * responsible for combining parameter pairs.
> + */
>  
> +#ifdef CONFIG_COMPAT
> +long compat_sys_mmap2(unsigned long addr, size_t len,
> +		      unsigned long prot, unsigned long flags,
> +		      unsigned long fd, unsigned long pgoff);
> +long compat_sys_ppc_pread64(unsigned int fd,
> +			    char __user *ubuf, compat_size_t count,
> +			    u32 reg6, u32 pos1, u32 pos2);
> +long compat_sys_ppc_pwrite64(unsigned int fd,
> +			     const char __user *ubuf, compat_size_t count,
> +			     u32 reg6, u32 pos1, u32 pos2);
> +long compat_sys_ppc_readahead(int fd, u32 r4,
> +			      u32 offset1, u32 offset2, u32 count);
> +long compat_sys_ppc_truncate64(const char __user *path, u32 reg4,
> +			       unsigned long len1, unsigned long len2);
> +long compat_sys_ppc_ftruncate64(unsigned int fd, u32 reg4,
> +				unsigned long len1, unsigned long len2);
>  long compat_sys_ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
>  				size_t len, int advice);
> +long compat_sys_ppc_sync_file_range2(int fd, unsigned int flags,
> +				     unsigned int offset1,
> +				     unsigned int offset2,
> +				     unsigned int nbytes1,
> +				     unsigned int nbytes2);
> +#endif /* CONFIG_COMPAT */
>  
> -long compat_sys_sync_file_range2(int fd, unsigned int flags,
> -				 unsigned int offset1, unsigned int offset2,
> -				 unsigned int nbytes1, unsigned int nbytes2);
> +#if defined(CONFIG_PPC32) || defined(CONFIG_COMPAT)
> +long sys_ppc_fadvise64_64(int fd, int advice,
> +			  u32 offset_high, u32 offset_low,
> +			  u32 len_high, u32 len_low);
>  #endif
>  
>  #endif /* __KERNEL__ */
> diff --git a/arch/powerpc/kernel/ppc32.h b/arch/powerpc/include/asm/syscalls_32.h
> similarity index 100%
> rename from arch/powerpc/kernel/ppc32.h
> rename to arch/powerpc/include/asm/syscalls_32.h
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index 157a7403e3eb..c114c7f25645 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -43,7 +43,7 @@
>  #include <asm/tm.h>
>  #include <asm/asm-prototypes.h>
>  #ifdef CONFIG_PPC64
> -#include "ppc32.h"
> +#include <asm/syscalls_32.h>
>  #include <asm/unistd.h>
>  #else
>  #include <asm/ucontext.h>
> diff --git a/arch/powerpc/perf/callchain_32.c b/arch/powerpc/perf/callchain_32.c
> index b83c47b7947f..ea8cfe3806dc 100644
> --- a/arch/powerpc/perf/callchain_32.c
> +++ b/arch/powerpc/perf/callchain_32.c
> @@ -19,7 +19,7 @@
>  #include "callchain.h"
>  
>  #ifdef CONFIG_PPC64
> -#include "../kernel/ppc32.h"
> +#include <asm/syscalls_32.h>
>  #else  /* CONFIG_PPC64 */
>  
>  #define __SIGNAL_FRAMESIZE32	__SIGNAL_FRAMESIZE
> -- 
> 2.34.1

[PATCH 17/23] powerpc: Enable compile-time check for syscall handlers

[Rohan McLure]

The table of syscall handlers and registered compatibility syscall
handlers has in past been produced using assembly, with function
references resolved at link time. This moves link-time errors to
compile-time, by rewriting systbl.S in C, and including the
linux/syscalls.h, linux/compat.h and asm/syscalls.h headers for
prototypes.

Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Reported-by: Nicholas Piggin <npiggin@gmail.com>
---
V1 -> V2: New patch.
V4 -> V5: For this patch only, represent handler function pointers as
unsigned long. Remove reference to syscall wrappers. Use asm/syscalls.h
which implies asm/syscall.h
---
 arch/powerpc/kernel/{systbl.S => systbl.c} | 28 ++++++++------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/kernel/systbl.S b/arch/powerpc/kernel/systbl.c
similarity index 61%
rename from arch/powerpc/kernel/systbl.S
rename to arch/powerpc/kernel/systbl.c
index 6c1db3b6de2d..ce52bd2ec292 100644
--- a/arch/powerpc/kernel/systbl.S
+++ b/arch/powerpc/kernel/systbl.c
@@ -10,32 +10,26 @@
  * PPC64 updates by Dave Engebretsen (engebret@us.ibm.com) 
  */
 
-#include <asm/ppc_asm.h>
+#include <linux/syscalls.h>
+#include <linux/compat.h>
+#include <asm/unistd.h>
+#include <asm/syscalls.h>
 
-.section .rodata,"a"
+#define __SYSCALL_WITH_COMPAT(nr, entry, compat) __SYSCALL(nr, entry)
+#define __SYSCALL(nr, entry) [nr] = (unsigned long) &entry,
 
-#ifdef CONFIG_PPC64
-	.p2align	3
-#define __SYSCALL(nr, entry)	.8byte entry
-#else
-	.p2align	2
-#define __SYSCALL(nr, entry)	.long entry
-#endif
-
-#define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, native)
-.globl sys_call_table
-sys_call_table:
+const unsigned long sys_call_table[] = {
 #ifdef CONFIG_PPC64
 #include <asm/syscall_table_64.h>
 #else
 #include <asm/syscall_table_32.h>
 #endif
+};
 
 #ifdef CONFIG_COMPAT
 #undef __SYSCALL_WITH_COMPAT
 #define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, compat)
-.globl compat_sys_call_table
-compat_sys_call_table:
-#define compat_sys_sigsuspend	sys_sigsuspend
+const unsigned long compat_sys_call_table[] = {
 #include <asm/syscall_table_32.h>
-#endif
+};
+#endif /* CONFIG_COMPAT */
-- 
2.34.1

Re: [PATCH 17/23] powerpc: Enable compile-time check for syscall handlers

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> The table of syscall handlers and registered compatibility syscall
> handlers has in past been produced using assembly, with function
> references resolved at link time. This moves link-time errors to
> compile-time, by rewriting systbl.S in C, and including the
> linux/syscalls.h, linux/compat.h and asm/syscalls.h headers for
> prototypes.
>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> Reported-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> V1 -> V2: New patch.
> V4 -> V5: For this patch only, represent handler function pointers as
> unsigned long. Remove reference to syscall wrappers. Use asm/syscalls.h
> which implies asm/syscall.h

Thanks, I think this is the right way to split the patches.

I'm not sure if I reported this issue, but I really like the patch.

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> ---
>  arch/powerpc/kernel/{systbl.S => systbl.c} | 28 ++++++++------------
>  1 file changed, 11 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/kernel/systbl.S b/arch/powerpc/kernel/systbl.c
> similarity index 61%
> rename from arch/powerpc/kernel/systbl.S
> rename to arch/powerpc/kernel/systbl.c
> index 6c1db3b6de2d..ce52bd2ec292 100644
> --- a/arch/powerpc/kernel/systbl.S
> +++ b/arch/powerpc/kernel/systbl.c
> @@ -10,32 +10,26 @@
>   * PPC64 updates by Dave Engebretsen (engebret@us.ibm.com) 
>   */
>  
> -#include <asm/ppc_asm.h>
> +#include <linux/syscalls.h>
> +#include <linux/compat.h>
> +#include <asm/unistd.h>
> +#include <asm/syscalls.h>
>  
> -.section .rodata,"a"
> +#define __SYSCALL_WITH_COMPAT(nr, entry, compat) __SYSCALL(nr, entry)
> +#define __SYSCALL(nr, entry) [nr] = (unsigned long) &entry,
>  
> -#ifdef CONFIG_PPC64
> -	.p2align	3
> -#define __SYSCALL(nr, entry)	.8byte entry
> -#else
> -	.p2align	2
> -#define __SYSCALL(nr, entry)	.long entry
> -#endif
> -
> -#define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, native)
> -.globl sys_call_table
> -sys_call_table:
> +const unsigned long sys_call_table[] = {
>  #ifdef CONFIG_PPC64
>  #include <asm/syscall_table_64.h>
>  #else
>  #include <asm/syscall_table_32.h>
>  #endif
> +};
>  
>  #ifdef CONFIG_COMPAT
>  #undef __SYSCALL_WITH_COMPAT
>  #define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, compat)
> -.globl compat_sys_call_table
> -compat_sys_call_table:
> -#define compat_sys_sigsuspend	sys_sigsuspend
> +const unsigned long compat_sys_call_table[] = {
>  #include <asm/syscall_table_32.h>
> -#endif
> +};
> +#endif /* CONFIG_COMPAT */
> -- 
> 2.34.1

[PATCH 18/23] powerpc: Use common syscall handler type

[Rohan McLure]

Cause syscall handlers to be typed as follows when called indirectly
throughout the kernel. This is to allow for better type checking.

typedef long (*syscall_fn)(unsigned long, unsigned long, unsigned long,
                           unsigned long, unsigned long, unsigned long);

Since both 32 and 64-bit abis allow for at least the first six
machine-word length parameters to a function to be passed by registers,
even handlers which admit fewer than six parameters may be viewed as
having the above type.

Coercing syscalls to syscall_fn requires a cast to void* to avoid
-Wcast-function-type.

Fixup comparisons in VDSO to avoid pointer-integer comparison. Introduce
explicit cast on systems with SPUs.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
V1 -> V2: New patch.
V2 -> V3: Remove unnecessary cast from const syscall_fn to syscall_fn
V4 -> V5: Update patch description.
---
 arch/powerpc/include/asm/syscall.h          | 7 +++++--
 arch/powerpc/include/asm/syscalls.h         | 1 +
 arch/powerpc/kernel/systbl.c                | 6 +++---
 arch/powerpc/kernel/vdso.c                  | 4 ++--
 arch/powerpc/platforms/cell/spu_callbacks.c | 6 +++---
 5 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
index 25fc8ad9a27a..d2a8dfd5de33 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -14,9 +14,12 @@
 #include <linux/sched.h>
 #include <linux/thread_info.h>
 
+typedef long (*syscall_fn)(unsigned long, unsigned long, unsigned long,
+			   unsigned long, unsigned long, unsigned long);
+
 /* ftrace syscalls requires exporting the sys_call_table */
-extern const unsigned long sys_call_table[];
-extern const unsigned long compat_sys_call_table[];
+extern const syscall_fn sys_call_table[];
+extern const syscall_fn compat_sys_call_table[];
 
 static inline int syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
 {
diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index 5d106acf7906..cc87168d6ecb 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -8,6 +8,7 @@
 #include <linux/types.h>
 #include <linux/compat.h>
 
+#include <asm/syscall.h>
 #ifdef CONFIG_PPC64
 #include <asm/syscalls_32.h>
 #endif
diff --git a/arch/powerpc/kernel/systbl.c b/arch/powerpc/kernel/systbl.c
index ce52bd2ec292..e5d419822b4e 100644
--- a/arch/powerpc/kernel/systbl.c
+++ b/arch/powerpc/kernel/systbl.c
@@ -16,9 +16,9 @@
 #include <asm/syscalls.h>
 
 #define __SYSCALL_WITH_COMPAT(nr, entry, compat) __SYSCALL(nr, entry)
-#define __SYSCALL(nr, entry) [nr] = (unsigned long) &entry,
+#define __SYSCALL(nr, entry) [nr] = (void *) entry,
 
-const unsigned long sys_call_table[] = {
+const syscall_fn sys_call_table[] = {
 #ifdef CONFIG_PPC64
 #include <asm/syscall_table_64.h>
 #else
@@ -29,7 +29,7 @@ const unsigned long sys_call_table[] = {
 #ifdef CONFIG_COMPAT
 #undef __SYSCALL_WITH_COMPAT
 #define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, compat)
-const unsigned long compat_sys_call_table[] = {
+const syscall_fn compat_sys_call_table[] = {
 #include <asm/syscall_table_32.h>
 };
 #endif /* CONFIG_COMPAT */
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index bf9574ec26ce..fcca06d200d3 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -304,10 +304,10 @@ static void __init vdso_setup_syscall_map(void)
 	unsigned int i;
 
 	for (i = 0; i < NR_syscalls; i++) {
-		if (sys_call_table[i] != (unsigned long)&sys_ni_syscall)
+		if (sys_call_table[i] != (void *)&sys_ni_syscall)
 			vdso_data->syscall_map[i >> 5] |= 0x80000000UL >> (i & 0x1f);
 		if (IS_ENABLED(CONFIG_COMPAT) &&
-		    compat_sys_call_table[i] != (unsigned long)&sys_ni_syscall)
+		    compat_sys_call_table[i] != (void *)&sys_ni_syscall)
 			vdso_data->compat_syscall_map[i >> 5] |= 0x80000000UL >> (i & 0x1f);
 	}
 }
diff --git a/arch/powerpc/platforms/cell/spu_callbacks.c b/arch/powerpc/platforms/cell/spu_callbacks.c
index fe0d8797a00a..e780c14c5733 100644
--- a/arch/powerpc/platforms/cell/spu_callbacks.c
+++ b/arch/powerpc/platforms/cell/spu_callbacks.c
@@ -34,15 +34,15 @@
  *	mbind, mq_open, ipc, ...
  */
 
-static void *spu_syscall_table[] = {
+static const syscall_fn spu_syscall_table[] = {
 #define __SYSCALL_WITH_COMPAT(nr, entry, compat) __SYSCALL(nr, entry)
-#define __SYSCALL(nr, entry) [nr] = entry,
+#define __SYSCALL(nr, entry) [nr] = (void *) entry,
 #include <asm/syscall_table_spu.h>
 };
 
 long spu_sys_callback(struct spu_syscall_block *s)
 {
-	long (*syscall)(u64 a1, u64 a2, u64 a3, u64 a4, u64 a5, u64 a6);
+	syscall_fn syscall;
 
 	if (s->nr_ret >= ARRAY_SIZE(spu_syscall_table)) {
 		pr_debug("%s: invalid syscall #%lld", __func__, s->nr_ret);
-- 
2.34.1

Re: [PATCH 18/23] powerpc: Use common syscall handler type

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> Cause syscall handlers to be typed as follows when called indirectly
> throughout the kernel. This is to allow for better type checking.
>
> typedef long (*syscall_fn)(unsigned long, unsigned long, unsigned long,
>                            unsigned long, unsigned long, unsigned long);
>
> Since both 32 and 64-bit abis allow for at least the first six
> machine-word length parameters to a function to be passed by registers,
> even handlers which admit fewer than six parameters may be viewed as
> having the above type.
>
> Coercing syscalls to syscall_fn requires a cast to void* to avoid
> -Wcast-function-type.

This could possibly be a comment in systbl.c as well?

>
> Fixup comparisons in VDSO to avoid pointer-integer comparison. Introduce
> explicit cast on systems with SPUs.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> V1 -> V2: New patch.
> V2 -> V3: Remove unnecessary cast from const syscall_fn to syscall_fn
> V4 -> V5: Update patch description.
> ---
>  arch/powerpc/include/asm/syscall.h          | 7 +++++--
>  arch/powerpc/include/asm/syscalls.h         | 1 +
>  arch/powerpc/kernel/systbl.c                | 6 +++---
>  arch/powerpc/kernel/vdso.c                  | 4 ++--
>  arch/powerpc/platforms/cell/spu_callbacks.c | 6 +++---

What's happened to arch/powerpc/kernel/syscall.c? We have
approximately the same type defined there with the same name.
That can just use your new type AFAIKS.

We're hopefully solving the rodata thing separately, so long
as you've got the const there that's enough.

Other than that,

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

>  5 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
> index 25fc8ad9a27a..d2a8dfd5de33 100644
> --- a/arch/powerpc/include/asm/syscall.h
> +++ b/arch/powerpc/include/asm/syscall.h
> @@ -14,9 +14,12 @@
>  #include <linux/sched.h>
>  #include <linux/thread_info.h>
>  
> +typedef long (*syscall_fn)(unsigned long, unsigned long, unsigned long,
> +			   unsigned long, unsigned long, unsigned long);
> +
>  /* ftrace syscalls requires exporting the sys_call_table */
> -extern const unsigned long sys_call_table[];
> -extern const unsigned long compat_sys_call_table[];
> +extern const syscall_fn sys_call_table[];
> +extern const syscall_fn compat_sys_call_table[];
>  
>  static inline int syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
>  {
> diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
> index 5d106acf7906..cc87168d6ecb 100644
> --- a/arch/powerpc/include/asm/syscalls.h
> +++ b/arch/powerpc/include/asm/syscalls.h
> @@ -8,6 +8,7 @@
>  #include <linux/types.h>
>  #include <linux/compat.h>
>  
> +#include <asm/syscall.h>
>  #ifdef CONFIG_PPC64
>  #include <asm/syscalls_32.h>
>  #endif
> diff --git a/arch/powerpc/kernel/systbl.c b/arch/powerpc/kernel/systbl.c
> index ce52bd2ec292..e5d419822b4e 100644
> --- a/arch/powerpc/kernel/systbl.c
> +++ b/arch/powerpc/kernel/systbl.c
> @@ -16,9 +16,9 @@
>  #include <asm/syscalls.h>
>  
>  #define __SYSCALL_WITH_COMPAT(nr, entry, compat) __SYSCALL(nr, entry)
> -#define __SYSCALL(nr, entry) [nr] = (unsigned long) &entry,
> +#define __SYSCALL(nr, entry) [nr] = (void *) entry,
>  
> -const unsigned long sys_call_table[] = {
> +const syscall_fn sys_call_table[] = {
>  #ifdef CONFIG_PPC64
>  #include <asm/syscall_table_64.h>
>  #else
> @@ -29,7 +29,7 @@ const unsigned long sys_call_table[] = {
>  #ifdef CONFIG_COMPAT
>  #undef __SYSCALL_WITH_COMPAT
>  #define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, compat)
> -const unsigned long compat_sys_call_table[] = {
> +const syscall_fn compat_sys_call_table[] = {
>  #include <asm/syscall_table_32.h>
>  };
>  #endif /* CONFIG_COMPAT */
> diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
> index bf9574ec26ce..fcca06d200d3 100644
> --- a/arch/powerpc/kernel/vdso.c
> +++ b/arch/powerpc/kernel/vdso.c
> @@ -304,10 +304,10 @@ static void __init vdso_setup_syscall_map(void)
>  	unsigned int i;
>  
>  	for (i = 0; i < NR_syscalls; i++) {
> -		if (sys_call_table[i] != (unsigned long)&sys_ni_syscall)
> +		if (sys_call_table[i] != (void *)&sys_ni_syscall)
>  			vdso_data->syscall_map[i >> 5] |= 0x80000000UL >> (i & 0x1f);
>  		if (IS_ENABLED(CONFIG_COMPAT) &&
> -		    compat_sys_call_table[i] != (unsigned long)&sys_ni_syscall)
> +		    compat_sys_call_table[i] != (void *)&sys_ni_syscall)
>  			vdso_data->compat_syscall_map[i >> 5] |= 0x80000000UL >> (i & 0x1f);
>  	}
>  }
> diff --git a/arch/powerpc/platforms/cell/spu_callbacks.c b/arch/powerpc/platforms/cell/spu_callbacks.c
> index fe0d8797a00a..e780c14c5733 100644
> --- a/arch/powerpc/platforms/cell/spu_callbacks.c
> +++ b/arch/powerpc/platforms/cell/spu_callbacks.c
> @@ -34,15 +34,15 @@
>   *	mbind, mq_open, ipc, ...
>   */
>  
> -static void *spu_syscall_table[] = {
> +static const syscall_fn spu_syscall_table[] = {
>  #define __SYSCALL_WITH_COMPAT(nr, entry, compat) __SYSCALL(nr, entry)
> -#define __SYSCALL(nr, entry) [nr] = entry,
> +#define __SYSCALL(nr, entry) [nr] = (void *) entry,
>  #include <asm/syscall_table_spu.h>
>  };
>  
>  long spu_sys_callback(struct spu_syscall_block *s)
>  {
> -	long (*syscall)(u64 a1, u64 a2, u64 a3, u64 a4, u64 a5, u64 a6);
> +	syscall_fn syscall;
>  
>  	if (s->nr_ret >= ARRAY_SIZE(spu_syscall_table)) {
>  		pr_debug("%s: invalid syscall #%lld", __func__, s->nr_ret);
> -- 
> 2.34.1

[PATCH 19/23] powerpc: Provide syscall wrapper

[Rohan McLure]

Implement syscall wrapper as per s390, x86, arm64. When enabled
cause handlers to accept parameters from a stack frame rather than
from user scratch register state. This allows for user registers to be
safely cleared in order to reduce caller influence on speculation
within syscall routine. The wrapper is a macro that emits syscall
handler symbols that call into the target handler, obtaining its
parameters from a struct pt_regs on the stack.

As registers are already saved to the stack prior to calling
system_call_exception, it appears that this function is executed more
efficiently with the new stack-pointer convention than with parameters
passed by registers, avoiding the allocation of a stack frame for this
method. On a 32-bit system, we see >20% performance increases on the
null_syscall microbenchmark, and on a Power 8 the performance gains
amortise the cost of clearing and restoring registers which is
implemented at the end of this series, seeing final result of ~5.6%
performance improvement on null_syscall.

Syscalls are wrapped in this fashion on all platforms except for the
Cell processor as this commit does not provide SPU support. This can be
quickly fixed in a successive patch, but requires spu_sys_callback to
allocate a pt_regs structure to satisfy the wrapped calling convention.

Co-developed-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
V1 -> V2: Generate prototypes for symbols produced by the wrapper.
V2 -> V3: Rebased to remove conflict with 1547db7d1f44
("powerpc: Move system_call_exception() to syscall.c"). Also remove copy
from gpr3 save slot on stackframe to orig_r3's slot. Fix whitespace with
preprocessor defines in system_call_exception.
V4 -> V5: Move systbl.c syscall wrapper support to this patch. Swap
calling convention for system_call_exception to be (&regs, r0)
---
 arch/powerpc/Kconfig                       |  1 +
 arch/powerpc/include/asm/interrupt.h       |  3 +-
 arch/powerpc/include/asm/syscall.h         |  4 +
 arch/powerpc/include/asm/syscall_wrapper.h | 84 ++++++++++++++++++++
 arch/powerpc/include/asm/syscalls.h        | 30 ++++++-
 arch/powerpc/kernel/entry_32.S             |  6 +-
 arch/powerpc/kernel/interrupt_64.S         | 28 +++++--
 arch/powerpc/kernel/syscall.c              | 31 +++-----
 arch/powerpc/kernel/systbl.c               |  8 ++
 arch/powerpc/kernel/vdso.c                 |  2 +
 10 files changed, 164 insertions(+), 33 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 4c466acdc70d..ef6c83e79c9b 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -137,6 +137,7 @@ config PPC
 	select ARCH_HAS_STRICT_KERNEL_RWX	if (PPC_BOOK3S || PPC_8xx || 40x) && !HIBERNATION
 	select ARCH_HAS_STRICT_KERNEL_RWX	if FSL_BOOKE && !HIBERNATION && !RANDOMIZE_BASE
 	select ARCH_HAS_STRICT_MODULE_RWX	if ARCH_HAS_STRICT_KERNEL_RWX
+	select ARCH_HAS_SYSCALL_WRAPPER		if !SPU_BASE
 	select ARCH_HAS_TICK_BROADCAST		if GENERIC_CLOCKEVENTS_BROADCAST
 	select ARCH_HAS_UACCESS_FLUSHCACHE
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
index 8069dbc4b8d1..48eec9cd1429 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -665,8 +665,7 @@ static inline void interrupt_cond_local_irq_enable(struct pt_regs *regs)
 		local_irq_enable();
 }
 
-long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8,
-			   unsigned long r0, struct pt_regs *regs);
+long system_call_exception(struct pt_regs *regs, unsigned long r0);
 notrace unsigned long syscall_exit_prepare(unsigned long r3, struct pt_regs *regs, long scv);
 notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs);
 notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs);
diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
index d2a8dfd5de33..3dd36c5e334a 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -14,8 +14,12 @@
 #include <linux/sched.h>
 #include <linux/thread_info.h>
 
+#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
+typedef long (*syscall_fn)(const struct pt_regs *);
+#else
 typedef long (*syscall_fn)(unsigned long, unsigned long, unsigned long,
 			   unsigned long, unsigned long, unsigned long);
+#endif
 
 /* ftrace syscalls requires exporting the sys_call_table */
 extern const syscall_fn sys_call_table[];
diff --git a/arch/powerpc/include/asm/syscall_wrapper.h b/arch/powerpc/include/asm/syscall_wrapper.h
new file mode 100644
index 000000000000..91bcfa40f740
--- /dev/null
+++ b/arch/powerpc/include/asm/syscall_wrapper.h
@@ -0,0 +1,84 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * syscall_wrapper.h - powerpc specific wrappers to syscall definitions
+ *
+ * Based on arch/{x86,arm64}/include/asm/syscall_wrapper.h
+ */
+
+#ifndef __ASM_SYSCALL_WRAPPER_H
+#define __ASM_SYSCALL_WRAPPER_H
+
+struct pt_regs;
+
+#define SC_POWERPC_REGS_TO_ARGS(x, ...)				\
+	__MAP(x,__SC_ARGS					\
+	      ,,regs->gpr[3],,regs->gpr[4],,regs->gpr[5]	\
+	      ,,regs->gpr[6],,regs->gpr[7],,regs->gpr[8])
+
+#ifdef CONFIG_COMPAT
+
+#define COMPAT_SYSCALL_DEFINEx(x, name, ...)						\
+	long __powerpc_compat_sys##name(const struct pt_regs *regs);			\
+	ALLOW_ERROR_INJECTION(__powerpc_compat_sys##name, ERRNO);			\
+	static long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));		\
+	static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));	\
+	long __powerpc_compat_sys##name(const struct pt_regs *regs)			\
+	{										\
+		return __se_compat_sys##name(SC_POWERPC_REGS_TO_ARGS(x,__VA_ARGS__));	\
+	}										\
+	static long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))		\
+	{										\
+		return __do_compat_sys##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__));	\
+	}										\
+	static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
+
+#define COMPAT_SYSCALL_DEFINE0(sname)						\
+	long __powerpc_compat_sys_##sname(const struct pt_regs *__unused);	\
+	ALLOW_ERROR_INJECTION(__powerpc_compat_sys_##sname, ERRNO);		\
+	long __powerpc_compat_sys_##sname(const struct pt_regs *__unused)
+
+#define COND_SYSCALL_COMPAT(name)						\
+	long __powerpc_compat_sys_##name(const struct pt_regs *regs);		\
+	long __weak __powerpc_compat_sys_##name(const struct pt_regs *regs)	\
+	{									\
+		return sys_ni_syscall();					\
+	}
+#define COMPAT_SYS_NI(name) \
+	SYSCALL_ALIAS(__powerpc_compat_sys_##name, sys_ni_posix_timers);
+
+#endif /* CONFIG_COMPAT */
+
+#define __SYSCALL_DEFINEx(x, name, ...)						\
+	long __powerpc_sys##name(const struct pt_regs *regs);			\
+	ALLOW_ERROR_INJECTION(__powerpc_sys##name, ERRNO);			\
+	static long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));		\
+	static inline long __do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));	\
+	long __powerpc_sys##name(const struct pt_regs *regs)			\
+	{									\
+		return __se_sys##name(SC_POWERPC_REGS_TO_ARGS(x,__VA_ARGS__));	\
+	}									\
+	static long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))		\
+	{									\
+		long ret = __do_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));	\
+		__MAP(x,__SC_TEST,__VA_ARGS__);					\
+		__PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__));		\
+		return ret;							\
+	}									\
+	static inline long __do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
+
+#define SYSCALL_DEFINE0(sname)							\
+	SYSCALL_METADATA(_##sname, 0);						\
+	long __powerpc_sys_##sname(const struct pt_regs *__unused);		\
+	ALLOW_ERROR_INJECTION(__powerpc_sys_##sname, ERRNO);			\
+	long __powerpc_sys_##sname(const struct pt_regs *__unused)
+
+#define COND_SYSCALL(name)							\
+	long __powerpc_sys_##name(const struct pt_regs *regs);			\
+	long __weak __powerpc_sys_##name(const struct pt_regs *regs)		\
+	{									\
+		return sys_ni_syscall();					\
+	}
+
+#define SYS_NI(name) SYSCALL_ALIAS(__powerpc_sys_##name, sys_ni_posix_timers);
+
+#endif /* __ASM_SYSCALL_WRAPPER_H */
diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index cc87168d6ecb..1ecdf6c071f6 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -15,6 +15,12 @@
 #include <asm/unistd.h>
 #include <asm/ucontext.h>
 
+#ifndef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
+long sys_ni_syscall(void);
+#else
+long sys_ni_syscall(const struct pt_regs *regs);
+#endif
+
 struct rtas_args;
 
 /*
@@ -29,12 +35,12 @@ struct rtas_args;
 #define merge_64(high, low) ((u64)high << 32) | low
 #endif
 
-long sys_ni_syscall(void);
-
 /*
  * PowerPC architecture-specific syscalls
  */
 
+#ifndef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
+
 long sys_rtas(struct rtas_args __user *uargs);
 
 #ifdef CONFIG_PPC64
@@ -114,5 +120,25 @@ long sys_ppc_fadvise64_64(int fd, int advice,
 			  u32 len_high, u32 len_low);
 #endif
 
+#else
+
+#define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, native)
+#define __SYSCALL(nr, entry) \
+	long __powerpc_##entry(const struct pt_regs *regs);
+
+#ifdef CONFIG_PPC64
+#include <asm/syscall_table_64.h>
+#else
+#include <asm/syscall_table_32.h>
+#endif /* CONFIG_PPC64 */
+
+#ifdef CONFIG_COMPAT
+#undef __SYSCALL_WITH_COMPAT
+#define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, compat)
+#include <asm/syscall_table_32.h>
+#endif /* CONFIG_COMPAT */
+
+#endif /* CONFIG_ARCH_HAS_SYSCALL_WRAPPER */
+
 #endif /* __KERNEL__ */
 #endif /* __ASM_POWERPC_SYSCALLS_H */
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index e4b694cebc44..96782aa72083 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -122,9 +122,9 @@ transfer_to_syscall:
 	SAVE_NVGPRS(r1)
 	kuep_lock
 
-	/* Calling convention has r9 = orig r0, r10 = regs */
-	addi	r10,r1,STACK_FRAME_OVERHEAD
-	mr	r9,r0
+	/* Calling convention has r3 = regs, r4 = orig r0 */
+	addi	r3,r1,STACK_FRAME_OVERHEAD
+	mr	r4,r0
 	bl	system_call_exception
 
 ret_from_syscall:
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 7d92a7a54727..16a1b44088e7 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -87,9 +87,11 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name)
 	std	r11,_TRAP(r1)
 	std	r12,_CCR(r1)
 	std	r3,ORIG_GPR3(r1)
-	addi	r10,r1,STACK_FRAME_OVERHEAD
+	/* Calling convention has r3 = regs, r4 = orig r0 */
+	addi	r3,r1,STACK_FRAME_OVERHEAD
+	mr	r4,r0
 	ld	r11,exception_marker@toc(r2)
-	std	r11,-16(r10)		/* "regshere" marker */
+	std	r11,-16(r3)		/* "regshere" marker */
 
 BEGIN_FTR_SECTION
 	HMT_MEDIUM
@@ -104,8 +106,12 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	 * but this is the best we can do.
 	 */
 
-	/* Calling convention has r9 = orig r0, r10 = regs */
-	mr	r9,r0
+	/*
+	 * Zero user registers to prevent influencing speculative execution
+	 * state of kernel code.
+	 */
+	ZEROIZE_GPRS(5, 12)
+	ZEROIZE_NVGPRS()
 	bl	system_call_exception
 
 .Lsyscall_vectored_\name\()_exit:
@@ -260,9 +266,11 @@ END_BTB_FLUSH_SECTION
 	std	r11,_TRAP(r1)
 	std	r12,_CCR(r1)
 	std	r3,ORIG_GPR3(r1)
-	addi	r10,r1,STACK_FRAME_OVERHEAD
+	/* Calling convention has r3 = regs, r4 = orig r0 */
+	addi	r3,r1,STACK_FRAME_OVERHEAD
+	mr	r4,r0
 	ld	r11,exception_marker@toc(r2)
-	std	r11,-16(r10)		/* "regshere" marker */
+	std	r11,-16(r3)		/* "regshere" marker */
 
 #ifdef CONFIG_PPC_BOOK3S
 	li	r11,1
@@ -283,8 +291,12 @@ END_BTB_FLUSH_SECTION
 	wrteei	1
 #endif
 
-	/* Calling convention has r9 = orig r0, r10 = regs */
-	mr	r9,r0
+	/*
+	 * Zero user registers to prevent influencing speculative execution
+	 * state of kernel code.
+	 */
+	ZEROIZE_GPRS(5, 12)
+	ZEROIZE_NVGPRS()
 	bl	system_call_exception
 
 .Lsyscall_exit:
diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
index 64102a64fd84..2f4dd7f0d819 100644
--- a/arch/powerpc/kernel/syscall.c
+++ b/arch/powerpc/kernel/syscall.c
@@ -12,12 +12,8 @@
 #include <asm/unistd.h>
 
 
-typedef long (*syscall_fn)(long, long, long, long, long, long);
-
 /* Has to run notrace because it is entered not completely "reconciled" */
-notrace long system_call_exception(long r3, long r4, long r5,
-				   long r6, long r7, long r8,
-				   unsigned long r0, struct pt_regs *regs)
+notrace long system_call_exception(struct pt_regs *regs, unsigned long r0)
 {
 	long ret;
 	syscall_fn f;
@@ -138,12 +134,6 @@ notrace long system_call_exception(long r3, long r4, long r5,
 		r0 = do_syscall_trace_enter(regs);
 		if (unlikely(r0 >= NR_syscalls))
 			return regs->gpr[3];
-		r3 = regs->gpr[3];
-		r4 = regs->gpr[4];
-		r5 = regs->gpr[5];
-		r6 = regs->gpr[6];
-		r7 = regs->gpr[7];
-		r8 = regs->gpr[8];
 
 	} else if (unlikely(r0 >= NR_syscalls)) {
 		if (unlikely(trap_is_unsupported_scv(regs))) {
@@ -160,18 +150,23 @@ notrace long system_call_exception(long r3, long r4, long r5,
 	if (unlikely(is_compat_task())) {
 		f = (void *)compat_sys_call_table[r0];
 
-		r3 &= 0x00000000ffffffffULL;
-		r4 &= 0x00000000ffffffffULL;
-		r5 &= 0x00000000ffffffffULL;
-		r6 &= 0x00000000ffffffffULL;
-		r7 &= 0x00000000ffffffffULL;
-		r8 &= 0x00000000ffffffffULL;
+		regs->gpr[3] &= 0x00000000ffffffffULL;
+		regs->gpr[4] &= 0x00000000ffffffffULL;
+		regs->gpr[5] &= 0x00000000ffffffffULL;
+		regs->gpr[6] &= 0x00000000ffffffffULL;
+		regs->gpr[7] &= 0x00000000ffffffffULL;
+		regs->gpr[8] &= 0x00000000ffffffffULL;
 
 	} else {
 		f = (void *)sys_call_table[r0];
 	}
 
-	ret = f(r3, r4, r5, r6, r7, r8);
+#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
+	ret = f(regs);
+#else
+	ret = f(regs->gpr[3], regs->gpr[4], regs->gpr[5],
+		regs->gpr[6], regs->gpr[7], regs->gpr[8]);
+#endif
 
 	/*
 	 * Ultimately, this value will get limited by KSTACK_OFFSET_MAX(),
diff --git a/arch/powerpc/kernel/systbl.c b/arch/powerpc/kernel/systbl.c
index e5d419822b4e..cb05e302ea58 100644
--- a/arch/powerpc/kernel/systbl.c
+++ b/arch/powerpc/kernel/systbl.c
@@ -15,8 +15,16 @@
 #include <asm/unistd.h>
 #include <asm/syscalls.h>
 
+#undef __SYSCALL_WITH_COMPAT
 #define __SYSCALL_WITH_COMPAT(nr, entry, compat) __SYSCALL(nr, entry)
+
+#undef __SYSCALL
+#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
+#define __SYSCALL(nr, entry) [nr] = __powerpc_##entry,
+#define __powerpc_sys_ni_syscall	sys_ni_syscall
+#else
 #define __SYSCALL(nr, entry) [nr] = (void *) entry,
+#endif
 
 const syscall_fn sys_call_table[] = {
 #ifdef CONFIG_PPC64
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index fcca06d200d3..e1f36fd61db3 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -39,6 +39,8 @@
 extern char vdso32_start, vdso32_end;
 extern char vdso64_start, vdso64_end;
 
+long sys_ni_syscall(void);
+
 /*
  * The vdso data page (aka. systemcfg for old ppc64 fans) is here.
  * Once the early boot kernel code no longer needs to muck around
-- 
2.34.1

Re: [PATCH 19/23] powerpc: Provide syscall wrapper

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> Implement syscall wrapper as per s390, x86, arm64. When enabled
> cause handlers to accept parameters from a stack frame rather than
> from user scratch register state. This allows for user registers to be
> safely cleared in order to reduce caller influence on speculation
> within syscall routine. The wrapper is a macro that emits syscall
> handler symbols that call into the target handler, obtaining its
> parameters from a struct pt_regs on the stack.
>
> As registers are already saved to the stack prior to calling
> system_call_exception, it appears that this function is executed more
> efficiently with the new stack-pointer convention than with parameters
> passed by registers, avoiding the allocation of a stack frame for this
> method. On a 32-bit system, we see >20% performance increases on the
> null_syscall microbenchmark, and on a Power 8 the performance gains
> amortise the cost of clearing and restoring registers which is
> implemented at the end of this series, seeing final result of ~5.6%
> performance improvement on null_syscall.
>
> Syscalls are wrapped in this fashion on all platforms except for the
> Cell processor as this commit does not provide SPU support. This can be
> quickly fixed in a successive patch, but requires spu_sys_callback to
> allocate a pt_regs structure to satisfy the wrapped calling convention.
>
> Co-developed-by: Andrew Donnellan <ajd@linux.ibm.com>
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> V1 -> V2: Generate prototypes for symbols produced by the wrapper.
> V2 -> V3: Rebased to remove conflict with 1547db7d1f44
> ("powerpc: Move system_call_exception() to syscall.c"). Also remove copy
> from gpr3 save slot on stackframe to orig_r3's slot. Fix whitespace with
> preprocessor defines in system_call_exception.
> V4 -> V5: Move systbl.c syscall wrapper support to this patch. Swap
> calling convention for system_call_exception to be (&regs, r0)
> ---
>  arch/powerpc/Kconfig                       |  1 +
>  arch/powerpc/include/asm/interrupt.h       |  3 +-
>  arch/powerpc/include/asm/syscall.h         |  4 +
>  arch/powerpc/include/asm/syscall_wrapper.h | 84 ++++++++++++++++++++
>  arch/powerpc/include/asm/syscalls.h        | 30 ++++++-
>  arch/powerpc/kernel/entry_32.S             |  6 +-
>  arch/powerpc/kernel/interrupt_64.S         | 28 +++++--
>  arch/powerpc/kernel/syscall.c              | 31 +++-----

Ah, this is where it went :)

I wouldn't normally mind except the previous patch might break the
build because it uses the same name, will it?

This *could* be two patches, one to change system_call_exception API,
the next to add the wrapper.

>  arch/powerpc/kernel/systbl.c               |  8 ++
>  arch/powerpc/kernel/vdso.c                 |  2 +
>  10 files changed, 164 insertions(+), 33 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 4c466acdc70d..ef6c83e79c9b 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -137,6 +137,7 @@ config PPC
>  	select ARCH_HAS_STRICT_KERNEL_RWX	if (PPC_BOOK3S || PPC_8xx || 40x) && !HIBERNATION
>  	select ARCH_HAS_STRICT_KERNEL_RWX	if FSL_BOOKE && !HIBERNATION && !RANDOMIZE_BASE
>  	select ARCH_HAS_STRICT_MODULE_RWX	if ARCH_HAS_STRICT_KERNEL_RWX
> +	select ARCH_HAS_SYSCALL_WRAPPER		if !SPU_BASE
>  	select ARCH_HAS_TICK_BROADCAST		if GENERIC_CLOCKEVENTS_BROADCAST
>  	select ARCH_HAS_UACCESS_FLUSHCACHE
>  	select ARCH_HAS_UBSAN_SANITIZE_ALL
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index 8069dbc4b8d1..48eec9cd1429 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -665,8 +665,7 @@ static inline void interrupt_cond_local_irq_enable(struct pt_regs *regs)
>  		local_irq_enable();
>  }
>  
> -long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8,
> -			   unsigned long r0, struct pt_regs *regs);
> +long system_call_exception(struct pt_regs *regs, unsigned long r0);
>  notrace unsigned long syscall_exit_prepare(unsigned long r3, struct pt_regs *regs, long scv);
>  notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs);
>  notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs);
> diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
> index d2a8dfd5de33..3dd36c5e334a 100644
> --- a/arch/powerpc/include/asm/syscall.h
> +++ b/arch/powerpc/include/asm/syscall.h
> @@ -14,8 +14,12 @@
>  #include <linux/sched.h>
>  #include <linux/thread_info.h>
>  
> +#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
> +typedef long (*syscall_fn)(const struct pt_regs *);
> +#else
>  typedef long (*syscall_fn)(unsigned long, unsigned long, unsigned long,
>  			   unsigned long, unsigned long, unsigned long);
> +#endif
>  
>  /* ftrace syscalls requires exporting the sys_call_table */
>  extern const syscall_fn sys_call_table[];
> diff --git a/arch/powerpc/include/asm/syscall_wrapper.h b/arch/powerpc/include/asm/syscall_wrapper.h
> new file mode 100644
> index 000000000000..91bcfa40f740
> --- /dev/null
> +++ b/arch/powerpc/include/asm/syscall_wrapper.h
> @@ -0,0 +1,84 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * syscall_wrapper.h - powerpc specific wrappers to syscall definitions
> + *
> + * Based on arch/{x86,arm64}/include/asm/syscall_wrapper.h
> + */
> +
> +#ifndef __ASM_SYSCALL_WRAPPER_H
> +#define __ASM_SYSCALL_WRAPPER_H
> +
> +struct pt_regs;
> +
> +#define SC_POWERPC_REGS_TO_ARGS(x, ...)				\
> +	__MAP(x,__SC_ARGS					\
> +	      ,,regs->gpr[3],,regs->gpr[4],,regs->gpr[5]	\
> +	      ,,regs->gpr[6],,regs->gpr[7],,regs->gpr[8])
> +
> +#ifdef CONFIG_COMPAT
> +
> +#define COMPAT_SYSCALL_DEFINEx(x, name, ...)						\
> +	long __powerpc_compat_sys##name(const struct pt_regs *regs);			\
> +	ALLOW_ERROR_INJECTION(__powerpc_compat_sys##name, ERRNO);			\
> +	static long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));		\
> +	static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));	\
> +	long __powerpc_compat_sys##name(const struct pt_regs *regs)			\
> +	{										\
> +		return __se_compat_sys##name(SC_POWERPC_REGS_TO_ARGS(x,__VA_ARGS__));	\
> +	}										\
> +	static long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))		\
> +	{										\
> +		return __do_compat_sys##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__));	\
> +	}										\
> +	static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
> +
> +#define COMPAT_SYSCALL_DEFINE0(sname)						\
> +	long __powerpc_compat_sys_##sname(const struct pt_regs *__unused);	\
> +	ALLOW_ERROR_INJECTION(__powerpc_compat_sys_##sname, ERRNO);		\
> +	long __powerpc_compat_sys_##sname(const struct pt_regs *__unused)
> +
> +#define COND_SYSCALL_COMPAT(name)						\
> +	long __powerpc_compat_sys_##name(const struct pt_regs *regs);		\
> +	long __weak __powerpc_compat_sys_##name(const struct pt_regs *regs)	\
> +	{									\
> +		return sys_ni_syscall();					\
> +	}
> +#define COMPAT_SYS_NI(name) \
> +	SYSCALL_ALIAS(__powerpc_compat_sys_##name, sys_ni_posix_timers);
> +
> +#endif /* CONFIG_COMPAT */
> +
> +#define __SYSCALL_DEFINEx(x, name, ...)						\
> +	long __powerpc_sys##name(const struct pt_regs *regs);			\
> +	ALLOW_ERROR_INJECTION(__powerpc_sys##name, ERRNO);			\
> +	static long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));		\
> +	static inline long __do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));	\
> +	long __powerpc_sys##name(const struct pt_regs *regs)			\
> +	{									\
> +		return __se_sys##name(SC_POWERPC_REGS_TO_ARGS(x,__VA_ARGS__));	\
> +	}									\
> +	static long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))		\
> +	{									\
> +		long ret = __do_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));	\
> +		__MAP(x,__SC_TEST,__VA_ARGS__);					\
> +		__PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__));		\
> +		return ret;							\
> +	}									\
> +	static inline long __do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
> +
> +#define SYSCALL_DEFINE0(sname)							\
> +	SYSCALL_METADATA(_##sname, 0);						\
> +	long __powerpc_sys_##sname(const struct pt_regs *__unused);		\
> +	ALLOW_ERROR_INJECTION(__powerpc_sys_##sname, ERRNO);			\
> +	long __powerpc_sys_##sname(const struct pt_regs *__unused)
> +
> +#define COND_SYSCALL(name)							\
> +	long __powerpc_sys_##name(const struct pt_regs *regs);			\
> +	long __weak __powerpc_sys_##name(const struct pt_regs *regs)		\
> +	{									\
> +		return sys_ni_syscall();					\
> +	}
> +
> +#define SYS_NI(name) SYSCALL_ALIAS(__powerpc_sys_##name, sys_ni_posix_timers);
> +
> +#endif /* __ASM_SYSCALL_WRAPPER_H */
> diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
> index cc87168d6ecb..1ecdf6c071f6 100644
> --- a/arch/powerpc/include/asm/syscalls.h
> +++ b/arch/powerpc/include/asm/syscalls.h
> @@ -15,6 +15,12 @@
>  #include <asm/unistd.h>
>  #include <asm/ucontext.h>
>  
> +#ifndef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
> +long sys_ni_syscall(void);
> +#else
> +long sys_ni_syscall(const struct pt_regs *regs);
> +#endif
> +
>  struct rtas_args;
>  
>  /*
> @@ -29,12 +35,12 @@ struct rtas_args;
>  #define merge_64(high, low) ((u64)high << 32) | low
>  #endif
>  
> -long sys_ni_syscall(void);
> -
>  /*
>   * PowerPC architecture-specific syscalls
>   */
>  
> +#ifndef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
> +
>  long sys_rtas(struct rtas_args __user *uargs);
>  
>  #ifdef CONFIG_PPC64
> @@ -114,5 +120,25 @@ long sys_ppc_fadvise64_64(int fd, int advice,
>  			  u32 len_high, u32 len_low);
>  #endif
>  
> +#else
> +
> +#define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, native)
> +#define __SYSCALL(nr, entry) \
> +	long __powerpc_##entry(const struct pt_regs *regs);
> +
> +#ifdef CONFIG_PPC64
> +#include <asm/syscall_table_64.h>
> +#else
> +#include <asm/syscall_table_32.h>
> +#endif /* CONFIG_PPC64 */
> +
> +#ifdef CONFIG_COMPAT
> +#undef __SYSCALL_WITH_COMPAT
> +#define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, compat)
> +#include <asm/syscall_table_32.h>
> +#endif /* CONFIG_COMPAT */
> +
> +#endif /* CONFIG_ARCH_HAS_SYSCALL_WRAPPER */
> +
>  #endif /* __KERNEL__ */
>  #endif /* __ASM_POWERPC_SYSCALLS_H */
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index e4b694cebc44..96782aa72083 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -122,9 +122,9 @@ transfer_to_syscall:
>  	SAVE_NVGPRS(r1)
>  	kuep_lock
>  
> -	/* Calling convention has r9 = orig r0, r10 = regs */
> -	addi	r10,r1,STACK_FRAME_OVERHEAD
> -	mr	r9,r0
> +	/* Calling convention has r3 = regs, r4 = orig r0 */
> +	addi	r3,r1,STACK_FRAME_OVERHEAD
> +	mr	r4,r0
>  	bl	system_call_exception
>  
>  ret_from_syscall:
> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
> index 7d92a7a54727..16a1b44088e7 100644
> --- a/arch/powerpc/kernel/interrupt_64.S
> +++ b/arch/powerpc/kernel/interrupt_64.S
> @@ -87,9 +87,11 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name)
>  	std	r11,_TRAP(r1)
>  	std	r12,_CCR(r1)
>  	std	r3,ORIG_GPR3(r1)
> -	addi	r10,r1,STACK_FRAME_OVERHEAD
> +	/* Calling convention has r3 = regs, r4 = orig r0 */
> +	addi	r3,r1,STACK_FRAME_OVERHEAD
> +	mr	r4,r0
>  	ld	r11,exception_marker@toc(r2)
> -	std	r11,-16(r10)		/* "regshere" marker */
> +	std	r11,-16(r3)		/* "regshere" marker */
>  
>  BEGIN_FTR_SECTION
>  	HMT_MEDIUM

I think the asm adjustments look okay. Should get a Christophe ack for
the 32-bit at least though :)

> @@ -104,8 +106,12 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  	 * but this is the best we can do.
>  	 */
>  
> -	/* Calling convention has r9 = orig r0, r10 = regs */
> -	mr	r9,r0
> +	/*
> +	 * Zero user registers to prevent influencing speculative execution
> +	 * state of kernel code.
> +	 */
> +	ZEROIZE_GPRS(5, 12)
> +	ZEROIZE_NVGPRS()
>  	bl	system_call_exception
>  
>  .Lsyscall_vectored_\name\()_exit:

Did this and the below hunk belong in a later patch?

> @@ -260,9 +266,11 @@ END_BTB_FLUSH_SECTION
>  	std	r11,_TRAP(r1)
>  	std	r12,_CCR(r1)
>  	std	r3,ORIG_GPR3(r1)
> -	addi	r10,r1,STACK_FRAME_OVERHEAD
> +	/* Calling convention has r3 = regs, r4 = orig r0 */
> +	addi	r3,r1,STACK_FRAME_OVERHEAD
> +	mr	r4,r0
>  	ld	r11,exception_marker@toc(r2)
> -	std	r11,-16(r10)		/* "regshere" marker */
> +	std	r11,-16(r3)		/* "regshere" marker */
>  
>  #ifdef CONFIG_PPC_BOOK3S
>  	li	r11,1
> @@ -283,8 +291,12 @@ END_BTB_FLUSH_SECTION
>  	wrteei	1
>  #endif
>  
> -	/* Calling convention has r9 = orig r0, r10 = regs */
> -	mr	r9,r0
> +	/*
> +	 * Zero user registers to prevent influencing speculative execution
> +	 * state of kernel code.
> +	 */
> +	ZEROIZE_GPRS(5, 12)
> +	ZEROIZE_NVGPRS()
>  	bl	system_call_exception
>  
>  .Lsyscall_exit:
> diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
> index 64102a64fd84..2f4dd7f0d819 100644
> --- a/arch/powerpc/kernel/syscall.c
> +++ b/arch/powerpc/kernel/syscall.c
> @@ -12,12 +12,8 @@
>  #include <asm/unistd.h>
>  
>  
> -typedef long (*syscall_fn)(long, long, long, long, long, long);
> -
>  /* Has to run notrace because it is entered not completely "reconciled" */
> -notrace long system_call_exception(long r3, long r4, long r5,
> -				   long r6, long r7, long r8,
> -				   unsigned long r0, struct pt_regs *regs)
> +notrace long system_call_exception(struct pt_regs *regs, unsigned long r0)
>  {
>  	long ret;
>  	syscall_fn f;
> @@ -138,12 +134,6 @@ notrace long system_call_exception(long r3, long r4, long r5,
>  		r0 = do_syscall_trace_enter(regs);
>  		if (unlikely(r0 >= NR_syscalls))
>  			return regs->gpr[3];
> -		r3 = regs->gpr[3];
> -		r4 = regs->gpr[4];
> -		r5 = regs->gpr[5];
> -		r6 = regs->gpr[6];
> -		r7 = regs->gpr[7];
> -		r8 = regs->gpr[8];
>  
>  	} else if (unlikely(r0 >= NR_syscalls)) {
>  		if (unlikely(trap_is_unsupported_scv(regs))) {
> @@ -160,18 +150,23 @@ notrace long system_call_exception(long r3, long r4, long r5,
>  	if (unlikely(is_compat_task())) {
>  		f = (void *)compat_sys_call_table[r0];
>  
> -		r3 &= 0x00000000ffffffffULL;
> -		r4 &= 0x00000000ffffffffULL;
> -		r5 &= 0x00000000ffffffffULL;
> -		r6 &= 0x00000000ffffffffULL;
> -		r7 &= 0x00000000ffffffffULL;
> -		r8 &= 0x00000000ffffffffULL;
> +		regs->gpr[3] &= 0x00000000ffffffffULL;
> +		regs->gpr[4] &= 0x00000000ffffffffULL;
> +		regs->gpr[5] &= 0x00000000ffffffffULL;
> +		regs->gpr[6] &= 0x00000000ffffffffULL;
> +		regs->gpr[7] &= 0x00000000ffffffffULL;
> +		regs->gpr[8] &= 0x00000000ffffffffULL;

This (possibly) changes regs->gpr values on the interrupt stack frame.

I suppose that's going to be okay.  Aside from debugging and tracing
stuff, could it cause slight differences to how signals are delivered?

Why do we actually do this anyway? Could we get rid of it (or relegate
it to !WRAPPER case)?

Thanks,
Nick

>  
>  	} else {
>  		f = (void *)sys_call_table[r0];
>  	}
>  
> -	ret = f(r3, r4, r5, r6, r7, r8);
> +#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
> +	ret = f(regs);
> +#else
> +	ret = f(regs->gpr[3], regs->gpr[4], regs->gpr[5],
> +		regs->gpr[6], regs->gpr[7], regs->gpr[8]);
> +#endif
>  
>  	/*
>  	 * Ultimately, this value will get limited by KSTACK_OFFSET_MAX(),
> diff --git a/arch/powerpc/kernel/systbl.c b/arch/powerpc/kernel/systbl.c
> index e5d419822b4e..cb05e302ea58 100644
> --- a/arch/powerpc/kernel/systbl.c
> +++ b/arch/powerpc/kernel/systbl.c
> @@ -15,8 +15,16 @@
>  #include <asm/unistd.h>
>  #include <asm/syscalls.h>
>  
> +#undef __SYSCALL_WITH_COMPAT
>  #define __SYSCALL_WITH_COMPAT(nr, entry, compat) __SYSCALL(nr, entry)
> +
> +#undef __SYSCALL
> +#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
> +#define __SYSCALL(nr, entry) [nr] = __powerpc_##entry,
> +#define __powerpc_sys_ni_syscall	sys_ni_syscall
> +#else
>  #define __SYSCALL(nr, entry) [nr] = (void *) entry,
> +#endif
>  
>  const syscall_fn sys_call_table[] = {
>  #ifdef CONFIG_PPC64
> diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
> index fcca06d200d3..e1f36fd61db3 100644
> --- a/arch/powerpc/kernel/vdso.c
> +++ b/arch/powerpc/kernel/vdso.c
> @@ -39,6 +39,8 @@
>  extern char vdso32_start, vdso32_end;
>  extern char vdso64_start, vdso64_end;
>  
> +long sys_ni_syscall(void);
> +
>  /*
>   * The vdso data page (aka. systemcfg for old ppc64 fans) is here.
>   * Once the early boot kernel code no longer needs to muck around
> -- 
> 2.34.1

Re: [PATCH 19/23] powerpc: Provide syscall wrapper

[Rohan McLure]

> On 20 Sep 2022, at 11:59 am, Nicholas Piggin <npiggin@gmail.com> wrote:
> 
> On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
>> Implement syscall wrapper as per s390, x86, arm64. When enabled
>> cause handlers to accept parameters from a stack frame rather than
>> from user scratch register state. This allows for user registers to be
>> safely cleared in order to reduce caller influence on speculation
>> within syscall routine. The wrapper is a macro that emits syscall
>> handler symbols that call into the target handler, obtaining its
>> parameters from a struct pt_regs on the stack.
>> 
>> As registers are already saved to the stack prior to calling
>> system_call_exception, it appears that this function is executed more
>> efficiently with the new stack-pointer convention than with parameters
>> passed by registers, avoiding the allocation of a stack frame for this
>> method. On a 32-bit system, we see >20% performance increases on the
>> null_syscall microbenchmark, and on a Power 8 the performance gains
>> amortise the cost of clearing and restoring registers which is
>> implemented at the end of this series, seeing final result of ~5.6%
>> performance improvement on null_syscall.
>> 
>> Syscalls are wrapped in this fashion on all platforms except for the
>> Cell processor as this commit does not provide SPU support. This can be
>> quickly fixed in a successive patch, but requires spu_sys_callback to
>> allocate a pt_regs structure to satisfy the wrapped calling convention.
>> 
>> Co-developed-by: Andrew Donnellan <ajd@linux.ibm.com>
>> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
>> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
>> ---
>> V1 -> V2: Generate prototypes for symbols produced by the wrapper.
>> V2 -> V3: Rebased to remove conflict with 1547db7d1f44
>> ("powerpc: Move system_call_exception() to syscall.c"). Also remove copy
>> from gpr3 save slot on stackframe to orig_r3's slot. Fix whitespace with
>> preprocessor defines in system_call_exception.
>> V4 -> V5: Move systbl.c syscall wrapper support to this patch. Swap
>> calling convention for system_call_exception to be (&regs, r0)
>> ---
>> arch/powerpc/Kconfig                       |  1 +
>> arch/powerpc/include/asm/interrupt.h       |  3 +-
>> arch/powerpc/include/asm/syscall.h         |  4 +
>> arch/powerpc/include/asm/syscall_wrapper.h | 84 ++++++++++++++++++++
>> arch/powerpc/include/asm/syscalls.h        | 30 ++++++-
>> arch/powerpc/kernel/entry_32.S             |  6 +-
>> arch/powerpc/kernel/interrupt_64.S         | 28 +++++--
>> arch/powerpc/kernel/syscall.c              | 31 +++-----
> 
> Ah, this is where it went :)
> 
> I wouldn't normally mind except the previous patch might break the
> build because it uses the same name, will it?
> 
> This *could* be two patches, one to change system_call_exception API,
> the next to add the wrapper.
> 
>> arch/powerpc/kernel/systbl.c               |  8 ++
>> arch/powerpc/kernel/vdso.c                 |  2 +
>> 10 files changed, 164 insertions(+), 33 deletions(-)
>> 
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 4c466acdc70d..ef6c83e79c9b 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -137,6 +137,7 @@ config PPC
>> 	select ARCH_HAS_STRICT_KERNEL_RWX	if (PPC_BOOK3S || PPC_8xx || 40x) && !HIBERNATION
>> 	select ARCH_HAS_STRICT_KERNEL_RWX	if FSL_BOOKE && !HIBERNATION && !RANDOMIZE_BASE
>> 	select ARCH_HAS_STRICT_MODULE_RWX	if ARCH_HAS_STRICT_KERNEL_RWX
>> +	select ARCH_HAS_SYSCALL_WRAPPER		if !SPU_BASE
>> 	select ARCH_HAS_TICK_BROADCAST		if GENERIC_CLOCKEVENTS_BROADCAST
>> 	select ARCH_HAS_UACCESS_FLUSHCACHE
>> 	select ARCH_HAS_UBSAN_SANITIZE_ALL
>> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
>> index 8069dbc4b8d1..48eec9cd1429 100644
>> --- a/arch/powerpc/include/asm/interrupt.h
>> +++ b/arch/powerpc/include/asm/interrupt.h
>> @@ -665,8 +665,7 @@ static inline void interrupt_cond_local_irq_enable(struct pt_regs *regs)
>> 		local_irq_enable();
>> }
>> 
>> -long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8,
>> -			   unsigned long r0, struct pt_regs *regs);
>> +long system_call_exception(struct pt_regs *regs, unsigned long r0);
>> notrace unsigned long syscall_exit_prepare(unsigned long r3, struct pt_regs *regs, long scv);
>> notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs);
>> notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs);
>> diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
>> index d2a8dfd5de33..3dd36c5e334a 100644
>> --- a/arch/powerpc/include/asm/syscall.h
>> +++ b/arch/powerpc/include/asm/syscall.h
>> @@ -14,8 +14,12 @@
>> #include <linux/sched.h>
>> #include <linux/thread_info.h>
>> 
>> +#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
>> +typedef long (*syscall_fn)(const struct pt_regs *);
>> +#else
>> typedef long (*syscall_fn)(unsigned long, unsigned long, unsigned long,
>> 			   unsigned long, unsigned long, unsigned long);
>> +#endif
>> 
>> /* ftrace syscalls requires exporting the sys_call_table */
>> extern const syscall_fn sys_call_table[];
>> diff --git a/arch/powerpc/include/asm/syscall_wrapper.h b/arch/powerpc/include/asm/syscall_wrapper.h
>> new file mode 100644
>> index 000000000000..91bcfa40f740
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/syscall_wrapper.h
>> @@ -0,0 +1,84 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * syscall_wrapper.h - powerpc specific wrappers to syscall definitions
>> + *
>> + * Based on arch/{x86,arm64}/include/asm/syscall_wrapper.h
>> + */
>> +
>> +#ifndef __ASM_SYSCALL_WRAPPER_H
>> +#define __ASM_SYSCALL_WRAPPER_H
>> +
>> +struct pt_regs;
>> +
>> +#define SC_POWERPC_REGS_TO_ARGS(x, ...)				\
>> +	__MAP(x,__SC_ARGS					\
>> +	      ,,regs->gpr[3],,regs->gpr[4],,regs->gpr[5]	\
>> +	      ,,regs->gpr[6],,regs->gpr[7],,regs->gpr[8])
>> +
>> +#ifdef CONFIG_COMPAT
>> +
>> +#define COMPAT_SYSCALL_DEFINEx(x, name, ...)						\
>> +	long __powerpc_compat_sys##name(const struct pt_regs *regs);			\
>> +	ALLOW_ERROR_INJECTION(__powerpc_compat_sys##name, ERRNO);			\
>> +	static long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));		\
>> +	static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));	\
>> +	long __powerpc_compat_sys##name(const struct pt_regs *regs)			\
>> +	{										\
>> +		return __se_compat_sys##name(SC_POWERPC_REGS_TO_ARGS(x,__VA_ARGS__));	\
>> +	}										\
>> +	static long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))		\
>> +	{										\
>> +		return __do_compat_sys##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__));	\
>> +	}										\
>> +	static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
>> +
>> +#define COMPAT_SYSCALL_DEFINE0(sname)						\
>> +	long __powerpc_compat_sys_##sname(const struct pt_regs *__unused);	\
>> +	ALLOW_ERROR_INJECTION(__powerpc_compat_sys_##sname, ERRNO);		\
>> +	long __powerpc_compat_sys_##sname(const struct pt_regs *__unused)
>> +
>> +#define COND_SYSCALL_COMPAT(name)						\
>> +	long __powerpc_compat_sys_##name(const struct pt_regs *regs);		\
>> +	long __weak __powerpc_compat_sys_##name(const struct pt_regs *regs)	\
>> +	{									\
>> +		return sys_ni_syscall();					\
>> +	}
>> +#define COMPAT_SYS_NI(name) \
>> +	SYSCALL_ALIAS(__powerpc_compat_sys_##name, sys_ni_posix_timers);
>> +
>> +#endif /* CONFIG_COMPAT */
>> +
>> +#define __SYSCALL_DEFINEx(x, name, ...)						\
>> +	long __powerpc_sys##name(const struct pt_regs *regs);			\
>> +	ALLOW_ERROR_INJECTION(__powerpc_sys##name, ERRNO);			\
>> +	static long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));		\
>> +	static inline long __do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));	\
>> +	long __powerpc_sys##name(const struct pt_regs *regs)			\
>> +	{									\
>> +		return __se_sys##name(SC_POWERPC_REGS_TO_ARGS(x,__VA_ARGS__));	\
>> +	}									\
>> +	static long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))		\
>> +	{									\
>> +		long ret = __do_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));	\
>> +		__MAP(x,__SC_TEST,__VA_ARGS__);					\
>> +		__PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__));		\
>> +		return ret;							\
>> +	}									\
>> +	static inline long __do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
>> +
>> +#define SYSCALL_DEFINE0(sname)							\
>> +	SYSCALL_METADATA(_##sname, 0);						\
>> +	long __powerpc_sys_##sname(const struct pt_regs *__unused);		\
>> +	ALLOW_ERROR_INJECTION(__powerpc_sys_##sname, ERRNO);			\
>> +	long __powerpc_sys_##sname(const struct pt_regs *__unused)
>> +
>> +#define COND_SYSCALL(name)							\
>> +	long __powerpc_sys_##name(const struct pt_regs *regs);			\
>> +	long __weak __powerpc_sys_##name(const struct pt_regs *regs)		\
>> +	{									\
>> +		return sys_ni_syscall();					\
>> +	}
>> +
>> +#define SYS_NI(name) SYSCALL_ALIAS(__powerpc_sys_##name, sys_ni_posix_timers);
>> +
>> +#endif /* __ASM_SYSCALL_WRAPPER_H */
>> diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
>> index cc87168d6ecb..1ecdf6c071f6 100644
>> --- a/arch/powerpc/include/asm/syscalls.h
>> +++ b/arch/powerpc/include/asm/syscalls.h
>> @@ -15,6 +15,12 @@
>> #include <asm/unistd.h>
>> #include <asm/ucontext.h>
>> 
>> +#ifndef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
>> +long sys_ni_syscall(void);
>> +#else
>> +long sys_ni_syscall(const struct pt_regs *regs);
>> +#endif
>> +
>> struct rtas_args;
>> 
>> /*
>> @@ -29,12 +35,12 @@ struct rtas_args;
>> #define merge_64(high, low) ((u64)high << 32) | low
>> #endif
>> 
>> -long sys_ni_syscall(void);
>> -
>> /*
>>  * PowerPC architecture-specific syscalls
>>  */
>> 
>> +#ifndef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
>> +
>> long sys_rtas(struct rtas_args __user *uargs);
>> 
>> #ifdef CONFIG_PPC64
>> @@ -114,5 +120,25 @@ long sys_ppc_fadvise64_64(int fd, int advice,
>> 			  u32 len_high, u32 len_low);
>> #endif
>> 
>> +#else
>> +
>> +#define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, native)
>> +#define __SYSCALL(nr, entry) \
>> +	long __powerpc_##entry(const struct pt_regs *regs);
>> +
>> +#ifdef CONFIG_PPC64
>> +#include <asm/syscall_table_64.h>
>> +#else
>> +#include <asm/syscall_table_32.h>
>> +#endif /* CONFIG_PPC64 */
>> +
>> +#ifdef CONFIG_COMPAT
>> +#undef __SYSCALL_WITH_COMPAT
>> +#define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, compat)
>> +#include <asm/syscall_table_32.h>
>> +#endif /* CONFIG_COMPAT */
>> +
>> +#endif /* CONFIG_ARCH_HAS_SYSCALL_WRAPPER */
>> +
>> #endif /* __KERNEL__ */
>> #endif /* __ASM_POWERPC_SYSCALLS_H */
>> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
>> index e4b694cebc44..96782aa72083 100644
>> --- a/arch/powerpc/kernel/entry_32.S
>> +++ b/arch/powerpc/kernel/entry_32.S
>> @@ -122,9 +122,9 @@ transfer_to_syscall:
>> 	SAVE_NVGPRS(r1)
>> 	kuep_lock
>> 
>> -	/* Calling convention has r9 = orig r0, r10 = regs */
>> -	addi	r10,r1,STACK_FRAME_OVERHEAD
>> -	mr	r9,r0
>> +	/* Calling convention has r3 = regs, r4 = orig r0 */
>> +	addi	r3,r1,STACK_FRAME_OVERHEAD
>> +	mr	r4,r0
>> 	bl	system_call_exception
>> 
>> ret_from_syscall:
>> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
>> index 7d92a7a54727..16a1b44088e7 100644
>> --- a/arch/powerpc/kernel/interrupt_64.S
>> +++ b/arch/powerpc/kernel/interrupt_64.S
>> @@ -87,9 +87,11 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name)
>> 	std	r11,_TRAP(r1)
>> 	std	r12,_CCR(r1)
>> 	std	r3,ORIG_GPR3(r1)
>> -	addi	r10,r1,STACK_FRAME_OVERHEAD
>> +	/* Calling convention has r3 = regs, r4 = orig r0 */
>> +	addi	r3,r1,STACK_FRAME_OVERHEAD
>> +	mr	r4,r0
>> 	ld	r11,exception_marker@toc(r2)
>> -	std	r11,-16(r10)		/* "regshere" marker */
>> +	std	r11,-16(r3)		/* "regshere" marker */
>> 
>> BEGIN_FTR_SECTION
>> 	HMT_MEDIUM
> 
> I think the asm adjustments look okay. Should get a Christophe ack for
> the 32-bit at least though :)
> 
>> @@ -104,8 +106,12 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>> 	 * but this is the best we can do.
>> 	 */
>> 
>> -	/* Calling convention has r9 = orig r0, r10 = regs */
>> -	mr	r9,r0
>> +	/*
>> +	 * Zero user registers to prevent influencing speculative execution
>> +	 * state of kernel code.
>> +	 */
>> +	ZEROIZE_GPRS(5, 12)
>> +	ZEROIZE_NVGPRS()
>> 	bl	system_call_exception
>> 
>> .Lsyscall_vectored_\name\()_exit:
> 
> Did this and the below hunk belong in a later patch?
> 
>> @@ -260,9 +266,11 @@ END_BTB_FLUSH_SECTION
>> 	std	r11,_TRAP(r1)
>> 	std	r12,_CCR(r1)
>> 	std	r3,ORIG_GPR3(r1)
>> -	addi	r10,r1,STACK_FRAME_OVERHEAD
>> +	/* Calling convention has r3 = regs, r4 = orig r0 */
>> +	addi	r3,r1,STACK_FRAME_OVERHEAD
>> +	mr	r4,r0
>> 	ld	r11,exception_marker@toc(r2)
>> -	std	r11,-16(r10)		/* "regshere" marker */
>> +	std	r11,-16(r3)		/* "regshere" marker */
>> 
>> #ifdef CONFIG_PPC_BOOK3S
>> 	li	r11,1
>> @@ -283,8 +291,12 @@ END_BTB_FLUSH_SECTION
>> 	wrteei	1
>> #endif
>> 
>> -	/* Calling convention has r9 = orig r0, r10 = regs */
>> -	mr	r9,r0
>> +	/*
>> +	 * Zero user registers to prevent influencing speculative execution
>> +	 * state of kernel code.
>> +	 */
>> +	ZEROIZE_GPRS(5, 12)
>> +	ZEROIZE_NVGPRS()
>> 	bl	system_call_exception
>> 
>> .Lsyscall_exit:
>> diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
>> index 64102a64fd84..2f4dd7f0d819 100644
>> --- a/arch/powerpc/kernel/syscall.c
>> +++ b/arch/powerpc/kernel/syscall.c
>> @@ -12,12 +12,8 @@
>> #include <asm/unistd.h>
>> 
>> 
>> -typedef long (*syscall_fn)(long, long, long, long, long, long);
>> -
>> /* Has to run notrace because it is entered not completely "reconciled" */
>> -notrace long system_call_exception(long r3, long r4, long r5,
>> -				   long r6, long r7, long r8,
>> -				   unsigned long r0, struct pt_regs *regs)
>> +notrace long system_call_exception(struct pt_regs *regs, unsigned long r0)
>> {
>> 	long ret;
>> 	syscall_fn f;
>> @@ -138,12 +134,6 @@ notrace long system_call_exception(long r3, long r4, long r5,
>> 		r0 = do_syscall_trace_enter(regs);
>> 		if (unlikely(r0 >= NR_syscalls))
>> 			return regs->gpr[3];
>> -		r3 = regs->gpr[3];
>> -		r4 = regs->gpr[4];
>> -		r5 = regs->gpr[5];
>> -		r6 = regs->gpr[6];
>> -		r7 = regs->gpr[7];
>> -		r8 = regs->gpr[8];
>> 
>> 	} else if (unlikely(r0 >= NR_syscalls)) {
>> 		if (unlikely(trap_is_unsupported_scv(regs))) {
>> @@ -160,18 +150,23 @@ notrace long system_call_exception(long r3, long r4, long r5,
>> 	if (unlikely(is_compat_task())) {
>> 		f = (void *)compat_sys_call_table[r0];
>> 
>> -		r3 &= 0x00000000ffffffffULL;
>> -		r4 &= 0x00000000ffffffffULL;
>> -		r5 &= 0x00000000ffffffffULL;
>> -		r6 &= 0x00000000ffffffffULL;
>> -		r7 &= 0x00000000ffffffffULL;
>> -		r8 &= 0x00000000ffffffffULL;
>> +		regs->gpr[3] &= 0x00000000ffffffffULL;
>> +		regs->gpr[4] &= 0x00000000ffffffffULL;
>> +		regs->gpr[5] &= 0x00000000ffffffffULL;
>> +		regs->gpr[6] &= 0x00000000ffffffffULL;
>> +		regs->gpr[7] &= 0x00000000ffffffffULL;
>> +		regs->gpr[8] &= 0x00000000ffffffffULL;
> 
> This (possibly) changes regs->gpr values on the interrupt stack frame.
> 
> I suppose that's going to be okay.  Aside from debugging and tracing
> stuff, could it cause slight differences to how signals are delivered?
> 
> Why do we actually do this anyway? Could we get rid of it (or relegate
> it to !WRAPPER case)?

Seems from the dissassembly that COMPAT_SYSCALL_DEFINEx supplied from
either asm/syscall_wrapper.h or linux/compat.h issue casts to the
type specified in the signature of the compat handler. In which case,
after the patch which expresses all such handlers with COMPAT_SYSCALL_DEFINE,
we should be able to get rid of these high-order word clears.

The casting macro used in each case is as follows:
#define __SC_DELOUSE(t,v) ((__force t)(unsigned long)(v))

> 
> Thanks,
> Nick
> 
>> 
>> 	} else {
>> 		f = (void *)sys_call_table[r0];
>> 	}
>> 
>> -	ret = f(r3, r4, r5, r6, r7, r8);
>> +#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
>> +	ret = f(regs);
>> +#else
>> +	ret = f(regs->gpr[3], regs->gpr[4], regs->gpr[5],
>> +		regs->gpr[6], regs->gpr[7], regs->gpr[8]);
>> +#endif
>> 
>> 	/*
>> 	 * Ultimately, this value will get limited by KSTACK_OFFSET_MAX(),
>> diff --git a/arch/powerpc/kernel/systbl.c b/arch/powerpc/kernel/systbl.c
>> index e5d419822b4e..cb05e302ea58 100644
>> --- a/arch/powerpc/kernel/systbl.c
>> +++ b/arch/powerpc/kernel/systbl.c
>> @@ -15,8 +15,16 @@
>> #include <asm/unistd.h>
>> #include <asm/syscalls.h>
>> 
>> +#undef __SYSCALL_WITH_COMPAT
>> #define __SYSCALL_WITH_COMPAT(nr, entry, compat) __SYSCALL(nr, entry)
>> +
>> +#undef __SYSCALL
>> +#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
>> +#define __SYSCALL(nr, entry) [nr] = __powerpc_##entry,
>> +#define __powerpc_sys_ni_syscall	sys_ni_syscall
>> +#else
>> #define __SYSCALL(nr, entry) [nr] = (void *) entry,
>> +#endif
>> 
>> const syscall_fn sys_call_table[] = {
>> #ifdef CONFIG_PPC64
>> diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
>> index fcca06d200d3..e1f36fd61db3 100644
>> --- a/arch/powerpc/kernel/vdso.c
>> +++ b/arch/powerpc/kernel/vdso.c
>> @@ -39,6 +39,8 @@
>> extern char vdso32_start, vdso32_end;
>> extern char vdso64_start, vdso64_end;
>> 
>> +long sys_ni_syscall(void);
>> +
>> /*
>>  * The vdso data page (aka. systemcfg for old ppc64 fans) is here.
>>  * Once the early boot kernel code no longer needs to muck around
>> -- 
>> 2.34.1

[PATCH 20/23] powerpc/64s: Clear/restore caller gprs in syscall interrupt/return

[Rohan McLure]

Clear user state in gprs (assign to zero) to reduce the influence of user
registers on speculation within kernel syscall handlers. Clears occur
at the very beginning of the sc and scv 0 interrupt handlers, with
restores occurring following the execution of the syscall handler.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
V1 -> V2: Update summary
V2 -> V3: Remove erroneous summary paragraph on syscall_exit_prepare
V3 -> V4: Use ZEROIZE instead of NULLIFY. Clear r0 also.
V4 -> V5: Move to end of patch series.
---
 arch/powerpc/kernel/interrupt_64.S | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 16a1b44088e7..40147558e1a6 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -70,7 +70,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name)
 	ld	r2,PACATOC(r13)
 	mfcr	r12
 	li	r11,0
-	/* Can we avoid saving r3-r8 in common case? */
+	/* Save syscall parameters in r3-r8 */
 	SAVE_GPRS(3, 8, r1)
 	/* Zero r9-r12, this should only be required when restoring all GPRs */
 	std	r11,GPR9(r1)
@@ -110,6 +110,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	 * Zero user registers to prevent influencing speculative execution
 	 * state of kernel code.
 	 */
+	ZEROIZE_GPR(0)
 	ZEROIZE_GPRS(5, 12)
 	ZEROIZE_NVGPRS()
 	bl	system_call_exception
@@ -140,6 +141,7 @@ BEGIN_FTR_SECTION
 	HMT_MEDIUM_LOW
 END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 
+	REST_NVGPRS(r1)
 	cmpdi	r3,0
 	bne	.Lsyscall_vectored_\name\()_restore_regs
 
@@ -243,7 +245,7 @@ END_BTB_FLUSH_SECTION
 	ld	r2,PACATOC(r13)
 	mfcr	r12
 	li	r11,0
-	/* Can we avoid saving r3-r8 in common case? */
+	/* Save syscall parameters in r3-r8 */
 	SAVE_GPRS(3, 8, r1)
 	/* Zero r9-r12, this should only be required when restoring all GPRs */
 	std	r11,GPR9(r1)
@@ -295,6 +297,7 @@ END_BTB_FLUSH_SECTION
 	 * Zero user registers to prevent influencing speculative execution
 	 * state of kernel code.
 	 */
+	ZEROIZE_GPR(0)
 	ZEROIZE_GPRS(5, 12)
 	ZEROIZE_NVGPRS()
 	bl	system_call_exception
@@ -337,6 +340,7 @@ BEGIN_FTR_SECTION
 	stdcx.	r0,0,r1			/* to clear the reservation */
 END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
 
+	REST_NVGPRS(r1)
 	cmpdi	r3,0
 	bne	.Lsyscall_restore_regs
 	/* Zero volatile regs that may contain sensitive kernel data */
@@ -364,7 +368,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 .Lsyscall_restore_regs:
 	ld	r3,_CTR(r1)
 	ld	r4,_XER(r1)
-	REST_NVGPRS(r1)
 	mtctr	r3
 	mtspr	SPRN_XER,r4
 	REST_GPR(0, r1)
-- 
2.34.1

Re: [PATCH 20/23] powerpc/64s: Clear/restore caller gprs in syscall interrupt/return

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> Clear user state in gprs (assign to zero) to reduce the influence of user
> registers on speculation within kernel syscall handlers. Clears occur
> at the very beginning of the sc and scv 0 interrupt handlers, with
> restores occurring following the execution of the syscall handler.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> V1 -> V2: Update summary
> V2 -> V3: Remove erroneous summary paragraph on syscall_exit_prepare
> V3 -> V4: Use ZEROIZE instead of NULLIFY. Clear r0 also.
> V4 -> V5: Move to end of patch series.
> ---

I think it looks okay. I'll have to take a better look with the series
applied.

>  arch/powerpc/kernel/interrupt_64.S | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
> index 16a1b44088e7..40147558e1a6 100644
> --- a/arch/powerpc/kernel/interrupt_64.S
> +++ b/arch/powerpc/kernel/interrupt_64.S
> @@ -70,7 +70,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name)
>  	ld	r2,PACATOC(r13)
>  	mfcr	r12
>  	li	r11,0
> -	/* Can we avoid saving r3-r8 in common case? */
> +	/* Save syscall parameters in r3-r8 */

These two comment changes could go in your system_call_exception API
change patch though.

Thanks,
Nick

>  	SAVE_GPRS(3, 8, r1)
>  	/* Zero r9-r12, this should only be required when restoring all GPRs */
>  	std	r11,GPR9(r1)
> @@ -110,6 +110,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  	 * Zero user registers to prevent influencing speculative execution
>  	 * state of kernel code.
>  	 */
> +	ZEROIZE_GPR(0)
>  	ZEROIZE_GPRS(5, 12)
>  	ZEROIZE_NVGPRS()
>  	bl	system_call_exception
> @@ -140,6 +141,7 @@ BEGIN_FTR_SECTION
>  	HMT_MEDIUM_LOW
>  END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  
> +	REST_NVGPRS(r1)
>  	cmpdi	r3,0
>  	bne	.Lsyscall_vectored_\name\()_restore_regs
>  
> @@ -243,7 +245,7 @@ END_BTB_FLUSH_SECTION
>  	ld	r2,PACATOC(r13)
>  	mfcr	r12
>  	li	r11,0
> -	/* Can we avoid saving r3-r8 in common case? */
> +	/* Save syscall parameters in r3-r8 */
>  	SAVE_GPRS(3, 8, r1)
>  	/* Zero r9-r12, this should only be required when restoring all GPRs */
>  	std	r11,GPR9(r1)
> @@ -295,6 +297,7 @@ END_BTB_FLUSH_SECTION
>  	 * Zero user registers to prevent influencing speculative execution
>  	 * state of kernel code.
>  	 */
> +	ZEROIZE_GPR(0)
>  	ZEROIZE_GPRS(5, 12)
>  	ZEROIZE_NVGPRS()
>  	bl	system_call_exception
> @@ -337,6 +340,7 @@ BEGIN_FTR_SECTION
>  	stdcx.	r0,0,r1			/* to clear the reservation */
>  END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
>  
> +	REST_NVGPRS(r1)
>  	cmpdi	r3,0
>  	bne	.Lsyscall_restore_regs
>  	/* Zero volatile regs that may contain sensitive kernel data */
> @@ -364,7 +368,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  .Lsyscall_restore_regs:
>  	ld	r3,_CTR(r1)
>  	ld	r4,_XER(r1)
> -	REST_NVGPRS(r1)
>  	mtctr	r3
>  	mtspr	SPRN_XER,r4
>  	REST_GPR(0, r1)
> -- 
> 2.34.1

Re: [PATCH 20/23] powerpc/64s: Clear/restore caller gprs in syscall interrupt/return

[Rohan McLure]

[-- Attachment #1: Type: text/plain, Size: 3755 bytes --]



> On 20 Sep 2022, at 12:03 pm, Nicholas Piggin <npiggin@gmail.com> wrote:
> 
> On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
>> Clear user state in gprs (assign to zero) to reduce the influence of user
>> registers on speculation within kernel syscall handlers. Clears occur
>> at the very beginning of the sc and scv 0 interrupt handlers, with
>> restores occurring following the execution of the syscall handler.
>> 
>> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
>> ---
>> V1 -> V2: Update summary
>> V2 -> V3: Remove erroneous summary paragraph on syscall_exit_prepare
>> V3 -> V4: Use ZEROIZE instead of NULLIFY. Clear r0 also.
>> V4 -> V5: Move to end of patch series.
>> ---
> 
> I think it looks okay. I'll have to take a better look with the series
> applied.


Your comments alerted me to the fact that general interrupt and syscalls
share their exit code in interrupt_return and its derivatives. Meaning
that disabling INTERRUPT_SANITIZE_REGISTERS also reverts restores of NVGPRS
to being optional, which makes it possible to clobber NVGPRS and then not
restore them. The cleanest way forward I belive is going to be to cause
INTERRUPT_SANITIZE_REGISTERS to perform sanitisation on all interrupt sources
rather than continuing with syscalls as their own special case. I’ll put
this out in a v6 soon.

> 
>> arch/powerpc/kernel/interrupt_64.S | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>> 
>> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
>> index 16a1b44088e7..40147558e1a6 100644
>> --- a/arch/powerpc/kernel/interrupt_64.S
>> +++ b/arch/powerpc/kernel/interrupt_64.S
>> @@ -70,7 +70,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name)
>> 	ld	r2,PACATOC(r13)
>> 	mfcr	r12
>> 	li	r11,0
>> -	/* Can we avoid saving r3-r8 in common case? */
>> +	/* Save syscall parameters in r3-r8 */
> 
> These two comment changes could go in your system_call_exception API
> change patch though.
> 
> Thanks,
> Nick
> 
>> 	SAVE_GPRS(3, 8, r1)
>> 	/* Zero r9-r12, this should only be required when restoring all GPRs */
>> 	std	r11,GPR9(r1)
>> @@ -110,6 +110,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>> 	 * Zero user registers to prevent influencing speculative execution
>> 	 * state of kernel code.
>> 	 */
>> +	ZEROIZE_GPR(0)
>> 	ZEROIZE_GPRS(5, 12)
>> 	ZEROIZE_NVGPRS()
>> 	bl	system_call_exception
>> @@ -140,6 +141,7 @@ BEGIN_FTR_SECTION
>> 	HMT_MEDIUM_LOW
>> END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>> 
>> +	REST_NVGPRS(r1)
>> 	cmpdi	r3,0
>> 	bne	.Lsyscall_vectored_\name\()_restore_regs
>> 
>> @@ -243,7 +245,7 @@ END_BTB_FLUSH_SECTION
>> 	ld	r2,PACATOC(r13)
>> 	mfcr	r12
>> 	li	r11,0
>> -	/* Can we avoid saving r3-r8 in common case? */
>> +	/* Save syscall parameters in r3-r8 */
>> 	SAVE_GPRS(3, 8, r1)
>> 	/* Zero r9-r12, this should only be required when restoring all GPRs */
>> 	std	r11,GPR9(r1)
>> @@ -295,6 +297,7 @@ END_BTB_FLUSH_SECTION
>> 	 * Zero user registers to prevent influencing speculative execution
>> 	 * state of kernel code.
>> 	 */
>> +	ZEROIZE_GPR(0)
>> 	ZEROIZE_GPRS(5, 12)
>> 	ZEROIZE_NVGPRS()
>> 	bl	system_call_exception
>> @@ -337,6 +340,7 @@ BEGIN_FTR_SECTION
>> 	stdcx.	r0,0,r1			/* to clear the reservation */
>> END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
>> 
>> +	REST_NVGPRS(r1)
>> 	cmpdi	r3,0
>> 	bne	.Lsyscall_restore_regs
>> 	/* Zero volatile regs that may contain sensitive kernel data */
>> @@ -364,7 +368,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>> .Lsyscall_restore_regs:
>> 	ld	r3,_CTR(r1)
>> 	ld	r4,_XER(r1)
>> -	REST_NVGPRS(r1)
>> 	mtctr	r3
>> 	mtspr	SPRN_XER,r4
>> 	REST_GPR(0, r1)
>> -- 
>> 2.34.1


[-- Attachment #2: Type: text/html, Size: 18285 bytes --]

Re: [PATCH 20/23] powerpc/64s: Clear/restore caller gprs in syscall interrupt/return

[Rohan McLure]

> On 20 Sep 2022, at 2:54 pm, Rohan McLure <rmclure@linux.ibm.com> wrote: 
> 
>> On 20 Sep 2022, at 12:03 pm, Nicholas Piggin <npiggin@gmail.com> wrote:
>> 
>> On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
>>> Clear user state in gprs (assign to zero) to reduce the influence of user
>>> registers on speculation within kernel syscall handlers. Clears occur
>>> at the very beginning of the sc and scv 0 interrupt handlers, with
>>> restores occurring following the execution of the syscall handler.
>>> 
>>> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
>>> ---
>>> V1 -> V2: Update summary
>>> V2 -> V3: Remove erroneous summary paragraph on syscall_exit_prepare
>>> V3 -> V4: Use ZEROIZE instead of NULLIFY. Clear r0 also.
>>> V4 -> V5: Move to end of patch series.
>>> ---
>> 
>> I think it looks okay. I'll have to take a better look with the series
>> applied.
> 
> 
> Your comments alerted me to the fact that general interrupt and syscalls
> share their exit code in interrupt_return and its derivatives. Meaning
> that disabling INTERRUPT_SANITIZE_REGISTERS also reverts restores of NVGPRS
> to being optional, which makes it possible to clobber NVGPRS and then not
> restore them. The cleanest way forward I belive is going to be to cause
> INTERRUPT_SANITIZE_REGISTERS to perform sanitisation on all interrupt sources
> rather than continuing with syscalls as their own special case. I’ll put
> this out in a v6 soon.

I think I managed to confuse myself here. Syscall handlers directly RFID, while
other interrupt sources share a common exit path.

>> 
>>> arch/powerpc/kernel/interrupt_64.S | 9 ++++++---
>>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
>>> index 16a1b44088e7..40147558e1a6 100644
>>> --- a/arch/powerpc/kernel/interrupt_64.S
>>> +++ b/arch/powerpc/kernel/interrupt_64.S
>>> @@ -70,7 +70,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name)
>>> 	ld	r2,PACATOC(r13)
>>> 	mfcr	r12
>>> 	li	r11,0
>>> -	/* Can we avoid saving r3-r8 in common case? */
>>> +	/* Save syscall parameters in r3-r8 */
>> 
>> These two comment changes could go in your system_call_exception API
>> change patch though.
>> 
>> Thanks,
>> Nick
>> 
>>> 	SAVE_GPRS(3, 8, r1)
>>> 	/* Zero r9-r12, this should only be required when restoring all GPRs */
>>> 	std	r11,GPR9(r1)
>>> @@ -110,6 +110,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>>> 	 * Zero user registers to prevent influencing speculative execution
>>> 	 * state of kernel code.
>>> 	 */
>>> +	ZEROIZE_GPR(0)
>>> 	ZEROIZE_GPRS(5, 12)
>>> 	ZEROIZE_NVGPRS()
>>> 	bl	system_call_exception
>>> @@ -140,6 +141,7 @@ BEGIN_FTR_SECTION
>>> 	HMT_MEDIUM_LOW
>>> END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>>> 
>>> +	REST_NVGPRS(r1)
>>> 	cmpdi	r3,0
>>> 	bne	.Lsyscall_vectored_\name\()_restore_regs
>>> 
>>> @@ -243,7 +245,7 @@ END_BTB_FLUSH_SECTION
>>> 	ld	r2,PACATOC(r13)
>>> 	mfcr	r12
>>> 	li	r11,0
>>> -	/* Can we avoid saving r3-r8 in common case? */
>>> +	/* Save syscall parameters in r3-r8 */
>>> 	SAVE_GPRS(3, 8, r1)
>>> 	/* Zero r9-r12, this should only be required when restoring all GPRs */
>>> 	std	r11,GPR9(r1)
>>> @@ -295,6 +297,7 @@ END_BTB_FLUSH_SECTION
>>> 	 * Zero user registers to prevent influencing speculative execution
>>> 	 * state of kernel code.
>>> 	 */
>>> +	ZEROIZE_GPR(0)
>>> 	ZEROIZE_GPRS(5, 12)
>>> 	ZEROIZE_NVGPRS()
>>> 	bl	system_call_exception
>>> @@ -337,6 +340,7 @@ BEGIN_FTR_SECTION
>>> 	stdcx.	r0,0,r1			/* to clear the reservation */
>>> END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
>>> 
>>> +	REST_NVGPRS(r1)
>>> 	cmpdi	r3,0
>>> 	bne	.Lsyscall_restore_regs
>>> 	/* Zero volatile regs that may contain sensitive kernel data */
>>> @@ -364,7 +368,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>>> .Lsyscall_restore_regs:
>>> 	ld	r3,_CTR(r1)
>>> 	ld	r4,_XER(r1)
>>> -	REST_NVGPRS(r1)
>>> 	mtctr	r3
>>> 	mtspr	SPRN_XER,r4
>>> 	REST_GPR(0, r1)
>>> -- 
>>> 2.34.1

Re: [PATCH 20/23] powerpc/64s: Clear/restore caller gprs in syscall interrupt/return

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> Clear user state in gprs (assign to zero) to reduce the influence of user
> registers on speculation within kernel syscall handlers. Clears occur
> at the very beginning of the sc and scv 0 interrupt handlers, with
> restores occurring following the execution of the syscall handler.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> V1 -> V2: Update summary
> V2 -> V3: Remove erroneous summary paragraph on syscall_exit_prepare
> V3 -> V4: Use ZEROIZE instead of NULLIFY. Clear r0 also.
> V4 -> V5: Move to end of patch series.
> ---

Oh I would say change the title more like the interrupt patches to avoid
mentioning the restore/return because that's implicit.

e.g., "Clear user GPRs in syscall interrupt entry"

Could we have the one config option that does both this and the interrupt
zeroing?

Thanks,
Nick

>  arch/powerpc/kernel/interrupt_64.S | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
> index 16a1b44088e7..40147558e1a6 100644
> --- a/arch/powerpc/kernel/interrupt_64.S
> +++ b/arch/powerpc/kernel/interrupt_64.S
> @@ -70,7 +70,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name)
>  	ld	r2,PACATOC(r13)
>  	mfcr	r12
>  	li	r11,0
> -	/* Can we avoid saving r3-r8 in common case? */
> +	/* Save syscall parameters in r3-r8 */
>  	SAVE_GPRS(3, 8, r1)
>  	/* Zero r9-r12, this should only be required when restoring all GPRs */
>  	std	r11,GPR9(r1)
> @@ -110,6 +110,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  	 * Zero user registers to prevent influencing speculative execution
>  	 * state of kernel code.
>  	 */
> +	ZEROIZE_GPR(0)
>  	ZEROIZE_GPRS(5, 12)
>  	ZEROIZE_NVGPRS()
>  	bl	system_call_exception
> @@ -140,6 +141,7 @@ BEGIN_FTR_SECTION
>  	HMT_MEDIUM_LOW
>  END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  
> +	REST_NVGPRS(r1)
>  	cmpdi	r3,0
>  	bne	.Lsyscall_vectored_\name\()_restore_regs
>  
> @@ -243,7 +245,7 @@ END_BTB_FLUSH_SECTION
>  	ld	r2,PACATOC(r13)
>  	mfcr	r12
>  	li	r11,0
> -	/* Can we avoid saving r3-r8 in common case? */
> +	/* Save syscall parameters in r3-r8 */
>  	SAVE_GPRS(3, 8, r1)
>  	/* Zero r9-r12, this should only be required when restoring all GPRs */
>  	std	r11,GPR9(r1)
> @@ -295,6 +297,7 @@ END_BTB_FLUSH_SECTION
>  	 * Zero user registers to prevent influencing speculative execution
>  	 * state of kernel code.
>  	 */
> +	ZEROIZE_GPR(0)
>  	ZEROIZE_GPRS(5, 12)
>  	ZEROIZE_NVGPRS()
>  	bl	system_call_exception
> @@ -337,6 +340,7 @@ BEGIN_FTR_SECTION
>  	stdcx.	r0,0,r1			/* to clear the reservation */
>  END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
>  
> +	REST_NVGPRS(r1)
>  	cmpdi	r3,0
>  	bne	.Lsyscall_restore_regs
>  	/* Zero volatile regs that may contain sensitive kernel data */
> @@ -364,7 +368,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  .Lsyscall_restore_regs:
>  	ld	r3,_CTR(r1)
>  	ld	r4,_XER(r1)
> -	REST_NVGPRS(r1)
>  	mtctr	r3
>  	mtspr	SPRN_XER,r4
>  	REST_GPR(0, r1)
> -- 
> 2.34.1

[PATCH 21/23] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig

[Rohan McLure]

Add Kconfig option for enabling clearing of registers on arrival in an
interrupt handler. This reduces the speculation influence of registers
on kernel internals. The option will be consumed by 64-bit systems that
feature speculation and wish to implement this mitigation.

This patch only introduces the Kconfig option, no actual mitigations.

The primary overhead of this mitigation lies in an increased number of
registers that must be saved and restored by interrupt handlers on
Book3S systems. Enable by default on Book3E systems, which prior to
this patch eagerly save and restore register state, meaning that the
mitigation when implemented will have minimal overhead.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
V4 -> V5: New patch
---
 arch/powerpc/Kconfig | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index ef6c83e79c9b..a643ebd83349 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -528,6 +528,15 @@ config HOTPLUG_CPU
 
 	  Say N if you are unsure.
 
+config INTERRUPT_SANITIZE_REGISTERS
+	bool "Clear gprs on interrupt arrival"
+	depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
+	default PPC_BOOK3E_64
+	help
+	  Reduce the influence of user register state on interrupt handlers and
+	  syscalls through clearing user state from registers before handling
+	  the exception.
+
 config PPC_QUEUED_SPINLOCKS
 	bool "Queued spinlocks" if EXPERT
 	depends on SMP
-- 
2.34.1

Re: [PATCH 21/23] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> Add Kconfig option for enabling clearing of registers on arrival in an
> interrupt handler. This reduces the speculation influence of registers
> on kernel internals. The option will be consumed by 64-bit systems that
> feature speculation and wish to implement this mitigation.
>
> This patch only introduces the Kconfig option, no actual mitigations.
>
> The primary overhead of this mitigation lies in an increased number of
> registers that must be saved and restored by interrupt handlers on
> Book3S systems. Enable by default on Book3E systems, which prior to
> this patch eagerly save and restore register state, meaning that the
> mitigation when implemented will have minimal overhead.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>

Thanks for adding this. Maybe I'm being sentimental and we might end
up just simplifying code by always doing it... for now I'm happier to
have the option.

Acked-by: Nicholas Piggin <npiggin@gmail.com>

> ---
> V4 -> V5: New patch
> ---
>  arch/powerpc/Kconfig | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index ef6c83e79c9b..a643ebd83349 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -528,6 +528,15 @@ config HOTPLUG_CPU
>  
>  	  Say N if you are unsure.
>  
> +config INTERRUPT_SANITIZE_REGISTERS
> +	bool "Clear gprs on interrupt arrival"
> +	depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
> +	default PPC_BOOK3E_64
> +	help
> +	  Reduce the influence of user register state on interrupt handlers and
> +	  syscalls through clearing user state from registers before handling
> +	  the exception.
> +
>  config PPC_QUEUED_SPINLOCKS
>  	bool "Queued spinlocks" if EXPERT
>  	depends on SMP
> -- 
> 2.34.1

[PATCH 22/23] powerpc/64s: Clear gprs on interrupt routine entry in Book3S

[Rohan McLure]

Zero GPRS r0, r2-r11, r14-r31, on entry into the kernel for all
other interrupt sources to limit influence of user-space values
in potential speculation gadgets. The remaining gprs are overwritten by
entry macros to interrupt handlers, irrespective of whether or not a
given handler consumes these register values.

Prior to this commit, r14-r31 are restored on a per-interrupt basis at
exit, but now they are always restored. Remove explicit REST_NVGPRS
invocations as non-volatiles must now always be restored. 32-bit systems
do not clear user registers on interrupt, and continue to depend on the
return value of interrupt_exit_user_prepare to determine whether or not
to restore non-volatiles.

The mmap_bench benchmark in selftests should rapidly invoke pagefaults.
See ~0.8% performance regression with this mitigation, but this
indicates the worst-case performance due to heavier-weight interrupt
handlers. This mitigation is disabled by default, but enabled with
CONFIG_INTERRUPT_SANITIZE_REGISTERS.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
V1 -> V2: Add benchmark data
V2 -> V3: Use ZEROIZE_GPR{,S} macro renames, clarify
interrupt_exit_user_prepare changes in summary.
V4 -> V5: Configurable now with INTERRUPT_SANITIZE_REGISTERS. Zero r12
(containing MSR) from common macro on per-interrupt basis with IOPTION.
---
 arch/powerpc/kernel/exceptions-64s.S | 37 ++++++++++++++++++++++++--
 arch/powerpc/kernel/interrupt_64.S   | 10 +++++++
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index a3b51441b039..be5e72caada1 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -111,6 +111,7 @@ name:
 #define ISTACK		.L_ISTACK_\name\()	/* Set regular kernel stack */
 #define __ISTACK(name)	.L_ISTACK_ ## name
 #define IKUAP		.L_IKUAP_\name\()	/* Do KUAP lock */
+#define IMSR_R12	.L_IMSR_R12_\name\()	/* Assumes MSR saved to r12 */
 
 #define INT_DEFINE_BEGIN(n)						\
 .macro int_define_ ## n name
@@ -176,6 +177,9 @@ do_define_int n
 	.ifndef IKUAP
 		IKUAP=1
 	.endif
+	.ifndef IMSR_R12
+		IMSR_R12=0
+	.endif
 .endm
 
 /*
@@ -502,6 +506,7 @@ DEFINE_FIXED_SYMBOL(\name\()_common_real, text)
 	std	r10,0(r1)		/* make stack chain pointer	*/
 	std	r0,GPR0(r1)		/* save r0 in stackframe	*/
 	std	r10,GPR1(r1)		/* save r1 in stackframe	*/
+	ZEROIZE_GPR(0)
 
 	/* Mark our [H]SRRs valid for return */
 	li	r10,1
@@ -544,8 +549,16 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	std	r9,GPR11(r1)
 	std	r10,GPR12(r1)
 	std	r11,GPR13(r1)
+	.if !IMSR_R12
+	ZEROIZE_GPRS(9, 12)
+	.else
+	ZEROIZE_GPRS(9, 11)
+	.endif
 
 	SAVE_NVGPRS(r1)
+#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
+	ZEROIZE_NVGPRS()
+#endif
 
 	.if IDAR
 	.if IISIDE
@@ -577,8 +590,8 @@ BEGIN_FTR_SECTION
 END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
 	ld	r10,IAREA+EX_CTR(r13)
 	std	r10,_CTR(r1)
-	std	r2,GPR2(r1)		/* save r2 in stackframe	*/
-	SAVE_GPRS(3, 8, r1)		/* save r3 - r8 in stackframe   */
+	SAVE_GPRS(2, 8, r1)		/* save r2 - r8 in stackframe   */
+	ZEROIZE_GPRS(2, 8)
 	mflr	r9			/* Get LR, later save to stack	*/
 	ld	r2,PACATOC(r13)		/* get kernel TOC into r2	*/
 	std	r9,_LINK(r1)
@@ -696,6 +709,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
 	mtlr	r9
 	ld	r9,_CCR(r1)
 	mtcr	r9
+#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
+	REST_NVGPRS(r1)
+#endif
 	REST_GPRS(2, 13, r1)
 	REST_GPR(0, r1)
 	/* restore original r1. */
@@ -1368,11 +1384,13 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
 	b	interrupt_return_srr
 
 1:	bl	do_break
+#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
 	/*
 	 * do_break() may have changed the NV GPRS while handling a breakpoint.
 	 * If so, we need to restore them with their updated values.
 	 */
 	REST_NVGPRS(r1)
+#endif
 	b	interrupt_return_srr
 
 
@@ -1598,7 +1616,9 @@ EXC_COMMON_BEGIN(alignment_common)
 	GEN_COMMON alignment
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	alignment_exception
+#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
 	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
+#endif
 	b	interrupt_return_srr
 
 
@@ -1708,7 +1728,9 @@ EXC_COMMON_BEGIN(program_check_common)
 .Ldo_program_check:
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	program_check_exception
+#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
 	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
+#endif
 	b	interrupt_return_srr
 
 
@@ -1726,6 +1748,7 @@ INT_DEFINE_BEGIN(fp_unavailable)
 #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
 	IKVM_REAL=1
 #endif
+	IMSR_R12=1
 INT_DEFINE_END(fp_unavailable)
 
 EXC_REAL_BEGIN(fp_unavailable, 0x800, 0x100)
@@ -2139,7 +2162,9 @@ EXC_COMMON_BEGIN(emulation_assist_common)
 	GEN_COMMON emulation_assist
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	emulation_assist_interrupt
+#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
 	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
+#endif
 	b	interrupt_return_hsrr
 
 
@@ -2347,6 +2372,7 @@ INT_DEFINE_BEGIN(altivec_unavailable)
 #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
 	IKVM_REAL=1
 #endif
+	IMSR_R12=1
 INT_DEFINE_END(altivec_unavailable)
 
 EXC_REAL_BEGIN(altivec_unavailable, 0xf20, 0x20)
@@ -2396,6 +2422,7 @@ INT_DEFINE_BEGIN(vsx_unavailable)
 #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
 	IKVM_REAL=1
 #endif
+	IMSR_R12=1
 INT_DEFINE_END(vsx_unavailable)
 
 EXC_REAL_BEGIN(vsx_unavailable, 0xf40, 0x20)
@@ -2457,7 +2484,9 @@ EXC_COMMON_BEGIN(facility_unavailable_common)
 	GEN_COMMON facility_unavailable
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	facility_unavailable_exception
+#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
 	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
+#endif
 	b	interrupt_return_srr
 
 
@@ -2485,7 +2514,9 @@ EXC_COMMON_BEGIN(h_facility_unavailable_common)
 	GEN_COMMON h_facility_unavailable
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	facility_unavailable_exception
+#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
 	REST_NVGPRS(r1) /* XXX Shouldn't be necessary in practice */
+#endif
 	b	interrupt_return_hsrr
 
 
@@ -2711,7 +2742,9 @@ EXC_COMMON_BEGIN(altivec_assist_common)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 #ifdef CONFIG_ALTIVEC
 	bl	altivec_assist_exception
+#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
 	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
+#endif
 #else
 	bl	unknown_exception
 #endif
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 40147558e1a6..edad0c17e47a 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -433,9 +433,11 @@ interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
 _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	interrupt_exit_user_prepare
+#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
 	cmpdi	r3,0
 	bne-	.Lrestore_nvgprs_\srr
 .Lrestore_nvgprs_\srr\()_cont:
+#endif
 	std	r1,PACA_EXIT_SAVE_R1(r13) /* save r1 for restart */
 #ifdef CONFIG_PPC_BOOK3S
 .Linterrupt_return_\srr\()_user_rst_start:
@@ -449,6 +451,9 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user)
 	stb	r11,PACAIRQHAPPENED(r13) # clear out possible HARD_DIS
 
 .Lfast_user_interrupt_return_\srr\():
+#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
+	REST_NVGPRS(r1)
+#endif
 #ifdef CONFIG_PPC_BOOK3S
 	.ifc \srr,srr
 	lbz	r4,PACASRR_VALID(r13)
@@ -518,9 +523,11 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
 	b	.	/* prevent speculative execution */
 .Linterrupt_return_\srr\()_user_rst_end:
 
+#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
 .Lrestore_nvgprs_\srr\():
 	REST_NVGPRS(r1)
 	b	.Lrestore_nvgprs_\srr\()_cont
+#endif
 
 #ifdef CONFIG_PPC_BOOK3S
 interrupt_return_\srr\()_user_restart:
@@ -562,6 +569,9 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_kernel)
 1:
 
 .Lfast_kernel_interrupt_return_\srr\():
+#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
+	REST_NVGPRS(r1)
+#endif
 	cmpdi	cr1,r3,0
 #ifdef CONFIG_PPC_BOOK3S
 	.ifc \srr,srr
-- 
2.34.1

Re: [PATCH 22/23] powerpc/64s: Clear gprs on interrupt routine entry in Book3S

[Nicholas Piggin]

On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> Zero GPRS r0, r2-r11, r14-r31, on entry into the kernel for all
> other interrupt sources to limit influence of user-space values
> in potential speculation gadgets. The remaining gprs are overwritten by
> entry macros to interrupt handlers, irrespective of whether or not a
> given handler consumes these register values.
>
> Prior to this commit, r14-r31 are restored on a per-interrupt basis at
> exit, but now they are always restored. Remove explicit REST_NVGPRS
> invocations as non-volatiles must now always be restored. 32-bit systems
> do not clear user registers on interrupt, and continue to depend on the
> return value of interrupt_exit_user_prepare to determine whether or not
> to restore non-volatiles.
>
> The mmap_bench benchmark in selftests should rapidly invoke pagefaults.
> See ~0.8% performance regression with this mitigation, but this
> indicates the worst-case performance due to heavier-weight interrupt
> handlers. This mitigation is disabled by default, but enabled with
> CONFIG_INTERRUPT_SANITIZE_REGISTERS.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> V1 -> V2: Add benchmark data
> V2 -> V3: Use ZEROIZE_GPR{,S} macro renames, clarify
> interrupt_exit_user_prepare changes in summary.
> V4 -> V5: Configurable now with INTERRUPT_SANITIZE_REGISTERS. Zero r12
> (containing MSR) from common macro on per-interrupt basis with IOPTION.

Thanks, this is looking pretty good. I'll have a closer look at the asm
though.

Possibly here and in some of the previous patches as well there could be
some macros that hide some of the ifdefs.

e.g., SANITIZE_ZEROIZE_NVGPRS.

And HANDLER_RESTORE_NVGPRS() could do the restore if !SANITIZE, etc.

Also I just realised you're Americanising arch/powerpc. Lizzy would not
have been amused.

Thanks,
Nick

> ---
>  arch/powerpc/kernel/exceptions-64s.S | 37 ++++++++++++++++++++++++--
>  arch/powerpc/kernel/interrupt_64.S   | 10 +++++++
>  2 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index a3b51441b039..be5e72caada1 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -111,6 +111,7 @@ name:
>  #define ISTACK		.L_ISTACK_\name\()	/* Set regular kernel stack */
>  #define __ISTACK(name)	.L_ISTACK_ ## name
>  #define IKUAP		.L_IKUAP_\name\()	/* Do KUAP lock */
> +#define IMSR_R12	.L_IMSR_R12_\name\()	/* Assumes MSR saved to r12 */
>  
>  #define INT_DEFINE_BEGIN(n)						\
>  .macro int_define_ ## n name
> @@ -176,6 +177,9 @@ do_define_int n
>  	.ifndef IKUAP
>  		IKUAP=1
>  	.endif
> +	.ifndef IMSR_R12
> +		IMSR_R12=0
> +	.endif
>  .endm
>  
>  /*
> @@ -502,6 +506,7 @@ DEFINE_FIXED_SYMBOL(\name\()_common_real, text)
>  	std	r10,0(r1)		/* make stack chain pointer	*/
>  	std	r0,GPR0(r1)		/* save r0 in stackframe	*/
>  	std	r10,GPR1(r1)		/* save r1 in stackframe	*/
> +	ZEROIZE_GPR(0)
>  
>  	/* Mark our [H]SRRs valid for return */
>  	li	r10,1
> @@ -544,8 +549,16 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  	std	r9,GPR11(r1)
>  	std	r10,GPR12(r1)
>  	std	r11,GPR13(r1)
> +	.if !IMSR_R12
> +	ZEROIZE_GPRS(9, 12)
> +	.else
> +	ZEROIZE_GPRS(9, 11)
> +	.endif
>  
>  	SAVE_NVGPRS(r1)
> +#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
> +	ZEROIZE_NVGPRS()
> +#endif
>  
>  	.if IDAR
>  	.if IISIDE
> @@ -577,8 +590,8 @@ BEGIN_FTR_SECTION
>  END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
>  	ld	r10,IAREA+EX_CTR(r13)
>  	std	r10,_CTR(r1)
> -	std	r2,GPR2(r1)		/* save r2 in stackframe	*/
> -	SAVE_GPRS(3, 8, r1)		/* save r3 - r8 in stackframe   */
> +	SAVE_GPRS(2, 8, r1)		/* save r2 - r8 in stackframe   */
> +	ZEROIZE_GPRS(2, 8)
>  	mflr	r9			/* Get LR, later save to stack	*/
>  	ld	r2,PACATOC(r13)		/* get kernel TOC into r2	*/
>  	std	r9,_LINK(r1)
> @@ -696,6 +709,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
>  	mtlr	r9
>  	ld	r9,_CCR(r1)
>  	mtcr	r9
> +#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
> +	REST_NVGPRS(r1)
> +#endif
>  	REST_GPRS(2, 13, r1)
>  	REST_GPR(0, r1)
>  	/* restore original r1. */
> @@ -1368,11 +1384,13 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
>  	b	interrupt_return_srr
>  
>  1:	bl	do_break
> +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
>  	/*
>  	 * do_break() may have changed the NV GPRS while handling a breakpoint.
>  	 * If so, we need to restore them with their updated values.
>  	 */
>  	REST_NVGPRS(r1)
> +#endif
>  	b	interrupt_return_srr
>  
>  
> @@ -1598,7 +1616,9 @@ EXC_COMMON_BEGIN(alignment_common)
>  	GEN_COMMON alignment
>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>  	bl	alignment_exception
> +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
>  	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
> +#endif
>  	b	interrupt_return_srr
>  
>  
> @@ -1708,7 +1728,9 @@ EXC_COMMON_BEGIN(program_check_common)
>  .Ldo_program_check:
>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>  	bl	program_check_exception
> +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
>  	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
> +#endif
>  	b	interrupt_return_srr
>  
>  
> @@ -1726,6 +1748,7 @@ INT_DEFINE_BEGIN(fp_unavailable)
>  #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
>  	IKVM_REAL=1
>  #endif
> +	IMSR_R12=1
>  INT_DEFINE_END(fp_unavailable)
>  
>  EXC_REAL_BEGIN(fp_unavailable, 0x800, 0x100)
> @@ -2139,7 +2162,9 @@ EXC_COMMON_BEGIN(emulation_assist_common)
>  	GEN_COMMON emulation_assist
>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>  	bl	emulation_assist_interrupt
> +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
>  	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
> +#endif
>  	b	interrupt_return_hsrr
>  
>  
> @@ -2347,6 +2372,7 @@ INT_DEFINE_BEGIN(altivec_unavailable)
>  #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
>  	IKVM_REAL=1
>  #endif
> +	IMSR_R12=1
>  INT_DEFINE_END(altivec_unavailable)
>  
>  EXC_REAL_BEGIN(altivec_unavailable, 0xf20, 0x20)
> @@ -2396,6 +2422,7 @@ INT_DEFINE_BEGIN(vsx_unavailable)
>  #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
>  	IKVM_REAL=1
>  #endif
> +	IMSR_R12=1
>  INT_DEFINE_END(vsx_unavailable)
>  
>  EXC_REAL_BEGIN(vsx_unavailable, 0xf40, 0x20)
> @@ -2457,7 +2484,9 @@ EXC_COMMON_BEGIN(facility_unavailable_common)
>  	GEN_COMMON facility_unavailable
>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>  	bl	facility_unavailable_exception
> +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
>  	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
> +#endif
>  	b	interrupt_return_srr
>  
>  
> @@ -2485,7 +2514,9 @@ EXC_COMMON_BEGIN(h_facility_unavailable_common)
>  	GEN_COMMON h_facility_unavailable
>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>  	bl	facility_unavailable_exception
> +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
>  	REST_NVGPRS(r1) /* XXX Shouldn't be necessary in practice */
> +#endif
>  	b	interrupt_return_hsrr
>  
>  
> @@ -2711,7 +2742,9 @@ EXC_COMMON_BEGIN(altivec_assist_common)
>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>  #ifdef CONFIG_ALTIVEC
>  	bl	altivec_assist_exception
> +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
>  	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
> +#endif
>  #else
>  	bl	unknown_exception
>  #endif
> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
> index 40147558e1a6..edad0c17e47a 100644
> --- a/arch/powerpc/kernel/interrupt_64.S
> +++ b/arch/powerpc/kernel/interrupt_64.S
> @@ -433,9 +433,11 @@ interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
>  _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user)
>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>  	bl	interrupt_exit_user_prepare
> +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
>  	cmpdi	r3,0
>  	bne-	.Lrestore_nvgprs_\srr
>  .Lrestore_nvgprs_\srr\()_cont:
> +#endif
>  	std	r1,PACA_EXIT_SAVE_R1(r13) /* save r1 for restart */
>  #ifdef CONFIG_PPC_BOOK3S
>  .Linterrupt_return_\srr\()_user_rst_start:
> @@ -449,6 +451,9 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user)
>  	stb	r11,PACAIRQHAPPENED(r13) # clear out possible HARD_DIS
>  
>  .Lfast_user_interrupt_return_\srr\():
> +#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
> +	REST_NVGPRS(r1)
> +#endif
>  #ifdef CONFIG_PPC_BOOK3S
>  	.ifc \srr,srr
>  	lbz	r4,PACASRR_VALID(r13)
> @@ -518,9 +523,11 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
>  	b	.	/* prevent speculative execution */
>  .Linterrupt_return_\srr\()_user_rst_end:
>  
> +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
>  .Lrestore_nvgprs_\srr\():
>  	REST_NVGPRS(r1)
>  	b	.Lrestore_nvgprs_\srr\()_cont
> +#endif
>  
>  #ifdef CONFIG_PPC_BOOK3S
>  interrupt_return_\srr\()_user_restart:
> @@ -562,6 +569,9 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_kernel)
>  1:
>  
>  .Lfast_kernel_interrupt_return_\srr\():
> +#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
> +	REST_NVGPRS(r1)
> +#endif
>  	cmpdi	cr1,r3,0
>  #ifdef CONFIG_PPC_BOOK3S
>  	.ifc \srr,srr
> -- 
> 2.34.1

[PATCH 23/23] powerpc/64e: Clear gprs on interrupt routine entry on Book3E

[Rohan McLure]

Zero GPRS r14-r31 on entry into the kernel for interrupt sources to
limit influence of user-space values in potential speculation gadgets.
Prior to this commit, all other GPRS are reassigned during the common
prologue to interrupt handlers and so need not be zeroised explicitly.

This may be done safely, without loss of register state prior to the
interrupt, as the common prologue saves the initial values of
non-volatiles, which are unconditionally restored in interrupt_64.S.
Mitigation defaults to enabled by INTERRUPT_SANITIZE_REGISTERS.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
V3 -> V4: New patch.
V4 -> V5: Depend on Kconfig option. Remove ZEROIZE_NVGPRS on bad kernel
stack handler.
---
 arch/powerpc/kernel/exceptions-64e.S | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 48c640ca425d..61748769ea29 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -365,6 +365,11 @@ ret_from_mc_except:
 	std	r14,PACA_EXMC+EX_R14(r13);				    \
 	std	r15,PACA_EXMC+EX_R15(r13)
 
+#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
+#define SANITIZE_NVGPRS	ZEROIZE_NVGPRS()
+#else
+#define SANITIZE_NVGPRS
+#endif
 
 /* Core exception code for all exceptions except TLB misses. */
 #define EXCEPTION_COMMON_LVL(n, scratch, excf)				    \
@@ -401,7 +406,8 @@ exc_##n##_common:							    \
 	std	r12,STACK_FRAME_OVERHEAD-16(r1); /* mark the frame */	    \
 	std	r3,_TRAP(r1);		/* set trap number		*/  \
 	std	r0,RESULT(r1);		/* clear regs->result */	    \
-	SAVE_NVGPRS(r1);
+	SAVE_NVGPRS(r1);						    \
+	SANITIZE_NVGPRS;		/* minimise speculation influence */
 
 #define EXCEPTION_COMMON(n) \
 	EXCEPTION_COMMON_LVL(n, SPRN_SPRG_GEN_SCRATCH, PACA_EXGEN)
-- 
2.34.1

Re: [PATCH 00/23] powerpc: Syscall wrapper and register clearing

[Rohan McLure]

> On 16 Sep 2022, at 3:32 pm, Rohan McLure <rmclure@linux.ibm.com> wrote:
> 
> V4 available here:
> 
> Link: https://lore.kernel.org/all/20220824020548.62625-1-rmclure@linux.ibm.com/
> 
> Implement a syscall wrapper, causing arguments to handlers to be passed
> via a struct pt_regs on the stack. The syscall wrapper is implemented
> for all platforms other than the Cell processor, from which SPUs expect
> the ability to directly call syscall handler symbols with the regular
> in-register calling convention.
> 
> Adopting syscall wrappers requires redefinition of architecture-specific
> syscalls and compatibility syscalls to use the SYSCALL_DEFINE and
> COMPAT_SYSCALL_DEFINE macros, as well as removal of direct-references to
> the emitted syscall-handler symbols from within the kernel. This work
> lead to the following modernisations of powerpc's syscall handlers:
> 
> - Replace syscall 82 semantics with sys_old_select and remove
>   ppc_select handler, which features direct call to both sys_old_select
>   and sys_select.
> - Use a generic fallocate compatibility syscall
> 
> Replace asm implementation of syscall table with C implementation for
> more compile-time checks.
> 
> Many compatibility syscalls are candidates to be removed in favour of
> generically defined handlers, but exhibit different parameter orderings
> and numberings due to 32-bit ABI support for 64-bit parameters. The
> parameter reorderings are however consistent with arm. A future patch
> series will serve to modernise syscalls by providing generic
> implementations featuring these reorderings.
> 
> The design of this syscall is very similar to the s390, x86 and arm64
> implementations. See also Commit 4378a7d4be30 (arm64: implement syscall wrappers).
> The motivation for this change is that it allows for the clearing of
> register state when entering the kernel via through interrupt handlers
> on 64-bit servers. This serves to reduce the influence of values in
> registers carried over from the interrupted process, e.g. syscall
> parameters from user space, or user state at the site of a pagefault.
> All values in registers are saved and zeroized at the entry to an
> interrupt handler and restored afterward. While this may sound like a
> heavy-weight mitigation, many gprs are already saved and restored on
> handling of an interrupt, and the mmap_bench benchmark on Power 9 guest,
> repeatedly invoking the pagefault handler suggests at most ~0.8%
> regression in performance. Realistic workloads are not constantly
> producing interrupts, and so this does not indicate realistic slowdown.
> 
> Using wrapped syscalls yields to a performance improvement of ~5.6% on
> the null_syscall benchmark on pseries guests, by removing the need for
> system_call_exception to allocate its own stack frame. This amortises
> the additional costs of saving and restoring non-volatile registers
> (register clearing is cheap on super scalar platforms), and so the
> final mitigation actually yields a net performance improvement of ~0.6%
> on the null_syscall benchmark.
> 
> The clearing of general purpose registers on interrupts other than
> syscalls is enabled by default only on Book3E 64-bit systems (where the
> mitigation is inexpensive), but available to other 64-bit systems via
> the INTERRUPT_SANITIZE_REGISTERS Kconfig option. This mitigation is
> optional, as the speculation influence of interrupts is likely less than
> that of syscalls.
> 
> Patch Changelog:
> 
> - Format orig_r3 handling as its own patch rather than just a revert.
> - Provide asm-generic BE implementation of long-long munging syscall
>   compatiblity arguments.
> - Syscall #82 now refers to generic sys_old_select or
>   comptat_sys_old_select.
> - Drop 'inline' on static helper functions for mmap, personality.
> - Remove arch-specific sys fallocate implementation that was meant to
>   have been removed in V2.
> - Remove references to syscall wrapper until it is introduced.
> - Rearrange patch series so the last five patches are syscall wrapper >
>   syscall register clears > interrupt register clears.
> - Whether non-syscall interrupts should clear registers is now
>   configurable by INTERRUPT_SANITIZE_REGISTERS.
> 
> Rohan McLure (23):
>  powerpc: Remove asmlinkage from syscall handler definitions
>  powerpc: Save caller r3 prior to system_call_exception
>  powerpc: Add ZEROIZE_GPRS macros for register clears
>  powerpc/64s: Use {ZEROIZE,SAVE,REST}_GPRS macros in sc, scv 0 handlers
>  powerpc/32: Clarify interrupt restores with REST_GPR macro in
>    entry_32.S
>  powerpc/64e: Clarify register saves and clears with
>    {SAVE,ZEROIZE}_GPRS
>  powerpc/64s: Fix comment on interrupt handler prologue
>  powerpc: Fix fallocate and fadvise64_64 compat parameter combination
>  asm-generic: compat: Support BE for long long args in 32-bit ABIs
>  powerpc: Use generic fallocate compatibility syscall
>  powerpc/32: Remove powerpc select specialisation
>  powerpc: Remove direct call to personality syscall handler
>  powerpc: Remove direct call to mmap2 syscall handlers
>  powerpc: Provide do_ppc64_personality helper
>  powerpc: Adopt SYSCALL_DEFINE for arch-specific syscall handlers
>  powerpc: Include all arch-specific syscall prototypes
>  powerpc: Enable compile-time check for syscall handlers
>  powerpc: Use common syscall handler type
>  powerpc: Provide syscall wrapper
>  powerpc/64s: Clear/restore caller gprs in syscall interrupt/return
>  powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig
>  powerpc/64s: Clear gprs on interrupt routine entry in Book3S
>  powerpc/64e: Clear gprs on interrupt routine entry on Book3E
> 
> arch/powerpc/Kconfig                         |  10 ++
> arch/powerpc/include/asm/interrupt.h         |   3 +-
> arch/powerpc/include/asm/ppc_asm.h           |  22 +++
> arch/powerpc/include/asm/syscall.h           |  11 +-
> arch/powerpc/include/asm/syscall_wrapper.h   |  84 ++++++++++
> arch/powerpc/include/asm/syscalls.h          | 148 +++++++++++++----
> .../ppc32.h => include/asm/syscalls_32.h}    |   0
> arch/powerpc/include/asm/unistd.h            |   1 +
> arch/powerpc/kernel/entry_32.S               |  40 ++---
> arch/powerpc/kernel/exceptions-64e.S         |  35 ++--
> arch/powerpc/kernel/exceptions-64s.S         |  41 ++++-
> arch/powerpc/kernel/interrupt_64.S           |  92 +++++-----
> arch/powerpc/kernel/signal_32.c              |   2 +-
> arch/powerpc/kernel/sys_ppc32.c              |  66 +++-----
> arch/powerpc/kernel/syscall.c                |  32 ++--
> arch/powerpc/kernel/syscalls.c               |  61 ++++---
> arch/powerpc/kernel/syscalls/syscall.tbl     |  24 +--
> arch/powerpc/kernel/{systbl.S => systbl.c}   |  30 ++--
> arch/powerpc/kernel/vdso.c                   |   6 +-
> arch/powerpc/perf/callchain_32.c             |   2 +-
> arch/powerpc/platforms/cell/spu_callbacks.c  |   6 +-
> include/asm-generic/compat.h                 |   9 +-
> .../arch/powerpc/entry/syscalls/syscall.tbl  |  24 +--
> 23 files changed, 491 insertions(+), 258 deletions(-)
> create mode 100644 arch/powerpc/include/asm/syscall_wrapper.h
> rename arch/powerpc/{kernel/ppc32.h => include/asm/syscalls_32.h} (100%)
> rename arch/powerpc/kernel/{systbl.S => systbl.c} (55%)
> 
> -- 
> 2.34.1
> 

Apologies. This is in fact V5. I can resend if anyone needs me to.

Rohan