Changeset 86121 in vbox for trunk/src/VBox/VMM/VMMR3

Timestamp:

Sep 14, 2020 4:56:09 PM (5 years ago)

Author:

vboxsync

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

140355

Message:

VMM/GIM: Fix handling KVM system-time struct. to work from both ring-0 and ring-3 (earlier it relied on MSR writes being handled in ring-0).

File:

• trunk/src/VBox/VMM/VMMR3/GIMKvm.cpp (modified) (4 diffs)

trunk/src/VBox/VMM/VMMR3/GIMKvm.cpp

@@ -86 +86 @@
 
 /**
+ * Updates the KVM VCPU system-time structure in guest memory.
+ *
+ * @returns VBox status code.
+ * @param   pVM     The cross context VM structure.
+ * @param   pVCpu   The cross context virtual CPU structure.
+ *
+ * @remarks This must be called after the system time MSR value has been updated.
+ */
+static int gimR3KvmUpdateSystemTime(PVM pVM, PVMCPU pVCpu)
+{
+    PGIMKVM    pKvm    = &pVM->gim.s.u.Kvm;
+    PGIMKVMCPU pKvmCpu = &pVCpu->gim.s.u.KvmCpu;
+
+    /*
+     * Validate the MSR has the enable bit and the guest's system time struct. address.
+     */
+    MSR_GIM_KVM_SYSTEM_TIME_IS_ENABLED(pKvmCpu->u64SystemTimeMsr);
+    if (!PGMPhysIsGCPhysNormal(pVM, pKvmCpu->GCPhysSystemTime))
+    {
+        LogRel(("GIM: KVM: VCPU%3d: Invalid physical addr requested for mapping system-time struct. GCPhysSystemTime=%#RGp\n",
+               pVCpu->idCpu, pKvmCpu->GCPhysSystemTime));
+        return VERR_GIM_OPERATION_FAILED;
+    }
+
+    VMSTATE const enmVMState = pVM->enmVMState;
+    bool const fRunning = VMSTATE_IS_RUNNING(enmVMState);
+    Assert(!(pKvmCpu->u32SystemTimeVersion & UINT32_C(1)));
+
+    /*
+     * Construct a system-time struct.
+     */
+    GIMKVMSYSTEMTIME SystemTime;
+    RT_ZERO(SystemTime);
+    SystemTime.u32Version  = pKvmCpu->u32SystemTimeVersion + !!fRunning;
+    SystemTime.u64NanoTS   = pKvmCpu->uVirtNanoTS;
+    SystemTime.u64Tsc      = pKvmCpu->uTsc;
+    SystemTime.fFlags      = pKvmCpu->fSystemTimeFlags | GIM_KVM_SYSTEM_TIME_FLAGS_TSC_STABLE;
+
+    /*
+     * How the guest calculates the system time (nanoseconds):
+     *
+     * tsc = rdtsc - SysTime.u64Tsc
+     * if (SysTime.i8TscShift >= 0)
+     *     tsc <<= i8TscShift;
+     * else
+     *     tsc >>= -i8TscShift;
+     * time = ((tsc * SysTime.u32TscScale) >> 32) + SysTime.u64NanoTS
+     */
+    uint64_t u64TscFreq   = pKvm->cTscTicksPerSecond;
+    SystemTime.i8TscShift = 0;
+    while (u64TscFreq > 2 * RT_NS_1SEC_64)
+    {
+        u64TscFreq >>= 1;
+        SystemTime.i8TscShift--;
+    }
+    uint32_t uTscFreqLo = (uint32_t)u64TscFreq;
+    while (uTscFreqLo <= RT_NS_1SEC)
+    {
+        uTscFreqLo <<= 1;
+        SystemTime.i8TscShift++;
+    }
+    SystemTime.u32TscScale = ASMDivU64ByU32RetU32(RT_NS_1SEC_64 << 32, uTscFreqLo);
+
+    /*
+     * For informational purposes, back-calculate the exact TSC frequency the guest will see.
+     * Note that the frequency is in kHz, not Hz, since that's what Linux uses.
+     */
+    uint64_t uTscKHz = (RT_NS_1MS_64 << 32) / SystemTime.u32TscScale;
+    if (SystemTime.i8TscShift < 0)
+        uTscKHz <<= -SystemTime.i8TscShift;
+    else
+        uTscKHz >>=  SystemTime.i8TscShift;
+
+    /*
+     * Update guest memory with the system-time struct.
+     *
+     * We update the struct with an incremented, odd version field to indicate to the guest
+     * that the memory is being updated concurrently by the host and it should discard any
+     * data from this struct when it reads an odd version.
+     *
+     * When the VM is not running, we don't need to do this two step update for obvious
+     * reasons and so we skip it.
+     */
+    if (fRunning)
+        Assert(SystemTime.u32Version & UINT32_C(1));
+    else
+        Assert(!(SystemTime.u32Version & UINT32_C(1)));
+
+    int rc = PGMPhysSimpleWriteGCPhys(pVM, pKvmCpu->GCPhysSystemTime, &SystemTime, sizeof(GIMKVMSYSTEMTIME));
+    if (RT_SUCCESS(rc))
+    {
+        LogRel(("GIM: KVM: VCPU%3d: Enabled system-time struct. at %#RGp - u32TscScale=%#RX32 i8TscShift=%d uVersion=%#RU32 "
+                "fFlags=%#x uTsc=%#RX64 uVirtNanoTS=%#RX64 TscKHz=%RU64\n", pVCpu->idCpu, pKvmCpu->GCPhysSystemTime,
+                SystemTime.u32TscScale, SystemTime.i8TscShift, SystemTime.u32Version + !!fRunning, SystemTime.fFlags,
+                pKvmCpu->uTsc, pKvmCpu->uVirtNanoTS, uTscKHz));
+        TMR3CpuTickParavirtEnable(pVM);
+    }
+    else
+    {
+        LogRel(("GIM: KVM: VCPU%3d: Failed to write system-time struct. at %#RGp. rc=%Rrc\n", pVCpu->idCpu,
+                pKvmCpu->GCPhysSystemTime, rc));
+    }
+
+    if (fRunning)
+    {
+        ++SystemTime.u32Version;
+        Assert(!(SystemTime.u32Version & UINT32_C(1)));
+        rc = PGMPhysSimpleWriteGCPhys(pVM, pKvmCpu->GCPhysSystemTime + RT_UOFFSETOF(GIMKVMSYSTEMTIME, u32Version),
+                                          &SystemTime.u32Version, sizeof(SystemTime.u32Version));
+        if (RT_FAILURE(rc))
+        {
+            LogRel(("GIM: KVM: VCPU%3d: Failed to write system-time struct. while updating version field at %#RGp. rc=%Rrc\n",
+                    pVCpu->idCpu, pKvmCpu->GCPhysSystemTime, rc));
+            return rc;
+        }
+
+        /* Update the version so our next write will start with an even value. */
+        pKvmCpu->u32SystemTimeVersion += 2;
+    }
+
+    return rc;
+}
+
+
+/**
  * Initializes the KVM GIM provider.
  *
@@ -340 +465 @@
             Assert(!TMVirtualIsTicking(pVM));       /* paranoia. */
             Assert(!TMCpuTickIsTicking(pVCpu));
-            gimR3KvmEnableSystemTime(pVM, pVCpu);
+            gimR3KvmUpdateSystemTime(pVM, pVCpu);
         }
     }
@@ -352 +477 @@
 
     return VINF_SUCCESS;
-}
-
-
-/**
- * Enables the KVM VCPU system-time structure.
- *
- * @returns VBox status code.
- * @param   pVM                The cross context VM structure.
- * @param   pVCpu              The cross context virtual CPU structure.
- *
- * @remarks Don't do any release assertions here, these can be triggered by
- *          guest R0 code.
- */
-VMMR3_INT_DECL(int) gimR3KvmEnableSystemTime(PVM pVM, PVMCPU pVCpu)
-{
-    PGIMKVM    pKvm    = &pVM->gim.s.u.Kvm;
-    PGIMKVMCPU pKvmCpu = &pVCpu->gim.s.u.KvmCpu;
-
-    /*
-     * Validate the mapping address first.
-     */
-    if (!PGMPhysIsGCPhysNormal(pVM, pKvmCpu->GCPhysSystemTime))
-    {
-        LogRel(("GIM: KVM: VCPU%3d: Invalid physical addr requested for mapping system-time struct. GCPhysSystemTime=%#RGp\n",
-               pVCpu->idCpu, pKvmCpu->GCPhysSystemTime));
-        return VERR_GIM_OPERATION_FAILED;
-    }
-
-    /*
-     * Construct the system-time struct.
-     */
-    GIMKVMSYSTEMTIME SystemTime;
-    RT_ZERO(SystemTime);
-    SystemTime.u32Version  = pKvmCpu->u32SystemTimeVersion;
-    SystemTime.u64NanoTS   = pKvmCpu->uVirtNanoTS;
-    SystemTime.u64Tsc      = pKvmCpu->uTsc;
-    SystemTime.fFlags      = pKvmCpu->fSystemTimeFlags | GIM_KVM_SYSTEM_TIME_FLAGS_TSC_STABLE;
-
-    /*
-     * How the guest calculates the system time (nanoseconds):
-     *
-     * tsc = rdtsc - SysTime.u64Tsc
-     * if (SysTime.i8TscShift >= 0)
-     *     tsc <<= i8TscShift;
-     * else
-     *     tsc >>= -i8TscShift;
-     * time = ((tsc * SysTime.u32TscScale) >> 32) + SysTime.u64NanoTS
-     */
-    uint64_t u64TscFreq   = pKvm->cTscTicksPerSecond;
-    SystemTime.i8TscShift = 0;
-    while (u64TscFreq > 2 * RT_NS_1SEC_64)
-    {
-        u64TscFreq >>= 1;
-        SystemTime.i8TscShift--;
-    }
-    uint32_t uTscFreqLo = (uint32_t)u64TscFreq;
-    while (uTscFreqLo <= RT_NS_1SEC)
-    {
-        uTscFreqLo <<= 1;
-        SystemTime.i8TscShift++;
-    }
-    SystemTime.u32TscScale = ASMDivU64ByU32RetU32(RT_NS_1SEC_64 << 32, uTscFreqLo);
-
-    /* For informational purposes, back-calculate the exact TSC frequency the guest will see.
-     * Note that the frequency is in kHz, not Hz, since that's what Linux uses.
-     */
-    uint64_t uTscKHz = (RT_NS_1MS_64 << 32) / SystemTime.u32TscScale;
-    if (SystemTime.i8TscShift < 0)
-        uTscKHz <<= -SystemTime.i8TscShift;
-    else
-        uTscKHz >>=  SystemTime.i8TscShift;
-
-    /*
-     * Update guest memory with the system-time struct. Technically we are cheating
-     * by only writing the struct once with the version incremented by two; in reality,
-     * the system-time struct is enabled once for each CPU at boot and not concurrently
-     * read by other VCPUs at the same time.
-     */
-    Assert(!(SystemTime.u32Version & UINT32_C(1)));
-    int rc = PGMPhysSimpleWriteGCPhys(pVM, pKvmCpu->GCPhysSystemTime, &SystemTime, sizeof(GIMKVMSYSTEMTIME));
-    if (RT_SUCCESS(rc))
-    {
-        LogRel(("GIM: KVM: VCPU%3d: Enabled system-time struct. at %#RGp - u32TscScale=%#RX32 i8TscShift=%d uVersion=%#RU32 "
-                "fFlags=%#x uTsc=%#RX64 uVirtNanoTS=%#RX64 TscKHz=%RU64\n", pVCpu->idCpu, pKvmCpu->GCPhysSystemTime, SystemTime.u32TscScale,
-                SystemTime.i8TscShift, SystemTime.u32Version, SystemTime.fFlags, pKvmCpu->uTsc, pKvmCpu->uVirtNanoTS, uTscKHz));
-        TMR3CpuTickParavirtEnable(pVM);
-    }
-    else
-        LogRel(("GIM: KVM: VCPU%3d: Failed to write system-time struct. at %#RGp. rc=%Rrc\n",
-                pVCpu->idCpu, pKvmCpu->GCPhysSystemTime, rc));
-
-    return rc;
 }
 
@@ -546 +579 @@
 }
 
+
+/**
+ * Enables the KVM system time structure.
+ *
+ * This can be done concurrently because the guest memory being updated is per-VCPU
+ * and the struct even has a "version" field which needs to be incremented
+ * before/after altering guest memory to allow concurrent updates from the host.
+ * Hence this is not being done in an EMT rendezvous. It -is- done in ring-3 since
+ * we call into ring-3 only TM code in the end.
+ *
+ * @returns VBox status code.
+ * @param   pVM             The cross context VM structure.
+ * @param   pVCpu           The cross context virtual CPU structure.
+ * @param   uMsrSystemTime  The system time MSR value being written.
+ */
+VMMR3_INT_DECL(int) gimR3KvmEnableSystemTime(PVMCC pVM, PVMCPUCC pVCpu, uint64_t uMsrSystemTime)
+{
+    Assert(uMsrSystemTime & MSR_GIM_KVM_SYSTEM_TIME_ENABLE_BIT);
+    PGIMKVM    pKvm    = &pVM->gim.s.u.Kvm;
+    PGIMKVMCPU pKvmCpu = &pVCpu->gim.s.u.KvmCpu;
+
+    /*
+     * Update the system-time struct.
+     * The system-time structs are usually placed at a different guest address for each VCPU.
+     *
+     * The rendezvous ensures that we don't have other VCPUs that might potentially be
+     * reading the guest memory which we're about to update.
+     */
+    pKvmCpu->uTsc                  = TMCpuTickGetNoCheck(pVCpu);
+    pKvmCpu->uVirtNanoTS           = ASMMultU64ByU32DivByU32(pKvmCpu->uTsc, RT_NS_1SEC, pKvm->cTscTicksPerSecond);
+    pKvmCpu->u64SystemTimeMsr      = uMsrSystemTime;
+    pKvmCpu->GCPhysSystemTime      = MSR_GIM_KVM_SYSTEM_TIME_GUEST_GPA(uMsrSystemTime);
+
+    int rc = gimR3KvmUpdateSystemTime(pVM, pVCpu);
+    if (RT_FAILURE(rc))
+    {
+        pKvmCpu->u64SystemTimeMsr = 0;
+        /* We shouldn't throw a #GP(0) here for buggy guests (neither does KVM apparently), see @bugref{8627}. */
+    }
+
+    return rc;
+}
+