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

Check MSR in each CPU/Thread #136

Merged
merged 3 commits into from
Mar 17, 2018

Conversation

abazhaniuk
Copy link
Contributor

When we check Spectre and Meltdown ucode patches and MSR configuration need to run all HW checks for each CPU.
In case we check just for CPU with index 0 (like /dev/cpu/0/msr) we missing check for rest CPUs. In case if ucode was not applied for all CPU or ucode has different behavior on other CPU we won't detect it.
This patch is reading MSR in each CPU and checking that result is the consistent in each CPU.

@abazhaniuk
Copy link
Contributor Author

any updates about this PR?

@speed47
Copy link
Owner

speed47 commented Feb 15, 2018

I'm working on getting v0.35 out with the pending fixes mostly for variant1 and other stuff that has already been merged to master before going offline for holidays... tomorrow!
Then we'll discuss your PR, it might need some adjustments, but I agree with the goal: we should check all cpu cores indeed.

@abazhaniuk
Copy link
Contributor Author

what adjustments you want me to make?

@speed47
Copy link
Owner

speed47 commented Feb 27, 2018

Your branch raises several shellcheck warnings, can you fix them?
(shellcheck -s sh spectre-meltdown-checker.sh)

@@ -1041,13 +1041,48 @@ sys_interface_check()
return 0
}

number_of_cpus()
{
n=$(cat /proc/cpuinfo | grep processor| wc -l)
Copy link
Owner

Choose a reason for hiding this comment

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

n=$(grep -c ^processor /proc/cpuinfo) ?

pstatus green YES
else
spec_ctrl_msr=1
pstatus green YES "(But not in all CPUs)"
Copy link
Owner

Choose a reason for hiding this comment

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

() are already added by pstatus on the 3rd param

if [ $cpu_mismatch -eq 0 ]; then
pstatus green YES
else
pstatus green YES "(But not in all CPUs)"
Copy link
Owner

Choose a reason for hiding this comment

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

() are already added by pstatus on the 3rd param

@abazhaniuk
Copy link
Contributor Author

Thanks for review. Fixed all issues/warnings.

@abazhaniuk
Copy link
Contributor Author

any suggestion? comments?

@speed47 speed47 merged commit ecdc448 into speed47:master Mar 17, 2018
@speed47
Copy link
Owner

speed47 commented Mar 17, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants