@@ -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 )
@@ -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);
@@ -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;
}
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@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Javi Merino <javi.merino@cloud.com> 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(-) base-commit: c8e3e39085bf97d1afb775d54884d239387e32cd prerequisite-patch-id: 1f86bfc85bb08e12c21535d5c527f555f192d4e7