[PATCH v2 0/4] Disable compat cruft on ppc64le v2

[PATCH v2 0/4] Disable compat cruft on ppc64le v2

[Michal Suchanek]

With endian switch disabled by default the ppc64le compat supports
ppc32le only which is something next to nobody has binaries for.

Less code means less bugs so drop the compat stuff.

I am not particularly sure about the best way to resolve the llseek
situation. I don't see anything in the syscal tables making it
32bit-only so I suppose it should be available on 64bit as well.

This is tested on ppc64le top of

https://patchwork.ozlabs.org/cover/1153556/

Changes in v2: saner CONFIG_COMPAT ifdefs

Thanks

Michal

Michal Suchanek (4):
  fs: always build llseek.
  powerpc: move common register copy functions from signal_32.c to
    signal.c
  powerpc/64: make buildable without CONFIG_COMPAT
  powerpc/64: Disable COMPAT if littleendian.

 arch/powerpc/Kconfig               |   2 +-
 arch/powerpc/include/asm/syscall.h |   2 +
 arch/powerpc/kernel/Makefile       |  15 ++-
 arch/powerpc/kernel/entry_64.S     |   2 +
 arch/powerpc/kernel/signal.c       | 146 ++++++++++++++++++++++++++++-
 arch/powerpc/kernel/signal_32.c    | 140 ---------------------------
 arch/powerpc/kernel/syscall_64.c   |   5 +-
 arch/powerpc/kernel/vdso.c         |   4 +-
 arch/powerpc/perf/callchain.c      |  14 ++-
 fs/read_write.c                    |   2 -
 10 files changed, 177 insertions(+), 155 deletions(-)

-- 
2.22.0

[PATCH v2 1/4] fs: always build llseek.

[Michal Suchanek]

64bit !COMPAT does not build because the llseek syscall is in the tables.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 fs/read_write.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index 5bbf587f5bc1..9db56931eb26 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -331,7 +331,6 @@ COMPAT_SYSCALL_DEFINE3(lseek, unsigned int, fd, compat_off_t, offset, unsigned i
 }
 #endif
 
-#if !defined(CONFIG_64BIT) || defined(CONFIG_COMPAT)
 SYSCALL_DEFINE5(llseek, unsigned int, fd, unsigned long, offset_high,
 		unsigned long, offset_low, loff_t __user *, result,
 		unsigned int, whence)
@@ -360,7 +359,6 @@ SYSCALL_DEFINE5(llseek, unsigned int, fd, unsigned long, offset_high,
 	fdput_pos(f);
 	return retval;
 }
-#endif
 
 int rw_verify_area(int read_write, struct file *file, const loff_t *ppos, size_t count)
 {
-- 
2.22.0

[PATCH v2 2/4] powerpc: move common register copy functions from signal_32.c to signal.c

[Michal Suchanek]

These functions are required for 64bit as well.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/kernel/signal.c    | 141 ++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/signal_32.c | 140 -------------------------------
 2 files changed, 141 insertions(+), 140 deletions(-)

diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index e6c30cee6abf..60436432399f 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -18,12 +18,153 @@
 #include <linux/syscalls.h>
 #include <asm/hw_breakpoint.h>
 #include <linux/uaccess.h>
+#include <asm/switch_to.h>
 #include <asm/unistd.h>
 #include <asm/debug.h>
 #include <asm/tm.h>
 
 #include "signal.h"
 
+#ifdef CONFIG_VSX
+unsigned long copy_fpr_to_user(void __user *to,
+			       struct task_struct *task)
+{
+	u64 buf[ELF_NFPREG];
+	int i;
+
+	/* save FPR copy to local buffer then write to the thread_struct */
+	for (i = 0; i < (ELF_NFPREG - 1) ; i++)
+		buf[i] = task->thread.TS_FPR(i);
+	buf[i] = task->thread.fp_state.fpscr;
+	return __copy_to_user(to, buf, ELF_NFPREG * sizeof(double));
+}
+
+unsigned long copy_fpr_from_user(struct task_struct *task,
+				 void __user *from)
+{
+	u64 buf[ELF_NFPREG];
+	int i;
+
+	if (__copy_from_user(buf, from, ELF_NFPREG * sizeof(double)))
+		return 1;
+	for (i = 0; i < (ELF_NFPREG - 1) ; i++)
+		task->thread.TS_FPR(i) = buf[i];
+	task->thread.fp_state.fpscr = buf[i];
+
+	return 0;
+}
+
+unsigned long copy_vsx_to_user(void __user *to,
+			       struct task_struct *task)
+{
+	u64 buf[ELF_NVSRHALFREG];
+	int i;
+
+	/* save FPR copy to local buffer then write to the thread_struct */
+	for (i = 0; i < ELF_NVSRHALFREG; i++)
+		buf[i] = task->thread.fp_state.fpr[i][TS_VSRLOWOFFSET];
+	return __copy_to_user(to, buf, ELF_NVSRHALFREG * sizeof(double));
+}
+
+unsigned long copy_vsx_from_user(struct task_struct *task,
+				 void __user *from)
+{
+	u64 buf[ELF_NVSRHALFREG];
+	int i;
+
+	if (__copy_from_user(buf, from, ELF_NVSRHALFREG * sizeof(double)))
+		return 1;
+	for (i = 0; i < ELF_NVSRHALFREG ; i++)
+		task->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = buf[i];
+	return 0;
+}
+
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+unsigned long copy_ckfpr_to_user(void __user *to,
+				  struct task_struct *task)
+{
+	u64 buf[ELF_NFPREG];
+	int i;
+
+	/* save FPR copy to local buffer then write to the thread_struct */
+	for (i = 0; i < (ELF_NFPREG - 1) ; i++)
+		buf[i] = task->thread.TS_CKFPR(i);
+	buf[i] = task->thread.ckfp_state.fpscr;
+	return __copy_to_user(to, buf, ELF_NFPREG * sizeof(double));
+}
+
+unsigned long copy_ckfpr_from_user(struct task_struct *task,
+					  void __user *from)
+{
+	u64 buf[ELF_NFPREG];
+	int i;
+
+	if (__copy_from_user(buf, from, ELF_NFPREG * sizeof(double)))
+		return 1;
+	for (i = 0; i < (ELF_NFPREG - 1) ; i++)
+		task->thread.TS_CKFPR(i) = buf[i];
+	task->thread.ckfp_state.fpscr = buf[i];
+
+	return 0;
+}
+
+unsigned long copy_ckvsx_to_user(void __user *to,
+				  struct task_struct *task)
+{
+	u64 buf[ELF_NVSRHALFREG];
+	int i;
+
+	/* save FPR copy to local buffer then write to the thread_struct */
+	for (i = 0; i < ELF_NVSRHALFREG; i++)
+		buf[i] = task->thread.ckfp_state.fpr[i][TS_VSRLOWOFFSET];
+	return __copy_to_user(to, buf, ELF_NVSRHALFREG * sizeof(double));
+}
+
+unsigned long copy_ckvsx_from_user(struct task_struct *task,
+					  void __user *from)
+{
+	u64 buf[ELF_NVSRHALFREG];
+	int i;
+
+	if (__copy_from_user(buf, from, ELF_NVSRHALFREG * sizeof(double)))
+		return 1;
+	for (i = 0; i < ELF_NVSRHALFREG ; i++)
+		task->thread.ckfp_state.fpr[i][TS_VSRLOWOFFSET] = buf[i];
+	return 0;
+}
+#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+#else
+inline unsigned long copy_fpr_to_user(void __user *to,
+				      struct task_struct *task)
+{
+	return __copy_to_user(to, task->thread.fp_state.fpr,
+			      ELF_NFPREG * sizeof(double));
+}
+
+inline unsigned long copy_fpr_from_user(struct task_struct *task,
+					void __user *from)
+{
+	return __copy_from_user(task->thread.fp_state.fpr, from,
+			      ELF_NFPREG * sizeof(double));
+}
+
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+inline unsigned long copy_ckfpr_to_user(void __user *to,
+					 struct task_struct *task)
+{
+	return __copy_to_user(to, task->thread.ckfp_state.fpr,
+			      ELF_NFPREG * sizeof(double));
+}
+
+inline unsigned long copy_ckfpr_from_user(struct task_struct *task,
+						 void __user *from)
+{
+	return __copy_from_user(task->thread.ckfp_state.fpr, from,
+				ELF_NFPREG * sizeof(double));
+}
+#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+#endif
+
 /* Log an error when sending an unhandled signal to a process. Controlled
  * through debug.exception-trace sysctl.
  */
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 98600b276f76..c93c937ea568 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -235,146 +235,6 @@ struct rt_sigframe {
 	int			abigap[56];
 };
 
-#ifdef CONFIG_VSX
-unsigned long copy_fpr_to_user(void __user *to,
-			       struct task_struct *task)
-{
-	u64 buf[ELF_NFPREG];
-	int i;
-
-	/* save FPR copy to local buffer then write to the thread_struct */
-	for (i = 0; i < (ELF_NFPREG - 1) ; i++)
-		buf[i] = task->thread.TS_FPR(i);
-	buf[i] = task->thread.fp_state.fpscr;
-	return __copy_to_user(to, buf, ELF_NFPREG * sizeof(double));
-}
-
-unsigned long copy_fpr_from_user(struct task_struct *task,
-				 void __user *from)
-{
-	u64 buf[ELF_NFPREG];
-	int i;
-
-	if (__copy_from_user(buf, from, ELF_NFPREG * sizeof(double)))
-		return 1;
-	for (i = 0; i < (ELF_NFPREG - 1) ; i++)
-		task->thread.TS_FPR(i) = buf[i];
-	task->thread.fp_state.fpscr = buf[i];
-
-	return 0;
-}
-
-unsigned long copy_vsx_to_user(void __user *to,
-			       struct task_struct *task)
-{
-	u64 buf[ELF_NVSRHALFREG];
-	int i;
-
-	/* save FPR copy to local buffer then write to the thread_struct */
-	for (i = 0; i < ELF_NVSRHALFREG; i++)
-		buf[i] = task->thread.fp_state.fpr[i][TS_VSRLOWOFFSET];
-	return __copy_to_user(to, buf, ELF_NVSRHALFREG * sizeof(double));
-}
-
-unsigned long copy_vsx_from_user(struct task_struct *task,
-				 void __user *from)
-{
-	u64 buf[ELF_NVSRHALFREG];
-	int i;
-
-	if (__copy_from_user(buf, from, ELF_NVSRHALFREG * sizeof(double)))
-		return 1;
-	for (i = 0; i < ELF_NVSRHALFREG ; i++)
-		task->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = buf[i];
-	return 0;
-}
-
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-unsigned long copy_ckfpr_to_user(void __user *to,
-				  struct task_struct *task)
-{
-	u64 buf[ELF_NFPREG];
-	int i;
-
-	/* save FPR copy to local buffer then write to the thread_struct */
-	for (i = 0; i < (ELF_NFPREG - 1) ; i++)
-		buf[i] = task->thread.TS_CKFPR(i);
-	buf[i] = task->thread.ckfp_state.fpscr;
-	return __copy_to_user(to, buf, ELF_NFPREG * sizeof(double));
-}
-
-unsigned long copy_ckfpr_from_user(struct task_struct *task,
-					  void __user *from)
-{
-	u64 buf[ELF_NFPREG];
-	int i;
-
-	if (__copy_from_user(buf, from, ELF_NFPREG * sizeof(double)))
-		return 1;
-	for (i = 0; i < (ELF_NFPREG - 1) ; i++)
-		task->thread.TS_CKFPR(i) = buf[i];
-	task->thread.ckfp_state.fpscr = buf[i];
-
-	return 0;
-}
-
-unsigned long copy_ckvsx_to_user(void __user *to,
-				  struct task_struct *task)
-{
-	u64 buf[ELF_NVSRHALFREG];
-	int i;
-
-	/* save FPR copy to local buffer then write to the thread_struct */
-	for (i = 0; i < ELF_NVSRHALFREG; i++)
-		buf[i] = task->thread.ckfp_state.fpr[i][TS_VSRLOWOFFSET];
-	return __copy_to_user(to, buf, ELF_NVSRHALFREG * sizeof(double));
-}
-
-unsigned long copy_ckvsx_from_user(struct task_struct *task,
-					  void __user *from)
-{
-	u64 buf[ELF_NVSRHALFREG];
-	int i;
-
-	if (__copy_from_user(buf, from, ELF_NVSRHALFREG * sizeof(double)))
-		return 1;
-	for (i = 0; i < ELF_NVSRHALFREG ; i++)
-		task->thread.ckfp_state.fpr[i][TS_VSRLOWOFFSET] = buf[i];
-	return 0;
-}
-#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
-#else
-inline unsigned long copy_fpr_to_user(void __user *to,
-				      struct task_struct *task)
-{
-	return __copy_to_user(to, task->thread.fp_state.fpr,
-			      ELF_NFPREG * sizeof(double));
-}
-
-inline unsigned long copy_fpr_from_user(struct task_struct *task,
-					void __user *from)
-{
-	return __copy_from_user(task->thread.fp_state.fpr, from,
-			      ELF_NFPREG * sizeof(double));
-}
-
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-inline unsigned long copy_ckfpr_to_user(void __user *to,
-					 struct task_struct *task)
-{
-	return __copy_to_user(to, task->thread.ckfp_state.fpr,
-			      ELF_NFPREG * sizeof(double));
-}
-
-inline unsigned long copy_ckfpr_from_user(struct task_struct *task,
-						 void __user *from)
-{
-	return __copy_from_user(task->thread.ckfp_state.fpr, from,
-				ELF_NFPREG * sizeof(double));
-}
-#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
-#endif
-
 /*
  * Save the current user registers on the user stack.
  * We only save the altivec/spe registers if the process has used
-- 
2.22.0

[PATCH v2 3/4] powerpc/64: make buildable without CONFIG_COMPAT

[Michal Suchanek]

There are numerous references to 32bit functions in generic and 64bit
code so ifdef them out.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v2:
- fix 32bit ifdef condition in signal.c
- simplify the compat ifdef condition in vdso.c - 64bit is redundant
- simplify the compat ifdef condition in callchain.c - 64bit is redundant
---
 arch/powerpc/include/asm/syscall.h |  2 ++
 arch/powerpc/kernel/Makefile       | 15 ++++++++++++---
 arch/powerpc/kernel/entry_64.S     |  2 ++
 arch/powerpc/kernel/signal.c       |  5 +++--
 arch/powerpc/kernel/syscall_64.c   |  5 +++--
 arch/powerpc/kernel/vdso.c         |  4 +++-
 arch/powerpc/perf/callchain.c      | 14 ++++++++++----
 7 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
index 38d62acfdce7..3ed3b75541a1 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -16,7 +16,9 @@
 
 /* ftrace syscalls requires exporting the sys_call_table */
 extern const unsigned long sys_call_table[];
+#ifdef CONFIG_COMPAT
 extern const unsigned long compat_sys_call_table[];
+#endif
 
 static inline int syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
 {
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 1d646a94d96c..b0db365b83d8 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -44,16 +44,25 @@ CFLAGS_btext.o += -DDISABLE_BRANCH_PROFILING
 endif
 
 obj-y				:= cputable.o ptrace.o syscalls.o \
-				   irq.o align.o signal_32.o pmc.o vdso.o \
+				   irq.o align.o pmc.o vdso.o \
 				   process.o systbl.o idle.o \
 				   signal.o sysfs.o cacheinfo.o time.o \
 				   prom.o traps.o setup-common.o \
 				   udbg.o misc.o io.o misc_$(BITS).o \
 				   of_platform.o prom_parse.o
-obj-$(CONFIG_PPC64)		+= setup_64.o sys_ppc32.o \
-				   signal_64.o ptrace32.o \
+ifndef CONFIG_PPC64
+obj-y				+= signal_32.o
+else
+ifdef CONFIG_COMPAT
+obj-y				+= signal_32.o
+endif
+endif
+obj-$(CONFIG_PPC64)		+= setup_64.o signal_64.o \
 				   paca.o nvram_64.o firmware.o \
 				   syscall_64.o
+ifdef CONFIG_COMPAT
+obj-$(CONFIG_PPC64)		+= sys_ppc32.o ptrace32.o
+endif
 obj-$(CONFIG_VDSO32)		+= vdso32/
 obj-$(CONFIG_PPC_WATCHDOG)	+= watchdog.o
 obj-$(CONFIG_HAVE_HW_BREAKPOINT)	+= hw_breakpoint.o
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 2ec825a85f5b..a2dbf216f607 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -51,8 +51,10 @@
 SYS_CALL_TABLE:
 	.tc sys_call_table[TC],sys_call_table
 
+#ifdef CONFIG_COMPAT
 COMPAT_SYS_CALL_TABLE:
 	.tc compat_sys_call_table[TC],compat_sys_call_table
+#endif
 
 /* This value is used to mark exception frames on the stack. */
 exception_marker:
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index 60436432399f..ffd045e9fb57 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -277,14 +277,15 @@ static void do_signal(struct task_struct *tsk)
 
 	rseq_signal_deliver(&ksig, tsk->thread.regs);
 
+#if !defined(CONFIG_PPC64) || defined(CONFIG_COMPAT)
 	if (is32) {
         	if (ksig.ka.sa.sa_flags & SA_SIGINFO)
 			ret = handle_rt_signal32(&ksig, oldset, tsk);
 		else
 			ret = handle_signal32(&ksig, oldset, tsk);
-	} else {
+	} else
+#endif /* 32bit */
 		ret = handle_rt_signal64(&ksig, oldset, tsk);
-	}
 
 	tsk->thread.regs->trap = 0;
 	signal_setup_done(ret, &ksig, test_thread_flag(TIF_SINGLESTEP));
diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
index 98ed970796d5..3f48262b512d 100644
--- a/arch/powerpc/kernel/syscall_64.c
+++ b/arch/powerpc/kernel/syscall_64.c
@@ -100,6 +100,7 @@ long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8,
 	/* May be faster to do array_index_nospec? */
 	barrier_nospec();
 
+#ifdef CONFIG_COMPAT
 	if (unlikely(ti_flags & _TIF_32BIT)) {
 		f = (void *)compat_sys_call_table[r0];
 
@@ -110,9 +111,9 @@ long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8,
 		r7 &= 0x00000000ffffffffULL;
 		r8 &= 0x00000000ffffffffULL;
 
-	} else {
+	} else
+#endif /* CONFIG_COMPAT */
 		f = (void *)sys_call_table[r0];
-	}
 
 	return f(r3, r4, r5, r6, r7, r8);
 }
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index d60598113a9f..a991b5d69010 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -667,7 +667,7 @@ static void __init vdso_setup_syscall_map(void)
 {
 	unsigned int i;
 	extern unsigned long *sys_call_table;
-#ifdef CONFIG_PPC64
+#ifdef CONFIG_COMPAT
 	extern unsigned long *compat_sys_call_table;
 #endif
 	extern unsigned long sys_ni_syscall;
@@ -678,9 +678,11 @@ static void __init vdso_setup_syscall_map(void)
 		if (sys_call_table[i] != sys_ni_syscall)
 			vdso_data->syscall_map_64[i >> 5] |=
 				0x80000000UL >> (i & 0x1f);
+#ifdef CONFIG_COMPAT
 		if (compat_sys_call_table[i] != sys_ni_syscall)
 			vdso_data->syscall_map_32[i >> 5] |=
 				0x80000000UL >> (i & 0x1f);
+#endif /* CONFIG_COMPAT */
 #else /* CONFIG_PPC64 */
 		if (sys_call_table[i] != sys_ni_syscall)
 			vdso_data->syscall_map_32[i >> 5] |=
diff --git a/arch/powerpc/perf/callchain.c b/arch/powerpc/perf/callchain.c
index c84bbd4298a0..b3dacc8bc98d 100644
--- a/arch/powerpc/perf/callchain.c
+++ b/arch/powerpc/perf/callchain.c
@@ -15,7 +15,7 @@
 #include <asm/sigcontext.h>
 #include <asm/ucontext.h>
 #include <asm/vdso.h>
-#ifdef CONFIG_PPC64
+#ifdef CONFIG_COMPAT
 #include "../kernel/ppc32.h"
 #endif
 #include <asm/pte-walk.h>
@@ -165,6 +165,7 @@ static int read_user_stack_64(unsigned long __user *ptr, unsigned long *ret)
 	return read_user_stack_slow(ptr, ret, 8);
 }
 
+#ifdef CONFIG_COMPAT
 static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
 {
 	if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
@@ -180,6 +181,7 @@ static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
 
 	return read_user_stack_slow(ptr, ret, 4);
 }
+#endif
 
 static inline int valid_user_sp(unsigned long sp, int is_64)
 {
@@ -341,6 +343,7 @@ static inline int valid_user_sp(unsigned long sp, int is_64)
 
 #endif /* CONFIG_PPC64 */
 
+#if !defined(CONFIG_PPC64) || defined(CONFIG_COMPAT)
 /*
  * Layout for non-RT signal frames
  */
@@ -482,12 +485,15 @@ static void perf_callchain_user_32(struct perf_callchain_entry_ctx *entry,
 		sp = next_sp;
 	}
 }
+#endif /* 32bit */
 
 void
 perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
 {
-	if (current_is_64bit())
-		perf_callchain_user_64(entry, regs);
-	else
+#if !defined(CONFIG_PPC64) || defined(CONFIG_COMPAT)
+	if (!current_is_64bit())
 		perf_callchain_user_32(entry, regs);
+	else
+#endif
+		perf_callchain_user_64(entry, regs);
 }
-- 
2.22.0

[PATCH v2 4/4] powerpc/64: Disable COMPAT if littleendian.

[Michal Suchanek]

ppc32le was never really a thing. Endian swap is already disabled by
default so this 32bit support is kind of useless on ppc64le.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 5bab0bb6b833..67cd1c4d1bae 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -265,7 +265,7 @@ config PANIC_TIMEOUT
 
 config COMPAT
 	bool
-	default y if PPC64
+	default y if PPC64 && !CPU_LITTLE_ENDIAN
 	select COMPAT_BINFMT_ELF
 	select ARCH_WANT_OLD_COMPAT_IPC
 	select COMPAT_OLD_SIGACTION
-- 
2.22.0

Re: [PATCH v2 0/4] Disable compat cruft on ppc64le v2

[Nicholas Piggin]

Michal Suchanek's on August 28, 2019 8:30 pm:
> With endian switch disabled by default the ppc64le compat supports
> ppc32le only which is something next to nobody has binaries for.
> 
> Less code means less bugs so drop the compat stuff.

Interesting patches, thanks for looking into it. I don't know much
about compat and wrong endian userspaces. I think sys_switch_endian
is enabled though, it's just a strange fast endian swap thing that
has been disabled by default.

The first patches look pretty good. Maybe for the last one it could
become a selectable option?


> I am not particularly sure about the best way to resolve the llseek
> situation. I don't see anything in the syscal tables making it
> 32bit-only so I suppose it should be available on 64bit as well.

It's for 32-bit userspace only. Can we just get rid of it, or is
there some old broken 64-bit BE userspace that tries to call it?

Thanks,
Nick

Re: [PATCH v2 3/4] powerpc/64: make buildable without CONFIG_COMPAT

[Christophe Leroy]

Le 28/08/2019 à 12:30, Michal Suchanek a écrit :
> There are numerous references to 32bit functions in generic and 64bit
> code so ifdef them out.

As far as possible, avoid opting things out with ifdefs. Ref 
https://www.kernel.org/doc/html/latest/process/coding-style.html#conditional-compilation

See comment below.

> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v2:
> - fix 32bit ifdef condition in signal.c
> - simplify the compat ifdef condition in vdso.c - 64bit is redundant
> - simplify the compat ifdef condition in callchain.c - 64bit is redundant
> ---
>   arch/powerpc/include/asm/syscall.h |  2 ++
>   arch/powerpc/kernel/Makefile       | 15 ++++++++++++---
>   arch/powerpc/kernel/entry_64.S     |  2 ++
>   arch/powerpc/kernel/signal.c       |  5 +++--
>   arch/powerpc/kernel/syscall_64.c   |  5 +++--
>   arch/powerpc/kernel/vdso.c         |  4 +++-
>   arch/powerpc/perf/callchain.c      | 14 ++++++++++----
>   7 files changed, 35 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
> index 38d62acfdce7..3ed3b75541a1 100644
> --- a/arch/powerpc/include/asm/syscall.h
> +++ b/arch/powerpc/include/asm/syscall.h
> @@ -16,7 +16,9 @@
>   
>   /* ftrace syscalls requires exporting the sys_call_table */
>   extern const unsigned long sys_call_table[];
> +#ifdef CONFIG_COMPAT
>   extern const unsigned long compat_sys_call_table[];
> +#endif

Leaving the declaration should be harmless.

>   
>   static inline int syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
>   {
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 1d646a94d96c..b0db365b83d8 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -44,16 +44,25 @@ CFLAGS_btext.o += -DDISABLE_BRANCH_PROFILING
>   endif
>   
>   obj-y				:= cputable.o ptrace.o syscalls.o \
> -				   irq.o align.o signal_32.o pmc.o vdso.o \
> +				   irq.o align.o pmc.o vdso.o \
>   				   process.o systbl.o idle.o \
>   				   signal.o sysfs.o cacheinfo.o time.o \
>   				   prom.o traps.o setup-common.o \
>   				   udbg.o misc.o io.o misc_$(BITS).o \
>   				   of_platform.o prom_parse.o
> -obj-$(CONFIG_PPC64)		+= setup_64.o sys_ppc32.o \
> -				   signal_64.o ptrace32.o \
> +ifndef CONFIG_PPC64
> +obj-y				+= signal_32.o
> +else
> +ifdef CONFIG_COMPAT
> +obj-y				+= signal_32.o
> +endif
> +endif
> +obj-$(CONFIG_PPC64)		+= setup_64.o signal_64.o \
>   				   paca.o nvram_64.o firmware.o \
>   				   syscall_64.o

That's still a bit messy. You could have:

obj-y = +=signal_$(BITS).o
obj-$(CONFIG_COMPAT) += signal_32.o

> +ifdef CONFIG_COMPAT
> +obj-$(CONFIG_PPC64)		+= sys_ppc32.o ptrace32.o
> +endif

AFAIK, CONFIG_COMPAT is only defined when CONFIG_PP64 is defined, so 
could be:

obj-$(CONFIG_COMPAT)		+= sys_ppc32.o ptrace32.o

And could be grouped with the above signal_32.o


>   obj-$(CONFIG_VDSO32)		+= vdso32/
>   obj-$(CONFIG_PPC_WATCHDOG)	+= watchdog.o
>   obj-$(CONFIG_HAVE_HW_BREAKPOINT)	+= hw_breakpoint.o
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 2ec825a85f5b..a2dbf216f607 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -51,8 +51,10 @@
>   SYS_CALL_TABLE:
>   	.tc sys_call_table[TC],sys_call_table
>   
> +#ifdef CONFIG_COMPAT
>   COMPAT_SYS_CALL_TABLE:
>   	.tc compat_sys_call_table[TC],compat_sys_call_table
> +#endif

Can we avoid this ifdef ?

>   
>   /* This value is used to mark exception frames on the stack. */
>   exception_marker:
> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
> index 60436432399f..ffd045e9fb57 100644
> --- a/arch/powerpc/kernel/signal.c
> +++ b/arch/powerpc/kernel/signal.c
> @@ -277,14 +277,15 @@ static void do_signal(struct task_struct *tsk)
>   
>   	rseq_signal_deliver(&ksig, tsk->thread.regs);
>   
> +#if !defined(CONFIG_PPC64) || defined(CONFIG_COMPAT)
>   	if (is32) {
>           	if (ksig.ka.sa.sa_flags & SA_SIGINFO)
>   			ret = handle_rt_signal32(&ksig, oldset, tsk);
>   		else
>   			ret = handle_signal32(&ksig, oldset, tsk);
> -	} else {
> +	} else

" if only one branch of a conditional statement is a single statement 
[...] use braces in both branches"

Ref 
https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces

> +#endif /* 32bit */

Having an #ifdef in a middle of a if/else is gross.

Check what are the possible values for is32. It will be always true 
which CONFIG_PPC32.
If you can make sure it is always false without CONFIG_COMPAT, you are 
done. If not, then combine the if(is32) with something involving 
IS_ENABLED(CONFIG_COMPAT).

>   		ret = handle_rt_signal64(&ksig, oldset, tsk);
> -	}
>   
>   	tsk->thread.regs->trap = 0;
>   	signal_setup_done(ret, &ksig, test_thread_flag(TIF_SINGLESTEP));
> diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
> index 98ed970796d5..3f48262b512d 100644
> --- a/arch/powerpc/kernel/syscall_64.c
> +++ b/arch/powerpc/kernel/syscall_64.c
> @@ -100,6 +100,7 @@ long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8,
>   	/* May be faster to do array_index_nospec? */
>   	barrier_nospec();
>   
> +#ifdef CONFIG_COMPAT
>   	if (unlikely(ti_flags & _TIF_32BIT)) {
>   		f = (void *)compat_sys_call_table[r0];

Don't opt out compat_sys_call_table[] declaration in .h file, and use:

	if (IS_ENABLED(CONFIG_COMPAT) && unlikely(ti_flags & _TIF_32BIT)) {

>   
> @@ -110,9 +111,9 @@ long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8,
>   		r7 &= 0x00000000ffffffffULL;
>   		r8 &= 0x00000000ffffffffULL;
>   
> -	} else {
> +	} else
> +#endif /* CONFIG_COMPAT */

Same comment above braces and #ifdefs in the middle of an if/else

>   		f = (void *)sys_call_table[r0];
> -	}
>   
>   	return f(r3, r4, r5, r6, r7, r8);
>   }
> diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
> index d60598113a9f..a991b5d69010 100644
> --- a/arch/powerpc/kernel/vdso.c
> +++ b/arch/powerpc/kernel/vdso.c
> @@ -667,7 +667,7 @@ static void __init vdso_setup_syscall_map(void)
>   {
>   	unsigned int i;
>   	extern unsigned long *sys_call_table;
> -#ifdef CONFIG_PPC64
> +#ifdef CONFIG_COMPAT

It should be possible to get rid of that #ifdef completely.

>   	extern unsigned long *compat_sys_call_table;
>   #endif
>   	extern unsigned long sys_ni_syscall;
> @@ -678,9 +678,11 @@ static void __init vdso_setup_syscall_map(void)
>   		if (sys_call_table[i] != sys_ni_syscall)
>   			vdso_data->syscall_map_64[i >> 5] |=
>   				0x80000000UL >> (i & 0x1f);
> +#ifdef CONFIG_COMPAT

Use if (IS_ENABLED(CONFIG_COMPAT && compat_sys_call_table[i] != 
sys_ni_syscall)

>   		if (compat_sys_call_table[i] != sys_ni_syscall)
>   			vdso_data->syscall_map_32[i >> 5] |=
>   				0x80000000UL >> (i & 0x1f);
> +#endif /* CONFIG_COMPAT */
>   #else /* CONFIG_PPC64 */
>   		if (sys_call_table[i] != sys_ni_syscall)
>   			vdso_data->syscall_map_32[i >> 5] |=
> diff --git a/arch/powerpc/perf/callchain.c b/arch/powerpc/perf/callchain.c
> index c84bbd4298a0..b3dacc8bc98d 100644
> --- a/arch/powerpc/perf/callchain.c
> +++ b/arch/powerpc/perf/callchain.c
> @@ -15,7 +15,7 @@
>   #include <asm/sigcontext.h>
>   #include <asm/ucontext.h>
>   #include <asm/vdso.h>
> -#ifdef CONFIG_PPC64
> +#ifdef CONFIG_COMPAT
>   #include "../kernel/ppc32.h"
>   #endif
>   #include <asm/pte-walk.h>
> @@ -165,6 +165,7 @@ static int read_user_stack_64(unsigned long __user *ptr, unsigned long *ret)
>   	return read_user_stack_slow(ptr, ret, 8);
>   }
>   
> +#ifdef CONFIG_COMPAT

Unneeded #ifdef

>   static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
>   {
>   	if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
> @@ -180,6 +181,7 @@ static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
>   
>   	return read_user_stack_slow(ptr, ret, 4);
>   }
> +#endif
>   
>   static inline int valid_user_sp(unsigned long sp, int is_64)
>   {
> @@ -341,6 +343,7 @@ static inline int valid_user_sp(unsigned long sp, int is_64)
>   
>   #endif /* CONFIG_PPC64 */
>   
> +#if !defined(CONFIG_PPC64) || defined(CONFIG_COMPAT)

You don't need to opt that out.

>   /*
>    * Layout for non-RT signal frames
>    */
> @@ -482,12 +485,15 @@ static void perf_callchain_user_32(struct perf_callchain_entry_ctx *entry,
>   		sp = next_sp;
>   	}
>   }
> +#endif /* 32bit */
>   
>   void
>   perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
>   {
> -	if (current_is_64bit())
> -		perf_callchain_user_64(entry, regs);
> -	else
> +#if !defined(CONFIG_PPC64) || defined(CONFIG_COMPAT)
> +	if (!current_is_64bit())
>   		perf_callchain_user_32(entry, regs);
> +	else
> +#endif
> +		perf_callchain_user_64(entry, regs);

Please rewrite using  IS_ENABLED() instead of #ifdefs.

Christophe

Re: [PATCH v2 0/4] Disable compat cruft on ppc64le v2

[Christophe Leroy]

On 08/28/2019 10:30 AM, Michal Suchanek wrote:
> With endian switch disabled by default the ppc64le compat supports
> ppc32le only which is something next to nobody has binaries for.
> 
> Less code means less bugs so drop the compat stuff.
> 
> I am not particularly sure about the best way to resolve the llseek
> situation. I don't see anything in the syscal tables making it
> 32bit-only so I suppose it should be available on 64bit as well.
> 
> This is tested on ppc64le top of

Really ?

I get a build failure with ppc64_defconfig + LITTLE_ENDIAN :

   CC      arch/powerpc/kernel/signal.o
arch/powerpc/kernel/signal.c: In function 'do_signal':
arch/powerpc/kernel/signal.c:250:6: error: unused variable 'is32' 
[-Werror=unused-variable]
   int is32 = is_32bit_task();
       ^~~~
cc1: all warnings being treated as errors
make[3]: *** [arch/powerpc/kernel/signal.o] Error 1

Christophe

> 
> https://patchwork.ozlabs.org/cover/1153556/
> 
> Changes in v2: saner CONFIG_COMPAT ifdefs
> 
> Thanks
> 
> Michal
> 
> Michal Suchanek (4):
>    fs: always build llseek.
>    powerpc: move common register copy functions from signal_32.c to
>      signal.c
>    powerpc/64: make buildable without CONFIG_COMPAT
>    powerpc/64: Disable COMPAT if littleendian.
> 
>   arch/powerpc/Kconfig               |   2 +-
>   arch/powerpc/include/asm/syscall.h |   2 +
>   arch/powerpc/kernel/Makefile       |  15 ++-
>   arch/powerpc/kernel/entry_64.S     |   2 +
>   arch/powerpc/kernel/signal.c       | 146 ++++++++++++++++++++++++++++-
>   arch/powerpc/kernel/signal_32.c    | 140 ---------------------------
>   arch/powerpc/kernel/syscall_64.c   |   5 +-
>   arch/powerpc/kernel/vdso.c         |   4 +-
>   arch/powerpc/perf/callchain.c      |  14 ++-
>   fs/read_write.c                    |   2 -
>   10 files changed, 177 insertions(+), 155 deletions(-)
> 

Re: [PATCH v2 3/4] powerpc/64: make buildable without CONFIG_COMPAT

[Michal Suchánek]

On Wed, 28 Aug 2019 14:49:16 +0200
Christophe Leroy <christophe.leroy@c-s.fr> wrote:

> Le 28/08/2019 à 12:30, Michal Suchanek a écrit :
> > There are numerous references to 32bit functions in generic and 64bit
> > code so ifdef them out.  
> 
> As far as possible, avoid opting things out with ifdefs. Ref 
> https://www.kernel.org/doc/html/latest/process/coding-style.html#conditional-compilation
> 
> See comment below.
> 
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > v2:
> > - fix 32bit ifdef condition in signal.c
> > - simplify the compat ifdef condition in vdso.c - 64bit is redundant
> > - simplify the compat ifdef condition in callchain.c - 64bit is redundant
> > ---
> >   arch/powerpc/include/asm/syscall.h |  2 ++
> >   arch/powerpc/kernel/Makefile       | 15 ++++++++++++---
> >   arch/powerpc/kernel/entry_64.S     |  2 ++
> >   arch/powerpc/kernel/signal.c       |  5 +++--
> >   arch/powerpc/kernel/syscall_64.c   |  5 +++--
> >   arch/powerpc/kernel/vdso.c         |  4 +++-
> >   arch/powerpc/perf/callchain.c      | 14 ++++++++++----
> >   7 files changed, 35 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
> > index 38d62acfdce7..3ed3b75541a1 100644
> > --- a/arch/powerpc/include/asm/syscall.h
> > +++ b/arch/powerpc/include/asm/syscall.h
> > @@ -16,7 +16,9 @@
> >   
> >   /* ftrace syscalls requires exporting the sys_call_table */
> >   extern const unsigned long sys_call_table[];
> > +#ifdef CONFIG_COMPAT
> >   extern const unsigned long compat_sys_call_table[];
> > +#endif  
> 
> Leaving the declaration should be harmless.

Yes, it only allows earlier check that the type is not used.

> 
> >   
> >   static inline int syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
> >   {
> > diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> > index 1d646a94d96c..b0db365b83d8 100644
> > --- a/arch/powerpc/kernel/Makefile
> > +++ b/arch/powerpc/kernel/Makefile
> > @@ -44,16 +44,25 @@ CFLAGS_btext.o += -DDISABLE_BRANCH_PROFILING
> >   endif
> >   
> >   obj-y				:= cputable.o ptrace.o syscalls.o \
> > -				   irq.o align.o signal_32.o pmc.o vdso.o \
> > +				   irq.o align.o pmc.o vdso.o \
> >   				   process.o systbl.o idle.o \
> >   				   signal.o sysfs.o cacheinfo.o time.o \
> >   				   prom.o traps.o setup-common.o \
> >   				   udbg.o misc.o io.o misc_$(BITS).o \
> >   				   of_platform.o prom_parse.o
> > -obj-$(CONFIG_PPC64)		+= setup_64.o sys_ppc32.o \
> > -				   signal_64.o ptrace32.o \
> > +ifndef CONFIG_PPC64
> > +obj-y				+= signal_32.o
> > +else
> > +ifdef CONFIG_COMPAT
> > +obj-y				+= signal_32.o
> > +endif
> > +endif
> > +obj-$(CONFIG_PPC64)		+= setup_64.o signal_64.o \
> >   				   paca.o nvram_64.o firmware.o \
> >   				   syscall_64.o  
> 
> That's still a bit messy. You could have:
> 
> obj-y = +=signal_$(BITS).o
> obj-$(CONFIG_COMPAT) += signal_32.o
> 
> > +ifdef CONFIG_COMPAT
> > +obj-$(CONFIG_PPC64)		+= sys_ppc32.o ptrace32.o
> > +endif  
> 
> AFAIK, CONFIG_COMPAT is only defined when CONFIG_PP64 is defined, so 
> could be:
> 
> obj-$(CONFIG_COMPAT)		+= sys_ppc32.o ptrace32.o
> 
> And could be grouped with the above signal_32.o
> 

Looks better.

> 
> >   obj-$(CONFIG_VDSO32)		+= vdso32/
> >   obj-$(CONFIG_PPC_WATCHDOG)	+= watchdog.o
> >   obj-$(CONFIG_HAVE_HW_BREAKPOINT)	+= hw_breakpoint.o
> > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> > index 2ec825a85f5b..a2dbf216f607 100644
> > --- a/arch/powerpc/kernel/entry_64.S
> > +++ b/arch/powerpc/kernel/entry_64.S
> > @@ -51,8 +51,10 @@
> >   SYS_CALL_TABLE:
> >   	.tc sys_call_table[TC],sys_call_table
> >   
> > +#ifdef CONFIG_COMPAT
> >   COMPAT_SYS_CALL_TABLE:
> >   	.tc compat_sys_call_table[TC],compat_sys_call_table
> > +#endif  
> 
> Can we avoid this ifdef ?

AFAICT it creates reference to non-existent table otherwise.

> 
> >   
> >   /* This value is used to mark exception frames on the stack. */
> >   exception_marker:
> > diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
> > index 60436432399f..ffd045e9fb57 100644
> > --- a/arch/powerpc/kernel/signal.c
> > +++ b/arch/powerpc/kernel/signal.c
> > @@ -277,14 +277,15 @@ static void do_signal(struct task_struct *tsk)
> >   
> >   	rseq_signal_deliver(&ksig, tsk->thread.regs);
> >   
> > +#if !defined(CONFIG_PPC64) || defined(CONFIG_COMPAT)
> >   	if (is32) {
> >           	if (ksig.ka.sa.sa_flags & SA_SIGINFO)
> >   			ret = handle_rt_signal32(&ksig, oldset, tsk);
> >   		else
> >   			ret = handle_signal32(&ksig, oldset, tsk);
> > -	} else {
> > +	} else  
> 
> " if only one branch of a conditional statement is a single statement 
> [...] use braces in both branches"
> 
> Ref 
> https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces
> 
> > +#endif /* 32bit */  
> 
> Having an #ifdef in a middle of a if/else is gross.
> 
> Check what are the possible values for is32. It will be always true 
> which CONFIG_PPC32.
> If you can make sure it is always false without CONFIG_COMPAT, you are 
> done. If not, then combine the if(is32) with something involving 
> IS_ENABLED(CONFIG_COMPAT).

The value of is32 is not a problem. References to non-existent
functions could be.

...

> > diff --git a/arch/powerpc/perf/callchain.c b/arch/powerpc/perf/callchain.c
> > index c84bbd4298a0..b3dacc8bc98d 100644
> > --- a/arch/powerpc/perf/callchain.c
> > +++ b/arch/powerpc/perf/callchain.c
> > @@ -15,7 +15,7 @@
> >   #include <asm/sigcontext.h>
> >   #include <asm/ucontext.h>
> >   #include <asm/vdso.h>
> > -#ifdef CONFIG_PPC64
> > +#ifdef CONFIG_COMPAT
> >   #include "../kernel/ppc32.h"

/srv/kernel/arch/powerpc/perf/../kernel/ppc32.h:50:2: error: unknown type name ‘compat_stack_t’

When required declarations are ifdefed in compat.h

> >   
> >   static inline int valid_user_sp(unsigned long sp, int is_64)
> >   {
> > @@ -341,6 +343,7 @@ static inline int valid_user_sp(unsigned long sp, int is_64)
> >   
> >   #endif /* CONFIG_PPC64 */
> >   
> > +#if !defined(CONFIG_PPC64) || defined(CONFIG_COMPAT)  
> 
> You don't need to opt that out.

You need to opt out here:

/srv/kernel/arch/powerpc/perf/callchain.c:349:22: error: field ‘sctx’ has incomplete type
  struct sigcontext32 sctx;
/srv/kernel/arch/powerpc/perf/callchain.c:359:2: error: unknown type name ‘compat_siginfo_t’
  compat_siginfo_t info;
...
> 
> >   /*
> >    * Layout for non-RT signal frames
> >    */
> > @@ -482,12 +485,15 @@ static void perf_callchain_user_32(struct perf_callchain_entry_ctx *entry,
> >   		sp = next_sp;
> >   	}
> >   }
> > +#endif /* 32bit */
> >   
> >   void
> >   perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
> >   {
> > -	if (current_is_64bit())
> > -		perf_callchain_user_64(entry, regs);
> > -	else
> > +#if !defined(CONFIG_PPC64) || defined(CONFIG_COMPAT)
> > +	if (!current_is_64bit())
> >   		perf_callchain_user_32(entry, regs);
> > +	else
> > +#endif
> > +		perf_callchain_user_64(entry, regs);  
> 
> Please rewrite using  IS_ENABLED() instead of #ifdefs.

And ifdef here.

The ifdefs could be potentially reduced in some places, though.

Thanks

Michal

Re: [PATCH v2 0/4] Disable compat cruft on ppc64le v2

[Michal Suchánek]

On Wed, 28 Aug 2019 13:08:48 +0000
Christophe Leroy <christophe.leroy@c-s.fr> wrote:

> On 08/28/2019 10:30 AM, Michal Suchanek wrote:
> > With endian switch disabled by default the ppc64le compat supports
> > ppc32le only which is something next to nobody has binaries for.
> > 
> > Less code means less bugs so drop the compat stuff.
> > 
> > I am not particularly sure about the best way to resolve the llseek
> > situation. I don't see anything in the syscal tables making it
> > 32bit-only so I suppose it should be available on 64bit as well.
> > 
> > This is tested on ppc64le top of  
> 
> Really ?

Really. It boots with the unused variable. It might depend on some
config options or gcc features if unused variables are fatal.

Thanks

Michal

Re: [PATCH v2 0/4] Disable compat cruft on ppc64le v2

[Michal Suchánek]

On Wed, 28 Aug 2019 20:57:48 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:

> Michal Suchanek's on August 28, 2019 8:30 pm:
> > With endian switch disabled by default the ppc64le compat supports
> > ppc32le only which is something next to nobody has binaries for.
> > 
> > Less code means less bugs so drop the compat stuff.  
> 
> Interesting patches, thanks for looking into it. I don't know much
> about compat and wrong endian userspaces. I think sys_switch_endian
> is enabled though, it's just a strange fast endian swap thing that
> has been disabled by default.
> 
> The first patches look pretty good. Maybe for the last one it could
> become a selectable option?

That sounds good.

> 
> 
> > I am not particularly sure about the best way to resolve the llseek
> > situation. I don't see anything in the syscal tables making it
> > 32bit-only so I suppose it should be available on 64bit as well.  
> 
> It's for 32-bit userspace only. Can we just get rid of it, or is
> there some old broken 64-bit BE userspace that tries to call it?

That sounds like a bug in creating these unified syscall tables then.
On architectures that have split tables the 64bit ones do not have
llseek. On architectures with one table the syscall is marked as common.

Thanks

Michal