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

kernel: fix cross compilation #33813

Merged
merged 8 commits into from Jan 16, 2018
Merged

Conversation

lopsided98
Copy link
Contributor

@lopsided98 lopsided98 commented Jan 13, 2018

This PR contains rebased versions of patches from #30882 to fix Linux kernel cross compilation.

Specifically, this includes:
5c6473c - kernel: Enable cross compiling
d1ccc5d - kernel: Use build kmod
5e91760 - kernel fixes
e915e4a - kernel: Fix cross-compilation

5e91760 has been squashed into 5c6473c because the changes are closely related.

There are a few things that I'm not sure about, or didn't seem right to me.

In 5c6473c, pkgs/os-specific/linux/kernel/perf.nix refers to a patch that no longer exists in master and did not exist in the tree that #30882 is based on. It also seems to revert the fix that was used to replace the patch. This seems like it could be due to a bad rebase at some point, but I wanted to make sure there wasn't another reason.

I put a few other concerns as comments on the diff.

I have tested cross building 4.14 kernels for armv7l and aarch64, as well as a native x86_64 kernel.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

} // extraMeta;
};
};
} // extraMeta;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't make sense to me. Why shouldn't extraMeta be merged with the existing meta attribute?

@lopsided98 lopsided98 changed the title Fix Linux kernel cross compilation kernel: fix cross compilation Jan 13, 2018
flex bison libiberty libaudit makeWrapper pkgconfig ];
buildInputs = [ elfutils python perl newt slang libunwind binutils zlib ] ++
flex bison libiberty libaudit makeWrapper pkgconfig python perl ];
buildInputs = [ elfutils newt slang libunwind binutils zlib ] ++
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if these dependencies are all correct.

@@ -58,37 +58,29 @@ let
in lib.concatStringsSep "\n" ([baseConfig] ++ configFromPatches);

configfile = stdenv.mkDerivation {
inherit ignoreConfigErrors;
#inherit ignoreConfigErrors;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover ?

@lopsided98
Copy link
Contributor Author

cc @bgamari @Ericson2314

arch = hostPlatform.platform.kernelArch;

# TODO(@Ericson2314): No null next hash break
ignoreConfigErrors = if stdenv.hostPlatform == stdenv.buildPlatform then null else true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "No null next hash break" mean?

Also, I just discovered this breaks builds with config errors, because ignoreConfigErrors does not get defined when not cross compiling and the scripts assume it is defined.

Use of uninitialized value $ENV{"ignoreConfigErrors"} in string ne at /nix/store/2dajr95g51fajq0jgi3jr4xmacjp3pj0-generate-config.pl line 141.

@teto
Copy link
Member

teto commented Jan 15, 2018

Quite excited to see some improvements on the kernel as it's the most annoying software to work with on nixos. Getting rid of crossplatform-specific code makes patching easir (I have some patches to handle the kernel config in a simpler way but I could not upstream them because they didn't support cross-platform).
Could your rebase this so that I can test it please ?

@teto
Copy link
Member

teto commented Jan 15, 2018

I rebased locally (you need to pay attention to b1ca851 ) but then I get

@[nix-shell:~/nixops]$ nixops deploy -d 828db7f5-f9a1-11e7-a652-309c233b770e --show-trace
error: while evaluating anonymous function at /home/teto/nixpkgs/lib/options.nix:116:41, called from /home/teto/nixpkgs/lib/attrsets.nix:199:52:
while evaluating ‘scrubOptionValue’ at /home/teto/nixpkgs/lib/options.nix:112:22, called from /home/teto/nixpkgs/lib/options.nix:116:44:
while evaluating ‘isDerivation’ at /home/teto/nixpkgs/lib/attrsets.nix:295:18, called from /home/teto/nixpkgs/lib/options.nix:113:8:
while evaluating the attribute ‘baseImage’ at /home/teto/nixpkgs/lib/attrsets.nix:199:44:
while evaluating anonymous function at /home/teto/nixpkgs/lib/modules.nix:75:45, called from /home/teto/nixpkgs/lib/attrsets.nix:199:52:
while evaluating the attribute ‘value’ at /home/teto/nixpkgs/lib/modules.nix:312:9:
while evaluating the option `deployment.libvirtd.baseImage':
while evaluating the attribute ‘mergedValue’ at /home/teto/nixpkgs/lib/modules.nix:339:5:
while evaluating anonymous function at /home/teto/nixpkgs/lib/modules.nix:339:32, called from /home/teto/nixpkgs/lib/modules.nix:339:19:
while evaluating ‘check’ at /home/teto/nixpkgs/lib/types.nix:318:15, called from /home/teto/nixpkgs/lib/modules.nix:340:10:
while evaluating ‘check’ at /home/teto/nixpkgs/lib/types.nix:230:15, called from /home/teto/nixpkgs/lib/types.nix:318:31:
while evaluating the attribute ‘args’ of the derivation ‘libvirtd-ssh-image’ at /home/teto/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:148:11:
while evaluating the attribute ‘text’ of the derivation ‘vm-run’ at /home/teto/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:148:11:
while evaluating the attribute ‘outPath’ at /home/teto/nixpkgs/lib/customisation.nix:155:7:
while evaluating the attribute ‘configurePhase’ of the derivation ‘linux-4.9.76’ at /home/teto/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:148:11:
while evaluating the attribute ‘arch’ of the derivation ‘linux-config-4.9.76’ at /home/teto/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:148:11:
attribute ‘platform’ missing, at /home/teto/nixpkgs/pkgs/os-specific/linux/kernel/generic.nix:75:12

which is strange since in repl, hostPlatform.platform does exist.

EDIT: nevermind, I had a freaking override in my overlays with a random platform.

#configWithPlatform = kernelPlatform: import ./common-config.nix
# { inherit stdenv version kernelPlatform extraConfig;
# features = passthru.features; # Ensure we know of all extra patches, etc.
# };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover

@teto
Copy link
Member

teto commented Jan 15, 2018

I pushed a rebased version with leftover removed https://github.com/teto/nixpkgs/tree/kernel_cross which seems ok for host builds. would be nice to check cross platform.

@bgamari
Copy link
Contributor

bgamari commented Jan 15, 2018

Thanks for doing this!

@lopsided98
Copy link
Contributor Author

lopsided98 commented Jan 15, 2018

I rebased and added some new commits to fix some cosmetic issues and remove some dead code left over from the old cross compiling system. Most of these changes were not hash breaking, but I did revert a change @bgamari made to how extraMeta is merged (80f6b8e, it didn't seem right, but let me know if it was that way for a reason), and I made perf inherit its makeFlags from the kernel. I was unable to test cross building perf because some of its dependencies don't build yet, but a native build worked.

Another thing I was wondering about is why do we need to ignore config errors when cross compiling? Config errors are already ignored on all non "pc" host platforms, so the only case I can think of where the new cross compiling condition would be used is when cross compiling on Darwin for x86_64 Linux.

Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this!

kernelBaseConfig = cp.kernelBaseConfig;
kernelTarget = cp.kernelTarget;
autoModules = cp.kernelAutoModules;
nativeBuildInputs = [ buildPackages.stdenv.cc perl ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be a depsBuildBuild

name = "linux-${version}";

enableParallelBuilding = true;

nativeBuildInputs = [ perl bc nettools openssl gmp libmpc mpfr ]
++ optional (stdenv.platform.kernelTarget == "uImage") ubootTools
nativeBuildInputs = [ perl bc nettools openssl gmp libmpc mpfr buildPackages.stdenv.cc ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildPackages.stdenv.cc should again be a depsBuildBuild.

@@ -23,8 +23,8 @@ stdenv.mkDerivation {
# perf refers both to newt and slang
# binutils is required for libbfd.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a libbfd derivation now.

@Ericson2314 Ericson2314 changed the base branch from master to staging January 16, 2018 17:58
@Ericson2314 Ericson2314 changed the base branch from staging to master January 16, 2018 17:58
@Ericson2314 Ericson2314 changed the base branch from master to staging January 16, 2018 18:06
@Ericson2314 Ericson2314 merged commit 782b63d into NixOS:staging Jan 16, 2018
@teto
Copy link
Member

teto commented Jan 17, 2018

Thanks for the patch.
sorry to hijack the thread but as the kernel configuration is system dependant (which makes tailoring the kernel much harder, buildLinux should accept the parameters directly and the system forcefully enable some but I digress), I have my kernel config defined in a custom platform

  test-platform = super.platforms.pc64_simplekernel // {
    kernelAutoModules = false;
    extraConfig = super.platforms.pc64_simplekernel
      + kvmConfig
      + debugConfig
      ;
  };

My understanding is that I need to crosscompile for the same architecture just to override some kernel settings :/ ?! I've never done any crosscompilation, what could be the command to compile the kernel with the previous platform please (in a nix-shell for instance) ?

NB: the naming hostPlatform.platform is confusing since hostPlatform looks like a platform already, shouldn't it be renamed to hostSystem or accept directly the platform argument ?

@teto teto mentioned this pull request Jan 17, 2018
@Ericson2314
Copy link
Member

All the *Platforms have a platform field, for the record. I agree its an ugly grab-bag of program-specific settings though, and I hope to gradually get rid of it.

You don't need to cross compile, you can pass localSystem to nixpkgs which has the same structure as the *Platforms. I'd write you an example now, but I'm on my phone. The manual is now a bit better in this stuff though.

@lopsided98 lopsided98 deleted the kernel-cross branch January 18, 2018 02:50
@teto
Copy link
Member

teto commented Jan 18, 2018

I managed to come up with
nix-build -A neovim --arg localSystem '(import <nixpkgs/lib>).systems.examples.aarch64-multiplatform' '<nixpkgs>' --show-trace thanks to the manual (I wish there were more examples though).
My system is not aarch64 though. Is it possible to override the platform component of the localSystem ? I believe it's defined in lib/systems/default.nix but as it's an impure thing I am not sure how to do this. My nix-info:
system: "x86_64-linux", multi-user?: yes, version: nix-env (Nix) 1.11.16,

@Ericson2314 Ericson2314 added the 6.topic: cross-compilation Building packages on a different sort platform than than they will be run on label Jan 18, 2018
@Ericson2314
Copy link
Member

@teto Yes, the syntax of --arg is --arg <identifier> <nixexpr> so you can put whatever you want in the second part.

@Ericson2314 Ericson2314 added this to After @bgamari's numerous fixes in Cross compilation Jan 18, 2018
@teto
Copy link
Member

teto commented Jan 19, 2018

Sorry I am a bit dense, I first tried this:
nix-build -A neovim --arg 'localSystem.platform' '(import <nixpkgs/lib>).systems.platforms.raspberrypi' '<nixpkgs>' --show-trace but it doesnt recompile anything. I believe the identifier might be wrong as I've read some issues where it mentioned specific rules.
In pseudo code I would like to do sthg like
nix-build -A neovim --arg 'localSystem' 'localSystem.platform // { kernelArch = "arm"; }' '<nixpkgs>' -vvv , i.e. not having to define localSystem from scratch.

NB: I would like to add your answer to the doc PR I prepare #29944

@Ericson2314
Copy link
Member

@teto that would be something like

$ nix-build -A neovim --arg 'localSystem' 'let lib = (import <nixpkgs/lib>); in lib.recursiveUpdate (lib.systems.elaborate { system = builtins.currentSystem; }) { platform.kernelArch = "arm"; }' '<nixpkgs>' --show-trace

But don't do something that drastic. You should only change the local system like that for a refinement of the build platform, e.g. building an ivybridge-specific kernel rather than generic x86_64. For x86_64 -> arm, you are unavoidably cross compiling and should use crossSystem normal.

@teto
Copy link
Member

teto commented Jan 20, 2018

My example was bad, I was more interested in changing the platform like platform = lib.systems.platforms.pc64_simplekernel; }' which works with your answer \o/
I am glad I asked since the answer is not so trivial. I'll add it to the manual as it is handy until kernel config becomes easier. Once again my apologies for having hijacked the thread.

@Ericson2314
Copy link
Member

@teto no worries and yeah it indeed is cumbersome. So if we were to remove the parameters from platform as you suggested, I'd love, if possible, to make it so the scenario I pointed out is impossible. For example maybe kernelArch could just be computed on the fly from hostPlatform.parsed.arch, whereas other things would be callPackage parameters.

teto added a commit to teto/nixpkgs that referenced this pull request Jan 22, 2018
hostPlatform=super.lib.platforms.pc64_simplekernel;
Also adds command shared by ericson1234 at
NixOS#33813 to further customize kernel
explaining how to really customize kernel config (apart from
	extraConfig).
@Ericson2314 Ericson2314 moved this from After @bgamari's numerous fixes to After big PR in Cross compilation Jan 22, 2018
@vcunat
Copy link
Member

vcunat commented Feb 12, 2018

This caused me quite a headache. Can some of you have a look at #34882 ? I'm not good in the cross stuff...

teto added a commit to teto/nixpkgs that referenced this pull request May 15, 2018
Presents the options available (linuxManualConfig versus overriding
extraConfig, ignoreConfigErrors, autoModules, kernelPreferBuiltin.

For advanced hostPlatform customization refer to the commands shared by ericson1234 at
NixOS#33813 but it is too advanced to
put in the doc.
dezgeg pushed a commit that referenced this pull request Jun 8, 2018
Presents the options available (linuxManualConfig versus overriding
extraConfig, ignoreConfigErrors, autoModules, kernelPreferBuiltin.

For advanced hostPlatform customization refer to the commands shared by ericson1234 at
#33813 but it is too advanced to
put in the doc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: cross-compilation Building packages on a different sort platform than than they will be run on 10.rebuild-darwin: 0 10.rebuild-linux: 501+
Projects
No open projects
Cross compilation
After big PR
Development

Successfully merging this pull request may close these issues.

None yet

6 participants