[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v4 17/30] KVM: x86: Simplify and comment kvm_get_time_scale()



From: David Woodhouse <dwmw@xxxxxxxxxxxx>

The kvm_get_time_scale() function was entirely opaque. Add comments
explaining what it does: compute a fixed-point multiplier and shift for
converting TSC ticks to nanoseconds via pvclock_scale_delta().

Rename the local variables from the cryptic tps64/tps32/scaled64 to
base_hz_u64/base32/scaled_hz_u64 to make the code self-documenting.
The "tps32" name stood for "Ticks Per Second" but was misleading since
it held the shifted base frequency, not a tick count.

No functional change.

Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
Reviewed-by: Paul Durrant <paul@xxxxxxx>
---
 arch/x86/kvm/x86.c | 55 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8e4993ef4f6b..980fc22ee05b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2472,32 +2472,57 @@ static uint32_t div_frac(uint32_t dividend, uint32_t 
divisor)
        return dividend;
 }
 
-static void kvm_get_time_scale(uint64_t scaled_hz, uint64_t base_hz,
+static void kvm_get_time_scale(u64 scaled_hz, u64 base_hz,
                               s8 *pshift, u32 *pmultiplier)
 {
-       uint64_t scaled64;
-       int32_t  shift = 0;
-       uint64_t tps64;
-       uint32_t tps32;
+       u64 scaled_hz_u64 = scaled_hz;
+       s32 shift = 0;
+       u64 base_hz_u64;
+       u32 base32;
 
-       tps64 = base_hz;
-       scaled64 = scaled_hz;
-       while (tps64 > scaled64*2 || tps64 & 0xffffffff00000000ULL) {
-               tps64 >>= 1;
+       /*
+        * This function calculates a fixed-point multiplier and shift such
+        * that:
+        *   time_ns = (tsc_cycles << shift) * multiplier >> 32
+        *
+        * Where tsc_cycles tick at base_hz, and time_ns should count at
+        * scaled_hz (typically NSEC_PER_SEC for a TSC→nanoseconds conversion).
+        *
+        * The multiplier is: (scaled_hz << 32) / base_hz, adjusted by shift
+        * to keep everything in range.
+        */
+
+       base_hz_u64 = base_hz;
+
+       /*
+        * Start by shifting base_hz right until it fits in 32 bits, and
+        * is lower than double the target rate. This introduces a negative
+        * shift value which would result in pvclock_scale_delta() shifting
+        * the actual tick count right before performing the multiplication.
+        */
+       while (base_hz_u64 > scaled_hz_u64 * 2 || base_hz_u64 >> 32) {
+               base_hz_u64 >>= 1;
                shift--;
        }
 
-       tps32 = (uint32_t)tps64;
-       while (tps32 <= scaled64 || scaled64 & 0xffffffff00000000ULL) {
-               if (scaled64 & 0xffffffff00000000ULL || tps32 & 0x80000000)
-                       scaled64 >>= 1;
+       /* Now the shifted base_hz fits in 32 bits. */
+       base32 = (u32)base_hz_u64;
+
+       /*
+        * Next, shift scaled_hz right until it fits in 32 bits, and ensure
+        * that the shifted base_hz is not larger (so that the result of the
+        * final division also fits in 32 bits).
+        */
+       while (base32 <= scaled_hz_u64 || scaled_hz_u64 >> 32) {
+               if (scaled_hz_u64 >> 32 || base32 & BIT(31))
+                       scaled_hz_u64 >>= 1;
                else
-                       tps32 <<= 1;
+                       base32 <<= 1;
                shift++;
        }
 
        *pshift = shift;
-       *pmultiplier = div_frac(scaled64, tps32);
+       *pmultiplier = div_frac(scaled_hz_u64, base32);
 }
 
 #ifdef CONFIG_X86_64
-- 
2.51.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.