Date: Mon, 15 Nov 1999 12:46:55 +0100 From: Marcel Moolenaar <marcel@scc.nl> To: Martin Cracauer <cracauer@cons.org> Cc: Bruce Evans <bde@zeta.org.au>, freebsd-arch@freebsd.org Subject: Re: cvs commit: src/sys/i386/include signal.h Message-ID: <382FF2AF.545C8C81@scc.nl> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
Martin Cracauer wrote:
>
> Marcel, Bruce,
>
> I moved this to -arch, since we have to make some long-term decisions
> here:
>
> 1) Does FreeBSD prefer to pass information like this as unnamed array
> of bytes or as structs with proper fields?
>
> 2) How big may we make struct sigcontext? The 512 bytes that is named
> for now is quite a big step from the 100 bytes it had before FPU
> state savings).
>
> I have a strong preference to anser 1) with "yes".
Do you want A or B? ... yes.
Euh, what is it? :-)
I'd say that we should find the balance. If more detail requires us to
rearrange a lot of the headers in /sys/<ARCH>/include and /sys/sys, then
I'd say we should use arrays.
> In <382FDBE2.D075594@scc.nl>, Marcel Moolenaar wrote:
> > Martin Cracauer wrote:
> >
> > > > Martin implemented saving it before you complicated things by changing
> > > > the signal handling :-). I didn't like it because it moved the definitions
> > > > of i386 FP structs to the wrong place (<machine/signal.h>). It's not
> > > > clear that there is a right place.
> > >
> > > The reasoning is like this:
> > >
> > > 0) If the state of the FPU is passed to a signal handler, it is
> > > preferrable to do so in a struct with proper names, not just an
> > > array of bytes.
> >
> > I disagree. Signal handlers don't use context information for (wild
> > guess) 99.9% of the time. Bloating signalling definitions with details
> > of the many different hardware configurations (even within a single
> > architecture) is not the way to go.
>
> Maybe we have a different definition of "bloat", but since the size is
> the same, this doesn't count as "bloat for me.
The bloat is in what you get when you include <signal.h>
> > No, it's
> > better to be vague in struct sigcontext and let the program use a cast
> > when it wants to dig in the gory details of the machine state.
>
> As I said, I disagree. FreeBSD/alpha also exposes its FPU state in
> struct sigcontext (although the FPU is simpler).
Sort of:
long sc_xxx2[3]; /* sc_fp_trap_pc, sc_fp_trigger_sum,
sc_fp_trigger_inst */
> > > 1) Applications that use signal handlers with extended parameters
> > > (SA_SIGINFO or old BSD-style three arguments) are supposed to
> > > include <signal.h>, but nothing else.
> >
> > SUSv2 defines sa_sigaction as:
> > void (*) (int, siginfo_t*, void*) sa_sigaction;
> >
> > Notice the `void*' where the `ucontext_t*' is supposed to be. SUSv2
> > simply says that the handler may cast the `void*' to get to the gory
> > details and ucontext_t itself contains an abstract type (mcontext_t)
> > that represents machine specific state.
>
> When I implemented SA_SIGINFO, you could get it in a type-save way as
> a member of the second argument. This capability has been removed by
> your newer signal changes.
Yes, because ucontext_t is not part of siginfo_t and adding it there
only creates unnecessary dependencies and pitfalls. Not to mention the
size increase and the shear unportability of the approach.
You don't want to implement RT signals with a bloated siginfo_t...
> > Point: You don't want low-level details embedded in high-level
> > definitions.
>
> We pass the FPU state some way or another. If we pass it, we should do
> it as type-save as possible.
As is *reasonably* possible.
> A proper description of the i386 FPU is already present in the kernel
> sources and used at other places, I just want to reuse the same
> definition in a ny place where is FPU is represented in a stored
> state.
A change in how the state is saved would then automaticly result in an
interface change while this should not be the case (I should know, I did
exactly the same thing :-)
> > > struct save87 is now 176 bytes, Marcel currently reserves 180. struct
> > > sigcontext without FPU state is 100 bytes, so we are at 276 bytes at
> > > least.
> > >
> > > Would it be unreasonable to bump it to 512 bytes to be done with it
> > > once and for all? Might have positive effects on cache behaviour as
> > > well.
> >
> > In that case, make ucontext_t 512 bytes and scale struct sigcontext
> > accordingly.
> >
> > > We may also need an way to tell an application at runtime how far the
> > > struct is filled.
> >
> > You need exactly 1 bit to tell the application if the FPU state is valid
> > or not, right?
>
> I don't think that is sufficient in the general case.
If you refer to the general case here, then also do it for the saved
state. In the general case, that may be very incompatible for each
different FPU and or emulator, which is not limited to what we have now.
> We don't want to know whether a kernel that is capable of storing the
> FPU state did so (it does so in any case). We want to know whether the
> kernel is new enough that is even knows that the FPU state might be
> stored here. If it doesn't it can't set this bit.
Thus, a new kernel sets a bit where an old kernel does not set it...
> Longer explanation:
>
> In a kernel version 1 'struct sigcontext' has a number of fields
> making 100 bytes and padding to 512 bytes.
>
> struct sigcontext {
> char bla[100];
> char padding[412];
> };
>
> In version 2, the FPU state is added and padding adjusted:
> struct sigcontext {
> char bla[100];
> char fpu[200]
> char padding[212];
> };
>
> In version 3, some other state is added:
> struct sigcontext {
> char bla[100];
> char fpu[200]
> cht otherstate[12]
> char padding[200];
> };
>
> Now, how can an application tell what is filled and what is not? A
> #define in the header file isn't suitable, since a binary compiled on
> one version might be moved to a didfferent kernel.
If a new kernel breaks old binaries, then it's not backwards compatible
:-)
--
Marcel Moolenaar mailto:marcel@scc.nl
SCC Internetworking & Databases http://www.scc.nl/
The FreeBSD project mailto:marcel@FreeBSD.org
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?382FF2AF.545C8C81>