Date: Sat, 4 Dec 1999 23:41:45 +0100 From: Martin Cracauer <cracauer@cons.org> To: Marcel Moolenaar <marcel@scc.nl> Cc: Bruce Evans <bde@zeta.org.au>, freebsd-arch@freebsd.org Subject: Re: cvs commit: src/sys/i386/include signal.h Message-ID: <19991204234145.A1221@cons.org> In-Reply-To: <382FF2AF.545C8C81@scc.nl>; from Marcel Moolenaar on Mon, Nov 15, 1999 at 12:46:55PM %2B0100 References: <382E8A1B.B7D9B7F0@scc.nl> <Pine.BSF.4.10.9911142303390.21828-100000@alphplex.bde.org> <19991115095729.A27507@cons.org> <382FDBE2.D075594@scc.nl> <19991115115552.A27870@cons.org> <382FF2AF.545C8C81@scc.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
--J2SCkAp4GZ/dPZZf Content-Type: text/plain; charset=us-ascii I thought over the issue of mcontext and the need for additional status fields that the application's signal handler may use. The following proposal allows both compile-time and tun-time checks for the presense of fields in mcontext_t. Also mcontext_t is proposed to be bumped to 512 bytes for possible future extensions (because changing the size once 4.0 is out is *bad*). ********************************************************************** It is important that we agree on the scheme to use and the size of mcontext_t before the 4.0 codefreeze (15.12.1999)! ***************************************************************** Example usage of the field presence test scheme: An application wants to use the FPU status word saved in mcontext_t.save87.sv_ex_sw. #ifdef MCONTEXT_FPU if (mcontext->extensions & MCONTEXT_FPU) foo = mcontext->save87.sv_ex_sw; #endif That way, the following error will be caught correctly: - Application is being compiled on a system that doesn't have the field in mcontext -> #ifdef will be nil, field will not be accessed. - Application was compiled on a system which has save87 in mcontext, but is running on an older system that hasn't. The runtime test will be nil, the field will not be accessed. If the application just accessed the field, it would either coredump or get junk (probably contents of a different field). - Application was compiled and is running on a system that has save87 in struct mcontext, but the save87 space is not filled in this situation. The runtime test will fail. This would allow us to introduce a kernel option not to save the FPU over signal handlers. This runtime test capability may not look important for the FPU, but for other extensions that are more processor-specific it may make sense. What is not covered by this scheme is using the same space in mcontext for different purposes. This may be useful for example when some space is allowed *either* by some Pentium-III specific set of registers *or* some AMD-specific set of registers. My scheme would require that space for both is always allocated but only one (or none) is filled. To solve this, the scheme could be changed from the bitmap test to a set of pointers for the different fields (which may be NULL). #ifdef MCONTEXT_FPU if (mcontext->save87 != NULL) foo = mcontext->save87->sv_ex_sw; #endif That has higher runtime overhead (not so much for the application, but for the kernel at signal send time). And it will only save space when we actually have some big status blocks to share space, since we would need 16 (or so) pointers as an initial overhead (compared to 16 bits in the constant-place solution). As least the alpha folks will eat us alive, IMHO. Decisions in order of importance: - Do you accept bumping mcontext_t to 512 bytes? - Do you like the bitmap scheme? - If you would prefer the pointer scheme, you probably have some processor-specific things that may end up in mcontext. Tell us! The implementation I appended should work, but I'm sure some will hate the choice of field and macro names. These may be changed later, the former decisions are more urgent. The implementation appended doesn't really support access to the FPU for a different reason. No good scheme has been found so far to move the struct save87 definition into scope of the user's signal handler. But the appended diff does the actual preserving of FPU state over the signal handler (which is required for ANSI C). Comments welcome, of course Martin -- %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% Martin Cracauer <cracauer@cons.org> http://www.cons.org/cracauer/ Tel.: (private) +4940 5221829 Fax.: (private) +4940 5228536 --J2SCkAp4GZ/dPZZf Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="diff.savefpu-4" Index: i386/i386/machdep.c =================================================================== RCS file: /home/CVS-FreeBSD/src/sys/i386/i386/machdep.c,v retrieving revision 1.379 diff -c -r1.379 machdep.c *** machdep.c 1999/11/24 01:02:58 1.379 --- machdep.c 1999/12/04 21:34:09 *************** *** 632,637 **** --- 632,643 ---- sf.sf_uc.uc_mcontext.mc_gs = rgs(); bcopy(regs, &sf.sf_uc.uc_mcontext.mc_fs, sizeof(struct trapframe)); + /* Fill additional fields in mcontext. */ + sf.sf_uc.uc_mcontext.mc_extensions = 0; + bcopy(&p->p_addr->u_pcb.pcb_savefpu, sf.sf_uc.uc_mcontext.mc_fpregs, + sizeof(sf.sf_uc.uc_mcontext.mc_fpregs) + + sizeof(sf.sf_uc.uc_mcontext.mc_fpemul)); + /* Allocate and validate space for the signal handler context. */ if ((p->p_flag & P_ALTSTACK) != 0 && !oonstack && SIGISMEMBER(psp->ps_sigonstack, sig)) { *************** *** 959,964 **** --- 965,975 ---- } bcopy(&ucp->uc_mcontext.mc_fs, regs, sizeof(struct trapframe)); } + + /* Restore FPU state */ + bcopy(ucp->uc_mcontext.mc_fpregs, &p->p_addr->u_pcb.pcb_savefpu, + sizeof(ucp->uc_mcontext.mc_fpregs) + + sizeof(ucp->uc_mcontext.mc_fpemul)); if (ucp->uc_mcontext.mc_onstack & 1) p->p_sigstk.ss_flags |= SS_ONSTACK; Index: i386/include/signal.h =================================================================== RCS file: /home/CVS-FreeBSD/src/sys/i386/include/signal.h,v retrieving revision 1.12 diff -c -r1.12 signal.h *** signal.h 1999/11/12 13:52:11 1.12 --- signal.h 1999/12/04 21:39:37 *************** *** 106,118 **** int sc_efl; int sc_esp; int sc_ss; ! /* ! * XXX FPU state is 27 * 4 bytes h/w, 1 * 4 bytes s/w (probably not ! * needed here), or that + 16 * 4 bytes for emulators (probably all ! * needed here). The "spare" bytes are mostly not spare. ! */ ! int sc_fpregs[28]; /* machine state (FPU): */ ! int sc_spare[17]; }; #define sc_sp sc_esp --- 106,117 ---- int sc_efl; int sc_esp; int sc_ss; ! ! int sc_extensions; ! ! int sc_fpregs[28]; ! int sc_fpemul[16]; ! int __morespare__[63]; }; #define sc_sp sc_esp Index: i386/include/ucontext.h =================================================================== RCS file: /home/CVS-FreeBSD/src/sys/i386/include/ucontext.h,v retrieving revision 1.4 diff -c -r1.4 ucontext.h *** ucontext.h 1999/10/11 20:33:09 1.4 --- ucontext.h 1999/12/04 21:36:41 *************** *** 58,65 **** int mc_esp; /* machine state */ int mc_ss; int mc_fpregs[28]; /* env87 + fpacc87 + u_long */ ! int __spare__[17]; } mcontext_t; #endif /* !_MACHINE_UCONTEXT_H_ */ --- 58,84 ---- int mc_esp; /* machine state */ int mc_ss; + /* + * The following fields may or may not be present. An application + * will be able to compile-time test for the presence of + * individual field using #ifdefs and runtime-test using the bitmap + * in mc_extensions. + * + * While the infrastructure to use these fields from an application's + * signal handler is not yet implemented, we already reserve the space + * for the runtime-test bitmap. + * + * This is upward compatible since none of the individual #defines is + * defined and mc_extensions is zero for now. + */ + + /* Allow access to field mc_extensions. */ + #define MCONTEXT_EXTENSIONS + int mc_extensions; /* Bitmap of filled fields below */ + int mc_fpregs[28]; /* env87 + fpacc87 + u_long */ ! int mc_fpemul[16]; /* Additional FPU emulator status */ ! int __morespare__[63]; /* For later additions, total = 512 */ } mcontext_t; #endif /* !_MACHINE_UCONTEXT_H_ */ --J2SCkAp4GZ/dPZZf-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?19991204234145.A1221>