Re: [PATCH v4 08/20] powerpc: Include all arch-specific syscall prototypes

On 12 Sep 2022, at 8:33 pm, Nicholas Piggin [off-list ref] wrote:

On Wed Aug 24, 2022 at 12:05 PM AEST, Rohan McLure wrote:

quoted

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 <redacted>
---
V1 -> V2: Explicitly include prototypes.
V2 -> V3: Remove extraneous #include <asm/compat.h> and ppc_fallocate
prototype. Rename header.
---
arch/powerpc/include/asm/syscalls.h          | 90 +++++++++++++-----
.../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, 70 insertions(+), 24 deletions(-)

diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index 3e3aff0835a6..91417dee534e 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h

@@ -8,45 +8,91 @@

#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;

+#ifndef CONFIG_ARCH_HAS_SYSCALL_WRAPPER

Do you need this ifdef?

Good spot. I’ll introduce that with syscall wrappers.

quoted

+
+/*
+ * PowerPC architecture-specific syscalls
+ */
+
+long sys_rtas(struct rtas_args __user *uargs);
+long sys_ni_syscall(void);
+
+#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 */
+
+/* Parameters are reordered for powerpc to avoid padding */
+long sys_ppc_fadvise64_64(int fd, int advice,
+			  u32 offset_high, u32 offset_low,
+			  u32 len_high, u32 len_low);

Should this be under PPC32 since you're adding the ifdefs?

Because you added a new comment here... This is register padding
to do with something, even/odd pair calling convention? I can't
remember the details would you be able to expand the comment a bit
because I'm sure I'll forget it again too.

Cool. I’ll neaten this up and change that comment.

Thanks,
Nick