[RFC PATCH v1] power: don't manage floating...

[RFC PATCH v1] power: don't manage floating point regs when no FPU

From: Christophe Leroy <hidden>
Date: 2020-08-07 16:14:08

There is no point in copying floating point regs when there
is no FPU and MATH_EMULATION is not selected.

Create a new CONFIG_PPC_FPU_REGS bool that is selected by
CONFIG_MATH_EMULATION and CONFIG_PPC_FPU, and use it to
opt out everything related to fp_state in thread_struct.

The following app runs in approx 10.50 seconds on an 8xx without
the patch, and in 9.45 seconds with the patch.

	void sigusr1(int sig) { }

	int main(int argc, char **argv)
	{
		int i = 100000;

		signal(SIGUSR1, sigusr1);
		for (;i--;)
			raise(SIGUSR1);
		exit(0);
	}

Signed-off-by: Christophe Leroy <redacted>
---
 arch/powerpc/Kconfig                      |  1 +
 arch/powerpc/include/asm/processor.h      |  2 ++
 arch/powerpc/kernel/asm-offsets.c         |  2 ++
 arch/powerpc/kernel/process.c             |  4 ++++
 arch/powerpc/kernel/ptrace/ptrace-novsx.c |  8 ++++++++
 arch/powerpc/kernel/ptrace/ptrace.c       |  4 ++++
 arch/powerpc/kernel/signal.c              | 12 +++++++++++-
 arch/powerpc/kernel/signal_32.c           |  4 ++++
 arch/powerpc/kernel/traps.c               |  4 ++++
 arch/powerpc/platforms/Kconfig.cputype    |  4 ++++
 10 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1f48bbfb3ce9..a2611880b904 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig

@@ -416,6 +416,7 @@ config HUGETLB_PAGE_SIZE_VARIABLE
 config MATH_EMULATION
 	bool "Math emulation"
 	depends on 4xx || PPC_8xx || PPC_MPC832x || BOOKE
+	select PPC_FPU_REGS
 	help
 	  Some PowerPC chips designed for embedded applications do not have
 	  a floating-point unit and therefore do not implement the

diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index ed0d633ab5aa..e20b0c5abe62 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h

@@ -175,8 +175,10 @@ struct thread_struct {
 #endif
 	/* Debug Registers */
 	struct debug_reg debug;
+#ifdef CONFIG_PPC_FPU_REGS
 	struct thread_fp_state	fp_state;
 	struct thread_fp_state	*fp_save_area;
+#endif
 	int		fpexc_mode;	/* floating-point exception mode */
 	unsigned int	align_ctl;	/* alignment handling control */
 #ifdef CONFIG_HAVE_HW_BREAKPOINT

diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 8711c2164b45..6cb36c341c70 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c

@@ -110,9 +110,11 @@ int main(void)
 #ifdef CONFIG_BOOKE
 	OFFSET(THREAD_NORMSAVES, thread_struct, normsave[0]);
 #endif
+#ifdef CONFIG_PPC_FPU
 	OFFSET(THREAD_FPEXC_MODE, thread_struct, fpexc_mode);
 	OFFSET(THREAD_FPSTATE, thread_struct, fp_state.fpr);
 	OFFSET(THREAD_FPSAVEAREA, thread_struct, fp_save_area);
+#endif
 	OFFSET(FPSTATE_FPSCR, thread_fp_state, fpscr);
 	OFFSET(THREAD_LOAD_FP, thread_struct, load_fp);
 #ifdef CONFIG_ALTIVEC

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 016bd831908e..7e0082ac0a39 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c

@@ -1694,7 +1694,9 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
 		p->thread.ptrace_bps[i] = NULL;
 #endif
 
+#ifdef CONFIG_PPC_FPU_REGS
 	p->thread.fp_save_area = NULL;
+#endif
 #ifdef CONFIG_ALTIVEC
 	p->thread.vr_save_area = NULL;
 #endif

@@ -1821,8 +1823,10 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)
 #endif
 	current->thread.load_slb = 0;
 	current->thread.load_fp = 0;
+#ifdef CONFIG_PPC_FPU_REGS
 	memset(&current->thread.fp_state, 0, sizeof(current->thread.fp_state));
 	current->thread.fp_save_area = NULL;
+#endif
 #ifdef CONFIG_ALTIVEC
 	memset(&current->thread.vr_state, 0, sizeof(current->thread.vr_state));
 	current->thread.vr_state.vscr.u[3] = 0x00010000; /* Java mode disabled */

diff --git a/arch/powerpc/kernel/ptrace/ptrace-novsx.c b/arch/powerpc/kernel/ptrace/ptrace-novsx.c
index b2dc4e92d11a..8f87a11f3f8c 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-novsx.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-novsx.c

@@ -21,6 +21,7 @@
 int fpr_get(struct task_struct *target, const struct user_regset *regset,
 	    unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf)
 {
+#ifdef CONFIG_PPC_FPU_REGS
 	BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
 		     offsetof(struct thread_fp_state, fpr[32]));

@@ -28,6 +29,9 @@ int fpr_get(struct task_struct *target, const struct user_regset *regset,
 
 	return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
 				   &target->thread.fp_state, 0, -1);
+#else
+	return 0;
+#endif
 }
 
 /*

@@ -47,6 +51,7 @@ int fpr_set(struct task_struct *target, const struct user_regset *regset,
 	    unsigned int pos, unsigned int count,
 	    const void *kbuf, const void __user *ubuf)
 {
+#ifdef CONFIG_PPC_FPU_REGS
 	BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
 		     offsetof(struct thread_fp_state, fpr[32]));

@@ -54,4 +59,7 @@ int fpr_set(struct task_struct *target, const struct user_regset *regset,
 
 	return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
 				  &target->thread.fp_state, 0, -1);
+#else
+	return 0;
+#endif
 }

diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
index f6e51be47c6e..f162bfec0d3f 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c

@@ -70,6 +70,7 @@ long arch_ptrace(struct task_struct *child, long request,
 			ret = ptrace_get_reg(child, (int) index, &tmp);
 			if (ret)
 				break;
+#ifdef CONFIG_PPC_FPU_REGS
 		} else {
 			unsigned int fpidx = index - PT_FPR0;

@@ -79,6 +80,7 @@ long arch_ptrace(struct task_struct *child, long request,
 				       sizeof(long));
 			else
 				tmp = child->thread.fp_state.fpscr;
+#endif
 		}
 		ret = put_user(tmp, datalp);
 		break;

@@ -103,6 +105,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		CHECK_FULL_REGS(child->thread.regs);
 		if (index < PT_FPR0) {
 			ret = ptrace_put_reg(child, index, data);
+#ifdef CONFIG_PPC_FPU_REGS
 		} else {
 			unsigned int fpidx = index - PT_FPR0;

@@ -113,6 +116,7 @@ long arch_ptrace(struct task_struct *child, long request,
 			else
 				child->thread.fp_state.fpscr = data;
 			ret = 0;
+#endif
 		}
 		break;
 	}

diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index d15a98c758b8..18dcbf538f8f 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c

@@ -133,7 +133,7 @@ unsigned long copy_ckvsx_from_user(struct task_struct *task,
 	return 0;
 }
 #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
-#else
+#elif defined(CONFIG_PPC_FPU_REGS)
 inline unsigned long copy_fpr_to_user(void __user *to,
 				      struct task_struct *task)
 {

@@ -163,6 +163,16 @@ inline unsigned long copy_ckfpr_from_user(struct task_struct *task,
 				ELF_NFPREG * sizeof(double));
 }
 #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+#else
+inline unsigned long copy_fpr_to_user(void __user *to, struct task_struct *task)
+{
+	return 0;
+}
+
+inline unsigned long copy_fpr_from_user(struct task_struct *task, void __user *from)
+{
+	return 0;
+}
 #endif
 
 /* Log an error when sending an unhandled signal to a process. Controlled

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 96950f189b5a..7b291707eb31 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c

@@ -814,7 +814,9 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
 	}
 	regs->link = tramp;
 
+#ifdef CONFIG_PPC_FPU_REGS
 	tsk->thread.fp_state.fpscr = 0;	/* turn off all fp exceptions */
+#endif
 
 	/* create a stack frame for the caller of the handler */
 	newsp = ((unsigned long)rt_sf) - (__SIGNAL_FRAMESIZE + 16);

@@ -1271,7 +1273,9 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
 
 	regs->link = tramp;
 
+#ifdef CONFIG_PPC_FPU_REGS
 	tsk->thread.fp_state.fpscr = 0;	/* turn off all fp exceptions */
+#endif
 
 	/* create a stack frame for the caller of the handler */
 	newsp = ((unsigned long)frame) - __SIGNAL_FRAMESIZE;

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index d1ebe152f210..ee9ec61e75b7 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c

@@ -1188,6 +1188,7 @@ static inline int __parse_fpscr(unsigned long fpscr)
 	return ret;
 }
 
+#ifdef CONFIG_PPC_FPU
 static void parse_fpe(struct pt_regs *regs)
 {
 	int code = 0;

@@ -1198,6 +1199,7 @@ static void parse_fpe(struct pt_regs *regs)
 
 	_exception(SIGFPE, regs, code, regs->nip);
 }
+#endif
 
 /*
  * Illegal instruction emulation support.  Originally written to

@@ -1477,11 +1479,13 @@ void program_check_exception(struct pt_regs *regs)
 	/* We can now get here via a FP Unavailable exception if the core
 	 * has no FPU, in that case the reason flags will be 0 */
 
+#ifdef CONFIG_PPC_FPU
 	if (reason & REASON_FP) {
 		/* IEEE FP exception */
 		parse_fpe(regs);
 		goto bail;
 	}
+#endif
 	if (reason & REASON_TRAP) {
 		unsigned long bugaddr;
 		/* Debugger is first in line to stop recursive faults in

diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 87737ec86d39..40ffcdba42b8 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype

@@ -225,9 +225,13 @@ config PPC_E500MC
 	  such as e5500/e6500), and must be disabled for running on
 	  e500v1 or e500v2.
 
+config PPC_FPU_REGS
+	bool
+
 config PPC_FPU
 	bool
 	default y if PPC64
+	select PPC_FPU_REGS
 
 config FSL_EMB_PERFMON
 	bool "Freescale Embedded Perfmon"

-- 
2.25.0

Re: [RFC PATCH v1] power: don't manage floating point regs when no FPU

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2020-08-11 12:07:10

Christophe Leroy [off-list ref] writes:

There is no point in copying floating point regs when there
is no FPU and MATH_EMULATION is not selected.

Yeah I guess you're right. I've never touched a system with neither, but
if such a thing exists then it does seem silly to copy regs around that
can't be used.

Create a new CONFIG_PPC_FPU_REGS bool that is selected by
CONFIG_MATH_EMULATION and CONFIG_PPC_FPU, and use it to
opt out everything related to fp_state in thread_struct.

The following app runs in approx 10.50 seconds on an 8xx without
the patch, and in 9.45 seconds with the patch.

	void sigusr1(int sig) { }

	int main(int argc, char **argv)
	{
		int i = 100000;

		signal(SIGUSR1, sigusr1);
		for (;i--;)
			raise(SIGUSR1);
		exit(0);
	}

Signed-off-by: Christophe Leroy <redacted>
---
 arch/powerpc/Kconfig                      |  1 +
 arch/powerpc/include/asm/processor.h      |  2 ++
 arch/powerpc/kernel/asm-offsets.c         |  2 ++
 arch/powerpc/kernel/process.c             |  4 ++++
 arch/powerpc/kernel/ptrace/ptrace-novsx.c |  8 ++++++++
 arch/powerpc/kernel/ptrace/ptrace.c       |  4 ++++
 arch/powerpc/kernel/signal.c              | 12 +++++++++++-
 arch/powerpc/kernel/signal_32.c           |  4 ++++
 arch/powerpc/kernel/traps.c               |  4 ++++
 arch/powerpc/platforms/Kconfig.cputype    |  4 ++++
 10 files changed, 44 insertions(+), 1 deletion(-)

In general this looks fine.

It's a bit #ifdef heavy. Maybe some of those can be cleaned up a bit
with some wrapper inlines?

quoted hunk

diff --git a/arch/powerpc/kernel/ptrace/ptrace-novsx.c b/arch/powerpc/kernel/ptrace/ptrace-novsx.c
index b2dc4e92d11a..8f87a11f3f8c 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-novsx.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-novsx.c

@@ -28,6 +29,9 @@ int fpr_get(struct task_struct *target, const struct user_regset *regset,
 
 	return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
 				   &target->thread.fp_state, 0, -1);
+#else
+	return 0;
+#endif

Should we return -ENODEV/EIO here? Wonder if another arch can give us a clue.

cheers

Re: [RFC PATCH v1] power: don't manage floating point regs when no FPU

From: Christophe Leroy <hidden>
Date: 2020-08-11 13:48:22


Le 11/08/2020 à 14:07, Michael Ellerman a écrit :

Christophe Leroy [off-list ref] writes:

quoted

There is no point in copying floating point regs when there
is no FPU and MATH_EMULATION is not selected.

Yeah I guess you're right. I've never touched a system with neither, but
if such a thing exists then it does seem silly to copy regs around that
can't be used.

Yes that exists, because glibc implements floating point emulation and 
it is definitely more efficient to rely of glibc emulation than kernel one.

quoted

  10 files changed, 44 insertions(+), 1 deletion(-)

In general this looks fine.

It's a bit #ifdef heavy. Maybe some of those can be cleaned up a bit
with some wrapper inlines?

Yes I'll try and respin, as part of a series I'm preparing to switch the 
32 bits signal code to using user_access_begin() logic and 
unsafe_put_user() and friends to reduce KUAP unlock/lock.

Christophe

Re: [RFC PATCH v1] power: don't manage floating point regs when no FPU

From: Christophe Leroy <hidden>
Date: 2020-08-11 14:06:56


Le 11/08/2020 à 14:07, Michael Ellerman a écrit :

Christophe Leroy [off-list ref] writes:

quoted

  10 files changed, 44 insertions(+), 1 deletion(-)

In general this looks fine.

It's a bit #ifdef heavy. Maybe some of those can be cleaned up a bit
with some wrapper inlines?

Looking at it once more, looks like more or less the same level of 
#ifdefs as things like CONFIG_ALTIVEC for instance. I can't really see 
much opportunities to clean it up.

quoted

diff --git a/arch/powerpc/kernel/ptrace/ptrace-novsx.c b/arch/powerpc/kernel/ptrace/ptrace-novsx.c
index b2dc4e92d11a..8f87a11f3f8c 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-novsx.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-novsx.c

@@ -28,6 +29,9 @@ int fpr_get(struct task_struct *target, const struct user_regset *regset,
  
  	return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
  				   &target->thread.fp_state, 0, -1);
+#else
+	return 0;
+#endif

Should we return -ENODEV/EIO here? Wonder if another arch can give us a clue.

Looks like we have to do another way  ... another #ifdef ... in the 
definition of native_regsets[] in ptrace-view.c . And then we should be 
able to not build ptrace-novsx.c at all. Will try that.

Christophe

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help