Revert "x86-64: Disable local APIC timer use on AMD systems with C1E"

This reverts commit e66485d747505e9d960b864fc6c37f8b2afafaf0, since
Rafael Wysocki noticed that the change only works for his in -mm, not in
mainline (and that both "noapictimer" _and_ "apicmaintimer" are broken
on his hardware, but that's apparently not a regression, just a symptom
of the same issue that causes the automatic apic timer disable to not
work).

It turns out that it really doesn't work correctly on x86-64, since
x86-64 doesn't use the generic clock events for timers yet.

Thanks to Rafal for testing, and here's the ugly details on x86-64 as
per Thomas:

  "I just looked into the code and the logic vs.  noapictimer on SMP is
   completely broken.

   On i386 the noapictimer option not only disables the local APIC
   timer, it also registers the CPUs for broadcasting via IPI on SMP
   systems.

   The x86-64 code uses the broadcast only when the local apic timer is
   active, i.e.  "noapictimer" is not on the command line.  This defeats
   the whole purpose of "noapictimer".  It should be there to make boxen
   work, where the local APIC timer actually has a hardware problem,
   e.g.  the nx6325.

   The current implementation of x86_64 only fixes the ACPI c-states
   related problem where the APIC timer stops in C3(2), nothing else.

   On nx6325 and other AMD X2 equipped systems which have the C1E
   enabled we run into the following:

   PIT keeps jiffies (and the system) running, but the local APIC timer
   interrupts can get out of sync due to this C1E effect.

   I don't think this is a critical problem, but it is wrong
   nevertheless.

   I think it's safe to revert the C1E patch and postpone the fix to the
   clock events conversion."

On further reflection, Thomas noted:

   "It's even worse than I thought on the first check:

    "noapictimer" on the command line of an SMP box prevents _ONLY_ the
    boot CPU apic timer from being used.  But the secondary CPU is still
    unconditionally setting up the APIC timer and uses the non
    calibrated variable calibration_result, which is of course 0, to
    setup the APIC timer.  Wreckage guaranteed."

so we'll just have to wait for the x86 merge to hopefully fix this up
for x86-64.

Tested-and-requested-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/arch/x86_64/kernel/setup.c b/arch/x86_64/kernel/setup.c
index 32054bf5ba405674ba4b008ab0aa4b998158643a..af838f6b0b7fc9b7e3ed4de0ab12992e48cf9f18 100644 (file)
--- a/arch/x86_64/kernel/setup.c
+++ b/arch/x86_64/kernel/setup.c
@@ -546,37 +546,6 @@ static void __init amd_detect_cmp(struct cpuinfo_x86 *c)
 #endif
 }
 
 #endif
 }
 

-#define ENABLE_C1E_MASK                0x18000000
-#define CPUID_PROCESSOR_SIGNATURE      1
-#define CPUID_XFAM             0x0ff00000
-#define CPUID_XFAM_K8          0x00000000
-#define CPUID_XFAM_10H         0x00100000
-#define CPUID_XFAM_11H         0x00200000
-#define CPUID_XMOD             0x000f0000
-#define CPUID_XMOD_REV_F       0x00040000
-
-/* AMD systems with C1E don't have a working lAPIC timer. Check for that. */
-static __cpuinit int amd_apic_timer_broken(void)
-{
-       u32 lo, hi;
-       u32 eax = cpuid_eax(CPUID_PROCESSOR_SIGNATURE);
-       switch (eax & CPUID_XFAM) {
-       case CPUID_XFAM_K8:
-               if ((eax & CPUID_XMOD) < CPUID_XMOD_REV_F)
-                       break;
-       case CPUID_XFAM_10H:
-       case CPUID_XFAM_11H:
-               rdmsr(MSR_K8_ENABLE_C1E, lo, hi);
-               if (lo & ENABLE_C1E_MASK)
-                       return 1;
-               break;
-       default:
-               /* err on the side of caution */
-               return 1;
-       }
-       return 0;
-}
-

 static void __cpuinit init_amd(struct cpuinfo_x86 *c)
 {
        unsigned level;
 static void __cpuinit init_amd(struct cpuinfo_x86 *c)
 {
        unsigned level;


@@ -648,9 +617,6 @@ static void __cpuinit init_amd(struct cpuinfo_x86 *c)
        /* Family 10 doesn't support C states in MWAIT so don't use it */
        if (c->x86 == 0x10 && !force_mwait)
                clear_bit(X86_FEATURE_MWAIT, &c->x86_capability);
        /* Family 10 doesn't support C states in MWAIT so don't use it */
        if (c->x86 == 0x10 && !force_mwait)
                clear_bit(X86_FEATURE_MWAIT, &c->x86_capability);

-
-       if (amd_apic_timer_broken())
-               disable_apic_timer = 1;

 }
 
 static void __cpuinit detect_ht(struct cpuinfo_x86 *c)
 }
 
 static void __cpuinit detect_ht(struct cpuinfo_x86 *c)

diff --git a/include/asm-x86_64/apic.h b/include/asm-x86_64/apic.h
index e4580203dde28c7a9eb19fa5063e4d8f62e287bf..85125ef3c4143a362b9758114544c17f6ee38686 100644 (file)
--- a/include/asm-x86_64/apic.h
+++ b/include/asm-x86_64/apic.h
@@ -20,7 +20,6 @@ extern int apic_verbosity;
 extern int apic_runs_main_timer;
 extern int ioapic_force;
 extern int apic_mapped;
 extern int apic_runs_main_timer;
 extern int ioapic_force;
 extern int apic_mapped;

-extern int disable_apic_timer;

 
 /*
  * Define the default level of output to be very little
 
 /*
  * Define the default level of output to be very little