The Freescale e500v2 PowerPC core does not use a standard FPU.
Instead, it uses a Signal Processing Engine (SPE)--a DSP-style vector processor
unit, which doubles as a FPU. The PowerPC SPE ABI is incompatible with the
stock powerpc ABI, so a new MACHINE_ARCH was created to deal with this.
Additionaly, the SPE opcodes overlap with Altivec, so these are mutually
exclusive. Taking advantage of this fact, a new file, powerpc/booke/spe.c, was
created with the same function set as in powerpc/powerpc/altivec.c, so it
becomes effectively a drop-in replacement. setjmp/longjmp were modified to save
the upper 32-bits of the now-64-bit GPRs (upper 32-bits are only accessible by
the SPE).
Details
Details
- Reviewers
nwhitehorn marcel imp bdrewery - Group Reviewers
PowerPC - Commits
- rS307761: Create a new MACHINE_ARCH for Freescale PowerPC e500v2
This was lightly tested on a RouterBoard RB800, compiled against the
new ABI. Base system utilities (/bin/sh, /bin/ls, etc) still function
appropriately, the system is able to boot multiuser.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
So I didn't check to see if the SPE details are right. Traps and that sort of thing.
I'd like to see a couple of minor things done, if possible, to avoid some of the issues we had on arm.
I'd like to see something that, if relatively easy, checks the ELF headers and such to see if the SPE mark is present or absent and based on the kernel refuse to run things.
I'd like to make sure that SPE is properly defined by the compiler (both gcc and clang)
I've not had a chance to dump the binaries you uploaded for me. This week has been insane... It started LAST week :(
| contrib/gcc/config/rs6000/freebsdspe.h | ||
|---|---|---|
| 73 ↗ | (On Diff #14443) | Are SPE binaries marked differently? I think they should be and we should understand that before it goes into the tree... |
| share/mk/sys.mk | ||
| 16 ↗ | (On Diff #14443) | Any chance you can reach out to Simon or Bryon to see if there's a way we can make this ugly expression a macro? It's used in half a dozen places in the tree now :( Some are compat for old systems and we can likely retire. Maybe it's beyond the scope, but if you have a few minutes to check into it, that would be great. |
| sys/conf/ldscript.powerpcspe | ||
| 3 ↗ | (On Diff #14443) | Same question about marking things differently... though that would have to be in binutils I guess... |
| sys/powerpc/include/param.h | ||
| 60 ↗ | (On Diff #14443) | I didn't see where this was added as an automatic define. Is it so defined by the compiler? |
Comment Actions
I'll take a closer look at the binaries. The ELF header doesn't look any different between powerpc and powerpcspe, though. Though, I just found the book-E binaries have a .PPC.EMB.apuinfo note section, which standard powerpc binaries don't have. So, yes, I guess these binaries are tagged differently.
| sys/powerpc/include/param.h | ||
|---|---|---|
| 60 ↗ | (On Diff #14443) | It's added automatically when you pass -mspe to the compiler. |
Comment Actions
Looking at the source for gas, this section is only added if needed. If the embedded features (SPE, ISEL, PMR, embedded floating point, BRLOCK, cache lock, or RMFCI) are used the section is added, otherwise it's not. Now, most of the time (probably >90%) code will use ISEL, and this won't be a problem. But, if a binary doesn't use any of these features, the section won't be added, and there's nothing else distinguishing it from a normal powerpc binary.
Comment Actions
Can I get more review on this? Especially the build changes. I've only tested that they work, not that they're entirely correct or complete (haven't even tried building ports). :)
| contrib/gcc/config/rs6000/freebsdspe.h | ||
|---|---|---|
| 73 ↗ | (On Diff #14443) | This is a tough one... when binaries *use* SPE, there's an ELF note to denote it, but the note is optional if it doesn't use any APUs (almost all do use the 'isel' extension APU, but it's not required). However, they all need to use the SPE ABI regardless of the note, and libc uses SPE in this arch. Is there an established way to add an extra note we can check on? Then we can block it by both the kernel and rtld (prevent linking of libraries that don't have the note). |
| usr.sbin/Makefile.powerpc | ||
| 3 ↗ | (On Diff #14443) | Oops, this change shouldn't be here. nvram should be built always, or guarded by !powerpcspe (will have to check) |
Comment Actions
On a slightly related note I've started collecting arch-specific details on a wiki page (and will move it when useful / complete), but add the details there if you like: https://wiki.freebsd.org/EdMaste/ArchitectureSpecifics
| contrib/gcc/config/rs6000/freebsdspe.h | ||
|---|---|---|
| 2 ↗ | (On Diff #14443) | Probably update this comment and add a reference to the file this one is based on. How should we go about upstreaming these changes? |
| gnu/usr.bin/binutils/Makefile.inc0 | ||
| 10 ↗ | (On Diff #14443) | What about powerpc(64|spe) as is done for mips/arm? |
| lib/libc/powerpcspe/Symbol.map | ||
| 59–61 ↗ | (On Diff #14443) | You should not need to export at least .curbrk, .minbrk, and .cerror. |
| lib/libc/powerpcspe/sys/cerror.S | ||
| 33 ↗ | (On Diff #14443) | Perhaps make this .hidden, as rS296474 for amd64? |
| sys/boot/fdt/dts/arm/zybo.dts | ||
| 1 ↗ | (On Diff #14443) | It looks like this is mistakenly included in the review? I notice it has CRLF line terminators and that should probably be addressed (separately from this change). |
| contrib/gcc/config/rs6000/freebsdspe.h | ||
|---|---|---|
| 2 ↗ | (On Diff #14443) | Yeah, I need to update this. You can see where I stole it from. As for upstreaming, the plan is to (after working out the kinks here) do the same copying on latest gcc. The file has changed considerably (reduced to only the ASM_DEFAULT_SPEC and TARGET_DEFAULT lines), so might be easier to test. I'll have to talk with someone who's already upstreamed stuff to gcc, when the time comes. |
| gnu/usr.bin/binutils/Makefile.inc0 | ||
| 10 ↗ | (On Diff #14443) | Yes, that would be much better, and should be applied to the other lines, too. |
| lib/libc/powerpcspe/Symbol.map | ||
| 59–61 ↗ | (On Diff #14443) | Thanks, I'll pull these. |
| sys/boot/fdt/dts/arm/zybo.dts | ||
| 1 ↗ | (On Diff #14443) | Hmm, I never even touched this file, wonder how it changed. Blame svn? Will pull it from my patch, that's for sure. |
Comment Actions
Address several comments.
One thing to note: In order for packages to be built appropriately, this *must*
be a separate arch. Just making it a toggle-able option on the powerpc arch is
insufficient.