Skip to content

lib/systems: strip kernel to avoid reference cycles#228018

Merged
1 commit merged intomasterfrom
unknown repository
Jul 20, 2023
Merged

lib/systems: strip kernel to avoid reference cycles#228018
1 commit merged intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Apr 24, 2023

Description of changes

lib/systems: strip kernel to avoid reference cycles

Since #221707 we must ensure that the image resulting from a kernel build is stripped. If we do not do this, there will be circular dependencies among its outpaths.

This commit fixes the breakage on mips.

Closes #244438

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ghost ghost requested review from Ericson2314, alyssais and matthewbauer as code owners April 24, 2023 20:41
@github-actions github-actions bot added the 6.topic: kernel The Linux kernel label Apr 24, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. labels Apr 24, 2023
@vkleen
Copy link
Contributor

vkleen commented Apr 24, 2023

It turns out that I can't actually get the PowerPC zImage to boot with kexec. Sorry for the half-baked suggestion... Unfortunately, for reasons that I don't understand, doing the same stripping on vmlinux as you're doing for MIPS doesn't seem to be enough to get rid of cyclic references.

Stupid workaround: the linux kernel build produces a vmlinux.strip.gz; if we set the platform kernel target to that instead of vmlinux, this might sidestep the issue.

@vkleen
Copy link
Contributor

vkleen commented Apr 25, 2023

Further testing: setting KBUILD_IMAGE=vmlinux.strip.gz and getting rid of the extra makeFlags produces a bootable image. At least after manually gunziping, either in the bootloader (which for my machine is a full Linux system in itself) or before.

@ghost
Copy link
Author

ghost commented Apr 26, 2023

It turns out that I can't actually get the PowerPC zImage to boot with kexec. Sorry for the half-baked suggestion... Unfortunately, for reasons that I don't understand, doing the same stripping on vmlinux as you're doing for MIPS doesn't seem to be enough to get rid of cyclic references.

MIPS kernels are actually fully ELF-compliant images, so a lot of stuff is easier there.

@ghost ghost marked this pull request as draft April 26, 2023 01:11
@ghost ghost changed the title linuxKernel: avoid reference cycles on powerpc64le, mips64el lib/systems: set default linux-kernel.target to avoid reference cycles May 15, 2023
@ghost
Copy link
Author

ghost commented May 15, 2023

I'm running test builds of this now...

@github-actions github-actions bot added 6.topic: kernel The Linux kernel and removed 6.topic: kernel The Linux kernel labels May 15, 2023
@ghost ghost changed the title lib/systems: set default linux-kernel.target to avoid reference cycles lib/systems: strip kernel to avoid reference cycles May 16, 2023
@ghost
Copy link
Author

ghost commented May 16, 2023

@alyssais wrote

This is what I was trying to avoid — I'd rather have the stripping done by the kernel build system where it supports it, because they'll do a better job of setting the right flags, I think.

I know that, stylistically, you were trying to avoid doing things this way, but no other solutions for the breakage caused by #221707 have arisen. I can't get anything else to work and I have run out of time.

@ghost ghost marked this pull request as ready for review May 16, 2023 07:32
@alyssais
Copy link
Member

(Originally posted here by mistake.)

I've done some investigation of my own. Here are the results:

PowerPC

PowerPC ends up with two sets of references. One is from kexec purgatory, and is probably an oversight. I expect I could upstream a patch to fix this, like I recently did for x86 and RISC-V1. The other is from the VDSO. Unlike every other architecture, PowerPC deliberately uses a non-stripped vdso, apparently because it made perf traces nicer. So unless things I've changed there (I'll try to have a look — it seems a bit strange that PowerPC would be special here), we won't be able to get the kernel build system to give us a reference-free PowerPC image. So even though I don't like it, ack to stripping manually on PowerPC.

MIPS

MIPS offers vmlinux, vmlinuz, and uImage. vmlinuz and uImage should both be reference-free, but I don't know enough about how MIPS boots to know whether either of those would be sensible default targets. I'd appreciate your insight here. If it's not feasible to change to either of those images, then we probably have to strip manually on MIPS too.

Footnotes

  1. the only thing getting in the way here is that it's a bit annoying to test, because our kexec test is currently broken. cc @RaitoBezarius.

Since #221707 we must ensure
that the image resulting from a kernel build is stripped.  If we do
not do this, there will be circular dependencies among its outpaths.

This commit fixes the breakage on mips.

Closes #224694
@roberth roberth added the 6.topic: lib The Nixpkgs function library label Jul 8, 2023
@ghost
Copy link
Author

ghost commented Jul 18, 2023

MIPS offers vmlinux, vmlinuz, and uImage. vmlinuz and uImage should both be reference-free, but I don't know enough about how MIPS boots to know whether either of those would be sensible default targets.

make vmlinuz requires CONFIG_SYS_SUPPORTS_ZBOOT, which isn't supported by all mips machines. Specifically Cavium Octeon routers, the most popular mips64 machines, don't have CONFIG_SYS_SUPPORTS_ZBOOT (the vendor-supplied bootloader is simply awful).

So make vmlinuz gives you:

CONFIG_SYS_SUPPORTS_ZBOOT is not enabled

I tried make uImage but it appears that is the same as make vmlinux, plus perhaps additional steps afterwards. It's also uboot-specific?

So even though I don't like it, ack to stripping manually on PowerPC.

@alyssais would you please update your review to reflect this? It still shows the big-red-changes-requested-of-doom.

@ghost ghost requested a review from alyssais July 18, 2023 09:10
@ghost
Copy link
Author

ghost commented Jul 18, 2023

@ofborg eval

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. and removed 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Jul 18, 2023
@ghost
Copy link
Author

ghost commented Jul 19, 2023

Rebased.

@ofborg ofborg bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. and removed 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. labels Jul 19, 2023
@github-actions github-actions bot removed the 6.topic: lib The Nixpkgs function library label Jul 20, 2023
@ghost ghost dismissed alyssais’s stale review July 20, 2023 02:45

@alyssais said "If it's not feasible to change to either of those images, then we probably have to strip manually on MIPS too." and that is the case #228018 (comment)

@ghost
Copy link
Author

ghost commented Jul 20, 2023

From #194153, which includes this PR:

pkgsCross.octeon.linux on x86_64-linux — Success Details

@ghost
Copy link
Author

ghost commented Jul 20, 2023

I have moved the powerpc fix into

#244436

@ghost
Copy link
Author

ghost commented Jul 20, 2023

Rebased.

It seems that ofborg-eval is on vacation today.

@ghost ghost merged commit 1247580 into NixOS:master Jul 20, 2023
@ghost ghost deleted the pr/fix/224694 branch July 20, 2023 03:43
@ghost ghost mentioned this pull request Jul 20, 2023
12 tasks
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: kernel The Linux kernel 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kernel builds on mips broken by #221707

3 participants