Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration updates to compile with GCC 8 #2803

Closed
wants to merge 2 commits into from

Conversation

LF-Luis
Copy link
Contributor

@LF-Luis LF-Luis commented Apr 17, 2019

No description provided.

@op-jenkins
Copy link
Contributor

Can one of the admins verify this patch?

@ghost
Copy link

ghost commented Apr 18, 2019

ok to test

@madscientist159
Copy link

Unfortunately this patch also yields a system that will not IPL, with no hostboot console output (openbmc/openbmc#3531). Reverting buildroot to GCC6 allows hostboot to start but it then immediately fails in targeting (open-power/hostboot#173)

@bofferdn
Copy link
Contributor

bofferdn commented May 2, 2019

Fixes for OCC gcc8 compile coming separately via occ package
Fix for crash in HB coming separately via hostboot package
Fix for PNOR layout changes to fit BOOTKERNEL for p8/p9 coming via pnor package
This will have to be resubmitted to bring all those in. Good news is we built whole driver in GCC8 and booted on a WSP, so should be out of the woods once we link in all these changes.

@LF-Luis LF-Luis force-pushed the master branch 6 times, most recently from 91ef3b3 to 28a0963 Compare May 21, 2019 23:08
@bofferdn
Copy link
Contributor

I attemped to build this patch on master on a RHEL 7 machine w/ witherspoon defconfig and it passed; I don't get why it's giving inconsistent results vs. local testing.

@LF-Luis LF-Luis force-pushed the master branch 4 times, most recently from 95ba593 to bd87ba8 Compare May 30, 2019 18:49
@LF-Luis
Copy link
Contributor Author

LF-Luis commented May 30, 2019

retest this please

2 similar comments
@bofferdn
Copy link
Contributor

retest this please

@LF-Luis
Copy link
Contributor Author

LF-Luis commented Jun 4, 2019

retest this please

@sammj
Copy link
Contributor

sammj commented Jun 5, 2019

I'm assuming this PR is for testing only and the included patches will go to their relevant repositories instead?

The Linux Makefile patch is interesting, @shenki can we handle that with a config option instead?

This should probably update the other defconfigs as well, not just P9 platforms.

@shenki
Copy link
Member

shenki commented Jun 5, 2019

The kernel changes are a non-starter. If we end up requiring this we will upstream them to the powerpc or top level makefile.

@LF-Luis LF-Luis force-pushed the master branch 2 times, most recently from 7dff595 to 2c43c72 Compare June 5, 2019 15:59
@LF-Luis
Copy link
Contributor Author

LF-Luis commented Jun 5, 2019

Running CI without the Kernel changes to see if new sizing still holds.

@sammj
Copy link
Contributor

sammj commented Jun 6, 2019

ERROR: PnorUtils::checkSpaceConstraints: Image provided (/tmp/hostboot-jenkins.swg-devops/jenkins-slave/workspace/Hostboot/OpenPower/GHPR/ubuntu-ppc64le-2242/CONFIG/witherspoon/OS/Ubuntu_18.04/build/op-build/output/host/powerpc64le-buildroot-linux-gnu/sysroot/openpower_pnor_scratch//zImage.epapr) has size (32505856) which is greater than allocated space (16252928) for section=BOOTKERNEL.  Aborting! at /tmp/hostboot-jenkins.swg-devops/jenkins-slave/workspace/Hostboot/OpenPower/GHPR/ubuntu-ppc64le-2242/CONFIG/witherspoon

Appears to have worked for some platforms but not others - that's 2x the normal size though I suspect something else is up there.

@madscientist159
Copy link

@sammj 2x the normal size is the scripting breaking down -- if you're even 1 byte over, dd spits out another block since the block size is set to the allowed partition size. Presto, double size for a slight overage.

It made debugging fun for a while until I figured out what was going on. 😄

@sammj
Copy link
Contributor

sammj commented Jun 6, 2019

@madscientist159 Haha, that does ring a bell. Ugh, my kingdom to find time to finish off the ffspart rework.

@bofferdn
Copy link
Contributor

@shenki Can we initiate the process to pull the kernel changes to upstream. This is the only way after a month of trying we have been able to fit everything in the space constraint. Otherwise, we'll need help to somehow prune the content down or something.

@shenki
Copy link
Member

shenki commented Jun 12, 2019

@shenki Can we initiate the process to pull the kernel changes to upstream. This is the only way after a month of trying we have been able to fit everything in the space constraint. Otherwise, we'll need help to somehow prune the content down or something.

Yes. Please hit me up on slack if you need instructions on how to send kernel patches.

Edit op-build config files to use GCC 8 compiler.
Temporarily added 0001-Fix-gcc8-signature-validation-anomaly HB patch.
Temporarily added 0002-GCC8-Code-Fixes HB patch.
Has 0003-space-saving-compile-options patch to reduce size of bootkernel.

Signed-off-by: Luis Fernandez <[email protected]>
For P8 configs use GCC 6, and for P9 configs use GCC 8.
For P10, GCC 8 will also be selected.

Signed-off-by: Luis Fernandez <[email protected]>
@ghost
Copy link

ghost commented Jul 16, 2019

#3004 removes the P8 platforms, so that solves the P8 problem.

@klauskiwi
Copy link

The ci/build* scrips have changed significantly since this was first proposed, and there are plenty of other PRs (here and on Hostboot, OCC etc) proposing solutions for compilation with modern compilers.. so far, looks like the theory of 'removing power8 entirely' is winning (#3145).

I'll politely close this, but if there's any aspect of this that we should still discuss, feel free to reopen/speak-up

@klauskiwi klauskiwi closed this Jul 24, 2020
@LF-Luis
Copy link
Contributor Author

LF-Luis commented Jul 24, 2020

Thank you for closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants