Page MenuHomeFreeBSD

Add support for microcode update on newer AMD CPUs (10h+)
ClosedPublic

Authored by avg on Oct 30 2016, 1:41 PM.

Details

Summary

This includes new code for parsing microcode files as well as
the kernel-side change to apply the update on all processors
at the same time.

Developed with help from Borislav Petkov, formerly bp@amd64.org.

Test Plan

Tested using Athlon II X2 processor on a system where BIOS does
not have the latest microcode version:
/boot/firmware/microcode_amd.bin: updating cpu /dev/cpuctl0 to revision 0x10000c7... done.

The microcode file is taken from here:
https://web.archive.org/web/20160528230514/http://www.amd64.org/microcode.html
(note that the site seems to be down at the moment)
It can also be found here:
https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/amd-ucode

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

avg updated this revision to Diff 21806.Oct 30 2016, 1:41 PM
avg retitled this revision from to Add support for microcode update on newer AMD CPUs (10h+).
avg updated this object.
avg edited the test plan for this revision. (Show Details)
avg added reviewers: kib, stas.
kib added inline comments.Oct 30 2016, 3:36 PM
sys/dev/cpuctl/cpuctl.c
60 ↗(On Diff #21806)

This is unrelated, commit separately (now ?).

396 ↗(On Diff #21806)

Why is this required ? I do not see any harm from allowing the update to run from arbitrary /dev/cpuctlN.

428 ↗(On Diff #21806)

Check for ptr != NULL is not needed. Use plain free(9).

usr.sbin/cpucontrol/amd10h.c
2 ↗(On Diff #21806)

Copyright line should be updated.

stas accepted this revision.Oct 31 2016, 8:07 AM
stas edited edge metadata.

Looks good besides what kib@ already mentioned.

Thanks!

sys/dev/cpuctl/cpuctl.c
396 ↗(On Diff #21806)

I agree. Unless there are restrictions on which cores the update can be performed on... But in that case the check will have to be a bit more sophisticated to account for multi-socket configurations.

usr.sbin/cpucontrol/amd10h.c
174 ↗(On Diff #21806)

Double space.

This revision is now accepted and ready to land.Oct 31 2016, 8:07 AM
avg updated this revision to Diff 21828.Oct 31 2016, 10:15 AM
avg edited edge metadata.

address comments from kib

This revision now requires review to proceed.Oct 31 2016, 10:15 AM
kib accepted this revision.Oct 31 2016, 12:21 PM
kib edited edge metadata.
kib added inline comments.
usr.sbin/cpucontrol/amd10h.c
37 ↗(On Diff #21828)

sys/ headers should come before the usermode headers.

299 ↗(On Diff #21828)

space after if

This revision is now accepted and ready to land.Oct 31 2016, 12:21 PM
avg updated this revision to Diff 21947.Nov 2 2016, 3:16 PM
avg edited edge metadata.

really address kib's comment and replace contigfree with free

This revision now requires review to proceed.Nov 2 2016, 3:16 PM
avg added inline comments.Nov 2 2016, 3:20 PM
sys/dev/cpuctl/cpuctl.c
396 ↗(On Diff #21806)

There is really no restriction. I just wanted to skip unnecessary writes to the update MSR in the default configuration with microcode_cpus="ALL". But that was not needed anyway, because the first invocation of cpucontrol would update all CPUs and subsequent invocations would see that the CPUs have the latest version.

avg updated this revision to Diff 21948.Nov 2 2016, 3:23 PM
avg edited edge metadata.

address comments from stas

avg updated this revision to Diff 21949.Nov 2 2016, 3:30 PM

add comments about the AMD update MSR being undocumented

This revision was automatically updated to reflect the committed changes.
avg added a comment.Nov 2 2016, 4:19 PM

Just as a data point:

$ cpucontrol -vvv -d /boot/firmware -u /dev/cpuctl0
cpucontrol: skipping /boot/firmware/.: is a directory
cpucontrol: skipping /boot/firmware/..: is a directory
cpucontrol: found cpu family 0xf model 0x6 stepping 0x2 extfamily 0x1 extmodel 0.
cpucontrol: microcode revision 0x1000098
cpucontrol: equiv_id: 1062
cpucontrol: selecting revision: 10000c7
cpucontrol: selected ucode size is 960
/boot/firmware/microcode_amd.bin: updating cpu /dev/cpuctl0 to revision 0x10000c7... done.
$ cpucontrol -vvv -d /boot/firmware -u /dev/cpuctl1
cpucontrol: skipping /boot/firmware/.: is a directory
cpucontrol: skipping /boot/firmware/..: is a directory
cpucontrol: found cpu family 0xf model 0x6 stepping 0x2 extfamily 0x1 extmodel 0.
cpucontrol: microcode revision 0x10000c7
cpucontrol: equiv_id: 1062