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

linuxManualConfig: forbid config errors on aarch64 #366004

Merged
merged 7 commits into from
Dec 24, 2024

Conversation

alyssais
Copy link
Member

It's not a nice experience to wait for a kernel build, only to notice the options you set didn't actually do anything, because Nixpkgs' kernel configuration script just prints a warning if an option that should have been set isn't set in the final kernel config.

Ideally, I think we'd never allow config errors, but we certainly don't have to torelate them for aarch64, when CI for kernel changes can ensure we don't accidentally break the build with x86-specific options.

I've tested that configuration of all mainline kernels still works on aarch64.

Further details of the required config cleanups in the commit messages.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 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.

Add a 👍 reaction to pull requests you find important.

@alyssais alyssais requested review from K900 and Ma27 December 17, 2024 21:40
@alyssais alyssais force-pushed the Kconfig-errors-aarch64 branch from 277d420 to 76b0977 Compare December 17, 2024 21:47
@alyssais alyssais marked this pull request as draft December 18, 2024 09:02
@alyssais alyssais force-pushed the Kconfig-errors-aarch64 branch from 76b0977 to f0fda1d Compare December 18, 2024 09:58
@alyssais alyssais marked this pull request as ready for review December 18, 2024 09:59
@alyssais
Copy link
Member Author

Verified that despite the rebuilds, there's no config change for either linux_testing on linux_5_4 for x86_64.

Here's the aarch64-linux diff for linux_testing:

diff --git 1/nix/store/jss4zq5lywz7hm91vzix9n6brk6g506g-linux-config-6.13-rc2 2/nix/store/3gm4nj2xnggm40rra7cc7sc85vs4j6py-linux-config-6.13-rc2
index 39fa2d14a988..aa154fa51d05 100644
--- 1/nix/store/jss4zq5lywz7hm91vzix9n6brk6g506g-linux-config-6.13-rc2
+++ 2/nix/store/3gm4nj2xnggm40rra7cc7sc85vs4j6py-linux-config-6.13-rc2
@@ -635,7 +635,11 @@ CONFIG_PM_WAKELOCKS=y
 CONFIG_PM_WAKELOCKS_LIMIT=100
 CONFIG_PM_WAKELOCKS_GC=y
 CONFIG_PM=y
-# CONFIG_PM_DEBUG is not set
+CONFIG_PM_DEBUG=y
+CONFIG_PM_ADVANCED_DEBUG=y
+# CONFIG_PM_TEST_SUSPEND is not set
+CONFIG_PM_SLEEP_DEBUG=y
+# CONFIG_DPM_WATCHDOG is not set
 CONFIG_PM_CLK=y
 CONFIG_PM_GENERIC_DOMAINS=y
 CONFIG_WQ_POWER_EFFICIENT_DEFAULT=y
@@ -1318,7 +1322,10 @@ CONFIG_NET_IP_TUNNEL=m
 CONFIG_NET_IPGRE=m
 # CONFIG_NET_IPGRE_BROADCAST is not set
 CONFIG_IP_MROUTE_COMMON=y
-# CONFIG_IP_MROUTE is not set
+CONFIG_IP_MROUTE=y
+CONFIG_IP_MROUTE_MULTIPLE_TABLES=y
+# CONFIG_IP_PIMSM_V1 is not set
+# CONFIG_IP_PIMSM_V2 is not set
 CONFIG_SYN_COOKIES=y
 CONFIG_NET_IPVTI=m
 CONFIG_NET_UDP_TUNNEL=m
@@ -4705,7 +4712,45 @@ CONFIG_MOUSE_VSXXXAA=m
 CONFIG_MOUSE_GPIO=m
 CONFIG_MOUSE_SYNAPTICS_I2C=m
 CONFIG_MOUSE_SYNAPTICS_USB=m
-# CONFIG_INPUT_JOYSTICK is not set
+CONFIG_INPUT_JOYSTICK=y
+CONFIG_JOYSTICK_ANALOG=m
+CONFIG_JOYSTICK_A3D=m
+CONFIG_JOYSTICK_ADC=m
+CONFIG_JOYSTICK_ADI=m
+CONFIG_JOYSTICK_COBRA=m
+CONFIG_JOYSTICK_GF2K=m
+CONFIG_JOYSTICK_GRIP=m
+CONFIG_JOYSTICK_GRIP_MP=m
+CONFIG_JOYSTICK_GUILLEMOT=m
+CONFIG_JOYSTICK_INTERACT=m
+CONFIG_JOYSTICK_SIDEWINDER=m
+CONFIG_JOYSTICK_TMDC=m
+CONFIG_JOYSTICK_IFORCE=m
+CONFIG_JOYSTICK_IFORCE_USB=m
+CONFIG_JOYSTICK_IFORCE_232=m
+CONFIG_JOYSTICK_WARRIOR=m
+CONFIG_JOYSTICK_MAGELLAN=m
+CONFIG_JOYSTICK_SPACEORB=m
+CONFIG_JOYSTICK_SPACEBALL=m
+CONFIG_JOYSTICK_STINGER=m
+CONFIG_JOYSTICK_TWIDJOY=m
+CONFIG_JOYSTICK_ZHENHUA=m
+CONFIG_JOYSTICK_DB9=m
+CONFIG_JOYSTICK_GAMECON=m
+CONFIG_JOYSTICK_TURBOGRAFX=m
+CONFIG_JOYSTICK_AS5011=m
+CONFIG_JOYSTICK_JOYDUMP=m
+CONFIG_JOYSTICK_XPAD=m
+CONFIG_JOYSTICK_XPAD_FF=y
+CONFIG_JOYSTICK_XPAD_LEDS=y
+CONFIG_JOYSTICK_WALKERA0701=m
+CONFIG_JOYSTICK_PSXPAD_SPI=m
+CONFIG_JOYSTICK_PSXPAD_SPI_FF=y
+CONFIG_JOYSTICK_PXRC=m
+CONFIG_JOYSTICK_QWIIC=m
+CONFIG_JOYSTICK_FSIA6B=m
+CONFIG_JOYSTICK_SENSEHAT=m
+CONFIG_JOYSTICK_SEESAW=m
 # CONFIG_INPUT_TABLET is not set
 CONFIG_INPUT_TOUCHSCREEN=y
 CONFIG_TOUCHSCREEN_ADS7846=m
@@ -13524,17 +13569,17 @@ CONFIG_EROFS_FS_ZIP=y
 # CONFIG_EROFS_FS_ONDEMAND is not set
 # CONFIG_EROFS_FS_PCPU_KTHREAD is not set
 CONFIG_NETWORK_FILESYSTEMS=y
-CONFIG_NFS_FS=y
+CONFIG_NFS_FS=m
 CONFIG_NFS_V2=m
-CONFIG_NFS_V3=y
+CONFIG_NFS_V3=m
 CONFIG_NFS_V3_ACL=y
-CONFIG_NFS_V4=y
+CONFIG_NFS_V4=m
 CONFIG_NFS_SWAP=y
 CONFIG_NFS_V4_1=y
 CONFIG_NFS_V4_2=y
-CONFIG_PNFS_FILE_LAYOUT=y
+CONFIG_PNFS_FILE_LAYOUT=m
 CONFIG_PNFS_BLOCK=m
-CONFIG_PNFS_FLEXFILE_LAYOUT=y
+CONFIG_PNFS_FLEXFILE_LAYOUT=m
 CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN="kernel.org"
 # CONFIG_NFS_V4_1_MIGRATION is not set
 CONFIG_NFS_V4_SECURITY_LABEL=y
@@ -13554,19 +13599,19 @@ CONFIG_NFSD_V4=y
 # CONFIG_NFSD_V4_2_INTER_SSC is not set
 CONFIG_NFSD_V4_SECURITY_LABEL=y
 CONFIG_NFSD_LEGACY_CLIENT_TRACKING=y
-CONFIG_GRACE_PERIOD=y
-CONFIG_LOCKD=y
+CONFIG_GRACE_PERIOD=m
+CONFIG_LOCKD=m
 CONFIG_LOCKD_V4=y
-CONFIG_NFS_ACL_SUPPORT=y
+CONFIG_NFS_ACL_SUPPORT=m
 CONFIG_NFS_COMMON=y
-CONFIG_NFS_COMMON_LOCALIO_SUPPORT=y
+CONFIG_NFS_COMMON_LOCALIO_SUPPORT=m
 CONFIG_NFS_LOCALIO=y
 CONFIG_NFS_V4_2_SSC_HELPER=y
-CONFIG_SUNRPC=y
-CONFIG_SUNRPC_GSS=y
+CONFIG_SUNRPC=m
+CONFIG_SUNRPC_GSS=m
 CONFIG_SUNRPC_BACKCHANNEL=y
 CONFIG_SUNRPC_SWAP=y
-CONFIG_RPCSEC_GSS_KRB5=y
+CONFIG_RPCSEC_GSS_KRB5=m
 CONFIG_RPCSEC_GSS_KRB5_ENCTYPES_AES_SHA1=y
 # CONFIG_RPCSEC_GSS_KRB5_ENCTYPES_CAMELLIA is not set
 # CONFIG_RPCSEC_GSS_KRB5_ENCTYPES_AES_SHA2 is not set

@alyssais alyssais force-pushed the Kconfig-errors-aarch64 branch 2 times, most recently from acc1674 to 06074aa Compare December 18, 2024 11:45
@@ -60,7 +60,8 @@ lib.makeOverridable ({ # The kernel source tarball.
# symbolic name and `patch' is the actual patch. The patch may
# optionally be compressed with gzip or bzip2.
kernelPatches ? []
, ignoreConfigErrors ? stdenv.hostPlatform.linux-kernel.name != "pc"
, ignoreConfigErrors ?
!lib.elem stdenv.hostPlatform.linux-kernel.name [ "aarch64-multiplatform" "pc" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make this a normal platform check or something? It took me a bit to figure out why pc is here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would like that in the end, but I want to be gradual about it — people using platforms other than the defaults are more likely to have weird configs that'll break, so I think we should start with this, make sure it doesn't break too much, and then further expand the ignoreConfigErrors default.

Copy link
Contributor

@K900 K900 left a comment

Choose a reason for hiding this comment

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

Overall LGTM

On x86, these get implictly defined, presumably through dependencies,
defconfig, etc.  On aarch64, they did not, which generated warnings
for the options now immediately following them in our config which
depended on them.
This leads to fewer warnings when configuring a kernel for other
platforms, and will make it possible to validate configs for other
platforms like we do for pc.
This is an internal option that isn't asked about.  All this line does
is produce a warning on architectures that
This is built in by default on aarch64, where the defconfig has a bad
habit of including way too much stuff.  It's particularly bad in this
case, because it prevents NFS_FSCACHE being set, because FSCACHE is a
module — for NFS_FSCACHE, both NFS and FSCACHE have to be built in, or
both have to be modules.
The option never existed on aarch64.
It's not a nice experience to wait for a kernel build, only to notice
the options you set didn't actually do anything, because Nixpkgs'
kernel configuration script just prints a warning if an option that
should have been set isn't set in the final kernel config.

Ideally, I think we'd never allow config errors, but we certainly
don't have to torelate them for aarch64, when CI for kernel changes
can ensure we don't accidentally break the build with x86-specific
options.

I've tested that configuration of all mainline kernels still works on
aarch64.
@alyssais alyssais force-pushed the Kconfig-errors-aarch64 branch from 06074aa to b44d3ea Compare December 19, 2024 10:32
@alyssais alyssais changed the base branch from master to staging December 20, 2024 18:35
@alyssais alyssais merged commit 37789d9 into NixOS:staging Dec 24, 2024
46 checks passed
@alyssais alyssais deleted the Kconfig-errors-aarch64 branch December 24, 2024 11:40
@misuzu
Copy link
Contributor

misuzu commented Jan 10, 2025

This broke eval on riscv64-linux (staging-next):

% nix-build . -A pkgs.linux --system riscv64-linux --dry-run
error:
       … in the condition of the assert statement
         at /home/misuzu/nixpkgs/lib/customisation.nix:417:9:
          416|       drvPath =
          417|         assert condition;
             |         ^
          418|         drv.drvPath;

       … while evaluating a branch condition
         at /home/misuzu/nixpkgs/pkgs/stdenv/generic/check-meta.nix:504:6:
          503|       inherit (validity) valid;
          504|   in if validity ? handled then validity else validity // {
             |      ^
          505|       # Throw an error if trying to evaluate a non-valid derivation

       … while evaluating the option `settings':

       … while evaluating definitions from `pkgs/os-specific/linux/kernel/common-config.nix':

       (stack trace truncated; use '--show-trace' to show the full, detailed trace)

       error: undefined variable 'isLoongarch64'
       at /home/misuzu/nixpkgs/pkgs/os-specific/linux/kernel/common-config.nix:512:51:
          511|           || (lib.versionAtLeast version "6.2" && isAarch64 && !stdenv.cc.isClang)
          512|           || (lib.versionAtLeast version "6.5" && isLoongarch64 && !stdenv.cc.isClang)
             |                                                   ^
          513|           || (lib.versionAtLeast version "6.10" && isRiscV64 && !stdenv.cc.isClang)

Reverting c4a1f9e fixes the issue

@misuzu
Copy link
Contributor

misuzu commented Jan 10, 2025

Opened #372726

@vcunat
Copy link
Member

vcunat commented Jan 18, 2025

5.4 doesn't seem to pass and a few others:
https://hydra.nixos.org/eval/1811099?filter=linuxKernel.kernels.linux_&compare=1811081#tabs-now-fail

/cc the staging-next PR #371501

@alyssais
Copy link
Member Author

Fix for 5.4: #375078

For the non-mainline kernels the maintainers of those should have a look, because it's likely that something unexpected is happening with their kernel configs.

brianmcgillion added a commit to brianmcgillion/jetpack-nixos that referenced this pull request Jan 22, 2025
NixOS/nixpkgs#366004 introduced a change that
warns and errors out when modules are defined and not acutally set for a
particular target.

This patch rectifies that by going back the the previous method of
ignoring those errors and not breaking the build. Commentary is left
inline to explain the situation and possible future solutions.

tested against orin agx target for ghaf.

there are similar solutions being deployed in nixpkgs to work around
this e.g.

NixOS/nixpkgs#375165

Signed-off-by: Brian McGillion <[email protected]>
brianmcgillion added a commit to brianmcgillion/ghaf that referenced this pull request Jan 22, 2025
NixOS/nixpkgs#366004

this addresses the issue with the jetpack-nixos boards

Signed-off-by: Brian McGillion <[email protected]>
brianmcgillion added a commit to brianmcgillion/nixos-hardware that referenced this pull request Jan 22, 2025
NixOS/nixpkgs#366004
introduced a breaking change that if a module is declared but it is not
being used it will fail.

Signed-off-by: Brian McGillion <[email protected]>
brianmcgillion added a commit to brianmcgillion/nixos-hardware that referenced this pull request Jan 22, 2025
this fixes the unused modules error that wad introduced by NixOS/nixpkgs#366004

Signed-off-by: Brian McGillion <[email protected]>
brianmcgillion added a commit to brianmcgillion/nixos-hardware that referenced this pull request Jan 22, 2025
this fixes the unused modules error that wad introduced by NixOS/nixpkgs#366004

Signed-off-by: Brian McGillion <[email protected]>
brianmcgillion added a commit to brianmcgillion/ghaf that referenced this pull request Jan 22, 2025
NixOS/nixpkgs#366004

this addresses the issue with the jetpack-nixos boards

Signed-off-by: Brian McGillion <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants