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

Re: [Xen-devel] [PATCH 1 of 3] apic: record local apic state on boot





On 18/05/11 19:49, Konrad Rzeszutek Wilk wrote:
On Wed, May 18, 2011 at 07:08:14PM +0100, Andrew Cooper 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.  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.
Which ACPI table requires this?
Cant remember offhand but linux (2.6.32) was doing finger pointing at the multi-processor tables
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
tare?
I fail at spelling - I meant "tear down"
+ */
+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(&current_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?
Email Vivek Goyal. He is the kexec/kdump maintainer.
He doesn't appear in the maintainers file, and I don't have an email address.
+             */
+            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());
This begs of a function to convert those enums to strings..
True - this was left over from my debugging - I shall fix that tomorrow.
+}
+
  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();
+
The spacing looks odd.
Does it? Looks fine for me - I have been following the 1 tab to 4 spaces convention in half the codebase.
        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 */
taredown?
same comment as above re. spelling
+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


 


Rackspace

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