Date: Sun, 11 Feb 2018 07:39:08 +0100 From: Stefan Blachmann <sblachmann@gmail.com> To: Sean Bruno <sbruno@freebsd.org> Cc: freebsd-hackers@freebsd.org Subject: Re: New microcode updating tool for FreeBSD Message-ID: <CACc-My27i=wjpO5_u=cKykf3VRzoQvth0qpvQp_qvbcG%2BwLW3g@mail.gmail.com> In-Reply-To: <4db65b52-5a82-15ff-1629-0371a619f89b@freebsd.org> References: <CACc-My1=y0huU11VxXesyYVQ0%2Bg1L5wV_PPnjCU7-YiQKkQhkQ@mail.gmail.com> <4db65b52-5a82-15ff-1629-0371a619f89b@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2/11/18, Sean Bruno <sbruno@freebsd.org> wrote: > > > On 02/08/18 23:15, Stefan Blachmann wrote: >> Hello, >> >> I would like to introduce you to my new microcode updater for FreeBSD. >> >> As the old devcpu-data port has serious bug and does not support the >> new multi-blobbed Intel microcode format, I have made this new tool. > > devcpu-data is a bunch of microcode files and a parser for the intel > legacy microcode.dat file. Do you mean cpucontrol in the base system > has a bug? I found reference to this in the code at the github download > link pointing at a patchless comment-type bugzilla entry here: > > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=192487 > > > Do you feel like trying to come up with a patchset or modification to > the existing usr.sbin/cpucontrol (which appears to be the basis of the > code in this github repository)? We could start using the Phabricator > review system to try and get your work into a form that would be > accepted into the base system. > > sean > > On 2/11/18, Sean Bruno <sbruno@freebsd.org> wrote: > > > On 02/08/18 23:15, Stefan Blachmann wrote: >> Hello, >> >> I would like to introduce you to my new microcode updater for FreeBSD. >> >> As the old devcpu-data port has serious bug and does not support the >> new multi-blobbed Intel microcode format, I have made this new tool. > > devcpu-data is a bunch of microcode files and a parser for the intel > legacy microcode.dat file. Do you mean cpucontrol in the base system > has a bug? I found reference to this in the code at the github download > link pointing at a patchless comment-type bugzilla entry here: > > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=192487 > > > Do you feel like trying to come up with a patchset or modification to > the existing usr.sbin/cpucontrol (which appears to be the basis of the > code in this github repository)? We could start using the Phabricator > review system to try and get your work into a form that would be > accepted into the base system. > > sean > > I think I should start with the background first: Because of the Meltdown/Spectre issues, I took a look into devcpu-data. The PR was written by the guy doing the Linux kernel microcode update stuff. No FreeBSD guy, not even the author of devcpu-data, cared about that important PR for more than three years! That bug is indeed serious, as while testing I myself observed the processor occasionally being incorrectly identified due to this. This indicated to me that devcpu-data is very littly tested and nobody actually seemed interested in dealing with it. Thus I concluded that the microcode update process should be *completely* reviewed and verified that each and every step is actually done *exactly* to Intel's specifications. The cpupdate program is the result of this work. I decided first to untangle the sparsely commented spaghetti code of devcpu-data, and to walk through the CPU identification/updating process step-by-step, while consulting the Intel manuals to make sure it is done according to Intel's specs. While doing this, I found that there are other issues with devcpu-data for which no PRs yet exist. For example, using bits that are reserved by Intel, instead of only using the bits that Intel specified to actually be used. Another thing I noticed when testing the new tool was that there is a not-yet-documented format change by Intel (at least I could not find any "official" reference). This regards microcode files which contain multiple update blobs (currently up to 7) which are targeted to CPUs of same model-family-stepping, but different CPU flags. There was a post from a Dell guy on Phabricator, hinting that in case one wonders why there are less files in intel-ucode/ than being created by iucode-tool, that this is because Intel now packs updates for same Family-Model-Stepping files into one single file containing multiple blobs, each for different processor flags. Sadly that thread got closed before I found it. So I could not post that devcpu-data would just fail these update files as invalid. (BTW: You can check the number of blobs in the update files yourself by using "cpupdate -I -vv -c <your intel-ucode dir>") The reason for me to practically completely rewrite the update functionality, instead of just fixing devcpu-data was that when I worked on it, aside of the bugs mentioned above, I found that it has just too many issues: 1. It is hard to understand. Few comments. Horrible spaghetti code. 2. It gives poor to no meaningful error information, making difficult to impossible for users to determine cause of problems. 3. It has no functionality to get information about cpu ID and revision. update files, and their contents. Please take a look at the intel.c module of devcpu-data. See how many gotos there are in the intel_update() function! In my eyes this is BASIC style spaghetti at its worst. And then compare this with cpupdate's intel.c module to see that the new code bears *very little* resemblance to this. Of course, the module is larger because of more functionality. Thus I do not see much sense in attempting to fix/patch devcpu-data's microcode update functionality. Instead I'd suggest to just remove/strip the microcode update functionality from devcpu-data. Then we would have two programs that do only one thing each, and do this good: - cpucontrol (= devcpu-data) for setting/reading CPU MSR registers - cpupdate for handling microcode updating related stuff. The cpupdate program, as it is now, is just an unpolished suggestion/proposal I am offering to the community. If there is interest in adding it to ports/system, I'd be very willing to make it "perfect". This would particularly include beautification/clean-up of code and output, stripping code and functionality of what can/should be stripped, creating manual page etc, adding AMD and VIA support, maybe later others like ARM, too. Regarding "getting my work into a form that would be accepted into the base system", I would appreciate if it could be reviewed, either by mailing list users or in the Phabricator, as suggestions and feedback would be very helpful to make the tool "perfect". Greetings, Stefan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACc-My27i=wjpO5_u=cKykf3VRzoQvth0qpvQp_qvbcG%2BwLW3g>