Changeset 54315 in vbox for trunk/src/VBox/HostDrivers

Timestamp:

Feb 19, 2015 9:33:21 PM (10 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

98390

Message:

SUPDrv: Eliminated the need for the GIP master to have a zero TSC delta value. Fixed incorrect RTCPUSET indexing (don't use idCpu!). Documented the tsc delta thred states. Made it transition from terminating to destroyed. Renamed supdrvMeasureTscDeltas to supdrvMeasureInitialTscDeltas and dropped the unused pidxMaster parameter.

Location:

trunk/src/VBox/HostDrivers/Support

Files:

• SUPDrv.c (modified) (27 diffs)
• SUPDrvInternal.h (modified) (2 diffs)

trunk/src/VBox/HostDrivers/Support/SUPDrv.c

@@ -179 +179 @@
                                                   RTCPUID idCpu, uint8_t idApic, uint64_t iTick);
 static void                 supdrvGipInitCpu(PSUPDRVDEVEXT pDevExt, PSUPGLOBALINFOPAGE pGip, PSUPGIPCPU pCpu, uint64_t u64NanoTS);
-static int                  supdrvMeasureTscDeltas(PSUPDRVDEVEXT pDevExt, uint32_t *pidxMaster);
+static int                  supdrvMeasureInitialTscDeltas(PSUPDRVDEVEXT pDevExt);
 static int                  supdrvMeasureTscDeltaOne(PSUPDRVDEVEXT pDevExt, uint32_t idxWorker);
 static int                  supdrvIOCtl_ResumeSuspendedKbds(void);
@@ -6051 +6051 @@
             {
                 pDevExt->enmTscDeltaThreadState = kTscDeltaThreadState_Measuring;
-                rc = RTSemEventSignal(pDevExt->hTscDeltaEvent);
+                rc = RTSemEventSignal(pDevExt->hTscDeltaEvent); /* (Safe on windows as long as spinlock isn't IRQ safe.) */
                 if (RT_FAILURE(rc))
                     return supdrvTscDeltaThreadButchered(pDevExt, true /* fSpinlockHeld */, "RTSemEventSignal", rc);
@@ -6070 +6070 @@
                     do
                     {
-                        rc = supdrvMeasureTscDeltas(pDevExt, NULL /* pidxMaster */);
+                        rc = supdrvMeasureInitialTscDeltas(pDevExt);
                         if (   RT_SUCCESS(rc)
                             || (   RT_FAILURE(rc)
@@ -6092 +6092 @@
                         PSUPGIPCPU pGipCpuWorker = &pGip->aCPUs[iCpu];
                         if (   pGipCpuWorker->i64TSCDelta == INT64_MAX
-                            && RTCpuSetIsMember(&pDevExt->TscDeltaCpuSet, pGipCpuWorker->idCpu))
+                            && RTCpuSetIsMember(&pDevExt->TscDeltaCpuSet, pGipCpuWorker->iCpuSet))
                         {
                             rc |= supdrvMeasureTscDeltaOne(pDevExt, iCpu);
@@ -6108 +6108 @@
 
             case kTscDeltaThreadState_Terminating:
+                pDevExt->enmTscDeltaThreadState = kTscDeltaThreadState_Destroyed;
                 RTSpinlockRelease(pDevExt->hTscDeltaSpinlock);
                 return VINF_SUCCESS;
@@ -6125 +6126 @@
  *
  * @returns VINF_SUCCESS on success, VERR_TIMEOUT if it doesn't respond in time,
- *        other error code on internal error.
+ *          other error code on internal error.
  *
  * @param   pThis           Pointer to the grant service instance data.
@@ -6769 +6770 @@
                     do
                     {
-                        rc = supdrvMeasureTscDeltas(pDevExt, NULL /* pidxMaster */);
+                        rc = supdrvMeasureInitialTscDeltas(pDevExt);
                         if (   rc != VERR_TRY_AGAIN
                             && rc != VERR_CPU_OFFLINE)
@@ -6826 +6827 @@
                 }
                 else
-                    OSDBGPRINT(("supdrvGipCreate: supdrvMeasureTscDeltas failed. rc=%Rrc\n", rc));
+                    OSDBGPRINT(("supdrvGipCreate: supdrvMeasureInitialTscDeltas failed. rc=%Rrc\n", rc));
             }
             else
@@ -7002 +7003 @@
  * @param   idCpu               The CPU ID.
  */
-static uint32_t supdrvGipCpuIndexFromCpuId(PSUPGLOBALINFOPAGE pGip, RTCPUID idCpu)
+static uint32_t supdrvGipFindOrAllocCpuIndexForCpuId(PSUPGLOBALINFOPAGE pGip, RTCPUID idCpu)
 {
     uint32_t i, cTries;
@@ -7030 +7031 @@
 
 /**
+ * Finds the GIP CPU index corresponding to @a idCpu.
+ *
+ * @returns GIP CPU array index, UINT32_MAX if not found.
+ * @param   pGip                The GIP.
+ * @param   idCpu               The CPU ID.
+ */
+static uint32_t supdrvGipFindCpuIndexForCpuId(PSUPGLOBALINFOPAGE pGip, RTCPUID idCpu)
+{
+    uint32_t i;
+    for (i = 0; i < pGip->cCpus; i++)
+        if (pGip->aCPUs[i].idCpu == idCpu)
+            return i;
+    return UINT32_MAX;
+}
+
+
+/**
  * The calling CPU should be accounted as online, update GIP accordingly.
  *
- * This is used by supdrvGipMpEvent as well as the supdrvGipCreate.
+ * This is used by supdrvGipCreate() as well as supdrvGipMpEvent().
  *
  * @param   pDevExt             The device extension.
@@ -7072 +7090 @@
      */
     u64NanoTS = RTTimeSystemNanoTS() - pGip->u32UpdateIntervalNS;
-    i = supdrvGipCpuIndexFromCpuId(pGip, idCpu);
+    i = supdrvGipFindOrAllocCpuIndexForCpuId(pGip, idCpu);
     supdrvGipInitCpu(pDevExt, pGip, &pGip->aCPUs[i], u64NanoTS);
     idApic = ASMGetApicId();
@@ -7091 +7109 @@
     if (pGip->enmUseTscDelta > SUPGIPUSETSCDELTA_ZERO_CLAIMED)
     {
-        RTCpuSetAdd(&pDevExt->TscDeltaCpuSet, idCpu);
+        RTCpuSetAdd(&pDevExt->TscDeltaCpuSet, iCpuSet);
 #ifdef SUPDRV_USE_TSC_DELTA_THREAD
         RTSpinlockAcquire(pDevExt->hTscDeltaSpinlock);
@@ -7153 +7171 @@
         ASMAtomicWriteS64(&pGip->aCPUs[i].i64TSCDelta, INT64_MAX);
         /* Remove this CPU from the set of CPUs that we have obtained the TSC deltas. */
-        RTCpuSetDel(&pDevExt->TscDeltaObtainedCpuSet, idCpu);
+        RTCpuSetDel(&pDevExt->TscDeltaObtainedCpuSet, iCpuSet);
     }
 
@@ -7210 +7228 @@
         {
             /*
-             * Find a new GIP master.
+             * The GIP master is going offline, find a new one.
              */
             bool        fIgnored;
             unsigned    i;
-            int64_t     iTSCDelta;
-            uint32_t    idxNewGipMaster;
             RTCPUID     idNewGipMaster = NIL_RTCPUID;
             RTCPUSET    OnlineCpus;
@@ -7234 +7250 @@
             ASMAtomicCmpXchgSize(&pDevExt->idGipMaster, idNewGipMaster, idGipMaster, fIgnored);
             NOREF(fIgnored);
-
-            /*
-             * Adjust all the TSC deltas against the new GIP master.
-             */
-            /** @todo this needs fixing, we cannot dynamically re-adjust the base unless
-             *        GIP isn't mapped and no other consumers of TSC-deltas. Basically,
-             *        we need to adjust the master's own TSC-delta (can be non-zero)
-             *        while computing the deltas. */
-            if (pGip)
-            {
-                idxNewGipMaster = supdrvGipCpuIndexFromCpuId(pGip, idNewGipMaster);
-                iTSCDelta       = pGip->aCPUs[idxNewGipMaster].i64TSCDelta;
-                Assert(iTSCDelta != INT64_MAX);
-                for (i = 0; i < pGip->cCpus; i++)
-                {
-                    PSUPGIPCPU pGipCpu      = &pGip->aCPUs[i];
-                    int64_t    iWorkerDelta = pGipCpu->i64TSCDelta;
-                    if (iWorkerDelta != INT64_MAX)
-                        iWorkerDelta -= iTSCDelta;
-                    ASMAtomicWriteS64(&pGipCpu->i64TSCDelta, iWorkerDelta);
-                }
-                Assert(pGip->aCPUs[idxNewGipMaster].i64TSCDelta == 0);
-            }
-        }
-    }
-}
-
-
-/**
- * Callback used by supdrvMeasureTscDeltas() to read the TSC on two CPUs and
- * compute the delta between them.
+        }
+    }
+}
+
+
+/**
+ * Callback used by supdrvMeasureInitialTscDeltas() to read the TSC on two CPUs
+ * and compute the delta between them.
  *
  * @param   idCpu       The CPU we are current scheduled on.
  * @param   pvUser1     Opaque pointer to the device instance data.
- * @param   pvUser2     Opaque pointer to the worker Cpu Id.
+ * @param   pvUser2     Pointer to the SUPGIPCPU entry of the worker CPU.
  *
  * @remarks Measuring TSC deltas between the CPUs is tricky because we need to
@@ -7297 +7290 @@
 static DECLCALLBACK(void) supdrvMeasureTscDeltaCallback(RTCPUID idCpu, void *pvUser1, void *pvUser2)
 {
-    PSUPDRVDEVEXT      pDevExt   = (PSUPDRVDEVEXT)pvUser1;
-    PSUPGLOBALINFOPAGE pGip      = pDevExt->pGip;
-    uint32_t          *pidWorker = (uint32_t *)pvUser2;
-    RTCPUID            idMaster  = ASMAtomicUoReadU32(&pDevExt->idTscDeltaInitiator);
-    unsigned           idxMaster = supdrvGipCpuIndexFromCpuId(pGip, idMaster);
-    unsigned           idxWorker = supdrvGipCpuIndexFromCpuId(pGip, *pidWorker);
-    PSUPGIPCPU         pGipCpuMaster = &pGip->aCPUs[idxMaster];
-    PSUPGIPCPU         pGipCpuWorker = &pGip->aCPUs[idxWorker];
-    int                cTriesLeft = 12;
+    PSUPDRVDEVEXT      pDevExt          = (PSUPDRVDEVEXT)pvUser1;
+    PSUPGLOBALINFOPAGE pGip             = pDevExt->pGip;
+    PSUPGIPCPU         pGipCpuWorker    = (PSUPGIPCPU)pvUser2;
+    uint32_t           idWorker         = pGipCpuWorker->idCpu;
+    RTCPUID            idMaster         = ASMAtomicUoReadU32(&pDevExt->idTscDeltaInitiator);
+    unsigned           idxMaster        = supdrvGipFindCpuIndexForCpuId(pGip, idMaster);
+    PSUPGIPCPU         pGipCpuMaster    = &pGip->aCPUs[idxMaster];
+    int                cTriesLeft;
 
     if (   idCpu != idMaster
-        && idCpu != *pidWorker)
+        && idCpu != idWorker)
         return;
 
@@ -7359 +7351 @@
     }
 
+    /*
+     * ...
+     */
     Assert(pGipCpuWorker->i64TSCDelta == INT64_MAX);
+    cTriesLeft = 12;
     while (cTriesLeft-- > 0)
     {
@@ -7397 +7393 @@
                     if (pGipCpuWorker->u64TSCSample != GIP_TSC_DELTA_RSVD)
                     {
-                        int64_t iDelta = pGipCpuWorker->u64TSCSample - pGipCpuMaster->u64TSCSample;
+                        int64_t iDelta = pGipCpuWorker->u64TSCSample
+                                       - (pGipCpuMaster->u64TSCSample - pGipCpuMaster->i64TSCDelta);
+                        /** @todo r=bird: Why isn't this code using absolute values? Guess it mostly works fine because
+                         * the detection code is biased thowards positive deltas (see tstSupTscDelta.cpp output) ... */
                         if (iDelta < pGipCpuWorker->i64TSCDelta)
                             pGipCpuWorker->i64TSCDelta = iDelta;
@@ -7464 +7463 @@
             if (idCpu == idMaster)
             {
-                RTCpuSetDel(&pDevExt->TscDeltaCpuSet, pGipCpuMaster->idCpu);
-                RTCpuSetAdd(&pDevExt->TscDeltaObtainedCpuSet, pGipCpuMaster->idCpu);
+                RTCpuSetDel(&pDevExt->TscDeltaCpuSet, pGipCpuMaster->iCpuSet);
+                RTCpuSetAdd(&pDevExt->TscDeltaObtainedCpuSet, pGipCpuMaster->iCpuSet);
             }
             else
             {
-                RTCpuSetDel(&pDevExt->TscDeltaCpuSet, pGipCpuWorker->idCpu);
-                RTCpuSetAdd(&pDevExt->TscDeltaObtainedCpuSet, pGipCpuWorker->idCpu);
+                RTCpuSetDel(&pDevExt->TscDeltaCpuSet, pGipCpuWorker->iCpuSet);
+                RTCpuSetAdd(&pDevExt->TscDeltaObtainedCpuSet, pGipCpuWorker->iCpuSet);
             }
             break;
@@ -7529 +7528 @@
     Assert(pGip->enmUseTscDelta > SUPGIPUSETSCDELTA_ZERO_CLAIMED);
 
+    /*
+     * Don't attempt measuring the delta for the GIP master.
+     */
     if (pGipCpuWorker->idCpu == idMaster)
     {
-        ASMAtomicWriteS64(&pGipCpuWorker->i64TSCDelta, 0);
+        if (pGipCpuWorker->i64TSCDelta == INT64_MAX) /* This shouldn't happen, but just in case. */
+            ASMAtomicWriteS64(&pGipCpuWorker->i64TSCDelta, 0);
         return VINF_SUCCESS;
     }
@@ -7550 +7553 @@
         ASMAtomicWriteS64(&pGipCpuWorker->i64TSCDelta, INT64_MAX);
         ASMAtomicWriteU32(&pDevExt->pTscDeltaSync->u, GIP_TSC_DELTA_SYNC_STOP);
-        rc = RTMpOnAll(supdrvMeasureTscDeltaCallback, pDevExt, &pGipCpuWorker->idCpu);
+        rc = RTMpOnAll(supdrvMeasureTscDeltaCallback, pDevExt, pGipCpuWorker);
         if (RT_SUCCESS(rc))
         {
@@ -7587 +7590 @@
 
 /**
- * Measures the TSC deltas between CPUs.
- *
+ * Performs the initial measurements of the TSC deltas between CPUs.
+ *
+ * This is called by supdrvGipCreate or triggered by it if threaded.
+ *
+ * @returns VBox status code.
  * @param   pDevExt     Pointer to the device instance data.
- * @param   pidxMaster  Where to store the index of the chosen master TSC if we
- *                      managed to determine the TSC deltas successfully.
- *                      Optional, can be NULL.
- *
- * @returns VBox status code.
+ *
  * @remarks Must be called only after supdrvGipInitOnCpu() as this function uses
  *          idCpu, GIP's online CPU set which are populated in
  *          supdrvGipInitOnCpu().
  */
-static int supdrvMeasureTscDeltas(PSUPDRVDEVEXT pDevExt, uint32_t *pidxMaster)
+static int supdrvMeasureInitialTscDeltas(PSUPDRVDEVEXT pDevExt)
 {
     PSUPGIPCPU pGipCpuMaster;
@@ -7607 +7609 @@
     int        rc             = VINF_SUCCESS;
     uint32_t   cMpOnOffEvents = ASMAtomicReadU32(&pDevExt->cMpOnOffEvents);
-    uint32_t   cOnlineCpus    = pGip->cOnlineCpus;
 
     Assert(pGip->enmUseTscDelta > SUPGIPUSETSCDELTA_ZERO_CLAIMED);
@@ -7638 +7639 @@
     ASMAtomicWriteSize(&pDevExt->idGipMaster, pGipCpuMaster->idCpu);
 
-    AssertReturn(cOnlineCpus > 0, VERR_INTERNAL_ERROR_5);
+    /*
+     * If there is only a single CPU online we have nothing to do.
+     */
     if (pGip->cOnlineCpus <= 1)
     {
-        if (pidxMaster)
-            *pidxMaster = idxMaster;
+        AssertReturn(pGip->cOnlineCpus > 0, VERR_INTERNAL_ERROR_5);
         return VINF_SUCCESS;
     }
 
+    /*
+     * Loop thru the GIP CPU array and get deltas for each CPU (except the master).
+     */
     for (iCpu = 0; iCpu < pGip->cCpus; iCpu++)
     {
         PSUPGIPCPU pGipCpuWorker = &pGip->aCPUs[iCpu];
         if (   iCpu != idxMaster
-            && RTCpuSetIsMember(&pDevExt->TscDeltaCpuSet, pGipCpuWorker->idCpu))
+            && RTCpuSetIsMember(&pDevExt->TscDeltaCpuSet, pGipCpuWorker->iCpuSet))
         {
             rc = supdrvMeasureTscDeltaOne(pDevExt, iCpu);
@@ -7669 +7674 @@
     }
 
-    if (   RT_SUCCESS(rc)
-        && !pGipCpuMaster->i64TSCDelta
-        && pidxMaster)
-    {
-        *pidxMaster = idxMaster;
-    }
     return rc;
 }
@@ -8267 +8266 @@
     if (RT_UNLIKELY(iTick == 1))
     {
-        iCpu = supdrvGipCpuIndexFromCpuId(pGip, idCpu);
+        iCpu = supdrvGipFindOrAllocCpuIndexForCpuId(pGip, idCpu);
         if (pGip->aCPUs[iCpu].enmState == SUPGIPCPUSTATE_OFFLINE)
             supdrvGipMpEventOnline(pDevExt, idCpu);
@@ -8375 +8374 @@
                  *        thread. Check if anyone uses/needs fAsync before implementing this. */
                 RTSpinlockAcquire(pDevExt->hTscDeltaSpinlock);
-                RTCpuSetAdd(&pDevExt->TscDeltaCpuSet, pGipCpuWorker->idCpu);
+                RTCpuSetAdd(&pDevExt->TscDeltaCpuSet, pGipCpuWorker->iCpuSet);
                 if (   pDevExt->enmTscDeltaThreadState == kTscDeltaThreadState_Listening
                     || pDevExt->enmTscDeltaThreadState == kTscDeltaThreadState_Measuring)

trunk/src/VBox/HostDrivers/Support/SUPDrvInternal.h

@@ -251 +251 @@
     /** Uninitialized/invalid value. */
     kTscDeltaThreadState_Invalid = 0,
-    /** The thread is being created. */
+    /** The thread is being created.
+     * Next state: Listening, Butchered, Terminating  */
     kTscDeltaThreadState_Creating,
-    /** The thread is listening for events. */
+    /** The thread is listening for events.
+     * Previous state: Creating, Measuring
+     * Next state: WaitAndMeasure, Butchered, Terminated */
     kTscDeltaThreadState_Listening,
-    /** The thread is sleeping before starting a measurement. */
+    /** The thread is sleeping before starting a measurement.
+     * Previous state: Listening, Measuring
+     * Next state:     Measuring, Butchered, Terminating
+     * @remarks The thread won't enter this state on its own, it is put into this
+     *          state by the GIP timer, the CPU online callback and by the
+     *          SUP_IOCTL_TSC_DELTA_MEASURE code. */
     kTscDeltaThreadState_WaitAndMeasure,
-    /** The thread is currently servicing a measurement request. */
+    /** The thread is currently servicing a measurement request.
+     * Previous state: WaitAndMeasure
+     * Next state:     Listening, WaitAndMeasure, Terminate */
     kTscDeltaThreadState_Measuring,
-    /** The thread is terminating. */
+    /** The thread is terminating.
+     * @remarks The thread won't enter this state on its own, is put into this state
+     *          by supdrvTscDeltaTerm. */
     kTscDeltaThreadState_Terminating,
-    /** The thread is butchered due to an unexpected error. */
+    /** The thread is butchered due to an unexpected error.
+     * Previous State: Creating, Listening, WaitAndMeasure  */
     kTscDeltaThreadState_Butchered,
-    /** The thread is destroyed. */
+    /** The thread is destroyed (final).
+     * Previous state: Terminating */
     kTscDeltaThreadState_Destroyed,
     /** The usual 32-bit blowup hack. */
     kTscDeltaThreadState_32BitHack = 0x7fffffff
-} SUPDRVTSCDELTATHREADSTATE, *PSUPDRVTSCDELTATHREADSTATE;
-#endif
+} SUPDRVTSCDELTATHREADSTATE;
+#endif /* SUPDRV_USE_TSC_DELTA_THREAD */
 
 /**
@@ -650 +664 @@
     uint32_t                        u32SystemTimerGranularityGrant;
     /** The CPU id of the GIP master.
-     * This CPU is responsible for the updating the common GIP data. */
+     * This CPU is responsible for the updating the common GIP data and it is
+     * the one used to calculate TSC deltas relative to.
+     * (The initial master will have a 0 zero value, but it it goes offline the
+     * new master may have a non-zero value.) */
     RTCPUID volatile                idGipMaster;