[announce] Intel BIOS Implementation Test Suite (BITS)

[announce] Intel BIOS Implementation Test Suite (BITS)

[Burt Triplett]

Intel is pleased to announce the BIOS Implementation Test Suite (BITS),
a bootable pre-OS environment for testing BIOSes and in particular their
initialization of Intel processors, hardware, and technologies. BITS can
verify your BIOS against many Intel recommendations. In addition, BITS
includes Intel's official reference code as provided to BIOS, which you
can use to override your BIOS's hardware initialization with a
known-good configuration, and then boot an OS.

BITS functionality fits in three broad categories:

- Testing: run test suites to check how the BIOS configured your
   platform hardware.
- Configuration: override the BIOS hardware configuration using a
   known-good configuration, such as by running Intel reference code
- Exploration: experimental tools and information gathering

BITS consists of a modified GRUB2 bootloader, with many additional
commands to probe and manipulate hardware configuration, as well as
scripts using these commands to test and reconfigure hardware.  GRUB2
already provided a very capable pre-OS runtime environment, including
many features commonly found in standard libraries, as well as a
scripting language, command line, and menu system; thanks to the GRUB2
developers for the great platform to build on.

To support the functionality we needed in BITS, we added:

- Support for initializing and running code on all CPUs in an SMP
   system.

- Many new commands for manipulating hardware resources such as
   model-specific registers (MSRs), CPUID, PCI and PCI Express devices,
   and memory addresses; in all cases these operations occur from a
   designated CPU, allowing access to CPU-specific resources and
   asymmetrically mapped system resources.

- Many new commands supporting the hardware configuration and BIOS
   test-suite infrastructure, including a full C expression evaluator for
   integer values (so you don't have to resort to implementing more
   commands in C just to shift, mask, and otherwise twiddle bits).

In addition to those changes to GRUB2 itself, BITS includes
configuration files which build a menu exposing the various BITS
functionality, including the test suites, hardware configuration, and
exploratory tools.  These scripts detect your system's CPU, and provide
menu entries for all the available functionality on your hardware
platform.  You can also access all of the new commands we've added
directly via the command line.

The current version of BITS focuses primarily on CPU configuration and
power management. It includes extensive support for Intel® processors
based on the microarchitecture code name Nehalem and newer, which
includes Intel Core i7, i5, and i3 desktop and mobile processors, and
corresponding Intel Xeon server processors.

Some examples of what you can currently do with BITS:

- Detect if your BIOS properly configures processor power management.

- Run Intel's power management reference code to override your BIOS's
   configuration.  This includes writing the ACPI tables for P-states and
   C-states, replacing those normally provided by your BIOS.  In
   particular, you can even turn off power management in your BIOS
   entirely, use BITS to initialize it, and then chainload Linux and run
   powertop to see full P-state and C-state support via cpufreq and
   cpuidle.

- Detect the frequency and duration of BIOS System Management
   Interrupts.  This tool shows the average duration of SMIs (grouped by
   orders of magnitude), the time between a few SMIs in each group, and
   the worst-case latency observed.

- Check how long a CPU core and the entire processor package spend in
   C-states (sleep states), by directly checking the CPU's residency
   counters (when available on your platform).  You can compare this
   C-state residency measured in a pre-OS environment to what you observe
   under Linux, to help attribute unexpectedly low C-state residency
   between Linux background tasks or configuration and BIOS
   configuration.

For more examples of using BITS, see the screenshots page at
<http://biosbits.org/screenshots/>.

Many pieces of functionality implemented in BITS demonstrate the use of
newly disclosed and documented hardware information, now provided in the
Intel 64 and IA-32 Architectures Software Developer's Manuals (SDM).

You can get BITS from the BITS homepage at <http://biosbits.org/>.  We
appreciate any feedback you might have on BITS, including questions,
comments, bug reports, feature requests, or patches.  In particular, the
BITS infrastructure makes it possible to implement a wide variety of
tests via simple text configuration files, and perform experimentation
directly on the GRUB command line; give it a try and let us know what
you think!

BITS handling of CPU microcode updates

[Henrique de Moraes Holschuh]

Burt,

The BITS handling of Intel CPU microcode updates does not match either the
official documentation I cold find, or the Linux kernel code.

The documentation clearly states that microcode revision levels are a
*signed* 32-bit number, and that implies it should be subject to signed
comparisons.

The Linux kernel code considers it an unsigned 32-bit number, and does an
unsigned comparison (this is likely a bug).

And the BITS code mentions something called a "BWG", and will always install
a microcode with a revision < 0, it will never install a microcode with a
revision of zero, and does a normal version comparison if the revision is
greater than zero.

AFAIK, just like extended signatures (which are yet to be seen in the wild),
microcoes with negative or zero revision levels have never been published to
operating system vendors, so those discrepancies have had, so far, no impact
in the field.  But they could well be latent bugs.

Can you please clarify what is the correct behaviour ?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: BITS handling of CPU microcode updates

[Burt Triplett]

Sorry for the delayed response; I wanted to make sure I could give you 
an answer that would agree with the Intel Software Developer's Manual, 
and that ended up meaning I needed to start the process of updating the 
relevant part of the SDM. :)

On 3/11/2011 6:30 PM, Henrique de Moraes Holschuh wrote:
> The BITS handling of Intel CPU microcode updates does not match either the
> official documentation I cold find, or the Linux kernel code.
>
> The documentation clearly states that microcode revision levels are a
> *signed* 32-bit number, and that implies it should be subject to signed
> comparisons.
>
> The Linux kernel code considers it an unsigned 32-bit number, and does an
> unsigned comparison (this is likely a bug).
>
> And the BITS code mentions something called a "BWG", and will always install
> a microcode with a revision<  0, it will never install a microcode with a
> revision of zero, and does a normal version comparison if the revision is
> greater than zero.
>
> AFAIK, just like extended signatures (which are yet to be seen in the wild),
> microcoes with negative or zero revision levels have never been published to
> operating system vendors, so those discrepancies have had, so far, no impact
> in the field.  But they could well be latent bugs.
>
> Can you please clarify what is the correct behaviour ?

The Intel SDM correctly identifies microcode revision numbers as signed. 
  However, a simple signed comparison doesn't actually capture the 
correct logic, nor does an unsigned comparison, though in both cases the 
problem doesn't tend to come up in common cases.

Negative microcode revision numbers only appear on microcodes used in 
test environments for debugging purposes.

For the following explanation, let X = the version of the microcode 
currently in the CPU, and let Z = the version of the microcode you 
potentially want to load depending on the revision check.

If you have a microcode with Z < 0, the user knows what they're doing 
and they want to load that microcode regardless of revision.  Always 
load such a microcode regardless of X (it doesn't matter if X < Z or X > Z).

If you have a microcode with Z > 0, and X > 0 as well (the CPU has a 
production microcode in it), then load the microcode only if newer (Z > 
X).  Since microcodes with negative revisions only appear in test lab 
environments (as you noted, they don't appear in the wild), and since 
with positive revisions this behavior matches either a simple signed or 
unsigned comparison, the subtly wrong results haven't appeared outside 
of test lab environments. :)

The interesting case comes up when X < 0 and Z > 0: the CPU already has 
a microcode loaded with a negative revision, and you have a production 
microcode you might want to load.  In this case, the correct behavior 
differs based on whether the microcode loader runs automatically (such 
as the tools that load microcode at Linux boot time), or acts with 
*explicit* user action (such as BITS, the BIOS int 15 handler, or the 
Linux tools *if* they can distinguish the case where the user explicitly 
ran them from the case of running automatically without user intervention).

Tools which run automatically, without explicit user action, should not 
attempt to load a microcode if (X < 0) and (Z > 0).  Doing so makes life 
very difficult for people in those test lab environments: they put a 
microcode they want to test in the BIOS or load it via BITS, but then 
the OS driver automatically overrides it with the latest production 
microcode.  So, tools which run automatically without explicit user 
action should follow this rule:
if ((Z < 0) || (Z > 0 && X > 0 && Z > X)) load_microcode();

Tools which load microcode in response to *explicit* user actions should 
override a negative-revision microcode with a positive-revision 
microcode.  Such tools should follow this rule:
if ((Z < 0) || (Z > 0 && Z > X)) load_microcode();


Currently, as far as I know, the Linux microcode driver and userspace 
tools do not distinguish these two cases: the logic invoked 
automatically at boot time matches the logic run if the user invokes 
microcode_ctl explicitly.  Given that, the revision logic in the kernel 
or microcode_ctl should match the "tools which run automatically, 
without explicit user action" case above:
if ((Z < 0) || (Z > 0 && X > 0 && Z > X)) load_microcode();

If microcode_ctl added an option to distinguish these two cases, it 
could apply the alternative logic when explicitly requested.

Note that you should *not* see production systems shipping with 
negative-revision microcodes.


I've started the process of getting the SDM changed to document the 
logic described above, and distinguish the two different types of 
microcode-loading tools.

Hope that helps,
Burt Triplett

Re: BITS handling of CPU microcode updates

[Henrique de Moraes Holschuh]

On Mon, 21 Mar 2011, Burt Triplett wrote:
> Sorry for the delayed response; I wanted to make sure I could give
> you an answer that would agree with the Intel Software Developer's
> Manual, and that ended up meaning I needed to start the process of
> updating the relevant part of the SDM. :)

Heh :-)

> The Intel SDM correctly identifies microcode revision numbers as
> signed.  However, a simple signed comparison doesn't actually
> capture the correct logic, nor does an unsigned comparison, though
> in both cases the problem doesn't tend to come up in common cases.

...

> Tools which run automatically, without explicit user action, should
> not attempt to load a microcode if (X < 0) and (Z > 0).  Doing so
> makes life very difficult for people in those test lab environments:
> they put a microcode they want to test in the BIOS or load it via
> BITS, but then the OS driver automatically overrides it with the
> latest production microcode.  So, tools which run automatically
> without explicit user action should follow this rule:
> if ((Z < 0) || (Z > 0 && X > 0 && Z > X)) load_microcode();

Very well, I will send patches to fix that behaviour, as well as some
other stuff I noticed that was not updated to match the recommended
actions documented by the SDM and some Intel application notes.

> If microcode_ctl added an option to distinguish these two cases, it
> could apply the alternative logic when explicitly requested.

Indeed, the userspace and internal kernel API don't support that at this
time.

Now, we could change them I suppose, but it doesn't look like a worthwhile
effort to change the microcode core and APIs for something that would be
of limited use even inside an Intel lab.

Unless there is a reason to support online microcode downgrading, that is.
Does it?  Currently, the rare case of a microcode downgrade will only
happen at the next boot.

> I've started the process of getting the SDM changed to document the
> logic described above, and distinguish the two different types of
> microcode-loading tools.

Thank you.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: BITS handling of CPU microcode updates

[Burt Triplett]

On 03/23/2011 05:14 PM, Henrique de Moraes Holschuh wrote:
> On Mon, 21 Mar 2011, Burt Triplett wrote:
>> The Intel SDM correctly identifies microcode revision numbers as
>> signed.  However, a simple signed comparison doesn't actually
>> capture the correct logic, nor does an unsigned comparison, though
>> in both cases the problem doesn't tend to come up in common cases.
>
> ...
>
>> Tools which run automatically, without explicit user action, should
>> not attempt to load a microcode if (X<  0) and (Z>  0).  Doing so
>> makes life very difficult for people in those test lab environments:
>> they put a microcode they want to test in the BIOS or load it via
>> BITS, but then the OS driver automatically overrides it with the
>> latest production microcode.  So, tools which run automatically
>> without explicit user action should follow this rule:
>> if ((Z < 0) || (Z > 0 && X > 0 && Z > X)) load_microcode();
>
> Very well, I will send patches to fix that behaviour, as well as some
> other stuff I noticed that was not updated to match the recommended
> actions documented by the SDM and some Intel application notes.

Thanks!

Out of curiosity, which other discrepancies did you encounter?

Also, please CC me on the patch, and I'll review the changes.

>> If microcode_ctl added an option to distinguish these two cases, it
>> could apply the alternative logic when explicitly requested.
>
> Indeed, the userspace and internal kernel API don't support that at this
> time.
>
> Now, we could change them I suppose, but it doesn't look like a worthwhile
> effort to change the microcode core and APIs for something that would be
> of limited use even inside an Intel lab.

Agreed; in general such microcode will get loaded before booting Linux, 
not via the Linux microcode driver.

> Unless there is a reason to support online microcode downgrading, that is.
> Does it?  Currently, the rare case of a microcode downgrade will only
> happen at the next boot.

No, the driver doesn't need to support this case.

- Burt Triplett

Re: BITS handling of CPU microcode updates

[Henrique de Moraes Holschuh]

On Thu, 24 Mar 2011, Burt Triplett wrote:
> Out of curiosity, which other discrepancies did you encounter?

That was a false alarm.  We use sync_core() instead of directly calling
CPUID(1), which right now is about the same, just more obfuscated and
complicated depending on some config options.  Well, as long as
sync_core() doesn't change in the future.

There are also SPARSE warnings in the driver, but the code itself is
correct.

> Also, please CC me on the patch, and I'll review the changes.

I will post it in a few minutes.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: BITS handling of CPU microcode updates

[Andi Kleen]

Henrique de Moraes Holschuh <hmh@hmh.eng.br> writes:
>
> Very well, I will send patches to fix that behaviour, as well as some
> other stuff I noticed that was not updated to match the recommended
> actions documented by the SDM and some Intel application notes.

I don't think you can easily -- there's no kernel interface
to distingush the "explicit user action" case from automatic loading.

This all would need a new kernel interface.

I suppose all the test labs have learned by now how to disable
automatic updates ...

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: BITS handling of CPU microcode updates

[Burt Triplett]

On 3/24/2011 2:46 PM, Andi Kleen wrote:
> Henrique de Moraes Holschuh<hmh@hmh.eng.br>  writes:
>>
>> Very well, I will send patches to fix that behaviour, as well as some
>> other stuff I noticed that was not updated to match the recommended
>> actions documented by the SDM and some Intel application notes.
>
> I don't think you can easily -- there's no kernel interface
> to distingush the "explicit user action" case from automatic loading.

Correct, but the microcode module can still implement the "automatic 
loading" case, as Henrique's patch does.

> I suppose all the test labs have learned by now how to disable
> automatic updates ...

The microcode driver's current unsigned revision checking happens to 
treat negative revisions as large positive revisions, so it happened to 
work OK in the test lab environments, though not by design. :)

- Burt Triplett

x86/microcode: intel: correctly handle negative revisions

[Henrique de Moraes Holschuh]

As per the Intel SDM vol 3A, microcode revisions are signed 32-bit
numbers.  The code was handling them as unsigned int in some places and as
an int in other places.

As per the clarification posted by Burt Triplett from the Intel BITS
project, negative microcode revisions are used internally at Intel and
should always get loaded.  Also, they should not be overriden unless we
can somehow differentiate "automated" loading from "forced" loading (which
we cannot at this time).  Burt says the SDM will be updated with this
information eventually.

The code should:

1. Ignore attempts to load a zero-revision microcode (that value is
reserved for the CPU to signal that it is running with the factory
microcode, and must not be present in a normal microcode update);

2. Always load negative revision microcodes, to help Intel's engineers;

3. Avoid upgrading from a BIOS-loaded negative revision microcode to
a normal microcode, to not get in the way of Intel's engineers.

4. Upgrade from revision 0 (no updates loaded in CPU) to any revision.

It was already doing some of that, but I don't feel like trying to track
down exactly how the old code with its mix of signed/unsigned handling of
revisions would behave in each of the above cases.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
LKML-Reference: <4D87E2CD.6020306@pbjtriplett.org>
Cc: Tigran Aivazian <tigran@aivazian.fsnet.co.uk>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Burt Triplett <burt@pbjtriplett.org>

diff --git a/arch/x86/kernel/microcode_intel.c b/arch/x86/kernel/microcode_intel.c
index 1a1b606..92b7cfe 100644
--- a/arch/x86/kernel/microcode_intel.c
+++ b/arch/x86/kernel/microcode_intel.c
@@ -89,7 +89,7 @@ MODULE_LICENSE("GPL");
 
 struct microcode_header_intel {
 	unsigned int            hdrver;
-	unsigned int            rev;
+	int                     rev;
 	unsigned int            date;
 	unsigned int            sig;
 	unsigned int            cksum;
@@ -178,10 +178,15 @@ static inline int update_match_cpu(struct cpu_signature *csig, int sig, int pf)
 	return (!sigmatch(sig, csig->sig, pf, csig->pf)) ? 0 : 1;
 }
 
+/*
+ * rev < 0 : devel/test microcode, always install
+ * rev = 0 : (cpu: no microcode installed.  data file: illegal)
+ * rev > 0 : normal microcode, avoid downgrading
+ */
 static inline int
 update_match_revision(struct microcode_header_intel *mc_header, int rev)
 {
-	return (mc_header->rev <= rev) ? 0 : 1;
+	return !!(mc_header->rev < 0 || (mc_header->rev > rev && rev >= 0));
 }
 
 static int microcode_sanity_check(void *mc)
@@ -298,7 +303,7 @@ static int apply_microcode(int cpu)
 {
 	struct microcode_intel *mc_intel;
 	struct ucode_cpu_info *uci;
-	unsigned int val[2];
+	int val[2];
 	int cpu_num;
 
 	cpu_num = raw_smp_processor_id();
-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: x86/microcode: intel: correctly handle negative revisions

[Burt Triplett]

On 3/24/2011 7:09 PM, Henrique de Moraes Holschuh wrote:
> As per the Intel SDM vol 3A, microcode revisions are signed 32-bit
> numbers.  The code was handling them as unsigned int in some places and as
> an int in other places.
> 
> As per the clarification posted by Burt Triplett from the Intel BITS
> project, negative microcode revisions are used internally at Intel and
> should always get loaded.  Also, they should not be overriden unless we
> can somehow differentiate "automated" loading from "forced" loading (which
> we cannot at this time).  Burt says the SDM will be updated with this
> information eventually.
> 
> The code should:
> 
> 1. Ignore attempts to load a zero-revision microcode (that value is
> reserved for the CPU to signal that it is running with the factory
> microcode, and must not be present in a normal microcode update);
> 
> 2. Always load negative revision microcodes, to help Intel's engineers;
> 
> 3. Avoid upgrading from a BIOS-loaded negative revision microcode to
> a normal microcode, to not get in the way of Intel's engineers.
> 
> 4. Upgrade from revision 0 (no updates loaded in CPU) to any revision.
> 
> It was already doing some of that, but I don't feel like trying to track
> down exactly how the old code with its mix of signed/unsigned handling of
> revisions would behave in each of the above cases.
> 
> Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> LKML-Reference: <4D87E2CD.6020306@pbjtriplett.org>
> Cc: Tigran Aivazian <tigran@aivazian.fsnet.co.uk>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Burt Triplett <burt@pbjtriplett.org>

Reviewed-by: Burt Triplett <burt@pbjtriplett.org>

Thanks,
Burt Triplett