Page MenuHomeFreeBSD

[PowerPC64LE] Set up powerpc.powerpc64le architecture
ClosedPublic

Authored by bdragon on Thu, Sep 10, 7:32 PM.

Details

Summary

This is the initial set up for PowerPC64LE.

As of rS365666, the base clang compiler is capable of building this.

The current plan is for this to remain an experimental arch for FreeBSD 13.

See https://github.com/freebsd/freebsd/compare/master...bdragon28:little-endian for my working branch. I have been pushing bits and pieces in as I go.

Currently the arch can self-host on POWER8 and POWER9 hardware. Poudriere has completed its first bulk build @ http://poudriere.rtk0.net/build.html?mastername=le_bootstrap-local&build=2020-09-12_20h20m54s

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bdragon created this revision.Thu, Sep 10, 7:32 PM
bdragon requested review of this revision.Thu, Sep 10, 7:32 PM
bdragon added inline comments.Thu, Sep 10, 8:19 PM
release/powerpc/powerpc64le.conf
10 ↗(On Diff #76900)

Regarding the temporal paradox here: This kernel config file is a later part of the main patchset. I can roll it in if necessary, but I'm planning on getting the main body of things in at the same time so the window where someone would notice this should be pretty small.

emaste added inline comments.Thu, Sep 10, 8:35 PM
release/powerpc/powerpc64le.conf
10 ↗(On Diff #76900)

You could also just leave the release/ change out until later, although I don't think it matters too much.

imp added inline comments.Mon, Sep 14, 7:41 PM
stand/defs.mk
186 ↗(On Diff #76900)

So this is weird to me. We normally build $NATIVE_ENDIAN for the loader. why wouldn't we do that here? The force_le thing is a hack for loading a big endian kernel with a little endian loader...

sys/conf/files.powerpc
139 ↗(On Diff #76900)

there's little endian AIM machines?

239 ↗(On Diff #76900)

So why isn't this just standard?

sys/conf/kern.mk
188 ↗(On Diff #76900)

The more times I see this, I wonder if we shouldn't be doing:
.if ${MACHINE_ARCH:Mpowerpc64*}
to match what we do with 64 bit mips.

andrew added a subscriber: andrew.Mon, Sep 14, 7:52 PM
andrew added inline comments.
share/mk/bsd.cpu.mk
131 ↗(On Diff #76900)

You can use ${MACHINE_ARCH:Mpowerpc64*} here.

sys/conf/files.powerpc
239 ↗(On Diff #76900)

I guess it could be !powerpcspe

emaste added inline comments.Mon, Sep 14, 8:02 PM
stand/defs.mk
186 ↗(On Diff #76900)

yeah, it seems like WITH_LOADER_FORCE_LE on a little-endian build should be a no-op

sys/conf/kern.mk
188 ↗(On Diff #76900)

Yeah, assuming we're not planning to grow other powerpc64 variants that are significantly different.

bdragon added inline comments.Mon, Sep 14, 8:15 PM
release/powerpc/powerpc64le.conf
10 ↗(On Diff #76900)

I have now split this out in my local until after I commit the kernel config.

share/mk/bsd.cpu.mk
131 ↗(On Diff #76900)

Thanks, will switch to doing that.

stand/defs.mk
186 ↗(On Diff #76900)

In this case, we are indeed loading a little endian kernel with a big endian loader.

SLOF is *always* big endian, so for pseries, this is correct.

186 ↗(On Diff #76900)

except we actually want big endian loaders on LE, because the only functioning one at the moment is for use with SLOF, which is *always* big endian.

Transition to little endian happens after entry into kernel.

The reason I constructed it like this was so that -mlittle-endian and -mbig-endian would never be on the same line.

I could simply have it .error if you try to set WITH_LOADER_FORCE_LE on LE.

sys/conf/files.powerpc
139 ↗(On Diff #76900)

aim is "all Book-S" as opposed to the Book-E processors.

239 ↗(On Diff #76900)

Technically it also shouldn't be built on e5500 either, but that's currently handled as custom CPUTYPE for powerpc64 and isn't really accessible from here.

sys/conf/kern.mk
188 ↗(On Diff #76900)

Good idea.

bdragon added inline comments.Mon, Sep 14, 10:38 PM
stand/defs.mk
186 ↗(On Diff #76900)

Submitted D26430 to rip out WITH_LOADER_FORCE_LE entirely.

Afterwards, will rework this to just unconditionally -mbig-endian on powerpc64le to ensure the loaders are built big-endian as intended.

emaste added inline comments.Mon, Sep 14, 10:51 PM
stand/defs.mk
186 ↗(On Diff #76900)

👍

bdragon updated this revision to Diff 77038.Tue, Sep 15, 6:18 AM

Addressing review comments.

Retested on POWER8.

bdragon marked 7 inline comments as done.Tue, Sep 15, 6:21 AM
bdragon added inline comments.
stand/defs.mk
186 ↗(On Diff #76900)

After removal of WITH_LOADER_FORCE_LE, it made more sense to move this up to the main block above.

sys/conf/files.powerpc
139 ↗(On Diff #76900)

At some point we could rename aim to books, but that's busywork for another day.

239 ↗(On Diff #76900)

jhibbits corrected me there, e5500 does need altivec.c, so yeah, !powerpcspe would make sense.

bdragon edited the summary of this revision. (Show Details)Tue, Sep 15, 6:24 AM
bdragon updated this revision to Diff 77087.Wed, Sep 16, 1:37 AM
bdragon marked an inline comment as done.

Addressing a review comment I missed.

Work branch reverified on powerpc64 (G5), powerpc (G4), powerpcspe (RB800)

bdragon marked 2 inline comments as done.Wed, Sep 16, 1:37 AM

LE retested on POWER8.

I didn't check files.powerpc extremely carefully, but overall LGTM

Makefile
503 ↗(On Diff #77087)

I suppose powerpcspe is, too. You could just leave this comment out, this whole section is for "rarely used, semi-supported architectures"

bdragon added inline comments.Wed, Sep 16, 2:38 AM
Makefile
503 ↗(On Diff #77087)

Actually I consider it a bug that powerpcspe hasn't been reenabled here, the clang issues were fixed and it's part of CI builds, so I occasionally have to fix stuff that someone's local tinderbox build didn't catch. It might actually get a few more users if the AmigaOne A1222 manages to make it out of development hell. (Fingers crossed there, I'm on the early adopters list for that)

Even though it's pretty much me and jhibbits that use it at the moment, it's a good way to ensure Book-E continues working correctly.

bdragon added inline comments.Tue, Sep 22, 11:39 PM
Makefile
503 ↗(On Diff #77087)

After rereading your comment, I see what you mean. Taking the comment back out.

This revision was not accepted when it landed; it landed in state Needs Review.Tue, Sep 22, 11:54 PM
This revision was automatically updated to reflect the committed changes.