linux-kernel - RE: [PATCH v7 01/12] timekeeping: Add base clock properties in clocksource structure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 9 May 2024 04:38:33 +0000
From: "D, Lakshmi Sowjanya" <lakshmi.sowjanya.d@...el.com>
To: Peter Hilber <peter.hilber@...nsynergy.com>, "tglx@...utronix.de"
	<tglx@...utronix.de>, "jstultz@...gle.com" <jstultz@...gle.com>,
	"giometti@...eenne.com" <giometti@...eenne.com>, "corbet@....net"
	<corbet@....net>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
CC: "x86@...nel.org" <x86@...nel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>, "intel-wired-lan@...ts.osuosl.org"
	<intel-wired-lan@...ts.osuosl.org>, "andriy.shevchenko@...ux.intel.com"
	<andriy.shevchenko@...ux.intel.com>, "Dong, Eddie" <eddie.dong@...el.com>,
	"Hall, Christopher S" <christopher.s.hall@...el.com>, "Brandeburg, Jesse"
	<jesse.brandeburg@...el.com>, "davem@...emloft.net" <davem@...emloft.net>,
	"alexandre.torgue@...s.st.com" <alexandre.torgue@...s.st.com>,
	"joabreu@...opsys.com" <joabreu@...opsys.com>, "mcoquelin.stm32@...il.com"
	<mcoquelin.stm32@...il.com>, "perex@...ex.cz" <perex@...ex.cz>,
	"linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>, "Nguyen, Anthony
 L" <anthony.l.nguyen@...el.com>, "N, Pandith" <pandith.n@...el.com>, "Mohan,
 Subramanian" <subramanian.mohan@...el.com>, "T R, Thejesh Reddy"
	<thejesh.reddy.t.r@...el.com>
Subject: RE: [PATCH v7 01/12] timekeeping: Add base clock properties in
 clocksource structure



> -----Original Message-----
> From: Peter Hilber <peter.hilber@...nsynergy.com>
> Sent: Tuesday, April 30, 2024 7:58 PM
> To: D, Lakshmi Sowjanya <lakshmi.sowjanya.d@...el.com>;
> tglx@...utronix.de; jstultz@...gle.com; giometti@...eenne.com;
> corbet@....net; linux-kernel@...r.kernel.org
> Cc: x86@...nel.org; netdev@...r.kernel.org; linux-doc@...r.kernel.org;
> intel-wired-lan@...ts.osuosl.org; andriy.shevchenko@...ux.intel.com; Dong,
> Eddie <eddie.dong@...el.com>; Hall, Christopher S
> <christopher.s.hall@...el.com>; Brandeburg, Jesse
> <jesse.brandeburg@...el.com>; davem@...emloft.net;
> alexandre.torgue@...s.st.com; joabreu@...opsys.com;
> mcoquelin.stm32@...il.com; perex@...ex.cz; linux-
> sound@...r.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@...el.com>;
> N, Pandith <pandith.n@...el.com>; Mohan, Subramanian
> <subramanian.mohan@...el.com>; T R, Thejesh Reddy
> <thejesh.reddy.t.r@...el.com>
> Subject: Re: [PATCH v7 01/12] timekeeping: Add base clock properties in
> clocksource structure
> 
> On 30.04.24 10:52, lakshmi.sowjanya.d@...el.com wrote:
> > From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@...el.com>
> >
> > 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 <tglx@...utronix.de>
> > Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> > Co-developed-by: Christopher S. Hall <christopher.s.hall@...el.com>
> > Signed-off-by: Christopher S. Hall <christopher.s.hall@...el.com>
> > Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@...el.com>
> > ---
> >  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