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

[installer] add processor.max_cstate=1 to intel kernel cmdline #16339

Merged

Conversation

Xichen96
Copy link
Contributor

@Xichen96 Xichen96 commented Aug 30, 2023

Why I did it

This is a fix for PR [kernel] Change grub cmdline to set c-states to 0 for "Intel" CPUs by shlomibitton · Pull Request #6051 · sonic-net/sonic-buildimage (github.com)

The original PR will disable intel idle driver but it cannot limit the max c-state to 1 due to system will fall back to acpi idle driver.

Currently intel_idle.max_cstate=0 is already present, which will disable intel idle driver. With the added option, common idle driver will be disabled as well, so there will not be idle management. This is to prevent a bug that can be triggered by idle instruction on intel platform.

Work item tracking
  • Microsoft ADO (number only): 24867921

How I did it

Add the option to installer file beside intel_idle.max_cstate=0

How to verify it

Manually changed kernel boot option to include the new parameter and reboot.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

20201231.82,20181130.101

Description for the changelog

add processor.max_cstate=0 to intel kernel cmdline

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@@ -481,7 +481,7 @@ bootloader_menu_config()
echo "Switch CPU vendor is: $CPUVENDOR"
if echo "$CPUVENDOR" | grep -i 'Intel' >/dev/null 2>&1; then
echo "Switch CPU cstates are: disabled"
CSTATES="intel_idle.max_cstate=0"
CSTATES="processor.max_cstate=0 intel_idle.max_cstate=0"
Copy link
Contributor

@Blueve Blueve Aug 30, 2023

Choose a reason for hiding this comment

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

Let's use 1 here.
According to document, system will change it to 1 in runtime when user set it to 0.
To avoid confusion, let's use 1 here.

@Xichen96 Xichen96 force-pushed the dev/xichenlin/master-fix-intel-idle branch from a478770 to 2fe873c Compare August 31, 2023 05:33
@Xichen96 Xichen96 changed the title [installer] add processor.max_cstate=0 to intel kernel cmdline [installer] add processor.max_cstate=1 to intel kernel cmdline Aug 31, 2023
@qiluo-msft
Copy link
Collaborator

Please provide the tested image version, not just the branch name.

@Xichen96
Copy link
Contributor Author

Please provide the tested image version, not just the branch name.

@qiluo-msft updated

@mssonicbld
Copy link
Collaborator

@Xichen96 PR conflicts with 202205 branch

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Aug 31, 2023
Why I did it
This is a fix for PR [kernel] Change grub cmdline to set c-states to 0 for "Intel" CPUs by shlomibitton · Pull Request sonic-net#6051 · sonic-net/sonic-buildimage (github.com)

The original PR will disable intel idle driver but it cannot limit the max c-state to 1 due to system will fall back to acpi idle driver.

Currently intel_idle.max_cstate=0 is already present, which will disable intel idle driver. With the added option, common idle driver will be disabled as well, so there will not be idle management. This is to prevent a bug that can be triggered by idle instruction on intel platform.

How I did it
Add the option to installer file beside intel_idle.max_cstate=0
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202211: #16365

mssonicbld pushed a commit that referenced this pull request Aug 31, 2023
Why I did it
This is a fix for PR [kernel] Change grub cmdline to set c-states to 0 for "Intel" CPUs by shlomibitton · Pull Request #6051 · sonic-net/sonic-buildimage (github.com)

The original PR will disable intel idle driver but it cannot limit the max c-state to 1 due to system will fall back to acpi idle driver.

Currently intel_idle.max_cstate=0 is already present, which will disable intel idle driver. With the added option, common idle driver will be disabled as well, so there will not be idle management. This is to prevent a bug that can be triggered by idle instruction on intel platform.

How I did it
Add the option to installer file beside intel_idle.max_cstate=0
@Xichen96
Copy link
Contributor Author

Xichen96 commented Sep 1, 2023

@yxieca changes to 202205 will be in another pr

@mssonicbld
Copy link
Collaborator

@Xichen96 PR conflicts with 202211 branch

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Sep 3, 2023
Why I did it
This is a fix for PR [kernel] Change grub cmdline to set c-states to 0 for "Intel" CPUs by shlomibitton · Pull Request sonic-net#6051 · sonic-net/sonic-buildimage (github.com)

The original PR will disable intel idle driver but it cannot limit the max c-state to 1 due to system will fall back to acpi idle driver.

Currently intel_idle.max_cstate=0 is already present, which will disable intel idle driver. With the added option, common idle driver will be disabled as well, so there will not be idle management. This is to prevent a bug that can be triggered by idle instruction on intel platform.

How I did it
Add the option to installer file beside intel_idle.max_cstate=0
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #16425

sonic-otn pushed a commit to sonic-otn/sonic-buildimage that referenced this pull request Sep 20, 2023
Why I did it
This is a fix for PR [kernel] Change grub cmdline to set c-states to 0 for "Intel" CPUs by shlomibitton · Pull Request sonic-net#6051 · sonic-net/sonic-buildimage (github.com)

The original PR will disable intel idle driver but it cannot limit the max c-state to 1 due to system will fall back to acpi idle driver.

Currently intel_idle.max_cstate=0 is already present, which will disable intel idle driver. With the added option, common idle driver will be disabled as well, so there will not be idle management. This is to prevent a bug that can be triggered by idle instruction on intel platform.

How I did it
Add the option to installer file beside intel_idle.max_cstate=0
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.

6 participants