tsc: improve initialization

include/villas/task.h

@@ -51,8 +51,6 @@ struct task {
 	int clock;			/**< CLOCK_{MONOTONIC,REALTIME} */
 
 #if PERIODIC_TASK_IMPL == RDTSC		/* We use cycle counts in RDTSC mode */
-	uint64_t frequency;
-
 	uint64_t period;
 	uint64_t next;
 #else
@@ -62,6 +60,8 @@ struct task {
 
 #if PERIODIC_TASK_IMPL == TIMERFD
 	int fd;				/**< The timerfd_create(2) file descriptior. */
+#elif PERIODIC_TASK_IMPL == RDTSC
+	struct rdtsc tsc;		/**< Initialized by tsc_init(). */
 #endif
 };
 

include/villas/tsc.h

@@ -0,0 +1,95 @@
+/** Measure time and sleep with IA-32 time-stamp counter.
+ *
+ * @file
+ * @author Steffen Vogel <stvogel@eonerc.rwth-aachen.de>
+ * @copyright 2018, Institute for Automation of Complex Power Systems, EONERC
+ * @license GNU General Public License (version 3)
+ *
+ * VILLASnode
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *********************************************************************************/
+
+#pragma once
+
+#include <stdbool.h>
+#include <cpuid.h>
+#include <inttypes.h>
+
+#include <villas/kernel/kernel.h>
+
+#ifdef __APPLE__
+  #include <sys/types.h>
+  #include <sys/sysctl.h>
+#endif
+
+#ifndef bit_TSC
+  #define bit_TSC (1 << 4)
+#endif
+
+#define bit_TSC_INVARIANT	(1 << 8)
+#define bit_RDTSCP		(1 << 27)
+
+#if !(__x86_64__ || __i386__)
+  #error this header is for x86 only
+#endif
+
+struct tsc {
+	uint64_t frequency;
+
+	bool rdtscp_supported;
+	bool is_invariant;
+};
+
+/** Get CPU timestep counter */
+__attribute__((unused,always_inline))
+static inline uint64_t rdtscp()
+{
+	uint64_t tsc;
+
+	__asm__ __volatile__(
+		 "rdtscp;"
+		 "shl $32, %%rdx;"
+		 "or %%rdx,%%rax"
+		: "=a" (tsc)
+		:
+		: "%rcx", "%rdx", "memory"
+	);
+
+	return tsc;
+}
+
+__attribute__((unused,always_inline))
+static inline uint64_t rdtsc()
+{
+	uint64_t tsc;
+
+	__asm__ __volatile__(
+		 "lfence;"
+		 "rdtsc;"
+		 "shl $32, %%rdx;"
+		 "or %%rdx,%%rax"
+		: "=a" (tsc)
+		:
+		: "%rcx", "%rdx", "memory"
+	);
+
+	return tsc;
+}
+
+int tsc_init(struct tsc *t);
+
+uint64_t tsc_rate_to_cyles(struct tsc *t, double rate);
+
+uint64_t tsc_now(struct tsc *t);

lib/task.c

@@ -31,7 +31,7 @@
 #if PERIODIC_TASK_IMPL == TIMERFD
   #include <sys/timerfd.h>
 #elif PERIODIC_TASK_IMPL == RDTSC
-  #include <villas/rdtsc.h>
+  #include <villas/tsc.h>
 #endif
 
 int task_init(struct task *t, double rate, int clock)
@@ -45,7 +45,7 @@ int task_init(struct task *t, double rate, int clock)
 	if (t->fd < 0)
 		return -1;
 #elif PERIODIC_TASK_IMPL == RDTSC
-	ret = rdtsc_init(&t->frequency);
+	ret = tsc_init(&t->tsc);
 	if (ret)
 		return ret;
 #endif
@@ -96,8 +96,8 @@ int task_set_rate(struct task *t, double rate)
 {
 
 #if PERIODIC_TASK_IMPL == RDTSC
-	t->period = t->frequency / rate * 1000;
-	t->next = rdtscp() + t->period;
+	t->period = tsc_rate_to_cycles(&t->tsc, rate);
+	t->next = tsc_now(&t->tsc) + t->period;
 #else
 	/* A rate of 0 will disarm the timer */
 	t->period = rate ? time_from_double(1.0 / rate) : (struct timespec) { 0, 0 };

lib/tsc.c

@@ -21,66 +21,24 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *********************************************************************************/
 
-#pragma once
+#include <villas/tsc.h>
 
-#include <stdint.h>
-#include <cpuid.h>
-#include <inttypes.h>
-
-#include <stdio.h>
-
-#ifdef __APPLE__
-  #include <sys/types.h>
-  #include <sys/sysctl.h>
-#endif
-
-
-#ifndef bit_TSC
-  #define bit_TSC (1 << 4)
-#endif
-
-#define bit_TSC_INVARIANT	(1 << 8)
-#define bit_RDTSCP		(1 << 27)
-
-#if !(__x86_64__ || __i386__)
-  #error this header is for x86 only
-#endif
-
-/** Get CPU timestep counter */
-__attribute__((always_inline))
-static inline uint64_t rdtscp()
-{
-	uint64_t tsc;
-
-	__asm__ __volatile__("rdtscp;"
-		 "shl $32, %%rdx;"
-		 "or %%rdx,%%rax"
-		: "=a" (tsc)
-		:
-		: "%rcx", "%rdx", "memory");
-
-	return tsc;
-}
-
-static int rdtsc_init(uint64_t *freq) __attribute__((unused));
-static int rdtsc_init(uint64_t *freq)
+int tsc_init(struct tsc *t)
 {
 	uint32_t eax, ebx, ecx, edx;
 
 	/** Check if TSC is supported */
 	__get_cpuid(0x1, &eax, &ebx, &ecx, &edx);
 	if (!(edx & bit_TSC))
-		return -1;
+		return -2;
 
 	/** Check if RDTSCP instruction is supported */
 	__get_cpuid(0x80000001, &eax, &ebx, &ecx, &edx);
-	if (!(edx & bit_RDTSCP))
-		return -1;
+	t->rdtscp_supported = edx & bit_RDTSCP;
 
 	/** Check if TSC is invariant */
 	__get_cpuid(0x80000007, &eax, &ebx, &ecx, &edx);
-	if (!(edx & bit_TSC_INVARIANT))
-		return -1;
+	t->is_invariant = edx & bit_TSC_INVARIANT;
 
 	/** Intel SDM Vol 3, Section 18.7.3:
 	 * Nominal TSC frequency = CPUID.15H.ECX[31:0] * CPUID.15H.EBX[31:0] ) ÷ CPUID.15H.EAX[31:0]
@@ -88,31 +46,39 @@ static int rdtsc_init(uint64_t *freq)
 	__get_cpuid(0x15, &eax, &ebx, &ecx, &edx);
 
 	if (ecx != 0)
-		*freq = ecx * ebx / eax;
+		t->frequency = ecx * ebx / eax;
 	else {
 		int ret;
 #ifdef __linux__
-		FILE *f = fopen(SYSFS_PATH "/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq", "r");
-		if (!f)
-			return -1;
-
-		ret = fscanf(f, "%" PRIu64, freq);
-
-		fclose(f);
-
-		if (ret != 1)
-			return -1;
+		ret = kernel_get_cpu_frequency(&t->frequency);
+		if (ret)
+			return ret;
 #elif defined(__APPLE__)
-		int64_t tscfreq;
+		int64_t frequency;
 		size_t lenp = sizeof(tscfreq);
 
-		ret = sysctlbyname("machdep.tsc.frequency", &tscfreq, &lenp, NULL, 0);
+		/** @todo: machdep.tsc.frequency seems to be a measured frequency (based on local APIC?
+		 *         We should figure out which frequency is more accurate */
+//		ret = sysctlbyname("hw.cpufrequency", &tscfreq, &lenp, NULL, 0);
+		ret = sysctlbyname("machdep.tsc.frequency", &frequency, &lenp, NULL, 0);
 		if (ret)
 			return ret;
 
-		*freq = tscfreq;
+		t->frequency = frequency;
 #endif
 	}
 
 	return 0;
 }
+
+uint64_t tsc_rate_to_cyles(struct tsc *t, double rate)
+{
+	return t->frequency / rate;
+}
+
+uint64_t tsc_now(struct tsc *t)
+{
+	return t->rdtscp_supported
+		? rdtscp()
+		: rdtsc();
+}

tests/unit/tsc.c

@@ -23,44 +23,52 @@
 #include <criterion/criterion.h>
 
 #include <villas/utils.h>
-#include <villas/rdtsc.h>
+#include <villas/tsc.h>
 #include <villas/timing.h>
 
-Test(rdtsc, increasing)
+#define CNT (1 << 18)
+
+Test(tsc, increasing)
 {
+	int ret;
+	struct tsc tsc;
 	uint64_t *cntrs;
 
-	cntrs = alloc(sizeof(uint64_t) << 18);
+	ret = tsc_init(&tsc);
+	cr_assert_eq(ret, 0);
+
+	cntrs = alloc(sizeof(uint64_t) * CNT);
 	cr_assert_not_null(cntrs);
 
-	for (int i = 0; i < ARRAY_LEN(cntrs); i++)
-		cntrs[i] = rdtscp();
+	for (int i = 0; i < CNT; i++)
+		cntrs[i] = tsc_now(&tsc);
 
-	for (int i = 1; i < ARRAY_LEN(cntrs); i++)
+	for (int i = 1; i < CNT; i++)
 		cr_assert_lt(cntrs[i-1], cntrs[i]);
 
 	free(cntrs);
 }
 
-Test(rdtsc, sleep)
+Test(tsc, sleep)
 {
 	int ret;
 	double delta, duration = 1;
 	struct timespec start, stop;
-	uint64_t freq, start_cycles, end_cycles;
+	struct tsc tsc;
+	uint64_t start_cycles, end_cycles;
 
-	ret = rdtsc_init(&freq);
+	ret = tsc_init(&tsc);
 	cr_assert_eq(ret, 0);
 
 	clock_gettime(CLOCK_MONOTONIC, &start);
 
-	start_cycles = rdtscp();
-	end_cycles = start_cycles + duration * freq;
+	start_cycles = tsc_now(&tsc);
+	end_cycles = start_cycles + duration * tsc.frequency;
 
-	while (rdtscp() < end_cycles);
+	while (tsc_now(&tsc) < end_cycles);
 
 	clock_gettime(CLOCK_MONOTONIC, &stop);
 	delta = time_delta(&start, &stop);
 
-	cr_assert_float_eq(delta, duration, 1e-3, "Error: %f, Delta: %lf, Freq: %llu", delta - duration, delta, freq);
+	cr_assert_float_eq(delta, duration, 1e-4, "Error: %f, Delta: %lf, Freq: %llu", delta - duration, delta, tsc.frequency);
 }