[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] x86/APIC: Remove workaround Pentium 3AP APIC_ESR erratum



The SDM instructs software to write 0 to ESR prior to reading it.  However,
due to an original Pentium erratum, most logic skips the write based on there
being more than 3 LVTs; a stand-in to identify the Pentium.

Xen, being 64bit, doesn't need compatibility for i586 processors.

Introduce a new apic_read_esr() helper, quoting the SDM to explain why a
function named apic_read_esr() has a write in it too.

Use the new helper throughout apic.c and smpboot.c, which allows us to remove
some useless reads of APIC_LVR.  This in turn removes the external callers of
get_maxlvt(), so make it local to apic.c

No practical change.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Javi Merino <javi.merino@xxxxxxxxx>

Based on Javi's patch correcting error_interrupt()

Bloat-o-meter reports:

  add/remove: 0/1 grow/shrink: 0/3 up/down: 0/-269 (-269)
  Function                                     old     new   delta
  get_maxlvt                                    48       -     -48
  __cpu_up                                    1465    1417     -48
  clear_local_APIC                            1109    1050     -59
  setup_local_APIC                             942     828    -114
---
 xen/arch/x86/apic.c             | 29 ++++++++++-------------------
 xen/arch/x86/include/asm/apic.h | 24 +++++++++++++++++++++++-
 xen/arch/x86/smpboot.c          | 17 ++++-------------
 3 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index b4f542d25918..017d97054b06 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -142,7 +142,7 @@ int get_physical_broadcast(void)
         return 0xf;
 }
 
-int get_maxlvt(void)
+static int get_maxlvt(void)
 {
     unsigned int v = apic_read(APIC_LVR);
 
@@ -209,9 +209,7 @@ void clear_local_APIC(void)
         apic_write(APIC_LDR, v);
     }
 
-    if (maxlvt > 3)        /* Due to Pentium errata 3AP and 11AP. */
-        apic_write(APIC_ESR, 0);
-    apic_read(APIC_ESR);
+    apic_read_esr();
 }
 
 void __init connect_bsp_APIC(void)
@@ -488,7 +486,7 @@ static void resume_x2apic(void)
 
 void setup_local_APIC(bool bsp)
 {
-    unsigned long oldvalue, value, maxlvt;
+    unsigned long oldvalue, value;
     int i, j;
 
     BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);
@@ -614,17 +612,13 @@ void setup_local_APIC(bool bsp)
         value = APIC_DM_NMI | APIC_LVT_MASKED;
     apic_write(APIC_LVT1, value);
 
-    maxlvt = get_maxlvt();
-    if (maxlvt > 3)     /* Due to the Pentium erratum 3AP. */
-        apic_write(APIC_ESR, 0);
-    oldvalue = apic_read(APIC_ESR);
+    oldvalue = apic_read_esr();
 
     value = ERROR_APIC_VECTOR;      // enables sending errors
     apic_write(APIC_LVTERR, value);
-    /* spec says clear errors after enabling vector. */
-    if (maxlvt > 3)
-        apic_write(APIC_ESR, 0);
-    value = apic_read(APIC_ESR);
+
+    value = apic_read_esr();
+
     if (value != oldvalue)
         apic_printk(APIC_VERBOSE,
                     "ESR value before enabling vector: %#lx  after: %#lx\n",
@@ -719,11 +713,9 @@ int lapic_resume(void)
     apic_write(APIC_LVTT, apic_pm_state.apic_lvtt);
     apic_write(APIC_TDCR, apic_pm_state.apic_tdcr);
     apic_write(APIC_TMICT, apic_pm_state.apic_tmict);
-    apic_write(APIC_ESR, 0);
-    apic_read(APIC_ESR);
+    apic_read_esr();
     apic_write(APIC_LVTERR, apic_pm_state.apic_lvterr);
-    apic_write(APIC_ESR, 0);
-    apic_read(APIC_ESR);
+    apic_read_esr();
     local_irq_restore(flags);
     return 0;
 }
@@ -1389,8 +1381,7 @@ static void cf_check error_interrupt(void)
     unsigned int i;
 
     /* First tickle the hardware, only then report what went on. -- REW */
-    apic_write(APIC_ESR, 0);
-    v = apic_read(APIC_ESR);
+    v = apic_read_esr();
     ack_APIC_irq();
 
     for ( i = 0; i < ARRAY_SIZE(entries); ++i )
diff --git a/xen/arch/x86/include/asm/apic.h b/xen/arch/x86/include/asm/apic.h
index d8eda6df6d86..337eb5cf6642 100644
--- a/xen/arch/x86/include/asm/apic.h
+++ b/xen/arch/x86/include/asm/apic.h
@@ -151,6 +151,29 @@ static inline u32 get_apic_id(void)
     return x2apic_enabled ? id : GET_xAPIC_ID(id);
 }
 
+static inline uint32_t apic_read_esr(void)
+{
+    /*
+     * The SDM states:
+     *   Before attempt to read from the ESR, software should first write to
+     *   it. (The value written does not affect the values read subsequently;
+     *   only zero may be written in x2APIC mode.) This write clears any
+     *   previously logged errors and updates the ESR with any errors detected
+     *   since the last write to the ESR. This write also rearms the APIC
+     *   error interrupt triggering mechanism.
+     */
+    if ( x2apic_enabled )
+    {
+        apic_wrmsr(APIC_ESR, 0);
+        return apic_rdmsr(APIC_ESR);
+    }
+    else
+    {
+        apic_mem_write(APIC_ESR, 0);
+        return apic_mem_read(APIC_ESR);
+    }
+}
+
 void apic_wait_icr_idle(void);
 
 int get_physical_broadcast(void);
@@ -161,7 +184,6 @@ static inline void ack_APIC_irq(void)
        apic_write(APIC_EOI, 0);
 }
 
-extern int get_maxlvt(void);
 extern void clear_local_APIC(void);
 extern void connect_bsp_APIC (void);
 extern void disconnect_bsp_APIC (int virt_wire_setup);
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 79a79c54c304..7c77125fe715 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -422,7 +422,7 @@ void asmlinkage start_secondary(void *unused)
 static int wakeup_secondary_cpu(int phys_apicid, unsigned long start_eip)
 {
     unsigned long send_status = 0, accept_status = 0;
-    int maxlvt, timeout, i;
+    int timeout, i;
 
     /*
      * Normal AP startup uses an INIT-SIPI-SIPI sequence.
@@ -444,8 +444,7 @@ static int wakeup_secondary_cpu(int phys_apicid, unsigned 
long start_eip)
     /*
      * Be paranoid about clearing APIC errors.
      */
-    apic_write(APIC_ESR, 0);
-    apic_read(APIC_ESR);
+    apic_read_esr();
 
     if ( send_INIT )
     {
@@ -495,13 +494,10 @@ static int wakeup_secondary_cpu(int phys_apicid, unsigned 
long start_eip)
         }
     }
 
-    maxlvt = get_maxlvt();
-
     for ( i = 0; i < 2; i++ )
     {
         Dprintk("Sending STARTUP #%d.\n", i+1);
-        apic_write(APIC_ESR, 0);
-        apic_read(APIC_ESR);
+        apic_read_esr();
         Dprintk("After apic_write.\n");
 
         /*
@@ -529,12 +525,7 @@ static int wakeup_secondary_cpu(int phys_apicid, unsigned 
long start_eip)
             udelay(200);
         }
 
-        /* Due to the Pentium erratum 3AP. */
-        if ( maxlvt > 3 )
-        {
-            apic_write(APIC_ESR, 0);
-        }
-        accept_status = (apic_read(APIC_ESR) & 0xEF);
+        accept_status = apic_read_esr() & 0xEF;
         if ( send_status || accept_status )
             break;
     }

base-commit: c8e3e39085bf97d1afb775d54884d239387e32cd
prerequisite-patch-id: 1f86bfc85bb08e12c21535d5c527f555f192d4e7
-- 
2.39.5




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.