[PATCH v5 25/32] powerpc: Convert to printbuf

2022-08-07 Thread Matthew Wilcox (Oracle)
From: Kent Overstreet 

This converts from seq_buf to printbuf. We're using printbuf in external
buffer mode, so it's a direct conversion, aside from some trivial
refactoring in cpu_show_meltdown() to make the code more consistent.

Signed-off-by: Kent Overstreet 
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/kernel/process.c | 16 +++--
 arch/powerpc/kernel/security.c| 75 ++-
 arch/powerpc/platforms/pseries/papr_scm.c | 34 +-
 3 files changed, 57 insertions(+), 68 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 0fbda89cd1bb..05654dbeb2c4 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -37,7 +37,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #include 
 #include 
@@ -1396,32 +1396,30 @@ void show_user_instructions(struct pt_regs *regs)
 {
unsigned long pc;
int n = NR_INSN_TO_PRINT;
-   struct seq_buf s;
char buf[96]; /* enough for 8 times 9 + 2 chars */
+   struct printbuf s = PRINTBUF_EXTERN(buf, sizeof(buf));
 
pc = regs->nip - (NR_INSN_TO_PRINT * 3 / 4 * sizeof(int));
 
-   seq_buf_init(&s, buf, sizeof(buf));
-
while (n) {
int i;
 
-   seq_buf_clear(&s);
+   printbuf_reset(&s);
 
for (i = 0; i < 8 && n; i++, n--, pc += sizeof(int)) {
int instr;
 
if (copy_from_user_nofault(&instr, (void __user *)pc,
sizeof(instr))) {
-   seq_buf_printf(&s, " ");
+   prt_printf(&s, " ");
continue;
}
-   seq_buf_printf(&s, regs->nip == pc ? "<%08x> " : "%08x 
", instr);
+   prt_printf(&s, regs->nip == pc ? "<%08x> " : "%08x ", 
instr);
}
 
-   if (!seq_buf_has_overflowed(&s))
+   if (printbuf_remaining(&s))
pr_info("%s[%d]: code: %s\n", current->comm,
-   current->pid, s.buffer);
+   current->pid, s.buf);
}
 }
 
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index d96fd14bd7c9..b34de62e65ce 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -10,7 +10,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 
 #include 
@@ -144,31 +144,28 @@ void __init setup_spectre_v2(void)
 #ifdef CONFIG_PPC_BOOK3S_64
 ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr, 
char *buf)
 {
+   struct printbuf s = PRINTBUF_EXTERN(buf, PAGE_SIZE);
bool thread_priv;
 
thread_priv = security_ftr_enabled(SEC_FTR_L1D_THREAD_PRIV);
 
if (rfi_flush) {
-   struct seq_buf s;
-   seq_buf_init(&s, buf, PAGE_SIZE - 1);
 
-   seq_buf_printf(&s, "Mitigation: RFI Flush");
+   prt_printf(&s, "Mitigation: RFI Flush");
if (thread_priv)
-   seq_buf_printf(&s, ", L1D private per thread");
-
-   seq_buf_printf(&s, "\n");
-
-   return s.len;
+   prt_printf(&s, ", L1D private per thread");
+
+   prt_printf(&s, "\n");
+   } else if (thread_priv) {
+   prt_printf(&s, "Vulnerable: L1D private per thread\n");
+   } else if (!security_ftr_enabled(SEC_FTR_L1D_FLUSH_HV) &&
+  !security_ftr_enabled(SEC_FTR_L1D_FLUSH_PR)) {
+   prt_printf(&s, "Not affected\n");
+   } else {
+   prt_printf(&s, "Vulnerable\n");
}
 
-   if (thread_priv)
-   return sprintf(buf, "Vulnerable: L1D private per thread\n");
-
-   if (!security_ftr_enabled(SEC_FTR_L1D_FLUSH_HV) &&
-   !security_ftr_enabled(SEC_FTR_L1D_FLUSH_PR))
-   return sprintf(buf, "Not affected\n");
-
-   return sprintf(buf, "Vulnerable\n");
+   return printbuf_written(&s);
 }
 
 ssize_t cpu_show_l1tf(struct device *dev, struct device_attribute *attr, char 
*buf)
@@ -179,70 +176,66 @@ ssize_t cpu_show_l1tf(struct device *dev, struct 
device_attribute *attr, char *b
 
 ssize_t cpu_show_spectre_v1(struct device *dev, struct device_attribute *attr, 
char *buf)
 {
-   struct seq_buf s;
-
-   seq_buf_init(&s, buf, PAGE_SIZE - 1);
+   struct printbuf s = PRINTBUF_EXTERN(buf, PAGE_SIZE);
 
if (security_ftr_enabled(SEC_FTR_BNDS_CHK_SPEC_BAR)) {
if (barrier_nospec_enabled)
-   seq_buf_printf(&s, "Mitigation: __user pointer 
sanitization");
+   prt_printf(&s, "Mitigation: __user pointer 
sanitization");
else
-   seq_buf_printf(&s, "Vulnerable");
+   prt_printf(&s, "Vulnerable");
 
  

Re: [PATCH v2 03/14] powerpc: Remove direct call to mmap2 syscall handlers

2022-08-07 Thread Andrew Donnellan
On Mon, 2022-07-25 at 16:25 +1000, Rohan McLure wrote:
> 
> diff --git a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
> b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
> index 54bb5834785f..437066f5c4b2 100644
> --- a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
> +++ b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
> @@ -243,7 +243,7 @@
>  189nospu   vfork   sys_vfork
>  190common  ugetrlimit  sys_getrlimit
>    compat_sys_getrlimit
>  191common  readahead   sys_readahead
>    compat_sys_ppc_readahead
> -
> 19232  mmap2   sys_mmap2  
>  compat_sys_ppc_mmap2
> +19232  mmap2   sys_mmap2
>    compat_sys_mmap2
>  19332  truncate64  sys_truncate64   
>    compat_sys_ppc_truncate64
>  19432  ftruncate64 sys_ftruncate64  
>    compat_sys_ppc_ftruncate64
>  19532  stat64  sys_stat64
> @@ -391,7 +391,7 @@
>  306common  timerfd_create  sys_timerfd_create
>  307common  eventfd sys_eventfd
>  308common  sync_file_range2sys_sync_file_range2 
>    compat_sys_ppc_sync_file_range2
> -
> 309nospu   fallocate   sys_fallocate  
>  compat_sys_ppc_fallocate
> +309nospu   fallocate   sys_fallocate
>    compat_sys_fallocate

This should be in patch 5?

>  310nospu   subpage_protsys_subpage_prot
>  31132  timerfd_settime sys_timerfd_settime32
>  31164  timerfd_settime sys_timerfd_settime

-- 
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com   IBM Australia Limited



Re: [PATCH v2 04/14] powerpc/32: Remove powerpc select specialisation

2022-08-07 Thread Andrew Donnellan
On Mon, 2022-07-25 at 16:26 +1000, Rohan McLure wrote:
> 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-163b0d29a...@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 for 32-bit binaries, in keeping with
> how
> other architectures support syscall #82.
> 
> Signed-off-by: Rohan McLure 
> ---
> V1 -> V2: Remove arch-specific select handler
> ---
>  arch/powerpc/kernel/syscalls.c   | 18 --
>  arch/powerpc/kernel/syscalls/syscall.tbl |  2 +-
>  .../arch/powerpc/entry/syscalls/syscall.tbl  |  2 +-
>  3 files changed, 2 insertions(+), 20 deletions(-)

You should remove the declaration from
arch/powerpc/include/asm/syscalls.h, which I see you end up doing in
patch #6.

Apart from that:

Reviewed-by: Andrew Donnellan 

> 
> diff --git a/arch/powerpc/kernel/syscalls.c
> b/arch/powerpc/kernel/syscalls.c
> index 9f339bcb433d..0afbcbd50433 100644
> --- a/arch/powerpc/kernel/syscalls.c
> +++ b/arch/powerpc/kernel/syscalls.c
> @@ -74,24 +74,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
> - */
> -SYSCALL_DEFINE5(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
>  static inline long do_ppc64_personality(unsigned long personality)
>  {
> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl
> b/arch/powerpc/kernel/syscalls/syscall.tbl
> index 59d9259dfbb5..c6cfcdf52c57 100644
> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> @@ -110,7 +110,7 @@
>  79 common  settimeofdaysys_settimeofday 
>    compat_sys_settimeofday
>  80 common  getgroups   sys_getgroups
>  81 common  setgroups   sys_setgroups
> -
> 82 32  select  sys_ppc_select 
>  sys_ni_syscall
> +82 32  select  sys_old_select   
>    sys_ni_syscall
>  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 437066f5c4b2..b4c970c9c6b1 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  settimeofdaysys_settimeofday 
>    compat_sys_settimeofday
>  80 common  getgroups   sys_getgroups
>  81 common  setgroups   sys_setgroups
> -
> 82 32  select  sys_ppc_select 
>  sys_ni_syscall
> +82 32  select  sys_old_select   
>    sys_ni_syscall
>  82 64  select  sys_ni_syscall
>  82 spu select  sys_ni_syscall
>  83 common  symlink sys_symlink

-- 
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com   IBM Australia Limited



Re: [PATCH v2 05/14] powerpc: Use generic fallocate compatibility syscall

2022-08-07 Thread Andrew Donnellan
On Mon, 2022-07-25 at 16:26 +1000, 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 
> Signed-off-by: Rohan McLure 
> ---
> V1 -> V2: Remove arch-specific fallocate handler.
> ---
>  arch/powerpc/include/asm/compat.h    | 5 +
>  arch/powerpc/include/asm/unistd.h    | 1 +
>  arch/powerpc/kernel/asm-offsets.c    | 1 +
>  arch/powerpc/kernel/sys_ppc32.c  | 8 
>  arch/powerpc/kernel/syscalls/syscall.tbl | 2 +-
>  5 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/compat.h
> b/arch/powerpc/include/asm/compat.h
> index dda4091fd012..f20caae3f019 100644
> --- a/arch/powerpc/include/asm/compat.h
> +++ b/arch/powerpc/include/asm/compat.h
> @@ -16,6 +16,11 @@ typedef u16  compat_ipc_pid_t;
>  #include 
>  
>  #ifdef __BIG_ENDIAN__
> +#define compat_arg_u64(name)   u32  name##_hi, u32 
> name##_lo
> +#define compat_arg_u64_dual(name)  u32, name##_hi, u32,
> name##_lo
> +#define compat_arg_u64_glue(name)  (((u64)name##_lo &
> 0xUL) | \
> +    ((u64)name##_hi << 32))
> +

Is there any reason this can't go into include/asm-generic/compat.h?

>  #define COMPAT_UTS_MACHINE "ppc\0\0"
>  #else
>  #define COMPAT_UTS_MACHINE "ppcle\0\0"
> 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/asm-offsets.c
> b/arch/powerpc/kernel/asm-offsets.c
> index 8c10f536e478..c669f1d1ee77 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -9,6 +9,7 @@
>   * #defines from the assembly-language output.
>   */
>  
> +#include 

What's this for?

>  #include 
>  #include 
>  #include 
> diff --git a/arch/powerpc/kernel/sys_ppc32.c
> b/arch/powerpc/kernel/sys_ppc32.c
> index 89e8c478fd6a..bd00b7dab7cd 100644
> --- a/arch/powerpc/kernel/sys_ppc32.c
> +++ b/arch/powerpc/kernel/sys_ppc32.c
> @@ -89,14 +89,6 @@ COMPAT_SYSCALL_DEFINE4(ppc_truncate64,
> return ksys_truncate(path, merge_64(len1, len2));
>  }
>  
> -COMPAT_SYSCALL_DEFINE6(ppc_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));
> -}
> -
>  COMPAT_SYSCALL_DEFINE4(ppc_ftruncate64,
>    unsigned int, fd, u32, reg4, unsigned long,
> len1,
>    unsigned long, len2)
> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl
> b/arch/powerpc/kernel/syscalls/syscall.tbl
> index c6cfcdf52c57..b4c970c9c6b1 100644
> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> @@ -391,7 +391,7 @@
>  306common  timerfd_create  sys_timerfd_create
>  307common  eventfd sys_eventfd
>  308common  sync_file_range2sys_sync_file_range2 
>    compat_sys_ppc_sync_file_range2
> -
> 309nospu   fallocate   sys_fallocate  
>  compat_sys_ppc_fallocate
> +309nospu   fallocate   sys_fallocate
>    compat_sys_fallocate
>  310nospu   subpage_protsys_subpage_prot
>  31132  timerfd_settime sys_timerfd_settime32
>  31164  timerfd_settime sys_timerfd_settime

-- 
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com   IBM Australia Limited



Re: [PATCH v2 13/14] powerpc/64s: Fix comment on interrupt handler prologue

2022-08-07 Thread Andrew Donnellan
On Mon, 2022-07-25 at 16:31 +1000, 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.
> 
> Signed-off-by: Rohan McLure 

Reviewed-by: Andrew Donnellan 

-- 
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com   IBM Australia Limited



Re: [PATCH v2 06/14] powerpc: Include all arch-specific syscall prototypes

2022-08-07 Thread Andrew Donnellan
On Mon, 2022-07-25 at 16:27 +1000, 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 



> ---
> V1 -> V2: Explicitly include prototypes.
> ---
>  arch/powerpc/{kernel => include/asm}/ppc32.h |   0
>  arch/powerpc/include/asm/syscalls.h  | 104 -
>  arch/powerpc/kernel/signal_32.c  |   2 +-
>  arch/powerpc/perf/callchain_32.c |   2 +-
>  4 files changed, 76 insertions(+), 32 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/ppc32.h
> b/arch/powerpc/include/asm/ppc32.h
> similarity index 100%
> rename from arch/powerpc/kernel/ppc32.h
> rename to arch/powerpc/include/asm/ppc32.h
> diff --git a/arch/powerpc/include/asm/syscalls.h
> b/arch/powerpc/include/asm/syscalls.h
> index 025d4b877161..8b2757d7f423 100644
> --- a/arch/powerpc/include/asm/syscalls.h
> +++ b/arch/powerpc/include/asm/syscalls.h
> @@ -8,49 +8,93 @@
>  #include 
>  #include 
>  
> +#ifdef CONFIG_PPC64
> +#include 
> +#endif
> +#include 
> +#include 
> +
>  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 sys_ppc64_personality(unsigned long personality);
> +#ifndef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
> +
> +/*
> + * PowerPC architecture-specific syscalls
> + */
> +
>  asmlinkage long sys_rtas(struct rtas_args __user *uargs);
> -int sys_ppc_select(int n, fd_set __user *inp, fd_set __user *outp,
> -  fd_set __user *exp, struct __kernel_old_timeval
> __user *tvp);

Move to patch 4?

> -long sys_ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32
> offset_low,
> - u32 len_high, u32 len_low);
> +asmlinkage long sys_ni_syscall(void);
>  
> +#ifdef CONFIG_PPC64
> +asmlinkage long sys_ppc64_personality(unsigned long personality);
>  #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);
> +asmlinkage long compat_sys_ppc64_personality(unsigned long
> personality);
> +#endif /* CONFIG_COMPAT */
> +#endif /* CONFIG_PPC64 */
>  
> -compat_ssize_t compat_sys_pread64(unsigned int fd, char __user
> *ubuf, compat_size_t count,
> - u32 reg6, u32 pos1, u32 pos2);
> +/* Parameters are reordered for powerpc to avoid padding */
> +asmlinkage long sys_ppc_fadvise64_64(int fd, int advice,
> +    u32 offset_high, u32 offset_low,
> +    u32 len_high, u32 len_low);
> +asmlinkage long sys_swapcontext(struct ucontext __user *old_ctx,
> +   struct ucontext __user *new_ctx, long
> ctx_size);
> +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 sys_switch_endian(void);
>  
> -compat_ssize_t compat_sys_pwrite64(unsigned int fd, const char
> __user *ubuf, compat_size_t count,
> -  u32 reg6, u32 pos1, u32 pos2);
> +#ifdef CONFIG_PPC32
> +asmlinkage long sys_sigreturn(void);
> +asmlinkage long sys_debug_setcontext(struct ucontext __user *ctx,
> int ndbg,
> +    struct sig_dbg_op __user *dbg);
> +#endif
>  
> -compat_ssize_t compat_sys_readahead(int fd, u32 r4, u32 offset1, u32
> offset2, u32 count);
> +asmlinkage long sys_rt_sigreturn(void);
>  
> -int compat_sys_truncate64(const char __user *path, u32 reg4,
> - unsigned long len1, unsigned long len2);
> +asmlinkage long sys_subpage_prot(unsigned long addr,
> +    unsigned long len, u32 __user *map);
>  
> -long compat_sys_fallocate(int fd, int mode, u32 offset1, u32
> offset2, u32 len1, u32 len2);

Move to patch 5?

> +#ifdef CONFIG_COMPAT
> +asmlinkage long compat_sys_swapcontext(struct ucontext32 __user
> *old_ctx,
> +  struct ucontext32 __user
> *new_ctx,
> +  int ctx_size);
> +as

Re: [PATCH v2 07/14] powerpc: Enable compile-time check for syscall handlers

2022-08-07 Thread Andrew Donnellan
On Mon, 2022-07-25 at 16:28 +1000, 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 
> Signed-off-by: Rohan McLure 
> ---
> V1 -> V2: New patch.
> ---
>  arch/powerpc/kernel/{systbl.S => systbl.c} | 27 ++--
>  1 file changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/systbl.S
> b/arch/powerpc/kernel/systbl.c
> similarity index 59%
> rename from arch/powerpc/kernel/systbl.S
> rename to arch/powerpc/kernel/systbl.c
> index cb3358886203..99ffdfef6b9c 100644
> --- a/arch/powerpc/kernel/systbl.S
> +++ b/arch/powerpc/kernel/systbl.c
> @@ -10,31 +10,32 @@
>   * PPC64 updates by Dave Engebretsen (engeb...@us.ibm.com) 
>   */
>  
> -#include 
> +#include 
> +#include 
> +#include 
> +#include 
>  
> -.section .rodata,"a"
> +#define __SYSCALL_WITH_COMPAT(nr, entry, compat) __SYSCALL(nr,
> entry)
>  
> -#ifdef CONFIG_PPC64
> -   .p2align3
> -#define __SYSCALL(nr, entry)   .8byte entry
> +#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
> +#define __SYSCALL(nr, entry) [nr] = __powerpc_##entry,
> +#define __powerpc_sys_ni_syscall   sys_ni_syscall

Keep this until patch 10.

Otherwise looks alright to me.

>  #else
> -#define __SYSCALL(nr, entry)   .long entry
> +#define __SYSCALL(nr, entry) [nr] = entry,
>  #endif
>  
> -#define __SYSCALL_WITH_COMPAT(nr, native, compat)  __SYSCALL(nr,
> native)
> -.globl sys_call_table
> -sys_call_table:
> +void *sys_call_table[] = {
>  #ifdef CONFIG_PPC64
>  #include 
>  #else
>  #include 
>  #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
> +void *compat_sys_call_table[] = {
>  #include 
> -#endif
> +};
> +#endif /* CONFIG_COMPAT */

-- 
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com   IBM Australia Limited



Re: [PATCH v4 3/8] dt-bindings: clock: Add ids for Lynx 10g PLLs

2022-08-07 Thread Krzysztof Kozlowski
On 05/08/2022 17:17, Sean Anderson wrote:
> 
> 
> On 8/5/22 2:53 AM, Krzysztof Kozlowski wrote:
>> On 05/08/2022 00:05, Sean Anderson wrote:
>>> This adds ids for the Lynx 10g SerDes's internal PLLs. These may be used
>>> witn assigned-clock* to specify a particular frequency to use.
>>>
>>> Signed-off-by: Sean Anderson 
>>> ---
>>>
>>> Changes in v4:
>>> - New
>>>
>>>  include/dt-bindings/clock/fsl,lynx-10g.h | 14 ++
>>>  1 file changed, 14 insertions(+)
>>>  create mode 100644 include/dt-bindings/clock/fsl,lynx-10g.h
>>>
>>> diff --git a/include/dt-bindings/clock/fsl,lynx-10g.h 
>>> b/include/dt-bindings/clock/fsl,lynx-10g.h
>>> new file mode 100644
>>> index ..f5b955658106
>>> --- /dev/null
>>> +++ b/include/dt-bindings/clock/fsl,lynx-10g.h
>>> @@ -0,0 +1,14 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>
>> This should be dual license.
> 
> This is just matching what the majority (263 out of 326) clock dt-bindings 
> headers do.

Then please license it just like bindings, so dual license with BSD.

> 
>>> +/*
>>> + * Copyright (C) 2022 Sean Anderson 
>>
>> It's confusing to see personal copyrights with company email. Either the
>> copyright is attributed to your employer or to you. If to you, use
>> private email.
> 
> I hold the copyright, and I would like inquiries to be directed to my work
> email (as I don't have this hardware at home).

OK, I guess I won't be the only one confused :). This entry here is not
parsed for any tools and only sometimes people look at it. The questions
are directed via entry in maintainers file or via git history, so you
can put company email just there.

> 
>>> + */
>>> +
>>> +#ifndef __DT_BINDINGS_CLK_LYNX_10G_H
>>> +#define __DT_BINDINGS_CLK_LYNX_10G_H
>>> +
>>> +#define LYNX10G_CLKS_PER_PLL 2
>>> +
>>> +#define LYNX10G_PLLa(a)((a) * LYNX10G_CLKS_PER_PLL)
>>> +#define LYNX10G_PLLa_EX_DLY(a) ((a) * LYNX10G_CLKS_PER_PLL + 1)
>>
>> These do not look like proper IDs for clocks for bindings. Numbering
>> starts from 0 or 1 and any "a" needs to be clearly explained. What do
>> you bind here?
> 
> This matches "a" is the index of the PLL. E.g. registers PLL1RSTCTL etc.
> This matches the notation used in the reference manual.

This is a file for bindings, not for storing register values. There is
no single need to store register values (offsets, indexes) as bindings
as it is not appropriate. Therefore if you do not use it as an ID, just
remove the bindings header.

> Although for
> convenience, this driver considers the PLLs to start at 0 instead of 1.

Best regards,
Krzysztof


Re: [PATCH v2 01/14] powerpc: Adopt SYSCALL_DEFINE for arch-specific syscall handlers

2022-08-07 Thread Rohan McLure
Thanks for reviewing my patches.

> I think this patch should be split in two patches. One where you just 
> change to using SYSCALL_DEFINE and COMPAT_SYSCALL_DEFINE, and a second 
> patch for everything else.
> 
> The first patch could then be linked to 
> https://github.com/linuxppc/issues/issues/146
> 
> Or in the reverse order if it makes more sense maybe.

My reasoning for completing all of these changes in a single patch is for
retaining bisectability. Adopting SYSCALL_DEFINE and COMPAT_SYSCALL_DEFINE
prepends {sys_, compat_sys_} to the symbol names, and so this must be
reflected immediately in the symbol tables.

> Is that 'asmlinkage' still needed ? Not all syscalls have it seems, and 
> as far as I can see that macro voids for powerpc.

Sorry, you commented on this in the previous revision, and I should have
acted on it then. I in fact touch all but one usages of ‘asmlinkage’ in 
arch/powerpc, and so am in a good position to get rid of all of them. I’ll
do that in next revision.



Re: [PATCH v2 05/14] powerpc: Use generic fallocate compatibility syscall

2022-08-07 Thread Rohan McLure
>> --- a/arch/powerpc/kernel/asm-offsets.c
>> +++ b/arch/powerpc/kernel/asm-offsets.c
>> @@ -9,6 +9,7 @@
>>   * #defines from the assembly-language output.
>>   */
>>  
>> +#include 
> 
> What's this for?

Good spot.