|
xen-devel
Re: [Xen-devel] [PATCH 1 of 3] apic: record local apic state on boot
On 19/05/11 00:40, Keir Fraser wrote:
On 18/05/2011 19:08, "Andrew Cooper"<andrew.cooper3@xxxxxxxxxx> wrote:
Xen does not store the boot local apic state which leads to problems
when shutting down for a kexec jump. This patch records the boot
state so we can return to the boot state when kexec'ing.
This is per CPU because all 3 bioses on the boxes I have tested dont
enabled all local apics on boot.
You mean some CPUs have APIC totally disabled?
That is what I meant but now I have double checked the trace, I realize
I was mis-interpreting the Xen APIC debug trace. As a result, I do
agree that most of the rest of this patch is based on false assumptions.
Impossible: you need the APIC
enabled to be able to INIT-SIPI bootstrap the secondary processors.
Nope. The MP spec section 3.8 states that all AP lapics must be
disabled when the bios passes over to the OS. Section 3.7.3 states that
the INIT-IPI is special and twiddles the PRST pin for external lapics,
or the INIT pin for internal lapics. The result in either case is the
lapic resetting, enabling themselves in the process, and reading the
reset vector.
And Xen
itself has no code to enable disabled APICs (except some very legacy code
that executes only in the uniprocessor case, which I'm pretty sure you're
not touching).
You call your new function so late in secondary CPU bringup that our own
APIC setup has already run (via
smp_callin->{x2apic_ap_setup,setup_local_APIC}). Which would indicate that
the boot state of the secondary CPUs is not of importance to you, since you
I'm sure tested your patch as working okay in this current form, and that
would imply that you don't need to record per-cpu boot state.
Hmm I thought I had traced the codepath but I clearly got that wrong.
Either way, given the new clarification from the MP spec, my suggestion
would be to fully disable the lapics in the nmi_crash_handler just
before looping over halt(). Experimental evidence shows however that
linux 2.6.32 kdump (and presumably earlier) can not function if its BSP
lapic is in x2apic mode when the bios would have left it disabled or in
xapic mode.
The singleshot BUG_ON in record_boot_APIC_mode is broken, since secondary
CPUs can be started up multiple times (we can offline/online them). You need
to bail on repeated invocations. Or not be recording boot state for
secondary cpus since apparently it isn't needed...
Fair enough - I didn't realize that it was possible to be called several
times
We're going to be going through these patches very slowly and painfully,
because frankly the arguments and assertions seem to be plenty full of
holes.
-- Keir
As a result, we have to return to
the bios state so the ACPI tables match up with the hardware state
for the booting kernel.
Signed-off-by: Andrew Cooper<andrew.cooper3@xxxxxxxxxx>
diff -r f531ed84b066 -r 62a8ce6595ad xen/arch/x86/apic.c
--- a/xen/arch/x86/apic.c Tue May 17 17:32:19 2011 +0100
+++ b/xen/arch/x86/apic.c Wed May 18 19:00:13 2011 +0100
@@ -74,6 +74,8 @@ u8 __read_mostly apic_verbosity;
static bool_t __initdata opt_x2apic = 1;
boolean_param("x2apic", opt_x2apic);
+DEFINE_PER_CPU_READ_MOSTLY(enum apic_mode, apic_boot_mode) =
APIC_MODE_INVALID;
+
bool_t __read_mostly x2apic_enabled = 0;
bool_t __read_mostly directed_eoi_enabled = 0;
@@ -1437,6 +1439,41 @@ int __init APIC_init_uniprocessor (void)
return 0;
}
+/* Needs to be called once per CPU during startup. It records the state the
BIOS
+ * leaves the local APIC so we can tare back down upon shutdown/crash
+ */
+void __init record_boot_APIC_mode(void)
+{
+ enum apic_mode this_apic_mode;
+ u64 msr_contents;
+
+ this_apic_mode = APIC_MODE_INVALID;
+
+ /* Sanity check - we should only ever run once */
+ BUG_ON( APIC_MODE_INVALID != this_cpu(apic_boot_mode) );
+
+ rdmsrl(MSR_IA32_APICBASE, msr_contents);
+
+ /* Reading EXTD bit from the MSR is only valid if CPUID says so, else
reserved */
+ if ( cpu_has(¤t_cpu_data, X86_FEATURE_X2APIC)
+&& (msr_contents& MSR_IA32_APICBASE_EXTD) )
+ this_apic_mode = APIC_MODE_X2APIC;
+ else
+ {
+ /* EN bit should always be valid as long as we can read the MSR
+ * Can anyone confirm this?
+ */
+ if ( msr_contents& MSR_IA32_APICBASE_ENABLE )
+ this_apic_mode = APIC_MODE_XAPIC;
+ else
+ this_apic_mode = APIC_MODE_DISABLED;
+ }
+
+ this_cpu(apic_boot_mode) = this_apic_mode;
+ apic_printk(APIC_DEBUG, "APIC boot state is %d on core #%d\n",
+ this_apic_mode, smp_processor_id());
+}
+
void check_for_unexpected_msi(unsigned int vector)
{
unsigned long v = apic_read(APIC_ISR + ((vector& ~0x1f)>> 1));
diff -r f531ed84b066 -r 62a8ce6595ad xen/arch/x86/genapic/probe.c
--- a/xen/arch/x86/genapic/probe.c Tue May 17 17:32:19 2011 +0100
+++ b/xen/arch/x86/genapic/probe.c Wed May 18 19:00:13 2011 +0100
@@ -60,6 +60,8 @@ void __init generic_apic_probe(void)
{
int i, changed;
+ record_boot_APIC_mode();
+
check_x2apic_preenabled();
cmdline_apic = changed = (genapic != NULL);
diff -r f531ed84b066 -r 62a8ce6595ad xen/arch/x86/smpboot.c
--- a/xen/arch/x86/smpboot.c Tue May 17 17:32:19 2011 +0100
+++ b/xen/arch/x86/smpboot.c Wed May 18 19:00:13 2011 +0100
@@ -388,6 +388,9 @@ void start_secondary(void *unused)
microcode_resume_cpu(cpu);
+ /* Record boot apic state */
+ record_boot_APIC_mode();
+
wmb();
startup_cpu_idle_loop();
}
diff -r f531ed84b066 -r 62a8ce6595ad xen/include/asm-x86/apic.h
--- a/xen/include/asm-x86/apic.h Tue May 17 17:32:19 2011 +0100
+++ b/xen/include/asm-x86/apic.h Wed May 18 19:00:13 2011 +0100
@@ -21,6 +21,16 @@
#define IO_APIC_REDIR_DEST_LOGICAL 0x00800
#define IO_APIC_REDIR_DEST_PHYSICAL 0x00000
+/* Possible APIC states */
+enum apic_mode { APIC_MODE_INVALID, /* Not set yet */
+ APIC_MODE_DISABLED, /* Some bioses disable by default for
compatability */
+ APIC_MODE_XAPIC, /* xAPIC mode - default upon chipset
reset */
+ APIC_MODE_X2APIC /* x2APIC mode - common for large smp
machines */
+};
+
+/* PerCPU local APIC boot mode - so we can taredown to bios state */
+DECLARE_PER_CPU(enum apic_mode, apic_boot_mode);
+
extern u8 apic_verbosity;
extern bool_t x2apic_enabled;
extern bool_t directed_eoi_enabled;
@@ -203,6 +213,7 @@ extern void disable_APIC_timer(void);
extern void enable_APIC_timer(void);
extern int lapic_suspend(void);
extern int lapic_resume(void);
+extern void record_boot_APIC_mode(void);
extern int check_nmi_watchdog (void);
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
--
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
|
|
Home