Changeset 83372 in vbox for trunk

Timestamp:

Mar 23, 2020 2:52:24 PM (5 years ago)

Author:

vboxsync

Message:

GIM/KVM: Update system-time struct on demand. Fixes guests that rely on the struct being frequently updated, e.g. SLES 10 SP4 (see bugref:7270).

Location:

trunk/src/VBox/VMM

Files:

• VMMAll/GIMAllKvm.cpp (modified) (2 diffs)
• VMMR0/GIMR0Kvm.cpp (modified) (2 diffs)
• VMMR3/GIMKvm.cpp (modified) (1 diff)

trunk/src/VBox/VMM/VMMAll/GIMAllKvm.cpp

@@ -240 +240 @@
             bool fEnable = RT_BOOL(uRawValue & MSR_GIM_KVM_SYSTEM_TIME_ENABLE_BIT);
 #ifndef IN_RING3
-            NOREF(fEnable); NOREF(pKvmCpu);
             gimR0KvmUpdateSystemTime(pVM, pVCpu);
-            return VINF_CPUM_R3_MSR_WRITE;
+            if (   fEnable
+                && MSR_GIM_KVM_SYSTEM_TIME_IS_ENABLED(pKvmCpu->u64SystemTimeMsr)
+                && MSR_GIM_KVM_SYSTEM_TIME_GUEST_GPA(uRawValue) == pKvmCpu->GCPhysSystemTime)
+            {
+                /*
+                 * Guest is asking for an update of the system-time struct. We only
+                 * need to update the TSC and system time fields, the rest stays unchanged.
+                 * We have to write the version twice in case another VCPU is reading the
+                 * system-time struct concurrently. Making the version number odd indicates
+                 * that the data is being updated.
+                 */
+                GIMKVMSYSTEMTIME SystemTime;
+                SystemTime.u32Version  = ++pKvmCpu->u32SystemTimeVersion;
+                SystemTime.u32Padding0 = 0;
+                SystemTime.u64NanoTS   = pKvmCpu->uVirtNanoTS;
+                SystemTime.u64Tsc      = pKvmCpu->uTsc;
+                AssertCompile(RT_UOFFSETOF(GIMKVMSYSTEMTIME, u32TscScale) == 3 * sizeof(uint64_t));
+                int rc2 = PGMPhysSimpleWriteGCPhys(pVM, pKvmCpu->GCPhysSystemTime, &SystemTime, RT_UOFFSETOF(GIMKVMSYSTEMTIME, u32TscScale));
+                if (RT_FAILURE(rc2))
+                    return rc2;
+
+                /* Make the version number even again to indicate the data is consistent. */
+                ++pKvmCpu->u32SystemTimeVersion;
+                rc2 = PGMPhysSimpleWriteGCPhys(pVM, pKvmCpu->GCPhysSystemTime, &pKvmCpu->u32SystemTimeVersion, sizeof(pKvmCpu->u32SystemTimeVersion));
+                if (RT_FAILURE(rc2))
+                    return rc2;
+                return VINF_SUCCESS;
+            }
+            else
+                return VINF_CPUM_R3_MSR_WRITE;
 #else /* IN_RING3 */
             if (!fEnable)
@@ -249 +277 @@
                 pKvmCpu->u64SystemTimeMsr = uRawValue;
                 return VINF_SUCCESS;
-            }
-
-            /* Is the system-time struct. already enabled? If so, get flags that need preserving. */
-            GIMKVMSYSTEMTIME SystemTime;
-            RT_ZERO(SystemTime);
-            if (   MSR_GIM_KVM_SYSTEM_TIME_IS_ENABLED(pKvmCpu->u64SystemTimeMsr)
-                && MSR_GIM_KVM_SYSTEM_TIME_GUEST_GPA(uRawValue) == pKvmCpu->GCPhysSystemTime)
-            {
-                int rc2 = PGMPhysSimpleReadGCPhys(pVM, &SystemTime, pKvmCpu->GCPhysSystemTime, sizeof(GIMKVMSYSTEMTIME));
-                if (RT_SUCCESS(rc2))
-                    pKvmCpu->fSystemTimeFlags = (SystemTime.fFlags & GIM_KVM_SYSTEM_TIME_FLAGS_GUEST_PAUSED);
             }
 

trunk/src/VBox/VMM/VMMR0/GIMR0Kvm.cpp

@@ -56 +56 @@
     uint64_t uVirtNanoTS;
     RTCCUINTREG fEFlags = ASMIntDisableFlags();
-    uTsc        = TMCpuTickGetNoCheck(pVCpu) | UINT64_C(1);
-    uVirtNanoTS = TMVirtualGetNoCheck(pVM)   | UINT64_C(1);
+    uTsc        = TMCpuTickGetNoCheck(pVCpu);
+    uVirtNanoTS = TMVirtualGetNoCheck(pVM);
     ASMSetFlags(fEFlags);
 
@@ -68 +68 @@
     {
         PGIMKVMCPU pKvmCpu = &VMCC_GET_CPU(pVM, idCpu)->gim.s.u.KvmCpu;
-        if (   !pKvmCpu->uTsc
-            && !pKvmCpu->uVirtNanoTS)
-        {
-            pKvmCpu->uTsc        = uTsc;
-            pKvmCpu->uVirtNanoTS = uVirtNanoTS;
-        }
+        pKvmCpu->uTsc        = uTsc;
+        pKvmCpu->uVirtNanoTS = uVirtNanoTS;
     }
     RTSpinlockRelease(pKvm->hSpinlockR0);

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

@@ -416 +416 @@
 
     /*
-     * Update guest memory with the system-time struct.
+     * 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 by the boot CPU and not concurrently read
+     * by other VCPUs at the same time.
      */
     Assert(!(SystemTime.u32Version & UINT32_C(1)));