[PATCH 1/6] powerpc: Put FP/VSX and VR state into structures

[Alexander Graf]

On 10.09.2013, at 05:20, Paul Mackerras wrote:

> This creates new 'thread_fp_state' and 'thread_vr_state' structures
> to store FP/VSX state (including FPSCR) and Altivec/VSX state
> (including VSCR), and uses them in the thread_struct.  In the
> thread_fp_state, the FPRs and VSRs are represented as u64 rather
> than double, since we rarely perform floating-point computations
> on the values, and this will enable the structures to be used
> in KVM code as well.  Similarly FPSCR is now a u64 rather than
> a structure of two 32-bit values.
> 
> This takes the offsets out of the macros such as SAVE_32FPRS,
> REST_32FPRS, etc.  This enables the same macros to be used for normal
> and transactional state, enabling us to delete the transactional
> versions of the macros.   This also removes the unused do_load_up_fpu
> and do_load_up_altivec, which were in fact buggy since they didn't
> create large enough stack frames to account for the fact that
> load_up_fpu and load_up_altivec are not designed to be called from C
> and assume that their caller's stack frame is an interrupt frame.
> 
> Signed-off-by: Paul Mackerras <paulus at samba.org>

I like the idea of this patch. However this really needs an ack from Ben (or get applied by him). I also found the patch pretty large and subtile, so there's a good chance I missed a register overwrite if there was any :).

> ---
> arch/powerpc/include/asm/ppc_asm.h     | 95 +++-------------------------------
> arch/powerpc/include/asm/processor.h   | 40 +++++++-------
> arch/powerpc/include/asm/sfp-machine.h |  2 +-
> arch/powerpc/kernel/align.c            |  6 +--
> arch/powerpc/kernel/asm-offsets.c      | 25 +++------
> arch/powerpc/kernel/fpu.S              | 59 +++++----------------
> arch/powerpc/kernel/process.c          |  8 ++-
> arch/powerpc/kernel/ptrace.c           | 49 +++++++++---------
> arch/powerpc/kernel/ptrace32.c         | 11 ++--
> arch/powerpc/kernel/signal_32.c        | 72 +++++++++++++-------------
> arch/powerpc/kernel/signal_64.c        | 29 ++++++-----
> arch/powerpc/kernel/tm.S               | 41 ++++++++-------
> arch/powerpc/kernel/traps.c            | 10 ++--
> arch/powerpc/kernel/vecemu.c           |  6 +--
> arch/powerpc/kernel/vector.S           | 50 ++++++------------
> arch/powerpc/kvm/book3s_pr.c           | 36 ++++++-------
> arch/powerpc/kvm/booke.c               | 19 ++++---
> 17 files changed, 200 insertions(+), 358 deletions(-)
> 
> 

[...]

> diff --git a/arch/powerpc/include/asm/sfp-machine.h b/arch/powerpc/include/asm/sfp-machine.h
> index 3a7a67a..d89beab 100644
> --- a/arch/powerpc/include/asm/sfp-machine.h
> +++ b/arch/powerpc/include/asm/sfp-machine.h
> @@ -125,7 +125,7 @@
> #define FP_EX_DIVZERO         (1 << (31 - 5))
> #define FP_EX_INEXACT         (1 << (31 - 6))
> 
> -#define __FPU_FPSCR	(current->thread.fpscr.val)
> +#define __FPU_FPSCR	(current->thread.fp_state.fpscr)
> 
> /* We only actually write to the destination register
>  * if exceptions signalled (if any) will not trap.
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index a27ccd5..eaa16bc 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -660,7 +660,7 @@ static int emulate_vsx(unsigned char __user *addr, unsigned int reg,
> 	if (reg < 32)
> 		ptr = (char *) &current->thread.TS_FPR(reg);
> 	else
> -		ptr = (char *) &current->thread.vr[reg - 32];
> +		ptr = (char *) &current->thread.vr_state.vr[reg - 32];
> 
> 	lptr = (unsigned long *) ptr;
> 
> @@ -897,7 +897,7 @@ int fix_alignment(struct pt_regs *regs)
> 				return -EFAULT;
> 		}
> 	} else if (flags & F) {
> -		data.dd = current->thread.TS_FPR(reg);
> +		data.ll = current->thread.TS_FPR(reg);

I don't understand this change. Could you please explain?

> 		if (flags & S) {
> 			/* Single-precision FP store requires conversion... */
> #ifdef CONFIG_PPC_FPU
> @@ -975,7 +975,7 @@ int fix_alignment(struct pt_regs *regs)
> 		if (unlikely(ret))
> 			return -EFAULT;
> 	} else if (flags & F)
> -		current->thread.TS_FPR(reg) = data.dd;
> +		current->thread.TS_FPR(reg) = data.ll;
> 	else
> 		regs->gpr[reg] = data.ll;
> 

[...]

> diff --git a/arch/powerpc/kernel/tm.S b/arch/powerpc/kernel/tm.S
> index 7b60b98..d781ca5 100644
> --- a/arch/powerpc/kernel/tm.S
> +++ b/arch/powerpc/kernel/tm.S
> @@ -12,16 +12,15 @@
> #include <asm/reg.h>
> 
> #ifdef CONFIG_VSX
> -/* See fpu.S, this is very similar but to save/restore checkpointed FPRs/VSRs */
> -#define __SAVE_32FPRS_VSRS_TRANSACT(n,c,base)	\
> +/* See fpu.S, this is borrowed from there */
> +#define __SAVE_32FPRS_VSRS(n,c,base)		\

Should this really be in tm.S with its new name?


Alex