Factor out TimerKHz and ShiftTSC calculation to tsctimer_configure().
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/timer.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/hw/timer.c b/src/hw/timer.c index bdcb3bfca211..ce3d63cd75f6 100644 --- a/src/hw/timer.c +++ b/src/hw/timer.c @@ -58,6 +58,18 @@ u8 ShiftTSC VARFSEG; * Internal timer setup ****************************************************************/
+static void +tsctimer_configure(u64 t) +{ + while (t >= (1<<24)) { + ShiftTSC++; + t = (t + 1) >> 1; + } + TimerKHz = DIV_ROUND_UP((u32)t, 1000 * PMTIMER_TO_PIT); + TimerPort = 0; + dprintf(1, "CPU Mhz=%u\n", (TimerKHz << ShiftTSC) / 1000); +} + #define CALIBRATE_COUNT 0x800 // Approx 1.7ms
// Calibrate the CPU time-stamp-counter @@ -87,14 +99,7 @@ tsctimer_setup(void) dprintf(6, "tsc calibrate start=%u end=%u diff=%u\n" , (u32)start, (u32)end, (u32)diff); u64 t = DIV_ROUND_UP(diff * PMTIMER_HZ, CALIBRATE_COUNT); - while (t >= (1<<24)) { - ShiftTSC++; - t = (t + 1) >> 1; - } - TimerKHz = DIV_ROUND_UP((u32)t, 1000 * PMTIMER_TO_PIT); - TimerPort = 0; - - dprintf(1, "CPU Mhz=%u\n", (TimerKHz << ShiftTSC) / 1000); + tsctimer_configure(t); }
// Setup internal timers.
Add function to set tsc frequency directly, without calibration. Also tweak timer setup functions a bit: skip setup in case TimerPort has not the default value any more, i.e. another timer has been setup already.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/util.h | 1 + src/hw/timer.c | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/src/util.h b/src/util.h index d96db788d1b8..0a75a6e96c95 100644 --- a/src/util.h +++ b/src/util.h @@ -171,6 +171,7 @@ void sdcard_setup(void); // hw/timer.c void timer_setup(void); void pmtimer_setup(u16 ioport); +void tsctimer_setfreq(u32 khz); u32 timer_calc(u32 msecs); u32 timer_calc_usec(u32 usecs); int timer_check(u32 end); diff --git a/src/hw/timer.c b/src/hw/timer.c index ce3d63cd75f6..a13f363b45a6 100644 --- a/src/hw/timer.c +++ b/src/hw/timer.c @@ -106,8 +106,10 @@ tsctimer_setup(void) void timer_setup(void) { - if (!CONFIG_TSC_TIMER || (CONFIG_PMTIMER && TimerPort != PORT_PIT_COUNTER0)) + if (!CONFIG_TSC_TIMER) return; + if (TimerPort != PORT_PIT_COUNTER0) + return; // have timer already
// Check if CPU has a timestamp counter u32 eax, ebx, ecx, edx, cpuid_features = 0; @@ -118,11 +120,25 @@ timer_setup(void) tsctimer_setup(); }
+void +tsctimer_setfreq(u32 khz) +{ + if (!CONFIG_TSC_TIMER) + return; + if (TimerPort != PORT_PIT_COUNTER0) + return; // have timer already + + tsctimer_configure((u64)khz * PMTIMER_TO_PIT * 1000); +} + void pmtimer_setup(u16 ioport) { if (!CONFIG_PMTIMER) return; + if (TimerPort != PORT_PIT_COUNTER0) + return; // have timer already + dprintf(1, "Using pmtimer, ioport 0x%x\n", ioport); TimerPort = ioport; TimerKHz = DIV_ROUND_UP(PMTIMER_HZ, 1000);
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/fw/paravirt.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index 4fcd8f570673..8b463af96c3e 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -67,6 +67,11 @@ static void kvm_detect(void) if (strcmp(signature, "KVMKVMKVM") == 0) { dprintf(1, "Running on KVM\n"); PlatformRunningOn |= PF_KVM; + if (eax >= KVM_CPUID_SIGNATURE + 0x10) { + cpuid(KVM_CPUID_SIGNATURE + 0x10, &eax, &ebx, &ecx, &edx); + dprintf(1, "kvm: have invtsc, freq %u kHz\n", eax); + tsctimer_setfreq(eax); + } } }
On 3/6/20 4:44 PM, Gerd Hoffmann wrote:
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
src/fw/paravirt.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index 4fcd8f570673..8b463af96c3e 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -67,6 +67,11 @@ static void kvm_detect(void) if (strcmp(signature, "KVMKVMKVM") == 0) { dprintf(1, "Running on KVM\n"); PlatformRunningOn |= PF_KVM;
if (eax >= KVM_CPUID_SIGNATURE + 0x10) {cpuid(KVM_CPUID_SIGNATURE + 0x10, &eax, &ebx, &ecx, &edx);dprintf(1, "kvm: have invtsc, freq %u kHz\n", eax);
What is "invtsc"? "tsc" alone maybe?
Reviewed-by: Philippe Mathieu-Daudé philmd@redhat.com
tsctimer_setfreq(eax);} }
On Fri, Mar 06, 2020 at 04:44:16PM +0100, Gerd Hoffmann wrote:
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
src/fw/paravirt.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index 4fcd8f570673..8b463af96c3e 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -67,6 +67,11 @@ static void kvm_detect(void) if (strcmp(signature, "KVMKVMKVM") == 0) { dprintf(1, "Running on KVM\n"); PlatformRunningOn |= PF_KVM;
if (eax >= KVM_CPUID_SIGNATURE + 0x10) {cpuid(KVM_CPUID_SIGNATURE + 0x10, &eax, &ebx, &ecx, &edx);dprintf(1, "kvm: have invtsc, freq %u kHz\n", eax);tsctimer_setfreq(eax);}}
That's odd that a khz value is being passed as eax. Is this a recent change?
-Kevin
On Fri, Mar 06, 2020 at 02:14:01PM -0500, Kevin O'Connor wrote:
On Fri, Mar 06, 2020 at 04:44:16PM +0100, Gerd Hoffmann wrote:
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
src/fw/paravirt.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index 4fcd8f570673..8b463af96c3e 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -67,6 +67,11 @@ static void kvm_detect(void) if (strcmp(signature, "KVMKVMKVM") == 0) { dprintf(1, "Running on KVM\n"); PlatformRunningOn |= PF_KVM;
if (eax >= KVM_CPUID_SIGNATURE + 0x10) {cpuid(KVM_CPUID_SIGNATURE + 0x10, &eax, &ebx, &ecx, &edx);dprintf(1, "kvm: have invtsc, freq %u kHz\n", eax);tsctimer_setfreq(eax);}}
That's odd that a khz value is being passed as eax. Is this a recent change?
KVM paravirtual extension. Exists for years. I think it's off by default (didn't check all cpu models though). You'll get it with "qemu -cpu $model,invtsc=on".
When available it is possible to use the tsc as reliable clock source because you don't need calibration which can be way off on a loaded host.
I'm looking for a clock source which works in case pmtimer is not available. Maybe I should just go for kvmclock instead ...
cheers, Gerd
On 3/6/20 4:44 PM, Gerd Hoffmann wrote:
Factor out TimerKHz and ShiftTSC calculation to tsctimer_configure().
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
src/hw/timer.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/hw/timer.c b/src/hw/timer.c index bdcb3bfca211..ce3d63cd75f6 100644 --- a/src/hw/timer.c +++ b/src/hw/timer.c @@ -58,6 +58,18 @@ u8 ShiftTSC VARFSEG;
- Internal timer setup
****************************************************************/
+static void +tsctimer_configure(u64 t) +{
- while (t >= (1<<24)) {
ShiftTSC++;t = (t + 1) >> 1;- }
- TimerKHz = DIV_ROUND_UP((u32)t, 1000 * PMTIMER_TO_PIT);
- TimerPort = 0;
- dprintf(1, "CPU Mhz=%u\n", (TimerKHz << ShiftTSC) / 1000);
+}
#define CALIBRATE_COUNT 0x800 // Approx 1.7ms
// Calibrate the CPU time-stamp-counter
@@ -87,14 +99,7 @@ tsctimer_setup(void) dprintf(6, "tsc calibrate start=%u end=%u diff=%u\n" , (u32)start, (u32)end, (u32)diff); u64 t = DIV_ROUND_UP(diff * PMTIMER_HZ, CALIBRATE_COUNT);
- while (t >= (1<<24)) {
ShiftTSC++;t = (t + 1) >> 1;- }
- TimerKHz = DIV_ROUND_UP((u32)t, 1000 * PMTIMER_TO_PIT);
- TimerPort = 0;
- dprintf(1, "CPU Mhz=%u\n", (TimerKHz << ShiftTSC) / 1000);
tsctimer_configure(t); }
// Setup internal timers.
Reviewed-by: Philippe Mathieu-Daudé philmd@redhat.com
On Fri, Mar 06, 2020 at 04:44:14PM +0100, Gerd Hoffmann wrote:
Factor out TimerKHz and ShiftTSC calculation to tsctimer_configure().
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
src/hw/timer.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/hw/timer.c b/src/hw/timer.c index bdcb3bfca211..ce3d63cd75f6 100644 --- a/src/hw/timer.c +++ b/src/hw/timer.c @@ -58,6 +58,18 @@ u8 ShiftTSC VARFSEG;
- Internal timer setup
****************************************************************/
+static void +tsctimer_configure(u64 t) +{
- while (t >= (1<<24)) {
ShiftTSC++;t = (t + 1) >> 1;- }
- TimerKHz = DIV_ROUND_UP((u32)t, 1000 * PMTIMER_TO_PIT);
- TimerPort = 0;
- dprintf(1, "CPU Mhz=%u\n", (TimerKHz << ShiftTSC) / 1000);
+}
#define CALIBRATE_COUNT 0x800 // Approx 1.7ms
// Calibrate the CPU time-stamp-counter @@ -87,14 +99,7 @@ tsctimer_setup(void) dprintf(6, "tsc calibrate start=%u end=%u diff=%u\n" , (u32)start, (u32)end, (u32)diff); u64 t = DIV_ROUND_UP(diff * PMTIMER_HZ, CALIBRATE_COUNT);
- while (t >= (1<<24)) {
ShiftTSC++;t = (t + 1) >> 1;- }
- TimerKHz = DIV_ROUND_UP((u32)t, 1000 * PMTIMER_TO_PIT);
- TimerPort = 0;
- dprintf(1, "CPU Mhz=%u\n", (TimerKHz << ShiftTSC) / 1000);
- tsctimer_configure(t);
}
The PMTIMER_TO_PIT in DIV_ROUND_UP((u32)t, 1000 * PMTIMER_TO_PIT) is directly tied to the measurement interval performed in tsctimer_setup(). Specifically, the code calculates the number of tsc ticks in n PIT intervals and then calculates "tsc_hz=pit_hz*elapsed_tsc/n". The complicated math is to avoid 64bit divides. I recommend against moving that low-level math to another function, as I fear the relationship would then be harder to understand.
-Kevin
-
Gerd Hoffmann -
Kevin O'Connor -
Philippe Mathieu-Daudé