[v1,2/6] x86/tsc: Convert Time Stamp Counter (TSC) value to Always Running Timer (ART)

Message ID 20231017052457.25287-3-lakshmi.sowjanya.d@intel.com
State New
Headers
Series Add support for Intel PPS Generator |

Commit Message

D, Lakshmi Sowjanya Oct. 17, 2023, 5:24 a.m. UTC
  From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

PPS generators trigger pulses according to system time/TSC.
Timed I/O hardware understands time in ART (Always Running Timer). There
is a need to convert TSC time to ART. The conversion is done using the
detected art_to_tsc_numerator and denominator.

ART = TSC * tsc_to_art_denominator / tsc_to_art_numerator

Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Eddie Dong <eddie.dong@intel.com>
---
 arch/x86/include/asm/tsc.h |  3 +++
 arch/x86/kernel/tsc.c      | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)
  

Comments

Thomas Gleixner Oct. 17, 2023, 9:29 a.m. UTC | #1
On Tue, Oct 17 2023 at 10:54, lakshmi.sowjanya.d@intel.com wrote:
>  
> +/*
> + * Converts input TSC to the corresponding ART value using conversion
> + * factors discovered by detect_art().
> + *
> + * Return: 0 on success, -errno on failure.

bool indicating success / fail ?

> + */
> +int convert_tsc_to_art(const struct system_counterval_t *system_counter,
> +		       u64 *art)
> +{
> +	u64 tmp, res, rem;
> +	/* ART = TSC * tsc_to_art_denominator / tsc_to_art_numerator */
> +	struct u32_fract tsc_to_art = {
> +		.numerator = art_to_tsc_denominator,
> +		.denominator = art_to_tsc_numerator,
> +	};

The purpose of this struct is to obfuscate the code, right?

The struct would make sense if a pointer would be handed to some other
function.

> +	if (system_counter->cs != art_related_clocksource)
> +		return -EINVAL;
> +
> +	res = system_counter->cycles - art_to_tsc_offset;
> +	rem = do_div(res, tsc_to_art.denominator);
> +
> +	tmp = rem * tsc_to_art.numerator;
> +	do_div(tmp, tsc_to_art.denominator);
> +	*art = res * tsc_to_art.numerator + tmp;

Yet another copy of the math in convert_art_to_tsc() and in
convert_art_ns_to_tsc(). Seriously?

Can we please have _one_ helper function which takes value, nominator,
denominator as arguments and clean up the copy&pasta instead of adding
more?

Thanks,

        tglx
  

Patch

diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index 594fce0ca744..f5cff8d4f61e 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -8,6 +8,8 @@ 
 #include <asm/processor.h>
 #include <asm/cpufeature.h>
 
+struct system_counterval_t;
+
 /*
  * Standard way to access the cycle counter.
  */
@@ -27,6 +29,7 @@  static inline cycles_t get_cycles(void)
 }
 #define get_cycles get_cycles
 
+extern int convert_tsc_to_art(const struct system_counterval_t *tsc, u64 *art);
 extern struct system_counterval_t convert_art_to_tsc(u64 art);
 extern struct system_counterval_t convert_art_ns_to_tsc(u64 art_ns);
 
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 15f97c0abc9d..92b800015d8f 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -2,6 +2,7 @@ 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/kernel.h>
+#include <linux/math.h>
 #include <linux/sched.h>
 #include <linux/sched/clock.h>
 #include <linux/init.h>
@@ -1294,6 +1295,37 @@  int unsynchronized_tsc(void)
 	return 0;
 }
 
+/*
+ * Converts input TSC to the corresponding ART value using conversion
+ * factors discovered by detect_art().
+ *
+ * Return: 0 on success, -errno on failure.
+ */
+int convert_tsc_to_art(const struct system_counterval_t *system_counter,
+		       u64 *art)
+{
+	u64 tmp, res, rem;
+	/* ART = TSC * tsc_to_art_denominator / tsc_to_art_numerator */
+	struct u32_fract tsc_to_art = {
+		.numerator = art_to_tsc_denominator,
+		.denominator = art_to_tsc_numerator,
+	};
+
+	if (system_counter->cs != art_related_clocksource)
+		return -EINVAL;
+
+	res = system_counter->cycles - art_to_tsc_offset;
+	rem = do_div(res, tsc_to_art.denominator);
+
+	tmp = rem * tsc_to_art.numerator;
+	do_div(tmp, tsc_to_art.denominator);
+
+	*art = res * tsc_to_art.numerator + tmp;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(convert_tsc_to_art);
+
 /*
  * Convert ART to TSC given numerator/denominator found in detect_art()
  */