Skip to content

Commit

Permalink
openrisc: Move FPU state out of pt_regs
Browse files Browse the repository at this point in the history
My orignal naive FPU support patch had the FPCSR register stored to both
the user pt_regs and the pt_regs stored for kernel state turing _switch.

This had problems as:
 1. it was wasteful to store and restore FPU state 2 times.
 2. it is problematic when we consider:
    - Accessing FPU state from ptrace/getregs, accessing and writing
      FPU state to user pt_regs.
    - Accessing FPU state when raising exceptions, FPU state was
      not being saved on the syscall fast path.

We fix this by moving the FPCSR state to `thread_struct` in
`task_stuct`.  We now only save FPCSR when:

 - in _switch
 - during syscalls if there is pending work for signals
   TODO need to restore!
 - during floating point exceptions
   TODO need to restore!

Signed-off-by: Stafford Horne <shorne@gmail.com>
  • Loading branch information
stffrdhrn committed Mar 14, 2024
1 parent 7f1e2fc commit 14f89b1
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 27 deletions.
9 changes: 9 additions & 0 deletions arch/openrisc/Kconfig
Expand Up @@ -187,6 +187,15 @@ config SMP

If you don't know what to do here, say N.

config FPU
bool "FPU support"
default y
help
Say N here if you want to disable all floating-point related procedures
in the kernel and reduce binary size.

If you don't know what to do here, say Y.

source "kernel/Kconfig.hz"

config OPENRISC_NO_SPR_SR_DSX
Expand Down
15 changes: 15 additions & 0 deletions arch/openrisc/include/asm/fpu.h
@@ -0,0 +1,15 @@
/* SPDX-License-Identifier: GPL-2.0 */
#ifndef __ASM_OPENRISC_FPU_H
#define __ASM_OPENRISC_FPU_H

struct task_struct;

#ifdef CONFIG_FPU
extern void save_fpu(struct task_struct *__tsk);
extern void restore_fpu(struct task_struct *__tsk);
#else
#define save_fpu(tsk) do { } while (0)
#define restore_fpu(tsk) do { } while (0)
#endif

#endif /* __ASM_OPENRISC_FPU_H */
1 change: 1 addition & 0 deletions arch/openrisc/include/asm/processor.h
Expand Up @@ -44,6 +44,7 @@
struct task_struct;

struct thread_struct {
long fpcsr; /* Floating point control status register. */
};

/*
Expand Down
3 changes: 1 addition & 2 deletions arch/openrisc/include/asm/ptrace.h
Expand Up @@ -59,7 +59,7 @@ struct pt_regs {
* -1 for all other exceptions.
*/
long orig_gpr11; /* For restarting system calls */
long fpcsr; /* Floating point control status register. */
long dummy; /* Cheap alignment fix */
long dummy2; /* Cheap alignment fix */
};

Expand Down Expand Up @@ -115,6 +115,5 @@ static inline long regs_return_value(struct pt_regs *regs)
#define PT_GPR31 124
#define PT_PC 128
#define PT_ORIG_GPR11 132
#define PT_FPCSR 136

#endif /* __ASM_OPENRISC_PTRACE_H */
1 change: 1 addition & 0 deletions arch/openrisc/kernel/Makefile
Expand Up @@ -13,5 +13,6 @@ obj-$(CONFIG_SMP) += smp.o sync-timer.o
obj-$(CONFIG_STACKTRACE) += stacktrace.o
obj-$(CONFIG_MODULES) += module.o
obj-$(CONFIG_OF) += prom.o
obj-$(CONFIG_FPU) += fpu.o

clean:
1 change: 1 addition & 0 deletions arch/openrisc/kernel/asm-offsets.c
Expand Up @@ -40,6 +40,7 @@ int main(void)
DEFINE(TASK_FLAGS, offsetof(struct task_struct, flags));
DEFINE(TASK_PTRACE, offsetof(struct task_struct, ptrace));
DEFINE(TASK_THREAD, offsetof(struct task_struct, thread));
DEFINE(TASK_THREAD_FPCSR, offsetof(struct task_struct, thread.fpcsr));
DEFINE(TASK_MM, offsetof(struct task_struct, mm));
DEFINE(TASK_ACTIVE_MM, offsetof(struct task_struct, active_mm));

Expand Down
13 changes: 4 additions & 9 deletions arch/openrisc/kernel/entry.S
Expand Up @@ -106,8 +106,6 @@
l.mtspr r0,r3,SPR_EPCR_BASE ;\
l.lwz r3,PT_SR(r1) ;\
l.mtspr r0,r3,SPR_ESR_BASE ;\
l.lwz r3,PT_FPCSR(r1) ;\
l.mtspr r0,r3,SPR_FPCSR ;\
l.lwz r2,PT_GPR2(r1) ;\
l.lwz r3,PT_GPR3(r1) ;\
l.lwz r4,PT_GPR4(r1) ;\
Expand Down Expand Up @@ -177,8 +175,6 @@ handler: ;\
/* r30 already save */ ;\
l.sw PT_GPR31(r1),r31 ;\
TRACE_IRQS_OFF_ENTRY ;\
l.mfspr r30,r0,SPR_FPCSR ;\
l.sw PT_FPCSR(r1),r30 ;\
/* Store -1 in orig_gpr11 for non-syscall exceptions */ ;\
l.addi r30,r0,-1 ;\
l.sw PT_ORIG_GPR11(r1),r30
Expand Down Expand Up @@ -219,8 +215,6 @@ handler: ;\
/* Store -1 in orig_gpr11 for non-syscall exceptions */ ;\
l.addi r30,r0,-1 ;\
l.sw PT_ORIG_GPR11(r1),r30 ;\
l.mfspr r30,r0,SPR_FPCSR ;\
l.sw PT_FPCSR(r1),r30 ;\
l.addi r3,r1,0 ;\
/* r4 is exception EA */ ;\
l.addi r5,r0,vector ;\
Expand Down Expand Up @@ -852,6 +846,7 @@ _syscall_badsys:

EXCEPTION_ENTRY(_fpe_trap_handler)
CLEAR_LWA_FLAG(r3)

/* r4: EA of fault (set by EXCEPTION_HANDLE) */
l.jal do_fpe_trap
l.addi r3,r1,0 /* pt_regs */
Expand Down Expand Up @@ -1100,9 +1095,9 @@ ENTRY(_switch)
l.sw PT_GPR28(r1),r28
l.sw PT_GPR30(r1),r30

/* Store the old FPU state to new pt_regs */
/* Store the current FPU state */
l.mfspr r29,r0,SPR_FPCSR
l.sw PT_FPCSR(r1),r29
l.sw TASK_THREAD_FPCSR(r10),r29

l.addi r11,r10,0 /* Save old 'current' to 'last' return value*/

Expand All @@ -1127,7 +1122,7 @@ ENTRY(_switch)
l.sw TI_KSP(r10),r29

/* Restore the old value of FPCSR */
l.lwz r29,PT_FPCSR(r1)
l.lwz r29,TASK_THREAD_FPCSR(r10)
l.mtspr r0,r29,SPR_FPCSR

/* ...and restore the registers, except r11 because the return value
Expand Down
17 changes: 17 additions & 0 deletions arch/openrisc/kernel/fpu.c
@@ -0,0 +1,17 @@
/* SPDX-License-Identifier: GPL-2.0 */

#include <linux/sched.h>

#include <asm/fpu.h>
#include <asm/spr.h>
#include <asm/spr_defs.h>

void save_fpu(struct task_struct *task)
{
task->thread.fpcsr = mfspr(SPR_FPCSR);
}

void restore_fpu(struct task_struct *task)
{
mtspr(SPR_FPCSR, task->thread.fpcsr);
}
12 changes: 3 additions & 9 deletions arch/openrisc/kernel/ptrace.c
Expand Up @@ -97,23 +97,17 @@ static int fpregs_get(struct task_struct *target,
const struct user_regset *regset,
struct membuf to)
{
const struct pt_regs *regs = task_pt_regs(target);

return membuf_store(&to, regs->fpcsr);
return membuf_store(&to, target->thread.fpcsr);
}

static int fpregs_set(struct task_struct *target,
const struct user_regset *regset,
unsigned int pos, unsigned int count,
const void *kbuf, const void __user *ubuf)
{
struct pt_regs *regs = task_pt_regs(target);
int ret;

/* FPCSR */
ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
&regs->fpcsr, 0, 4);
return ret;
return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
&target->thread.fpcsr, 0, 4);
}

/*
Expand Down
11 changes: 9 additions & 2 deletions arch/openrisc/kernel/signal.c
Expand Up @@ -23,6 +23,7 @@
#include <linux/stddef.h>
#include <linux/resume_user_mode.h>

#include <asm/fpu.h>
#include <asm/processor.h>
#include <asm/syscall.h>
#include <asm/ucontext.h>
Expand Down Expand Up @@ -55,7 +56,7 @@ static int restore_sigcontext(struct pt_regs *regs,
err |= __copy_from_user(regs, sc->regs.gpr, 32 * sizeof(unsigned long));
err |= __copy_from_user(&regs->pc, &sc->regs.pc, sizeof(unsigned long));
err |= __copy_from_user(&regs->sr, &sc->regs.sr, sizeof(unsigned long));
err |= __copy_from_user(&regs->fpcsr, &sc->fpcsr, sizeof(unsigned long));
err |= __copy_from_user(&current->thread.fpcsr, &sc->fpcsr, sizeof(unsigned long));

/* make sure the SM-bit is cleared so user-mode cannot fool us */
regs->sr &= ~SPR_SR_SM;
Expand All @@ -67,6 +68,9 @@ static int restore_sigcontext(struct pt_regs *regs,
* use it now ?
*/

/* Before returning to userspace restore the FPU */
restore_fpu(current);

return err;
}

Expand Down Expand Up @@ -111,14 +115,17 @@ static int setup_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
{
int err = 0;

/* Sync the user FPU state so we can copy to sigcontext */
save_fpu(current);

/* copy the regs */
/* There should be no need to save callee-saved registers here...
* ...but we save them anyway. Revisit this
*/
err |= __copy_to_user(sc->regs.gpr, regs, 32 * sizeof(unsigned long));
err |= __copy_to_user(&sc->regs.pc, &regs->pc, sizeof(unsigned long));
err |= __copy_to_user(&sc->regs.sr, &regs->sr, sizeof(unsigned long));
err |= __copy_to_user(&sc->fpcsr, &regs->fpcsr, sizeof(unsigned long));
err |= __copy_to_user(&sc->fpcsr, &current->thread.fpcsr, sizeof(unsigned long));

return err;
}
Expand Down
14 changes: 9 additions & 5 deletions arch/openrisc/kernel/traps.c
Expand Up @@ -31,6 +31,7 @@
#include <linux/uaccess.h>

#include <asm/bug.h>
#include <asm/fpu.h>
#include <asm/io.h>
#include <asm/processor.h>
#include <asm/unwinder.h>
Expand Down Expand Up @@ -84,9 +85,8 @@ void show_registers(struct pt_regs *regs)
in_kernel = 0;

printk("CPU #: %d\n"
" PC: %08lx SR: %08lx SP: %08lx FPCSR: %08lx\n",
smp_processor_id(), regs->pc, regs->sr, regs->sp,
regs->fpcsr);
" PC: %08lx SR: %08lx SP: %08lx\n",
smp_processor_id(), regs->pc, regs->sr, regs->sp);
printk("GPR00: %08lx GPR01: %08lx GPR02: %08lx GPR03: %08lx\n",
0L, regs->gpr[1], regs->gpr[2], regs->gpr[3]);
printk("GPR04: %08lx GPR05: %08lx GPR06: %08lx GPR07: %08lx\n",
Expand Down Expand Up @@ -181,7 +181,10 @@ asmlinkage void unhandled_exception(struct pt_regs *regs, int ea, int vector)
asmlinkage void do_fpe_trap(struct pt_regs *regs, unsigned long address)
{
int code = FPE_FLTUNK;
unsigned long fpcsr = regs->fpcsr;
unsigned long fpcsr;

save_fpu(current);
fpcsr = current->thread.fpcsr;

if (fpcsr & SPR_FPCSR_IVF)
code = FPE_FLTINV;
Expand All @@ -195,7 +198,8 @@ asmlinkage void do_fpe_trap(struct pt_regs *regs, unsigned long address)
code = FPE_FLTRES;

/* Clear all flags */
regs->fpcsr &= ~SPR_FPCSR_ALLF;
current->thread.fpcsr &= ~SPR_FPCSR_ALLF;
restore_fpu(current);

force_sig_fault(SIGFPE, code, (void __user *)regs->pc);
}
Expand Down

0 comments on commit 14f89b1

Please sign in to comment.