[PATCH 0/3] x86: TSC_DEADLINE vs TSC_ADJUST and microcode revisions

[PATCH 0/3] x86: TSC_DEADLINE vs TSC_ADJUST and microcode revisions

[Peter Zijlstra]

Hi all,

These patches rely on the latest microcode data files from Intel [*]

Without this microcode loaded, we'll print a FW_BUG informing the user to
update their microcode image and disable TSC_DEADLINE support.

This in turn allows us to remove the TSC_ADJUST workarounds which were required
due to errata.

---
[*] https://downloadcenter.intel.com/download/26798/Linux-Processor-Microcode-Data-File

[PATCH 1/3] x86/apic: Change the lapic name in deadline mode

[Peter Zijlstra]

[-- Attachment #1: peterz-lapic-name.patch --]
[-- Type: text/plain, Size: 598 bytes --]

So that we can more easily see in what mode the lapic timer operates.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kernel/apic/apic.c |    1 +
 1 file changed, 1 insertion(+)

--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -563,6 +563,7 @@ static void setup_APIC_timer(void)
 	levt->cpumask = cpumask_of(smp_processor_id());
 
 	if (this_cpu_has(X86_FEATURE_TSC_DEADLINE_TIMER)) {
+		levt->name = "lapic-deadline";
 		levt->features &= ~(CLOCK_EVT_FEAT_PERIODIC |
 				    CLOCK_EVT_FEAT_DUMMY);
 		levt->set_next_event = lapic_next_deadline;

[tip:x86/timers] x86/apic: Change the lapic name in deadline mode

[tip-bot for Peter Zijlstra]

Commit-ID:  c6e9f42bbeecbc10cd4fbcca474b5859aba1de67
Gitweb:     http://git.kernel.org/tip/c6e9f42bbeecbc10cd4fbcca474b5859aba1de67
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 31 May 2017 17:52:02 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 4 Jun 2017 21:55:52 +0200

x86/apic: Change the lapic name in deadline mode

So that we can more easily see in what mode the lapic timer operates.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: kevin.b.stanton@intel.com
Link: http://lkml.kernel.org/r/20170531155305.989808008@infradead.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/apic/apic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 2d75faf..87d721a1 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -563,6 +563,7 @@ static void setup_APIC_timer(void)
 	levt->cpumask = cpumask_of(smp_processor_id());
 
 	if (this_cpu_has(X86_FEATURE_TSC_DEADLINE_TIMER)) {
+		levt->name = "lapic-deadline";
 		levt->features &= ~(CLOCK_EVT_FEAT_PERIODIC |
 				    CLOCK_EVT_FEAT_DUMMY);
 		levt->set_next_event = lapic_next_deadline;

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

[Peter Zijlstra]

[-- Attachment #1: peterz-apic-deadline-microcode.patch --]
[-- Type: text/plain, Size: 3338 bytes --]

Due to errata it is possible for the TSC_DEADLINE timer to misbehave
after using TSC_ADJUST. A microcode update is available to fix this
situation.

Avoid using the TSC_DEADLINE timer if it is affected by this issue and
report the required microcode version.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kernel/apic/apic.c |   79 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -54,6 +54,8 @@
 #include <asm/mce.h>
 #include <asm/tsc.h>
 #include <asm/hypervisor.h>
+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
 
 unsigned int num_processors;
 
@@ -545,6 +547,81 @@ static struct clock_event_device lapic_c
 };
 static DEFINE_PER_CPU(struct clock_event_device, lapic_events);
 
+#define DEADLINE_MODEL_MATCH_FUNC(model, func)	\
+	{ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned long)&func }
+
+#define DEADLINE_MODEL_MATCH_REV(model, rev)	\
+	{ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned long)rev }
+
+static u32 hsx_deadline_rev(void)
+{
+	switch (boot_cpu_data.x86_mask) {
+	case 0x02: return 0x3a; /* EP */
+	case 0x04: return 0x0f; /* EX */
+	}
+
+	return ~0U;
+}
+
+static u32 bdx_deadline_rev(void)
+{
+	switch (boot_cpu_data.x86_mask) {
+	case 0x02: return 0x00000011;
+	case 0x03: return 0x0700000e;
+	case 0x04: return 0x0f00000c;
+	case 0x05: return 0x0e000003;
+	}
+
+	return ~0U;
+}
+
+static const struct x86_cpu_id deadline_match[] = {
+	DEADLINE_MODEL_MATCH_FUNC( INTEL_FAM6_HASWELL_X,	hsx_deadline_rev),
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_BROADWELL_X,	0x0b000020),
+	DEADLINE_MODEL_MATCH_FUNC( INTEL_FAM6_BROADWELL_XEON_D,	bdx_deadline_rev),
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_SKYLAKE_X,	0x02000014),
+
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_HASWELL_CORE,	0x22),
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_HASWELL_ULT,	0x20),
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_HASWELL_GT3E,	0x17),
+
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_BROADWELL_CORE,	0x25),
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_BROADWELL_GT3E,	0x17),
+
+	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),
+
+	{},
+};
+
+static void apic_detect_deadline(void)
+{
+	const struct x86_cpu_id *m = x86_match_cpu(deadline_match);
+	u32 rev;
+
+	if (!m)
+		return;
+
+	/*
+	 * Function pointers will have the MSB set due to address layout,
+	 * immediate revisions will not.
+	 */
+	if ((long)m->driver_data < 0)
+		rev = ((u32 (*)(void))(m->driver_data))();
+	else
+		rev = (u32)m->driver_data;
+
+	if (boot_cpu_data.microcode >= rev)
+		return;
+
+	setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE_TIMER);
+	pr_err(FW_BUG "TSC_DEADLINE disabled due to Errata; "
+	       "please update microcode to version: 0x%x (or later)\n", rev);
+}
+
 /*
  * Setup the local APIC timer for this CPU. Copy the initialized values
  * of the boot CPU and register the clock event in the framework.
@@ -1780,6 +1857,8 @@ void __init init_apic_mappings(void)
 {
 	unsigned int new_apicid;
 
+	apic_detect_deadline();
+
 	if (x2apic_mode) {
 		boot_cpu_physical_apicid = read_apic_id();
 		return;

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

[Henrique de Moraes Holschuh]

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?

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

-- 
  Henrique Holschuh

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.

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

[Thomas Gleixner]

On Thu, 1 Jun 2017, Peter Zijlstra wrote:
> 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.

Avoiding the superflous reload is dangerous. If the BIOS/FIT nonsense gets
fixed (however unlikely that is) then we run into an even worse problem
because then the kernel will not load version 5 if the CPU says version 4.
That's way worse than reloading the same version for nothing.

Thanks,

	tglx

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

[Henrique de Moraes Holschuh]

On Thu, 01 Jun 2017, Peter Zijlstra wrote:
> 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..

That would work, yes.  But that would breed confusion, it is visible to
userspace, and it and might create unforeseen issues in the future.  I
feel it is best to not go that way.

> > 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

In the case of your patch, it would avoid issuing an incorrect warning
to the user and needlessly triggering the errata workaround if the
system UEFI has exactly the revision of the microcode that first fixed
the issue, and it was loaded from FIT.

OTOH, it will add a further (if rather small) nail to the coffin of any
attempts to depend on the fact that microcode was loaded from FIT -- and
anything that does so would get in the way of users and distros fixing
systems with an operating system microcode update.

On those grounds, I retract my suggestion to handle the microcode
revision oddity related to SGX.

Please keep the patch as is.

> 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.

Yes, it works.  That is the reason why I never proposed a fix for our
microcode loader in these two years, actually.  It works just fine right
now, and if Intel really wanted a different behavior, they should have
requested such a change or sent in a patch in the first place.  Instead,
the changed behavior wasn't even documented in the Intel SDM, since it
is backwards-compatible.

In fact, as far as I am concerned, loading the same microcode update
over itself just to destroy the "loaded from FIT" marker IS the lesser
evil, at least as things currently stand.

> 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.

I think we want to keep our current behavior in the microcode loader as
well.  It is not there by mistake: it is exactly what the Intel SDM
recommends.

I believe it was no coincidence that this microcode-from-FIT marker was
designed by Intel in a way that non-modified operating systems (and
advanced boot loaders capable of microcode updates) in the field would
by default always attempt to override the FIT microcode and reset that
flag.  Just like we're doing.

-- 
  Henrique Holschuh

[tip:x86/timers] x86/apic: Add TSC_DEADLINE quirk due to errata

[tip-bot for Peter Zijlstra]

Commit-ID:  bd9240a18edfbfa72e957fc2ba831cf1f13ea073
Gitweb:     http://git.kernel.org/tip/bd9240a18edfbfa72e957fc2ba831cf1f13ea073
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 31 May 2017 17:52:03 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 4 Jun 2017 21:55:53 +0200

x86/apic: Add TSC_DEADLINE quirk due to errata

Due to errata it is possible for the TSC_DEADLINE timer to misbehave
after using TSC_ADJUST. A microcode update is available to fix this
situation.

Avoid using the TSC_DEADLINE timer if it is affected by this issue and
report the required microcode version.

[ tglx: Renamed function to apic_check_deadline_errata() ]

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: kevin.b.stanton@intel.com
Link: http://lkml.kernel.org/r/20170531155306.050849877@infradead.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/apic/apic.c | 79 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 87d721a1..3b215ff 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -54,6 +54,8 @@
 #include <asm/mce.h>
 #include <asm/tsc.h>
 #include <asm/hypervisor.h>
+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
 
 unsigned int num_processors;
 
@@ -545,6 +547,81 @@ static struct clock_event_device lapic_clockevent = {
 };
 static DEFINE_PER_CPU(struct clock_event_device, lapic_events);
 
+#define DEADLINE_MODEL_MATCH_FUNC(model, func)	\
+	{ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned long)&func }
+
+#define DEADLINE_MODEL_MATCH_REV(model, rev)	\
+	{ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned long)rev }
+
+static u32 hsx_deadline_rev(void)
+{
+	switch (boot_cpu_data.x86_mask) {
+	case 0x02: return 0x3a; /* EP */
+	case 0x04: return 0x0f; /* EX */
+	}
+
+	return ~0U;
+}
+
+static u32 bdx_deadline_rev(void)
+{
+	switch (boot_cpu_data.x86_mask) {
+	case 0x02: return 0x00000011;
+	case 0x03: return 0x0700000e;
+	case 0x04: return 0x0f00000c;
+	case 0x05: return 0x0e000003;
+	}
+
+	return ~0U;
+}
+
+static const struct x86_cpu_id deadline_match[] = {
+	DEADLINE_MODEL_MATCH_FUNC( INTEL_FAM6_HASWELL_X,	hsx_deadline_rev),
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_BROADWELL_X,	0x0b000020),
+	DEADLINE_MODEL_MATCH_FUNC( INTEL_FAM6_BROADWELL_XEON_D,	bdx_deadline_rev),
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_SKYLAKE_X,	0x02000014),
+
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_HASWELL_CORE,	0x22),
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_HASWELL_ULT,	0x20),
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_HASWELL_GT3E,	0x17),
+
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_BROADWELL_CORE,	0x25),
+	DEADLINE_MODEL_MATCH_REV ( INTEL_FAM6_BROADWELL_GT3E,	0x17),
+
+	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),
+
+	{},
+};
+
+static void apic_check_deadline_errata(void)
+{
+	const struct x86_cpu_id *m = x86_match_cpu(deadline_match);
+	u32 rev;
+
+	if (!m)
+		return;
+
+	/*
+	 * Function pointers will have the MSB set due to address layout,
+	 * immediate revisions will not.
+	 */
+	if ((long)m->driver_data < 0)
+		rev = ((u32 (*)(void))(m->driver_data))();
+	else
+		rev = (u32)m->driver_data;
+
+	if (boot_cpu_data.microcode >= rev)
+		return;
+
+	setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE_TIMER);
+	pr_err(FW_BUG "TSC_DEADLINE disabled due to Errata; "
+	       "please update microcode to version: 0x%x (or later)\n", rev);
+}
+
 /*
  * Setup the local APIC timer for this CPU. Copy the initialized values
  * of the boot CPU and register the clock event in the framework.
@@ -1780,6 +1857,8 @@ void __init init_apic_mappings(void)
 {
 	unsigned int new_apicid;
 
+	apic_check_deadline_errata();
+
 	if (x2apic_mode) {
 		boot_cpu_physical_apicid = read_apic_id();
 		return;

[PATCH 3/3] x86/tsc: Remove the TSC_ADJUST clamp

[Peter Zijlstra]

[-- Attachment #1: peterz-tscadj-normalize.patch --]
[-- Type: text/plain, Size: 2047 bytes --]

Now that all affected platforms have a microcode update; and we check
this and disable TSC_DEADLINE and print a microcode revision update
error if its too old, we can remove the TSC_ADJUST clamp.

This should help with systems where the second socket runs ahead of
the first socket and needs a negative adjustment. In this case we'd
hit the 0 clamp and give up for not achieving synchronization.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kernel/tsc_sync.c |   21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

--- a/arch/x86/kernel/tsc_sync.c
+++ b/arch/x86/kernel/tsc_sync.c
@@ -71,13 +71,8 @@ static void tsc_sanitize_first_cpu(struc
 	 * non zero. We don't do that on non boot cpus because physical
 	 * hotplug should have set the ADJUST register to a value > 0 so
 	 * the TSC is in sync with the already running cpus.
-	 *
-	 * But we always force positive ADJUST values. Otherwise the TSC
-	 * deadline timer creates an interrupt storm. We also have to
-	 * prevent values > 0x7FFFFFFF as those wreckage the timer as well.
 	 */
-	if ((bootcpu && bootval != 0) || (!bootcpu && bootval < 0) ||
-	    (bootval > 0x7FFFFFFF)) {
+	if (bootcpu && bootval != 0) {
 		pr_warn(FW_BUG "TSC ADJUST: CPU%u: %lld force to 0\n", cpu,
 			bootval);
 		wrmsrl(MSR_IA32_TSC_ADJUST, 0);
@@ -451,20 +446,6 @@ void check_tsc_sync_target(void)
 	 */
 	cur->adjusted += cur_max_warp;
 
-	/*
-	 * TSC deadline timer stops working or creates an interrupt storm
-	 * with adjust values < 0 and > x07ffffff.
-	 *
-	 * To allow adjust values > 0x7FFFFFFF we need to disable the
-	 * deadline timer and use the local APIC timer, but that requires
-	 * more intrusive changes and we do not have any useful information
-	 * from Intel about the underlying HW wreckage yet.
-	 */
-	if (cur->adjusted < 0)
-		cur->adjusted = 0;
-	if (cur->adjusted > 0x7FFFFFFF)
-		cur->adjusted = 0x7FFFFFFF;
-
 	pr_warn("TSC ADJUST compensate: CPU%u observed %lld warp. Adjust: %lld\n",
 		cpu, cur_max_warp, cur->adjusted);
 

[tip:x86/timers] x86/tsc: Remove the TSC_ADJUST clamp

[tip-bot for Peter Zijlstra]

Commit-ID:  855615eee9b1989cac8ec5eaae4562db081a239b
Gitweb:     http://git.kernel.org/tip/855615eee9b1989cac8ec5eaae4562db081a239b
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 31 May 2017 17:52:04 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 4 Jun 2017 21:55:53 +0200

x86/tsc: Remove the TSC_ADJUST clamp

Now that all affected platforms have a microcode update; and we check
this and disable TSC_DEADLINE and print a microcode revision update
error if its too old, we can remove the TSC_ADJUST clamp.

This should help with systems where the second socket runs ahead of
the first socket and needs a negative adjustment. In this case we'd
hit the 0 clamp and give up for not achieving synchronization.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: kevin.b.stanton@intel.com
Link: http://lkml.kernel.org/r/20170531155306.100950003@infradead.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/tsc_sync.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/arch/x86/kernel/tsc_sync.c b/arch/x86/kernel/tsc_sync.c
index 728f753..7842371 100644
--- a/arch/x86/kernel/tsc_sync.c
+++ b/arch/x86/kernel/tsc_sync.c
@@ -71,13 +71,8 @@ static void tsc_sanitize_first_cpu(struct tsc_adjust *cur, s64 bootval,
 	 * non zero. We don't do that on non boot cpus because physical
 	 * hotplug should have set the ADJUST register to a value > 0 so
 	 * the TSC is in sync with the already running cpus.
-	 *
-	 * But we always force positive ADJUST values. Otherwise the TSC
-	 * deadline timer creates an interrupt storm. We also have to
-	 * prevent values > 0x7FFFFFFF as those wreckage the timer as well.
 	 */
-	if ((bootcpu && bootval != 0) || (!bootcpu && bootval < 0) ||
-	    (bootval > 0x7FFFFFFF)) {
+	if (bootcpu && bootval != 0) {
 		pr_warn(FW_BUG "TSC ADJUST: CPU%u: %lld force to 0\n", cpu,
 			bootval);
 		wrmsrl(MSR_IA32_TSC_ADJUST, 0);
@@ -451,20 +446,6 @@ retry:
 	 */
 	cur->adjusted += cur_max_warp;
 
-	/*
-	 * TSC deadline timer stops working or creates an interrupt storm
-	 * with adjust values < 0 and > x07ffffff.
-	 *
-	 * To allow adjust values > 0x7FFFFFFF we need to disable the
-	 * deadline timer and use the local APIC timer, but that requires
-	 * more intrusive changes and we do not have any useful information
-	 * from Intel about the underlying HW wreckage yet.
-	 */
-	if (cur->adjusted < 0)
-		cur->adjusted = 0;
-	if (cur->adjusted > 0x7FFFFFFF)
-		cur->adjusted = 0x7FFFFFFF;
-
 	pr_warn("TSC ADJUST compensate: CPU%u observed %lld warp. Adjust: %lld\n",
 		cpu, cur_max_warp, cur->adjusted);
 

Re: [PATCH 0/3] x86: TSC_DEADLINE vs TSC_ADJUST and microcode revisions

[Henrique de Moraes Holschuh]

On Wed, 31 May 2017, Peter Zijlstra wrote:
> These patches rely on the latest microcode data files from Intel [*]
> 
> Without this microcode loaded, we'll print a FW_BUG informing the user to
> update their microcode image and disable TSC_DEADLINE support.
> 
> This in turn allows us to remove the TSC_ADJUST workarounds which were required
> due to errata.

Thanks!

On a related note, maybe it would be possible to do something about
SKL150/SKX150/KBL095 as well?

Working around that errata requires disabling hyperthreads on Skylake
when microcode < 0xb9 (confirmed to fix the issue by users suffering
from SKL150), and for Kabylake, disable hyperthreading when microcode <
0x5d (personal best guess based on microcode release date -- someone
@intel might want to confirm it).

-- 
  Henrique Holschuh