From b5125ceda9d70c44ef3139a5f5232fa9b2a67569 Mon Sep 17 00:00:00 2001 From: miod Date: Fri, 29 Mar 2024 21:14:31 +0000 Subject: [PATCH] Clean the fpu trap code: - since there are no hardware fpu operation queues on real sparc64 hardware, don't bother declaring the relevant struct and fields. - when an fpu instruction needs to be emulated, pass it directly to fpu_cleanup rather than fake its appearance in the fpu queue. While there, also pass the ready-to-use union sigval computed in trap() in case a signal needs to be delivered. ok claudio@ kettenis@ --- sys/arch/sparc64/fpu/fpu.c | 98 ++++++++-------------- sys/arch/sparc64/fpu/fpu_extern.h | 5 +- sys/arch/sparc64/include/reg.h | 19 +---- sys/arch/sparc64/sparc64/emul.c | 5 +- sys/arch/sparc64/sparc64/genassym.cf | 6 +- sys/arch/sparc64/sparc64/process_machdep.c | 9 +- sys/arch/sparc64/sparc64/trap.c | 38 +++------ 7 files changed, 58 insertions(+), 122 deletions(-) diff --git a/sys/arch/sparc64/fpu/fpu.c b/sys/arch/sparc64/fpu/fpu.c index 537aae8f3e2..1bf84272ad4 100644 --- a/sys/arch/sparc64/fpu/fpu.c +++ b/sys/arch/sparc64/fpu/fpu.c @@ -1,4 +1,4 @@ -/* $OpenBSD: fpu.c,v 1.25 2024/03/29 21:08:10 miod Exp $ */ +/* $OpenBSD: fpu.c,v 1.26 2024/03/29 21:14:31 miod Exp $ */ /* $NetBSD: fpu.c,v 1.11 2000/12/06 01:47:50 mrg Exp $ */ /* @@ -143,14 +143,14 @@ fpu_dumpstate(struct fpstate *fs) #define X8(x) X4(x),X4(x) #define X16(x) X8(x),X8(x) -static char cx_to_trapx[] = { +static const char cx_to_trapx[] = { X1(FSR_NX), X2(FSR_DZ), X4(FSR_UF), X8(FSR_OF), X16(FSR_NV) }; -static u_char fpu_codes[] = { +static const u_char fpu_codes[] = { X1(FPE_FLTINEX_TRAP), X2(FPE_FLTDIV_TRAP), X4(FPE_FLTUND_TRAP), @@ -158,7 +158,7 @@ static u_char fpu_codes[] = { X16(FPE_FLTOPERR_TRAP) }; -static int fpu_types[] = { +static const int fpu_types[] = { X1(FPE_FLTRES), X2(FPE_FLTDIV), X4(FPE_FLTUND), @@ -180,64 +180,48 @@ fpu_fcopy(u_int *src, u_int *dst, int type) } /* - * The FPU gave us an exception. Clean up the mess. Note that the - * fp queue can only have FPops in it, never load/store FP registers - * nor FBfcc instructions. Experiments with `crashme' prove that - * unknown FPops do enter the queue, however. + * The FPU gave us an exception. Clean up the mess. */ void -fpu_cleanup(struct proc *p, struct fpstate *fs) +fpu_cleanup(struct proc *p, struct fpstate *fs, union instr instr, + union sigval sv) { int i, fsr = fs->fs_fsr, error; - union instr instr; - union sigval sv; struct fpemu fe; - sv.sival_int = p->p_md.md_tf->tf_pc; /* XXX only approximate */ - switch ((fsr >> FSR_FTT_SHIFT) & FSR_FTT_MASK) { - case FSR_TT_NONE: -#if 0 - /* XXX I'm not sure how we get here, but ignoring the trap */ - /* XXX seems to work in my limited tests */ - /* XXX More research to be done =) */ - panic("fpu_cleanup 1"); /* ??? */ -#else - printf("fpu_cleanup 1\n"); +#ifdef DEBUG + printf("fpu_cleanup: invoked although no exception\n"); #endif - break; - + return; case FSR_TT_IEEE: if ((i = fsr & FSR_CX) == 0) panic("fpu ieee trap, but no exception"); trapsignal(p, SIGFPE, fpu_codes[i - 1], fpu_types[i - 1], sv); - break; /* XXX should return, but queue remains */ - + return; case FSR_TT_UNFIN: - if (fs->fs_qsize == 0) { - printf("fpu_cleanup: unfinished fpop"); - /* The book says reexecute or emulate. */ + if (instr.i_int == 0) { +#ifdef DEBUG + printf("fpu_cleanup: unfinished fpop\n"); +#endif return; } break; case FSR_TT_UNIMP: - if (fs->fs_qsize == 0) - panic("fpu_cleanup: unimplemented fpop"); + if (instr.i_int == 0) + panic("fpu_cleanup: unimplemented fpop without insn"); break; - case FSR_TT_SEQ: panic("fpu sequence error"); /* NOTREACHED */ - case FSR_TT_HWERR: log(LOG_ERR, "fpu hardware error (%s[%d])\n", p->p_p->ps_comm, p->p_p->ps_pid); uprintf("%s[%d]: fpu hardware error\n", p->p_p->ps_comm, p->p_p->ps_pid); trapsignal(p, SIGFPE, -1, FPE_FLTINV, sv); /* ??? */ - goto out; - + return; default: printf("fsr=0x%x\n", fsr); panic("fpu error"); @@ -245,36 +229,24 @@ fpu_cleanup(struct proc *p, struct fpstate *fs) /* emulate the instructions left in the queue */ fe.fe_fpstate = fs; - for (i = 0; i < fs->fs_qsize; i++) { - instr.i_int = fs->fs_queue[i].fq_instr; - if (instr.i_any.i_op != IOP_reg || - (instr.i_op3.i_op3 != IOP3_FPop1 && - instr.i_op3.i_op3 != IOP3_FPop2)) - panic("bogus fpu queue"); - error = fpu_execute(p, &fe, instr); - switch (error) { - - case 0: - continue; - - case FPE: - trapsignal(p, SIGFPE, - fpu_codes[(fs->fs_fsr & FSR_CX) - 1], - fpu_types[(fs->fs_fsr & FSR_CX) - 1], sv); - break; - - case NOTFPU: - trapsignal(p, SIGILL, 0, ILL_COPROC, sv); - break; - - default: - panic("fpu_cleanup 3"); - /* NOTREACHED */ - } - /* XXX should stop here, but queue remains */ + if (instr.i_any.i_op != IOP_reg || + (instr.i_op3.i_op3 != IOP3_FPop1 && + instr.i_op3.i_op3 != IOP3_FPop2)) + panic("bogus fpu instruction to emulate"); + error = fpu_execute(p, &fe, instr); + switch (error) { + case 0: + break; + case FPE: + trapsignal(p, SIGFPE, + fpu_codes[(fs->fs_fsr & FSR_CX) - 1], + fpu_types[(fs->fs_fsr & FSR_CX) - 1], sv); + break; + case NOTFPU: + default: + trapsignal(p, SIGILL, 0, ILL_COPROC, sv); + break; } -out: - fs->fs_qsize = 0; } /* diff --git a/sys/arch/sparc64/fpu/fpu_extern.h b/sys/arch/sparc64/fpu/fpu_extern.h index 8f7c907fc0d..b54ed041bd1 100644 --- a/sys/arch/sparc64/fpu/fpu_extern.h +++ b/sys/arch/sparc64/fpu/fpu_extern.h @@ -1,4 +1,4 @@ -/* $OpenBSD: fpu_extern.h,v 1.8 2024/03/29 21:07:11 miod Exp $ */ +/* $OpenBSD: fpu_extern.h,v 1.9 2024/03/29 21:14:31 miod Exp $ */ /* $NetBSD: fpu_extern.h,v 1.4 2000/08/03 18:32:08 eeh Exp $ */ /*- @@ -36,9 +36,10 @@ struct trapframe; union instr; struct fpemu; struct fpn; +union sigval; /* fpu.c */ -void fpu_cleanup(struct proc *, struct fpstate *); +void fpu_cleanup(struct proc *, struct fpstate *, union instr, union sigval); int fpu_emulate(struct proc *, struct trapframe *, struct fpstate *); int fpu_execute(struct proc *, struct fpemu *, union instr); diff --git a/sys/arch/sparc64/include/reg.h b/sys/arch/sparc64/include/reg.h index edbddc11baa..dc5e7bf42d1 100644 --- a/sys/arch/sparc64/include/reg.h +++ b/sys/arch/sparc64/include/reg.h @@ -1,4 +1,4 @@ -/* $OpenBSD: reg.h,v 1.8 2024/03/29 21:08:11 miod Exp $ */ +/* $OpenBSD: reg.h,v 1.9 2024/03/29 21:14:31 miod Exp $ */ /* $NetBSD: reg.h,v 1.8 2001/06/19 12:59:16 wiz Exp $ */ /* @@ -70,29 +70,12 @@ struct reg { /* * FP coprocessor registers. - * - * FP_QSIZE is the maximum coprocessor instruction queue depth - * of any implementation on which the kernel will run. David Hough: - * ``I'd suggest allowing 16 ... allowing an indeterminate variable - * size would be even better''. Of course, we cannot do that; we - * need to malloc these. - * - * XXXX UltraSPARC processors don't implement a floating point queue. */ -#define FP_QSIZE 16 -#define ALIGNFPSTATE(f) ((struct fpstate *)(((long)(f))&(~BLOCK_ALIGN))) - -struct fp_qentry { - int *fq_addr; /* the instruction's address */ - int fq_instr; /* the instruction itself */ -}; struct fpstate { u_int fs_regs[64]; /* our view is 64 32-bit registers */ int64_t fs_fsr; /* %fsr */ int fs_gsr; /* graphics state reg */ - int fs_qsize; /* actual queue depth */ - struct fp_qentry fs_queue[FP_QSIZE]; /* queue contents */ }; /* diff --git a/sys/arch/sparc64/sparc64/emul.c b/sys/arch/sparc64/sparc64/emul.c index 0a3c89d80af..4989b4e5ae0 100644 --- a/sys/arch/sparc64/sparc64/emul.c +++ b/sys/arch/sparc64/sparc64/emul.c @@ -1,4 +1,4 @@ -/* $OpenBSD: emul.c,v 1.27 2022/10/21 18:55:42 miod Exp $ */ +/* $OpenBSD: emul.c,v 1.28 2024/03/29 21:14:31 miod Exp $ */ /* $NetBSD: emul.c,v 1.8 2001/06/29 23:58:40 eeh Exp $ */ /*- @@ -67,7 +67,7 @@ swap_quad(int64_t *p) int emul_qf(int32_t insv, struct proc *p, union sigval sv, struct trapframe *tf) { - extern struct fpstate initfpstate; + extern const struct fpstate initfpstate; struct fpstate *fs = p->p_md.md_fpstate; int64_t addr, buf[2]; union instr ins; @@ -125,7 +125,6 @@ emul_qf(int32_t insv, struct proc *p, union sigval sv, struct trapframe *tf) /* don't currently have an fpu context, get one */ fs = malloc(sizeof(*fs), M_SUBPROC, M_WAITOK); *fs = initfpstate; - fs->fs_qsize = 0; p->p_md.md_fpstate = fs; KERNEL_UNLOCK(); } else diff --git a/sys/arch/sparc64/sparc64/genassym.cf b/sys/arch/sparc64/sparc64/genassym.cf index 4a38fd32942..4432b9528c0 100644 --- a/sys/arch/sparc64/sparc64/genassym.cf +++ b/sys/arch/sparc64/sparc64/genassym.cf @@ -1,4 +1,4 @@ -# $OpenBSD: genassym.cf,v 1.41 2024/03/29 21:08:11 miod Exp $ +# $OpenBSD: genassym.cf,v 1.42 2024/03/29 21:14:31 miod Exp $ # $NetBSD: genassym.cf,v 1.23 2001/08/08 00:09:30 eeh Exp $ # @@ -162,10 +162,6 @@ struct fpstate member fs_regs member fs_fsr member fs_gsr -member fs_qsize -member fs_queue -define FS_SIZE sizeof(struct fpstate) -export FSR_QNE export FPRS_FEF export FPRS_DU export FPRS_DL diff --git a/sys/arch/sparc64/sparc64/process_machdep.c b/sys/arch/sparc64/sparc64/process_machdep.c index 8355962133e..518fa878b02 100644 --- a/sys/arch/sparc64/sparc64/process_machdep.c +++ b/sys/arch/sparc64/sparc64/process_machdep.c @@ -1,4 +1,4 @@ -/* $OpenBSD: process_machdep.c,v 1.14 2022/10/21 18:55:42 miod Exp $ */ +/* $OpenBSD: process_machdep.c,v 1.15 2024/03/29 21:14:31 miod Exp $ */ /* $NetBSD: process_machdep.c,v 1.10 2000/09/26 22:05:50 eeh Exp $ */ /* @@ -71,7 +71,7 @@ #include #include -/* Unfortunately we need to convert v9 trapframe to v8 regs */ +/* Unfortunately we need to convert struct trapframe to struct reg */ int process_read_regs(struct proc *p, struct reg *regs) { @@ -95,8 +95,8 @@ process_read_regs(struct proc *p, struct reg *regs) int process_read_fpregs(struct proc *p, struct fpreg *regs) { - extern struct fpstate initfpstate; - struct fpstate *statep = &initfpstate; + extern const struct fpstate initfpstate; + const struct fpstate *statep = &initfpstate; /* NOTE: struct fpreg == struct fpstate */ if (p->p_md.md_fpstate) { @@ -160,7 +160,6 @@ process_write_fpregs(struct proc *p, struct fpreg *regs) /* copy in fregs */ bcopy(regs, p->p_md.md_fpstate, sizeof(struct fpreg)); - p->p_md.md_fpstate->fs_qsize = 0; return 0; } diff --git a/sys/arch/sparc64/sparc64/trap.c b/sys/arch/sparc64/sparc64/trap.c index 35bdd3eaa15..a015cf3825d 100644 --- a/sys/arch/sparc64/sparc64/trap.c +++ b/sys/arch/sparc64/sparc64/trap.c @@ -1,4 +1,4 @@ -/* $OpenBSD: trap.c,v 1.122 2024/03/29 21:09:04 miod Exp $ */ +/* $OpenBSD: trap.c,v 1.123 2024/03/29 21:14:31 miod Exp $ */ /* $NetBSD: trap.c,v 1.73 2001/08/09 01:03:01 eeh Exp $ */ /* @@ -87,8 +87,7 @@ * set, no matter how it is interpreted. Appendix N of the Sparc V8 document * seems to imply that we should do this, and it does make sense. */ -__asm(".align 64"); -struct fpstate initfpstate = { +const struct fpstate initfpstate = { { ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0, ~0 } }; @@ -475,26 +474,11 @@ dopanic: if (fs == NULL) { KERNEL_LOCK(); - /* NOTE: fpstate must be 64-bit aligned */ fs = malloc((sizeof *fs), M_SUBPROC, M_WAITOK); *fs = initfpstate; - fs->fs_qsize = 0; p->p_md.md_fpstate = fs; KERNEL_UNLOCK(); } - - /* - * We may have more FPEs stored up and/or ops queued. - * If they exist, handle them and get out. Otherwise, - * resolve the FPU state, turn it on, and try again. - * - * Ultras should never have a FPU queue. - */ - if (fs->fs_qsize) { - printf("trap: Warning fs_qsize is %d\n",fs->fs_qsize); - fpu_cleanup(p, fs); - break; - } if (fpproc != p) { /* we do not have it */ /* but maybe another CPU has it? */ fpusave_proc(p, 1); @@ -567,6 +551,9 @@ dopanic: case T_FP_IEEE_754: case T_FP_OTHER: + { + union instr ins; + /* * Clean up after a floating point exception. * fpu_cleanup can (and usually does) modify the @@ -582,20 +569,19 @@ dopanic: fpproc = NULL; intr_restore(s); /* tf->tf_psr &= ~PSR_EF; */ /* share_fpu will do this */ - if (type == T_FP_OTHER && p->p_md.md_fpstate->fs_qsize == 0) { + if (type == T_FP_OTHER) { /* - * Push the faulting instruction on the queue; + * Read the faulting instruction; * we might need to emulate it. */ - (void)copyinsn(p, pc, - &p->p_md.md_fpstate->fs_queue[0].fq_instr); - p->p_md.md_fpstate->fs_queue[0].fq_addr = (int *)pc; - p->p_md.md_fpstate->fs_qsize = 1; - } + (void)copyinsn(p, pc, &ins.i_int); + } else + ins.i_int = 0; ADVANCE; - fpu_cleanup(p, p->p_md.md_fpstate); + fpu_cleanup(p, p->p_md.md_fpstate, ins, sv); /* fpu_cleanup posts signals if needed */ break; + } case T_TAGOF: trapsignal(p, SIGEMT, 0, EMT_TAGOVF, sv); /* XXX code? */ -- 2.20.1