Date: Mon, 18 Jan 2016 14:01:35 -0800 From: John Baldwin <jhb@freebsd.org> To: Nathan Whitehorn <nwhitehorn@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r279189 - in head/sys/powerpc: aim fpu include powerpc Message-ID: <3368361.OPgXEjftIo@ralph.baldwin.cx> In-Reply-To: <569D5977.8050402@freebsd.org> References: <201502222140.t1MLeSFg075690@svn.freebsd.org> <11668266.h2pHzfGYxF@ralph.baldwin.cx> <569D5977.8050402@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, January 18, 2016 01:30:31 PM Nathan Whitehorn wrote: > > On 01/18/16 12:49, John Baldwin wrote: > > On Sunday, February 22, 2015 09:40:28 PM Nathan Whitehorn wrote: > >> Author: nwhitehorn > >> Date: Sun Feb 22 21:40:27 2015 > >> New Revision: 279189 > >> URL: https://svnweb.freebsd.org/changeset/base/279189 > >> > >> Log: > >> Kernel support for the Vector-Scalar eXtension (VSX) found on the POWER7 > >> and POWER8. This instruction set unifies the 32 64-bit scalar floating > >> point registers with the 32 128-bit vector registers into a single bank > >> of 64 128-bit registers. Kernel support mostly amounts to saving and > >> restoring the wider version of the floating point registers and making > >> sure that both scalar FP and vector registers are enabled once a VSX > >> instruction is executed. get_mcontext() and friends currently cannot > >> see the high bits, which will require a little more work. > >> > >> As the system compiler (GCC 4.2) does not support VSX, making use of this > >> from userland requires either newer GCC or clang. > >> > >> Relnotes: yes > >> Sponsored by: FreeBSD Foundation > >> > >> Modified: > >> head/sys/powerpc/aim/trap.c > >> head/sys/powerpc/aim/trap_subr64.S > >> head/sys/powerpc/fpu/fpu_emu.c > >> head/sys/powerpc/fpu/fpu_explode.c > >> head/sys/powerpc/include/cpu.h > >> head/sys/powerpc/include/pcb.h > >> head/sys/powerpc/include/psl.h > >> head/sys/powerpc/include/reg.h > >> head/sys/powerpc/include/trap.h > >> head/sys/powerpc/powerpc/cpu.c > >> head/sys/powerpc/powerpc/db_trace.c > >> head/sys/powerpc/powerpc/exec_machdep.c > >> head/sys/powerpc/powerpc/fpu.c > >> > >> Modified: head/sys/powerpc/include/reg.h > >> ============================================================================== > >> --- head/sys/powerpc/include/reg.h Sun Feb 22 21:32:57 2015 (r279188) > >> +++ head/sys/powerpc/include/reg.h Sun Feb 22 21:40:27 2015 (r279189) > >> @@ -20,7 +20,10 @@ struct reg { > >> > >> /* Must match pcb.pcb_fpu */ > >> struct fpreg { > >> - double fpreg[32]; > >> + union { > >> + double fpr; > >> + uint64_t vsr[2]; > >> + } fpreg[32]; > >> double fpscr; > >> }; > > This breaks the ABI of struct fpreg which changes the format of coredumps. > > It also breaks the ABI of older versions of programs (such as debuggers) that > > use ptrace(PT_GETFPREGS) (the kernel will now overflow the user-allocated > > buffer if a debugger built on 10.x is run under an 11.0 kernel, and > > PT_SETFPREGS is going to also buffer overflow and store random garbage in the > > FP regs). Did you mean to alter the structure's layout? > > > > I can maybe fix upstream gdb to cope with either size, but it's a bit of a > > PITA. In particular, gdb assumes that all the floating point registers in > > struct fpreg are in a packed array (so it can use starting_offset + 8 * n to > > extract register 'n' and only the initial offset is something that different > > platform targets have to configure), so fixing this means having to add a lot > > of special cases and duplicate code that is otherwise shared across > > platforms. > > > > Hmm, I see that you preserved the ABI of mcontext by just copying the > > doubles. I think you should do the same for 'struct fpreg' restoring its > > ABI and we can use MD ptrace requests to fetch the VSX state. This is what > > Linux effectively does. > > > > Drat. I hadn't appreciated that fpreg was part of the public API when I > wrote this code! It would be easy enough to follow what mcontext does, > and that seems like the right solution, but I won't have time for a > couple of weeks at least. If you do have time, I would be happy to > review and/or polish patches before that. > > Thanks for catching this! No worries, I was testing my GDB threading patches on ppc (which do work, whee!) and started getting warnings about ".reg2" being missized. Building kernels is dreadfully slow in qemu so it will take a while to test, but I'll see what I can come up with. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3368361.OPgXEjftIo>