tpwrules / nixos-apple-silicon

Resources to install NixOS bare metal on Apple Silicon Macs
MIT License
888 stars 92 forks source link

Would like to accept patches that add BTF/BPF support? #133

Open inclyc opened 10 months ago

inclyc commented 10 months ago

Recently I made patches for enable BTF & BPF in kernel config for eBPF programs. Would you like to accept such patches?

This basically changes the kernel config file, and it is not enabled by our upstream (asahi linux) by default, but enabled in NixOS.

Link: https://github.com/inclyc/nixos-apple-silicon/commit/18fe3ffd1bf24fac83943c93269b39eefcd0abe1

I can make the draft patch cleaner if the idea gets approved, thanks!

zzywysm commented 10 months ago

I added BPF support in https://github.com/tpwrules/nixos-apple-silicon/pull/134

I don't think BTF is going to be useful for most users.

inclyc commented 10 months ago

I don't think BTF is going to be useful for most users.

But most desktop distributions enable this option (e.g. Debian/Arch/NixOS (nixpkgs)) and I do need this feature. Why asahi - NixOS is special among these distributions / or our upstream NixOS ?

zzywysm commented 10 months ago

BTF is really great if you're developing BPF programs. BPF programs are intended to run within the kernel itself.

In other words, if you find BTF useful, you're likely to be a kernel developer, which means you probably know how to enable DEBUG_INFO_BTF within your NixOS kernel configuration.

By leaving it off, we avoid generating a bunch of debug crud that most folks will never use or care about.

inclyc commented 10 months ago

In other words, if you find BTF useful, you're likely to be a kernel developer, which means you probably know how to enable DEBUG_INFO_BTF within your NixOS kernel configuration.

Yes, I have enabled this in my own fork. So I just kindly ask may I merge this setting into upstream.

By leaving it off, we avoid generating a bunch of debug crud that most folks will never use or care about.

Just FYI NixOS enabled this option by default. This is not a useless option but necessary for many BPF tools.

This is used by bpftrace and lots of new eBPF-based tooling to avoid a dependency on LLVM on the host.

https://github.com/NixOS/nixpkgs/blob/98a0c372a314729fa4d811063ea2ad7f230b2f9b/pkgs/os-specific/linux/kernel/common-config.nix#L47

Did you mean that NixOS (in nixpkgs) is generating "a bunch of debug crud that most folks will never use or care about"?

never use or care about.

We introduced BTF in nixpkgs in this patch: https://github.com/NixOS/nixpkgs/pull/127922

And quote:

BTF debug information is enabled on all major distributions: Fedora 31+, RHEL 8.2+, Ubuntu 20.10, Debian 11 and ArchLinux all have enabled it.

I just cannot image that such "useless" feature is enabled by default in those major distros.

zzywysm commented 10 months ago

I just cannot image that such "useless" feature is enabled by default in those major distros.

Then you should take a closer look at some of the major distribution's kernel configurations. Holy mackerel do they enable a lot of stuff that will never ever be used. The Fedora Asahi Remix kernel config is an excellent example -- dozens if not hundreds of useless drivers!

https://copr-dist-git.fedorainfracloud.org/cgit/@asahi/kernel/kernel.git/tree/kernel-aarch64-16k-fedora.config?h=f39

This reality gets even more offensive when you learn about return-oriented progamming, and realize that this extra code is giving attackers more gadgets to play with.

https://en.wikipedia.org/wiki/Return-oriented_programming

inclyc commented 10 months ago

Then you should take a closer look at some of the major distribution's kernel configurations. Holy mackerel do they enable a lot of stuff that will never ever be used. The Fedora Asahi Remix kernel config is an excellent example -- dozens if not hundreds of useless drivers!

I totally agree that these useless drivers should not be enabled in our (asahi-linux) kernel config, and thanks for you excellent patch to address this!

However my point is: BTF is not a "useless" feature, major BPF tools do utilize BTF. e.g.

bpftrace: https://github.com/iovisor/bpftrace/blob/1c5fac13d75ba250dfb3f51b3367571021867228/docs/reference_guide.md?plain=1#L3862 libbpf: https://github.com/andreasgerstmayr/bcc/blob/5f748492a3d2ae95fb6c2934b0251e1d5b92dac3/libbpf-tools/README.md?plain=1#L96

This reality gets even more offensive when you learn about return-oriented progamming, and realize that this extra code is giving attackers more gadgets to play with.

That's a good point! I suppose you are talking about those drivers, not BTF? In my humble opinion "debug info" does not affect compiled code at all.

inclyc commented 10 months ago

Also kindly ping @tpwrules , WDYT?

psanford commented 10 months ago

The only thing that makes sense long term is to use the default nixpkgs aarch64 kernel config + asahi specific modules. That will give the most users the behavior that they expect (for drivers to be available without requiring a custom config).

Folks who want non-standard kernel options can then do that themselves using normal nix kernel configuration.

zzywysm commented 10 months ago

The only thing that makes sense long term is to use the default nixpkgs aarch64 kernel config + asahi specific modules. That will give the most users the behavior that they expect (for drivers to be available without requiring a custom config).

Folks who want non-standard kernel options can then do that themselves using normal nix kernel configuration.

When NixOS on Apple Silicon is officially supported by the NixOS/nixpkgs organization, then I agree with your logic 100%. I believe that @tpwrules has talked with them about that possibility, and I imagine they are likely waiting on the Asahi Linux code to get upstreamed into the vanilla Linux kernel. (There are still hundreds of commits that haven't been rebased and submitted for consideration by Hector and friends.). Perhaps this project can become part of the nixos-hardware project at that time.

https://github.com/NixOS/nixos-hardware (Note the first line of the nixos-hardware README is "NixOS profiles to optimize settings for different hardware." Eeeeew, who would ever want that?! 🤣)

But this is a project that is specifically about NixOS on Apple Silicon. At this point I don't understand the concern about making every conceivable driver available -- we should endeavor to enable the drivers for hardware that folks might plug into their Mac, and make it easy for power users and developers to enable the options they need, e.g. DEBUG_INFO_BTF. I think this project is succeeding at that.

This project already ships a custom sound system, a custom OpenGL codebase, a custom bootloader stack, and a custom installation ISO image. And yet @psanford (and @tpwrules elsewhere) are very concerned that our kernel isn't standard enough. To which I respond: huh?!

For reference, I am attaching the default NixOS arm64 kernel configuration, which I obtained by creating a NixOS VM inside QEMU/KVM, injecting the IKCONFIG option into the kernel and rebuilding it, then after a reboot running zcat /proc/config.gz > hotmess.txt . Note that just the size of the config file itself is almost twice the size of the configs I suggest in my pull requests. It has support for every conceivable arm64 SoC that Linux itself supports, even though this project will never boot on those SoCs, nor is it intended to. The technical term for this is "bloat", and it makes @tpwrules' build process take longer, and it fills up your EFI System Partition really fast.

hotmess.txt

psanford commented 10 months ago

Note that just the size of the config file itself is almost twice the size of the configs I suggest in my pull requests. It has support for every conceivable arm64 SoC that Linux itself supports, even though this project will never boot on those SoCs, nor is it intended to. The technical term for this is "bloat", and it makes @tpwrules' build process take longer, and it fills up your EFI System Partition really fast.

Not really. The reason we compile most things as kernel modules is so that we don't have to load them if they are not needed, but they are available on demand. Its also why we only include the required modules in the initrd. Looking at a stock nixos amd64 install, the kernel images and initrd files are roughly the same size as my nixos-apple-silicon install.

The main benefit to using the stock nixos kernel config with asahi specific changes applied is that it lowers the ongoing maintenance cost. We don't need to have constant discussions/debates about what should or should not be enabled.

If you want to run an optimized kernel, squeezing out those last few kb of kernel memory you can, and nixos makes that very easy to do. But for most folks the default kernel will give the expected experience of installing linux and having all the drivers available.

zzywysm commented 10 months ago

Not really. The reason we compile most things as kernel modules is so that we don't have to load them if they are not needed, but they are available on demand. Its also why we only include the required modules in the initrd. Looking at a stock nixos amd64 install, the kernel images and initrd files are roughly the same size as my nixos-apple-silicon install.

This still doesn't explain why it's desirable to use the NixOS default config at this time. I doubt that GitHub is thrilled about us running our builds on their infrastructure when a lot of the compiling happening is completely pointless. Or should we be wasting more electricity, not less?

The main benefit to using the stock nixos kernel config with asahi specific changes applied is that it lowers the ongoing maintenance cost. We don't need to have constant discussions/debates about what should or should not be enabled.

This particular complaint is not directed at you, @psanford, but I am making it in a reply to you. My bad.

You worry about maintenance cost. I have been offering my time to do maintenance of this project's kernel situation for months now. Often my pull requests are closed and not pulled. Currently I have two pull requests sitting for 5-7 days without any reply. If we are concerned about maintenance cost, why are y'all disrespecting free maintenance?

If you want to run an optimized kernel, squeezing out those last few kb of kernel memory you can, and nixos makes that very easy to do. But for most folks the default kernel will give the expected experience of installing linux and having all the drivers available.

In my experience spending days enabling and disabling config options, rebuilding kernels, and occasionally noticing very visible performance penalties, you can quite easily make wrong choices. Which choices has NixOS made that aren't very good? Do you know? Do you know how to find out? Do you really think that NixOS on arm64 gets anywhere the same tender love & care that amd64 receives?

psanford commented 10 months ago

Currently I have two pull requests sitting for 5-7 days without any reply.

Reviewing PRs is a burden on maintainers. If you use the stock kernel config that is fewer ongoing PRs to review. It also fixes issues like this one where a default config option (enabling BPF/BTF) is present. That's the maintenance cost I'm talking about.

In my experience spending days enabling and disabling config options, rebuilding kernels, and occasionally noticing very visible performance penalties, you can quite easily make wrong choices. Which choices has NixOS made that aren't very good? Do you know? Do you know how to find out? Do you really think that NixOS on arm64 gets anywhere the same tender love & care that amd64 receives?

As a much younger person (2003-2004), I ran gentoo and spent way too much time "optimizing" my kernel and compiler settings. For all intents and purposes, that was a complete waste of time.

My professional career involves running linux systems at scale where small performance improvements can save millions of dollars. I in fact know quite a bit about systems optimization. The correct way to do performance optimization requires a rigorous methodology. You need to have well defined workloads with measurable performance characteristics. Any change you make for the sake of performance needs to be justified by demonstrating the actual measured change in those metrics.

None of your PRs have included any actual performance metrics or reproducible test cases. I can't tell from your PRs if they make performance better or worse because you have not measured anything.

It seems like you are really excited to contribute to this project. I don't want to discourage that, but I would like to nudge you in the direction of contributions that are more likely to be accepted. I think a PR for using the stock nixos kernel config is more likely to be accepted than your past kernel PRs (I don't speak for tpwrules so I might be wrong here).

In any case, I've said everything I care to on this topic so I'm going to leave this conversation here. I hope you are able to find productive ways to contribute to the project. Cheers!

zzywysm commented 10 months ago

That's a good point! I suppose you are talking about those drivers, not BTF? In my humble opinion "debug info" does not affect compiled code at all.

From the kernel page on the NixOS wiki (which I swear I didn't put there):

Troubleshooting
Build fails
Too high ram usage

turn off `DEBUG_INFO_BTF`

(Source: https://nixos.wiki/wiki/Kernel )

Clearly this is something everyone wants on their Mac which has 8GB RAM in the most common configuration.

inclyc commented 10 months ago

This reality gets even more offensive when you learn about return-oriented progamming, and realize that this extra code is giving attackers more gadgets to play with.

That's a good point! I suppose you are talking about those drivers, not BTF? In my humble opinion "debug info" does not affect compiled code at all.

Clearly this is something everyone wants on their Mac which has 8GB RAM in the most common configuration.

Your previous discussion was about security issues. What I want to point out is that adding debugging information does not cause any CVEs you mentioned. The link you provided, as per my understanding, seems to only reduce memory consumption during "compile time" instead of "compiled code".

Clearly this is something everyone wants on their Mac which has 8GB RAM in the most common configuration.

Regarding reducing memory consumption during the compile time, I think @psanford has already had a detailed & professional discussion with you. I also kindly suggest you considering his opinions.

My professional career involves running linux systems at scale where small performance improvements can save millions of dollars. I in fact know quite a bit about systems optimization. The correct way to do performance optimization requires a rigorous methodology. You need to have well defined workloads with measurable performance characteristics. Any change you make for the sake of performance needs to be justified by demonstrating the actual measured change in those metrics.

zzywysm commented 10 months ago

Your previous discussion was about security issues. What I want to point out is that adding debugging information does not cause any CVEs you mentioned. The link you provided, as per my understanding, seems to only reduce memory consumption during "compile time" instead of "compiled code".

  1. I hesitate to share this next piece of information with you, because you may find it novel and shocking, but an idea can be bad for multiple reasons. If you tell me you're visiting the top of a volcano on vacation, and I warn that you might fall in, there is still a possibility the volcano will erupt.
  2. From tpwrules' own mouth in the installation guide: Building the image requires downloading of a large amount of data and compilation of a number of packages, including the kernel.
inclyc commented 10 months ago

IIRC systemd's next version depends on BTF. Just for a note.

https://github.com/systemd/systemd/pull/26826/files#diff-30d8f6be6320feeacf686be94f48c70869b52630e01ea625f0f15adc0d57c3e4R1856

zzywysm commented 10 months ago

IIRC systemd's next version depends on BTF. Just for a note.

https://github.com/systemd/systemd/pull/26826/files#diff-30d8f6be6320feeacf686be94f48c70869b52630e01ea625f0f15adc0d57c3e4R1856

If by "depends on BTF" you mean "systemd will try to use BTF as a fallback if it can't find a vmlinux image using its primary preferred method", then yes, you are correct.

Or in other words: read the large comment above the line you linked to.

rowanG077 commented 10 months ago

Please desalinate the comments here. This is just a discussion on whether to include some features. No reason to move this discussion in such a hostile direction.

yu-re-ka commented 9 months ago

The only thing that makes sense long term is to use the default nixpkgs aarch64 kernel config + asahi specific modules. That will give the most users the behavior that they expect (for drivers to be available without requiring a custom config).

If you are on nixos-unstable, you can try my branch https://github.com/yu-re-ka/nixos-m1/tree/minimize-patches which does exactly this.