Re: [PATCH 2/3] x86/apic: Add TSC_DEADLINE quirk due to errata

[Peter Zijlstra]

On Wed, May 31, 2017 at 06:58:55PM -0300, Henrique de Moraes Holschuh wrote:
> On Wed, 31 May 2017, Peter Zijlstra wrote:
> > +	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_SKYLAKE_X,	0x02000014),
> ...
> > +	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_SKYLAKE_MOBILE,	0xb2),
> > +	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_SKYLAKE_DESKTOP,	0xb2),
> ...
> > +	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_KABYLAKE_MOBILE,	0x52),
> > +	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_KABYLAKE_DESKTOP,	0x52),
> 
> Maybe these revisions need to be decreased by one, so that the SGX
> behavior of messing with the microcode revision is taken into account?

If anything, we should fix whatever sets our boot_cpu_data.microcode
number, not muck with this table. However..

> When the processor microcode is auto-updated from the (signed) FIT, and
> SGX's PRMRR feature is supported, the processor will report its
> microcode revision as one less.  Therefore, a microcode update with
> revision 0xb2 auto-loaded from FIT would be reported by RDMSR(0x8b) as
> revision 0xb1.
> 
> I know about this SGX-related behavior from a coreboot commit from ~two
> years ago (link below).  As far as I can tell, the Intel 64/IA32 SDM is
> *still* missing any mention about this behavior in vol 3A section 9.11
> (where it describes microcode updates).
> 
> https://review.coreboot.org/cgit/coreboot.git/commit/?id=5042aad4ded1651638ae9b60e34114b65e4f211e

That commit also states that is avoids a superfluous microcode load. And
we've verified on affected systems (both Thomas and I have a SKL system
with the PRMRR bit set in MTRRCAP) that when we manually load the
microcode image the reported revision number matches the one from the
image.

 [    0.000000] microcode: microcode updated early to revision 0xba, date = 2017-04-09
 [    2.297894] microcode: sig=0x506e3, pf=0x2, revision=0xba

And:

# hexdump /lib/firmware/intel-ucode/06-5e-03 | head -1
0000000 0001 0000 00ba 0000 2017 0409 06e3 0005
                  ^^^^^^^^^

So aside from a possible OS re-load of the microcode, the issue doesn't
appear to have any negative effect.


The microcode people (Cc'ed) might want to further look into this is
they care about avoiding the superfluous reload, but for the purpose of
this patch all is well.