[Xen-devel] [PATCH 2 of 3] apic: remove 'enabled_via_apicbase' variable

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

The use of this varable was only sensible when the choice for lapic mode
was between enabled or disabled.  Now that x2apic is about, it is wrong,
and causes a protection fault in certain cases when trying to tare down
x2apic mode.

The only place where its use is relevent in the code is in disable_local_APIC
which has been changed to correctly tare down the local APIC without a
protection fault (which leads to a general protection fault).

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

diff -r 62a8ce6595ad -r e80b5280fe2f xen/arch/x86/apic.c
--- a/xen/arch/x86/apic.c       Wed May 18 19:00:13 2011 +0100
+++ b/xen/arch/x86/apic.c       Wed May 18 19:00:13 2011 +0100
@@ -165,8 +165,6 @@ void __init apic_intr_init(void)
 /* Using APIC to generate smp_local_timer_interrupt? */
 static bool_t __read_mostly using_apic_timer;
 
-static bool_t __read_mostly enabled_via_apicbase;
-
 int get_physical_broadcast(void)
 {
     if (modern_apic())
@@ -330,6 +328,9 @@ void disconnect_bsp_APIC(int virt_wire_s
 
 void disable_local_APIC(void)
 {
+    u64 msr_contents;
+    enum apic_mode current_mode;
+
     clear_local_APIC();
 
     /*
@@ -339,10 +340,54 @@ void disable_local_APIC(void)
     apic_write_around(APIC_SPIV,
         apic_read(APIC_SPIV) & ~APIC_SPIV_APIC_ENABLED);
 
-    if (enabled_via_apicbase) {
-        uint64_t msr_content;
-        rdmsrl(MSR_IA32_APICBASE, msr_content);
-        wrmsrl(MSR_IA32_APICBASE, msr_content & ~MSR_IA32_APICBASE_ENABLE);
+    /* Work out what apic mode we are in */
+    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) )
+        current_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 )
+                current_mode = APIC_MODE_XAPIC;
+            else
+                current_mode = APIC_MODE_DISABLED;
+        }
+
+    /* See what (if anything) we need to do to revert to boot mode */
+    if( current_mode != this_cpu(apic_boot_mode) )
+    {
+        rdmsrl(MSR_IA32_APICBASE, msr_contents);
+        msr_contents &= ~ ( MSR_IA32_APICBASE_ENABLE | MSR_IA32_APICBASE_EXTD 
);
+        wrmsrl(MSR_IA32_APICBASE, msr_contents);
+
+        switch(this_cpu(apic_boot_mode))
+        {
+        case APIC_MODE_DISABLED:
+            break; /* Nothing to do - we did this above */
+        case APIC_MODE_XAPIC:
+        {
+            msr_contents |= MSR_IA32_APICBASE_ENABLE;
+            wrmsrl(MSR_IA32_APICBASE, msr_contents);
+            break;
+        }
+        case APIC_MODE_X2APIC:
+        {
+            msr_contents |= ( MSR_IA32_APICBASE_ENABLE | 
MSR_IA32_APICBASE_EXTD );
+            wrmsrl(MSR_IA32_APICBASE, msr_contents);
+            break;
+        }
+        default:
+        {
+            printk("Hit default case when reverting lapic to boot state on 
core #%d\n",
+                   smp_processor_id());
+            break;
+        }
+        }
     }
 }
 
@@ -874,7 +919,6 @@ static int __init detect_init_APIC (void
             wrmsrl(MSR_IA32_APICBASE,
                 msr_content | MSR_IA32_APICBASE_ENABLE
                 | APIC_DEFAULT_PHYS_BASE);
-            enabled_via_apicbase = 1;
         }
     }
     /*

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel