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.

Sign up for GitHub

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

Jump to bottom

msr-tools: nixos module to avoid CPU throttling #29764

Closed
wants to merge 1 commit into from
Closed

msr-tools: nixos module to avoid CPU throttling #29764

wants to merge 1 commit into from

Conversation

peterhoeg
Copy link
Member

Motivation for this change

The BIOS on the Dell Latitude E6x00 series is badly broken (despite 38 tries at getting a working version) and the BIOS will throttle the CPU even with just a little load.

This PR keeps kicking the CPU into the maximum power state to avoid any kind of throttling.

As a consequence the hardware will get QUITE hot, but at least it will perform.

I'm not entirely sure what is the best way to ship a feature that is capable of physically harming the hardware. Do we need more warnings?

Additionally, I don't know if the magic numbers being written to the registers will also work on other CPUs/BIOSes.

I could make the registers configurable (or hide them behind a model variable that sets them in case anyone has any good ideas.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • 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.

@mention-bot
Copy link

@peterhoeg, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @offlinehacker and @bjornfor to be potential reviewers.

@joachifm
Copy link
Contributor

Half joking but I think the option needs a scarier name, like meltMyCpuPlease.

@peterhoeg
Copy link
Member Author

I've made it possible to add more hardware in a structured way and emphasized the danger in the description.

@edolstra
Copy link
Member

I think that modules to work around bugs in very specific pieces of hardware might be more appropriate for the https://github.com/NixOS/nixos-hardware repo.

@peterhoeg
Copy link
Member Author

peterhoeg commented Sep 26, 2017 •
edited

The hardware repo is really for "recommended settings for particular pieces of hardware" which probably makes that repo equally unsuitable for this PR...

We can today configure many aspects of the various pieces hardware supported by NixOS - I think this PR is just one aspect of that.

The questions as I see them:

  1. does this type of PR belong in the main repo?
    a) if not, is it then nixos-hardware?
    b) if not nixos-hardware, where then?
  2. assuming it does belong, are the warnings sufficiently strongly worded?
  3. I did contemplate putting in a remark about running with this option enabled for ~6 months on a machine dedicated to kodi without any problems, but just because it hasn't gone up in smoke doesn't mean it won't.

@peterhoeg peterhoeg requested a review from infinisil as a code owner May 2, 2019 02:04
JohnAZoidberg
JohnAZoidberg reviewed May 2, 2019
View reviewed changes
nixos/modules/hardware/cpu/intel-peg.nix
machineConfigs = {
dell_latitude_e6x00 = {
link = "https://wiki.archlinux.org/index.php/Dell_Latitude_E6x00";
read = "44:32 0xCE";
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you could explain (or at least link to something that explains) how to figure out which values to use for a particular machine.

@JohnAZoidberg
Copy link
Member

JohnAZoidberg commented May 2, 2019 •
edited

I see it like this:
The module itself belongs in nixpkgs, as it is a convenient way to use msr-tools.
But if we're going to add more hardware specific settings like machineConfigs.dell_latitude_e6x00 that belongs in nixos-hardware. Maybe we could offload that part to the hardware repo.
If we had to choose between either repos, I'd keep it here - because it can be used without hardware presets.

I also think the graveness of this option should be conveyed in the option name itself.

@c0bw3b
Copy link
Contributor

c0bw3b commented May 15, 2019

There is some similar work in nixos-hardware by now.
This is better suited for this repo and it can probably re-use the common base.

@c0bw3b c0bw3b closed this May 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohnAZoidberg JohnAZoidberg JohnAZoidberg left review comments

@infinisil infinisil Awaiting requested review from infinisil

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@peterhoeg @mention-bot @joachifm @edolstra @JohnAZoidberg @c0bw3b