Page MenuHomeFreeBSD

[PowerPC] Book-E clang support
ClosedPublic

Authored by bdragon on Oct 12 2019, 6:08 PM.
Referenced Files
F101853140: D21999.id63888.diff
Mon, Nov 4, 5:23 PM
F101852170: D21999.id63569.diff
Mon, Nov 4, 5:07 PM
Unknown Object (File)
Fri, Nov 1, 3:26 PM
Unknown Object (File)
Oct 4 2024, 8:11 AM
Unknown Object (File)
Sep 25 2024, 8:33 AM
Unknown Object (File)
Sep 21 2024, 5:21 AM
Unknown Object (File)
Sep 18 2024, 5:21 AM
Unknown Object (File)
Sep 17 2024, 6:38 PM
Subscribers

Details

Summary

The main fixes needed are

A) we need to avoid certain mnemonics referring to SPRs that are different between Book-E and Book-S.

B) The boot page text relocations need eliminated.

I have not yet tested / validated 32-bit yet.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

adding some misc. notes inline

sys/powerpc/booke/locore.S
83

I am thinking maybe we should preprocess the linker script. There are several things in there that are suboptimal because of how the same linker script needs to be used across all of powerpc64, without being able to differentiate on platform and ABI.

978

Not sure whether this is clang specific or not, but lld doesn't like toc entries to nonexistent labels. Noticed while trying to do a uniprocessor build.

sys/powerpc/booke/pmap.c
1677

These debugf() changes are needed for enabling DEBUG for pmap on any ppc64 and isn't directly clang related.

sys/powerpc/conf/dpaa/config.dpaa
9

This part is not GCC compatible. There needs to be some way to make these compiler-specific, becuase gcc and clang both need different sets of permitted warnings.

sys/powerpc/powerpc/pmap_dispatch.c
147

This line is not directly related.

I really like that you took the time to annotate the existing code. It's tough to read and understand it the first several times through.

sys/powerpc/booke/locore.S
46

Why do you need to double the stack size?

83

The ultimate goal is to merge AIM and Book-E, I think (maybe it doesn't make sense, but I'm still thinking to do it). So, maybe the better thing to do isn't to preprocess the linker script, but instead to put the stub in a single file in powerpc/powerpc, which just jumps to start. When we do the massive merge we'll rename the individual start's to start_{aim,booke}, with the global start deciding which. But no need to do that in this patch.

sys/powerpc/booke/pmap.c
1677

We're moving to '%jx' and casting to uintmax_t.

1858–1859

This should really be splled as '%jx' with casts to uintmax_t. We're trying to get away from PRI*.

Addressing review comments.

updating inline comments

sys/powerpc/booke/locore.S
46

I think increasing it was something that somehow was letting me bypass a bug. Since it seems to boot OK again without the bump, changing back to the original value.

sys/powerpc/booke/pmap.c
4310

Looks like I need to take this one back out.

bdragon marked an inline comment as done and an inline comment as not done.Oct 23 2019, 2:23 AM

Remove an unnecessary patch band.

sys/powerpc/conf/dpaa/config.dpaa
9–13

Are these necessary? I built for powerpcspe, and didn't need these flags.

sys/powerpc/mpc85xx/platform_mpc85xx.c
360–365

Although they don't hurt, I don't think the syncs here are necessary. The cpu_flush_dcache() will take care of the necessary flushing and syncing.

Update patch to properly fix gcc / clang differences in warning flags.

I tested X5000 build and run on elfv2 and elfv1.

sys/powerpc/mpc85xx/platform_mpc85xx.c
360–365

I was being extra paranoid about ensuring everything else was explicitly written and pushed all the way through first, before writing the low word, because the spin logic is all triggered off of the low word, so as soon as the cpu picks up on the low word change it immediately continues on its way.

I'm almost certainly being too paranoid, but I did spend quite a while banging my head on this part.

This revision is now accepted and ready to land.Nov 2 2019, 8:47 PM