Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 18 Jul 2012 08:09:58 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Bruce Evans <brde@optusnet.com.au>
Cc:        Mark Linimon <linimon@lonesome.com>, freebsd-amd64@freebsd.org
Subject:   Re: amd64/169927: siginfo, si_code for fpe errors when error occurs using the SSE math processor
Message-ID:  <20120718050958.GQ2676@deviant.kiev.zoral.com.ua>
In-Reply-To: <20120718140523.X1862@besplex.bde.org>
References:  <201207172150.q6HLoFvB096742@freefall.freebsd.org> <20120718103210.E834@besplex.bde.org> <20120718035220.GO2676@deviant.kiev.zoral.com.ua> <20120718140523.X1862@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--s3R87C3fwYeCSZ0b
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Jul 18, 2012 at 02:36:16PM +1000, Bruce Evans wrote:
> On Wed, 18 Jul 2012, Konstantin Belousov wrote:
>=20
> >On Wed, Jul 18, 2012 at 10:32:23AM +1000, Bruce Evans wrote:
> >>OK.  Also, you can tell instruction that faulted, provided there are
> >>no spurious faults.  I think spurious faults can only occur for IRQ13
> >>exception handling which was never supported for amd64 and is no longer
> >>supported for i386.  My old test program that is broken by not clobberi=
ng
> >>the status was mainly for finding and fixing such spurious faults.
> >>I never owned an i387 or i486SX and had to break i486 exception handling
> >>to test IRQ13.
> >486SX+487 never used irq13 as well, since 487 was the 'full' DX package,
> >which turned off SX socket.
>=20
> The 487 is still a separate package.  I thought it had to use IRQ13 to
> communicate with the 486SX since the NE# (?) and ERR# (?) pins only
> work for a single package.  Do they actually work for the separate
> package?
487 packaged the whole CPU, it is not add-on FPU like 387.

486SX was turned off (electrically, AFAIR) when 487 was installed.
>=20
> >>Just noticed another style/efficiency bug: this should use curpcb,
> >>not curthread->td_pcb.  I don't like the curpcb global, but as
> >>long as it exists, it should be used.  Every time I looked at
> >>removing this global, it seemed better to keep it.  C code can
> >>easily use curthread->td_pcb instead of curpcb, and this is faster
> >>if curthread is cached.  But CURPCB is easier to use in asm.
> >I changed to use curpcb (its use is indeed mainly for asm context switch
> >code). I do want to use pcb_save though, see below.
> >>...
> >>I now quote your newer patch for the fputrap_x87() cleanup:
> >>
> >>>@@ -531,8 +534,9 @@ static char fpetable[128] =3D {
> >>>  * solution for signals other than SIGFPE.
> >>>  */
> >>> int
> >>>-fputrap()
> >>>+fputrap_x87(void)
> >>> {
> >>>+	struct savefpu *pcb_save;
> >>
> >>No need for this before or after.  Let the compiler cache it.  Just use
> >>curpcb now.
> >The curpcb access shall be spelled as PCPU_GET(curpcb). Please note that
> >compiler is not allowed to cache the accesses, since there is __asm
> >__volatile expression to indirect through %gs-prefixed read.
>=20
> Fix curpcb then.  curthread was de-pessimized to use __pure2 and to not
> use __volatile.  I could never this to work to cache curthread when I=20
> ried it, but it apparently works in -current.
>=20
> curpcb is no more fragile than curthread without the __volatile.
> Especially in fputrap*() where they are accessed under critical_enter().
> They are switched at the same time.  Any locking in pcpu access functions
> for them would be useless, since it goes away when the function returns.
> Apparently they are sufficiently locked by the logic of the code
> (unlike fpucurthread, which requires the critical_enter()).
>=20
> curthread is the only pcpu variable important enough to have a special
> access function in amd64 pcpu.h.  curpcb is not very important, but
> spelling it curpcb =3D __curpcb() instead of PCPU_GET(curpcb) is
> simpler for clients as well as faster.
>=20
> >diff --git a/sys/amd64/amd64/fpu.c b/sys/amd64/amd64/fpu.c
> >...
>=20
> OK except for the curpcb caching and the fnclex removal, which should
> be done separately.

curpcb() could be implemented like this for amd64 only:

diff --git a/sys/amd64/include/pcb.h b/sys/amd64/include/pcb.h
index 22cbbe2..3417c52 100644
--- a/sys/amd64/include/pcb.h
+++ b/sys/amd64/include/pcb.h
@@ -144,6 +144,17 @@ void	makectx(struct trapframe *, struct pcb *);
 int	savectx(struct pcb *) __returns_twice;
 void	resumectx(struct pcb *);
=20
+static __inline __pure2 struct pcb *
+__curpcb(void)
+{
+	struct pcb *pcb;
+
+	__asm("movq %%gs:%1,%0" : "=3Dr" (pcb)
+	    : "i" (offsetof(struct pcpu, pc_curpcb)));
+	return (pcb);
+}
+#define	curpcb		(__curpcb())
+
 #endif
=20
 #endif /* _AMD64_PCB_H_ */
diff --git a/sys/sys/user.h b/sys/sys/user.h
index accb7c3..ab1c7a7 100644
--- a/sys/sys/user.h
+++ b/sys/sys/user.h
@@ -35,6 +35,7 @@
 #ifndef _SYS_USER_H_
 #define _SYS_USER_H_
=20
+#include <sys/pcpu.h>
 #include <machine/pcb.h>
 #ifndef _KERNEL
 /* stuff that *used* to be included by user.h, or is now needed */

Please note the location in pcb.h an not in machine/pcpu.h, where it
cannot work for technical reasons (struct pcpu is not defined yet).
This should be committed separetely, together with the pass over amd64/
to convert PCPU_GET(curpcb) into curpcb().

Do you agree with committing the PR fix as is and adding the curpcb() later=
 ?
And removing fnclex() later (it seems I convinced enough for its removal).


--s3R87C3fwYeCSZ0b
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (FreeBSD)

iEYEARECAAYFAlAGRSYACgkQC3+MBN1Mb4hZqgCfT2huy84BV5OvSd3MX/KFG6kr
+twAoOxaFP3EY9GfZSG2Nso+Rka0Qi2V
=bPlt
-----END PGP SIGNATURE-----

--s3R87C3fwYeCSZ0b--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120718050958.GQ2676>