Changeset 49442 in vbox for trunk/src/VBox/Devices

Timestamp:

Nov 11, 2013 9:52:32 PM (11 years ago)

Author:

vboxsync

Message:

DevAPIC: Always set the APIC CPUID feature bits when the device is constructed, regardless of the CPU vendor. I.e. just like apicR3Reset always did.

File:

• trunk/src/VBox/Devices/PC/DevAPIC.cpp (modified) (10 diffs)

trunk/src/VBox/Devices/PC/DevAPIC.cpp

@@ -284 +284 @@
     /* Physical APIC id - not visible to user, constant */
     PhysApicId              phys_id;
-    /** @todo: is it logical or physical? Not really used anyway now. */
+    /** @todo is it logical or physical? Not really used anyway now. */
     PhysApicId              arb_id;
     uint32_t                spurious_vec;
@@ -493 +493 @@
             return MSR_IA32_APICBASE_ENABLE;
         case PDMAPICVERSION_X2APIC:
-            return MSR_IA32_APICBASE_ENABLE | MSR_IA32_APICBASE_X2ENABLE ;
+            return MSR_IA32_APICBASE_ENABLE | MSR_IA32_APICBASE_X2ENABLE;
         default:
             AssertMsgFailed(("Unsupported APIC version %d\n", pDev->enmVersion));
@@ -588 +588 @@
     Log(("apicSetBase: %016RX64\n", val));
 
-    /** @todo: do we need to lock here ? */
+    /** @todo do we need to lock here ? */
     /* APIC_LOCK_VOID(pDev, VERR_INTERNAL_ERROR); */
     /** @todo If this change is valid immediately, then we should change the MMIO registration! */
@@ -610 +610 @@
             }
             case PDMAPICVERSION_APIC:
-                /** @todo: map MMIO ranges, if needed */
+                /** @todo map MMIO ranges, if needed */
                 break;
             case PDMAPICVERSION_X2APIC:
-                /** @todo: unmap MMIO ranges of this APIC, according to the spec */
+                /** @todo unmap MMIO ranges of this APIC, according to the spec.  This is how
+                 *       real hw works! (Remember the problem disabling NMI watchdog timers in
+                 *       the world switchers when host used x2apic?)! */
                 break;
             default:
@@ -1853 +1855 @@
     Assert(cb == 4);
 
-    /** @todo: add LAPIC range validity checks (multiple LAPICs can theoretically have
-     *         different physical addresses, see @bugref{3092}) */
+    /** @todo add LAPIC range validity checks (multiple LAPICs can theoretically
+     *        have different physical addresses, see @bugref{3092}) */
 
     STAM_COUNTER_INC(&CTXSUFF(pDev->StatMMIOWrite));
@@ -2048 +2050 @@
     apicR3LiveExec(pDevIns, pSSM, SSM_PASS_FINAL);
 
-    /* save all APICs data */ /** @todo: is it correct? */
+    /* save all APICs data */ /** @todo is it correct? */
     APIC_FOREACH_BEGIN(pDev);
         apic_save(pSSM, pCurApic);
@@ -2090 +2092 @@
         return VINF_SUCCESS;
 
-    /* load all APICs data */ /** @todo: is it correct? */
+    /* load all APICs data */ /** @todo is it correct? */
     APIC_LOCK(pDev, VERR_INTERNAL_ERROR_3);
 
@@ -2134 +2136 @@
         apicCpuClearInterrupt(pDev, pApic);
     }
-    /** @todo r=bird: Why is this done everytime, while the constructor first
-     *        checks the CPUID?  Who is right? */
+
+    LogRel(("DevAPIC: Re-activating Local APIC\n"));
     pDev->pApicHlpR3->pfnChangeFeature(pDev->pDevInsR3, pDev->enmVersion);
 
@@ -2231 +2233 @@
     pDev->cCpus      = cCpus;
     pDev->fIoApic    = fIoApic;
-    /* Use PDMAPICVERSION_X2APIC to activate x2APIC mode */
+    /** @todo Finish X2APIC implementation.  Must, among other things, set
+     *        PDMAPICVERSION_X2APIC here when X2APIC is configured. */
     pDev->enmVersion = PDMAPICVERSION_APIC;
 
@@ -2323 +2326 @@
      * The CPUID feature bit.
      */
-    /** @todo r=bird: See remark in the apicR3Reset. */
-    uint32_t u32Eax, u32Ebx, u32Ecx, u32Edx;
-    PDMDevHlpGetCpuId(pDevIns, 0, &u32Eax, &u32Ebx, &u32Ecx, &u32Edx);
-    if (u32Eax >= 1)
-    {
-        if (   fIoApic                       /* If IOAPIC is enabled, enable Local APIC in any case */
-            || (   u32Ebx == X86_CPUID_VENDOR_INTEL_EBX
-                && u32Ecx == X86_CPUID_VENDOR_INTEL_ECX
-                && u32Edx == X86_CPUID_VENDOR_INTEL_EDX /* GenuineIntel */)
-            || (   u32Ebx == X86_CPUID_VENDOR_AMD_EBX
-                && u32Ecx == X86_CPUID_VENDOR_AMD_ECX
-                && u32Edx == X86_CPUID_VENDOR_AMD_EDX   /* AuthenticAMD */))
-        {
-            LogRel(("Activating Local APIC\n"));
-            pDev->pApicHlpR3->pfnChangeFeature(pDevIns, pDev->enmVersion);
-        }
-    }
+    LogRel(("DevAPIC: Activating Local APIC\n"));
+    pDev->pApicHlpR3->pfnChangeFeature(pDevIns, pDev->enmVersion);
 
     /*
      * Register the MMIO range.
      */
-    /** @todo: shall reregister, if base changes. */
+    /** @todo shall reregister, if base changes. */
     uint32_t ApicBase = pDev->paLapicsR3[0].apicbase & ~0xfff;
     rc = PDMDevHlpMMIORegister(pDevIns, ApicBase, 0x1000, pDev,