2024-04-30 08:54:52

by D, Lakshmi Sowjanya

[permalink] [raw]
Subject: [PATCH v7 01/12] timekeeping: Add base clock properties in clocksource structure

From: Lakshmi Sowjanya D <[email protected]>

Add base clock hardware abstraction in clocksource structure.

Provide generic functionality in convert_base_to_cs() to convert base
clock timestamps to system clocksource without requiring architecture
specific parameters.

This is intended to replace convert_art_to_tsc() and
convert_art_ns_to_tsc() functions which are specific to convert ART
(Always Running Timer) time to the corresponding TSC value.

Add the infrastructure in get_device_system_crosststamp().

Co-developed-by: Thomas Gleixner <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Co-developed-by: Christopher S. Hall <[email protected]>
Signed-off-by: Christopher S. Hall <[email protected]>
Signed-off-by: Lakshmi Sowjanya D <[email protected]>
---
include/linux/clocksource.h | 27 +++++++++++++++++++++++++
include/linux/timekeeping.h | 2 ++
kernel/time/timekeeping.c | 39 +++++++++++++++++++++++++++++++++++--
3 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 0ad8b550bb4b..d35b677b08fe 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -21,6 +21,7 @@
#include <asm/div64.h>
#include <asm/io.h>

+struct clocksource_base;
struct clocksource;
struct module;

@@ -50,6 +51,7 @@ struct module;
* multiplication
* @name: Pointer to clocksource name
* @list: List head for registration (internal)
+ * @freq_khz: Clocksource frequency in khz.
* @rating: Rating value for selection (higher is better)
* To avoid rating inflation the following
* list should give you a guide as to how
@@ -70,6 +72,8 @@ struct module;
* validate the clocksource from which the snapshot was
* taken.
* @flags: Flags describing special properties
+ * @base: Hardware abstraction for clock on which a clocksource
+ * is based
* @enable: Optional function to enable the clocksource
* @disable: Optional function to disable the clocksource
* @suspend: Optional suspend function for the clocksource
@@ -107,10 +111,12 @@ struct clocksource {
u64 max_cycles;
const char *name;
struct list_head list;
+ u32 freq_khz;
int rating;
enum clocksource_ids id;
enum vdso_clock_mode vdso_clock_mode;
unsigned long flags;
+ struct clocksource_base *base;

int (*enable)(struct clocksource *cs);
void (*disable)(struct clocksource *cs);
@@ -306,4 +312,25 @@ static inline unsigned int clocksource_get_max_watchdog_retry(void)

void clocksource_verify_percpu(struct clocksource *cs);

+/**
+ * struct clocksource_base - hardware abstraction for clock on which a clocksource
+ * is based
+ * @id: Defaults to CSID_GENERIC. The id value is used for conversion
+ * functions which require that the current clocksource is based
+ * on a clocksource_base with a particular ID in certain snapshot
+ * functions to allow callers to validate the clocksource from
+ * which the snapshot was taken.
+ * @freq_khz: Nominal frequency of the base clock in kHz
+ * @offset: Offset between the base clock and the clocksource
+ * @numerator: Numerator of the clock ratio between base clock and the clocksource
+ * @denominator: Denominator of the clock ratio between base clock and the clocksource
+ */
+struct clocksource_base {
+ enum clocksource_ids id;
+ u32 freq_khz;
+ u64 offset;
+ u32 numerator;
+ u32 denominator;
+};
+
#endif /* _LINUX_CLOCKSOURCE_H */
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 0ea7823b7f31..b2ee182d891e 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -310,10 +310,12 @@ struct system_device_crosststamp {
* timekeeping code to verify comparability of two cycle values.
* The default ID, CSID_GENERIC, does not identify a specific
* clocksource.
+ * @use_nsecs: @cycles is in nanoseconds.
*/
struct system_counterval_t {
u64 cycles;
enum clocksource_ids cs_id;
+ bool use_nsecs;
};

/*
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index b58dffc58a8f..4e5e931e766a 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1193,6 +1193,42 @@ static bool timestamp_in_interval(u64 start, u64 end, u64 ts)
return false;
}

+static bool convert_clock(u64 *val, u32 numerator, u32 denominator)
+{
+ u64 rem, res;
+
+ if (!numerator || !denominator)
+ return false;
+
+ res = div64_u64_rem(*val, denominator, &rem) * numerator;
+ *val = res + div_u64(rem * numerator, denominator);
+ return true;
+}
+
+static bool convert_base_to_cs(struct system_counterval_t *scv)
+{
+ struct clocksource *cs = tk_core.timekeeper.tkr_mono.clock;
+ struct clocksource_base *base = cs->base;
+ u32 num, den;
+
+ /* The timestamp was taken from the time keeper clock source */
+ if (cs->id == scv->cs_id)
+ return true;
+
+ /* Check whether cs_id matches the base clock */
+ if (!base || base->id != scv->cs_id)
+ return false;
+
+ num = scv->use_nsecs ? cs->freq_khz : base->numerator;
+ den = scv->use_nsecs ? USEC_PER_SEC : base->denominator;
+
+ if (!convert_clock(&scv->cycles, num, den))
+ return false;
+
+ scv->cycles += base->offset;
+ return true;
+}
+
/**
* get_device_system_crosststamp - Synchronously capture system/device timestamp
* @get_time_fn: Callback to get simultaneous device time and
@@ -1238,8 +1274,7 @@ int get_device_system_crosststamp(int (*get_time_fn)
* system counter value is the same as for the currently
* installed timekeeper clocksource
*/
- if (system_counterval.cs_id == CSID_GENERIC ||
- tk->tkr_mono.clock->id != system_counterval.cs_id)
+ if (!convert_base_to_cs(&system_counterval))
return -ENODEV;
cycles = system_counterval.cycles;

--
2.35.3



2024-04-30 14:48:22

by Peter Hilber

[permalink] [raw]
Subject: Re: [PATCH v7 01/12] timekeeping: Add base clock properties in clocksource structure

On 30.04.24 10:52, [email protected] wrote:
> From: Lakshmi Sowjanya D <[email protected]>
>
> Add base clock hardware abstraction in clocksource structure.
>
> Provide generic functionality in convert_base_to_cs() to convert base
> clock timestamps to system clocksource without requiring architecture
> specific parameters.
>
> This is intended to replace convert_art_to_tsc() and
> convert_art_ns_to_tsc() functions which are specific to convert ART
> (Always Running Timer) time to the corresponding TSC value.
>
> Add the infrastructure in get_device_system_crosststamp().
>
> Co-developed-by: Thomas Gleixner <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Co-developed-by: Christopher S. Hall <[email protected]>
> Signed-off-by: Christopher S. Hall <[email protected]>
> Signed-off-by: Lakshmi Sowjanya D <[email protected]>
> ---
> include/linux/clocksource.h | 27 +++++++++++++++++++++++++
> include/linux/timekeeping.h | 2 ++
> kernel/time/timekeeping.c | 39 +++++++++++++++++++++++++++++++++++--
> 3 files changed, 66 insertions(+), 2 deletions(-)
>
[...]
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index b58dffc58a8f..4e5e931e766a 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -1193,6 +1193,42 @@ static bool timestamp_in_interval(u64 start, u64 end, u64 ts)
> return false;
> }
>
> +static bool convert_clock(u64 *val, u32 numerator, u32 denominator)
> +{
> + u64 rem, res;
> +
> + if (!numerator || !denominator)
> + return false;
> +
> + res = div64_u64_rem(*val, denominator, &rem) * numerator;
> + *val = res + div_u64(rem * numerator, denominator);
> + return true;
> +}
> +
> +static bool convert_base_to_cs(struct system_counterval_t *scv)
> +{
> + struct clocksource *cs = tk_core.timekeeper.tkr_mono.clock;
> + struct clocksource_base *base = cs->base;
> + u32 num, den;
> +
> + /* The timestamp was taken from the time keeper clock source */
> + if (cs->id == scv->cs_id)
> + return true;
> +
> + /* Check whether cs_id matches the base clock */
> + if (!base || base->id != scv->cs_id)
> + return false;
> +
> + num = scv->use_nsecs ? cs->freq_khz : base->numerator;
> + den = scv->use_nsecs ? USEC_PER_SEC : base->denominator;
> +
> + if (!convert_clock(&scv->cycles, num, den))
> + return false;
> +
> + scv->cycles += base->offset;
> + return true;
> +}
> +
> /**
> * get_device_system_crosststamp - Synchronously capture system/device timestamp
> * @get_time_fn: Callback to get simultaneous device time and
> @@ -1238,8 +1274,7 @@ int get_device_system_crosststamp(int (*get_time_fn)
> * system counter value is the same as for the currently
> * installed timekeeper clocksource
> */
> - if (system_counterval.cs_id == CSID_GENERIC ||
> - tk->tkr_mono.clock->id != system_counterval.cs_id)
> + if (!convert_base_to_cs(&system_counterval))
> return -ENODEV;

AFAIU the error condition

system_counterval.cs_id == CSID_GENERIC

is silently dropped by this patch, but shouldn't be.

get_device_system_crosststamp() can only check the identity of a
clocksource (base) for non-generic ids.

Regards,

Peter

2024-05-09 04:39:01

by D, Lakshmi Sowjanya

[permalink] [raw]
Subject: RE: [PATCH v7 01/12] timekeeping: Add base clock properties in clocksource structure



> -----Original Message-----
> From: Peter Hilber <[email protected]>
> Sent: Tuesday, April 30, 2024 7:58 PM
> To: D, Lakshmi Sowjanya <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Dong,
> Eddie <[email protected]>; Hall, Christopher S
> <[email protected]>; Brandeburg, Jesse
> <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Nguyen, Anthony L <[email protected]>;
> N, Pandith <[email protected]>; Mohan, Subramanian
> <[email protected]>; T R, Thejesh Reddy
> <[email protected]>
> Subject: Re: [PATCH v7 01/12] timekeeping: Add base clock properties in
> clocksource structure
>
> On 30.04.24 10:52, [email protected] wrote:
> > From: Lakshmi Sowjanya D <[email protected]>
> >
> > Add base clock hardware abstraction in clocksource structure.
> >
> > Provide generic functionality in convert_base_to_cs() to convert base
> > clock timestamps to system clocksource without requiring architecture
> > specific parameters.
> >
> > This is intended to replace convert_art_to_tsc() and
> > convert_art_ns_to_tsc() functions which are specific to convert ART
> > (Always Running Timer) time to the corresponding TSC value.
> >
> > Add the infrastructure in get_device_system_crosststamp().
> >
> > Co-developed-by: Thomas Gleixner <[email protected]>
> > Signed-off-by: Thomas Gleixner <[email protected]>
> > Co-developed-by: Christopher S. Hall <[email protected]>
> > Signed-off-by: Christopher S. Hall <[email protected]>
> > Signed-off-by: Lakshmi Sowjanya D <[email protected]>
> > ---
> > include/linux/clocksource.h | 27 +++++++++++++++++++++++++
> > include/linux/timekeeping.h | 2 ++
> > kernel/time/timekeeping.c | 39
> +++++++++++++++++++++++++++++++++++--
> > 3 files changed, 66 insertions(+), 2 deletions(-)
> >
> [...]
> > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> > index b58dffc58a8f..4e5e931e766a 100644
> > --- a/kernel/time/timekeeping.c
> > +++ b/kernel/time/timekeeping.c
> > @@ -1193,6 +1193,42 @@ static bool timestamp_in_interval(u64 start, u64
> end, u64 ts)
> > return false;
> > }
> >
> > +static bool convert_clock(u64 *val, u32 numerator, u32 denominator) {
> > + u64 rem, res;
> > +
> > + if (!numerator || !denominator)
> > + return false;
> > +
> > + res = div64_u64_rem(*val, denominator, &rem) * numerator;
> > + *val = res + div_u64(rem * numerator, denominator);
> > + return true;
> > +}
> > +
> > +static bool convert_base_to_cs(struct system_counterval_t *scv) {
> > + struct clocksource *cs = tk_core.timekeeper.tkr_mono.clock;
> > + struct clocksource_base *base = cs->base;
> > + u32 num, den;
> > +
> > + /* The timestamp was taken from the time keeper clock source */
> > + if (cs->id == scv->cs_id)
> > + return true;
> > +
> > + /* Check whether cs_id matches the base clock */
> > + if (!base || base->id != scv->cs_id)
> > + return false;
> > +
> > + num = scv->use_nsecs ? cs->freq_khz : base->numerator;
> > + den = scv->use_nsecs ? USEC_PER_SEC : base->denominator;
> > +
> > + if (!convert_clock(&scv->cycles, num, den))
> > + return false;
> > +
> > + scv->cycles += base->offset;
> > + return true;
> > +}
> > +
> > /**
> > * get_device_system_crosststamp - Synchronously capture
> system/device timestamp
> > * @get_time_fn: Callback to get simultaneous device time and
> > @@ -1238,8 +1274,7 @@ int get_device_system_crosststamp(int
> (*get_time_fn)
> > * system counter value is the same as for the currently
> > * installed timekeeper clocksource
> > */
> > - if (system_counterval.cs_id == CSID_GENERIC ||
> > - tk->tkr_mono.clock->id != system_counterval.cs_id)
> > + if (!convert_base_to_cs(&system_counterval))
> > return -ENODEV;
>
> AFAIU the error condition
>
> system_counterval.cs_id == CSID_GENERIC
>
> is silently dropped by this patch, but shouldn't be.
>
> get_device_system_crosststamp() can only check the identity of a
> clocksource (base) for non-generic ids.
>
> Regards,
>
> Peter

Thanks Peter,
Noted. The check will be added as below:

if (system_counterval.cs_id == CSID_GENERIC ||
!convert_base_to_cs(&system_counterval))

Regards,
Sowjanya