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.
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
adding some misc. notes inline
| sys/powerpc/booke/locore.S | ||
|---|---|---|
| 83 ↗ | (On Diff #63190) | 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 ↗ | (On Diff #63190) | 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 ↗ | (On Diff #63190) | 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 ↗ | (On Diff #63190) | 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 ↗ | (On Diff #63190) | This line is not directly related. |
Comment Actions
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 ↗ | (On Diff #63190) | Why do you need to double the stack size? |
| 83 ↗ | (On Diff #63190) | 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 ↗ | (On Diff #63190) | We're moving to '%jx' and casting to uintmax_t. |
| 1858–1859 ↗ | (On Diff #63190) | This should really be splled as '%jx' with casts to uintmax_t. We're trying to get away from PRI*. |
Comment Actions
updating inline comments
| sys/powerpc/booke/locore.S | ||
|---|---|---|
| 46 ↗ | (On Diff #63190) | 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 | ||
| 4305 ↗ | (On Diff #63567) | Looks like I need to take this one back out. |
| sys/powerpc/conf/dpaa/config.dpaa | ||
|---|---|---|
| 9–13 ↗ | (On Diff #63569) | Are these necessary? I built for powerpcspe, and didn't need these flags. |
| sys/powerpc/mpc85xx/platform_mpc85xx.c | ||
| 360–364 ↗ | (On Diff #63569) | 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. |
Comment Actions
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–364 ↗ | (On Diff #63569) | 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. |