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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 6 Mar 2022 08:36:06 -0800 From: Richard Cochran <richardcochran@...il.com> To: Gerhard Engleder <gerhard@...leder-embedded.com> Cc: yangbo.lu@....com, davem@...emloft.net, kuba@...nel.org, mlichvar@...hat.com, vinicius.gomes@...el.com, netdev@...r.kernel.org Subject: Re: [RFC PATCH net-next 3/6] ptp: Add free running time support On Sun, Mar 06, 2022 at 09:56:55AM +0100, Gerhard Engleder wrote: > ptp vclocks require a clock with free running time for the timecounter. > Currently only a physical clock forced to free running is supported. > If vclocks are used, then the physical clock cannot be synchronized > anymore. The synchronized time is not available in hardware in this > case. As a result, timed transmission with ETF/TAPRIO hardware support > is not possible anymore. > > If hardware would support a free running time additionally to the > physical clock, then the physical clock does not need to be forced to > free running. Thus, the physical clocks can still be synchronized while > vclocks are in use. > > The physical clock could be used to synchronize the time domain of the > TSN network and trigger ETF/TAPRIO. In parallel vclocks can be used to > synchronize other time domains. > > Allow read and cross time stamp of additional free running time for > physical clocks. Let vclocks use free running time if available. > > Signed-off-by: Gerhard Engleder <gerhard@...leder-embedded.com> > --- > drivers/ptp/ptp_vclock.c | 20 +++++++++++++++----- > include/linux/ptp_clock_kernel.h | 27 +++++++++++++++++++++++++++ > 2 files changed, 42 insertions(+), 5 deletions(-) > > diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c > index cb179a3ea508..3715d75ee8bd 100644 > --- a/drivers/ptp/ptp_vclock.c > +++ b/drivers/ptp/ptp_vclock.c > @@ -68,7 +68,10 @@ static int ptp_vclock_gettimex(struct ptp_clock_info *ptp, > int err; > u64 ns; > > - err = pptp->info->gettimex64(pptp->info, &pts, sts); > + if (pptp->info->getfreeruntimex64) > + err = pptp->info->getfreeruntimex64(pptp->info, &pts, sts); > + else > + err = pptp->info->gettimex64(pptp->info, &pts, sts); Why all this extra if/then/else here and at registration? Just provide new ptp_vclock_ helpers and drop the run time conditionals. > if (err) > return err; > > @@ -104,7 +107,10 @@ static int ptp_vclock_getcrosststamp(struct ptp_clock_info *ptp, > int err; > u64 ns; > > - err = pptp->info->getcrosststamp(pptp->info, xtstamp); > + if (pptp->info->getfreeruncrosststamp) > + err = pptp->info->getfreeruncrosststamp(pptp->info, xtstamp); > + else > + err = pptp->info->getcrosststamp(pptp->info, xtstamp); same here > if (err) > return err; > > @@ -143,7 +149,9 @@ static u64 ptp_vclock_read(const struct cyclecounter *cc) > struct ptp_clock *ptp = vclock->pclock; > struct timespec64 ts = {}; > > - if (ptp->info->gettimex64) > + if (ptp->info->getfreeruntimex64) > + ptp->info->getfreeruntimex64(ptp->info, &ts, NULL); > + else if (ptp->info->gettimex64) > ptp->info->gettimex64(ptp->info, &ts, NULL); > else > ptp->info->gettime64(ptp->info, &ts); > @@ -168,11 +176,13 @@ struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock) > > vclock->pclock = pclock; > vclock->info = ptp_vclock_info; > - if (pclock->info->gettimex64) > + if (pclock->info->getfreeruntimex64 || pclock->info->gettimex64) > vclock->info.gettimex64 = ptp_vclock_gettimex; > else > vclock->info.gettime64 = ptp_vclock_gettime; > - if (pclock->info->getcrosststamp) > + if ((pclock->info->getfreeruntimex64 && > + pclock->info->getfreeruncrosststamp) || > + pclock->info->getcrosststamp) > vclock->info.getcrosststamp = ptp_vclock_getcrosststamp; > vclock->cc = ptp_vclock_cc; > > diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h > index 554454cb8693..b291517fc7c8 100644 > --- a/include/linux/ptp_clock_kernel.h > +++ b/include/linux/ptp_clock_kernel.h > @@ -108,6 +108,28 @@ struct ptp_system_timestamp { > * @settime64: Set the current time on the hardware clock. > * parameter ts: Time value to set. > * > + * @getfreeruntimex64: Reads the current free running time from the hardware > + * clock and optionally also the system clock. This > + * operation requires hardware support for an additional > + * free running time including support for hardware time > + * stamps based on that free running time. > + * The free running time must be completely independet from > + * the actual time of the PTP clock. It must be monotonic > + * and its frequency must be constant. > + * parameter ts: Holds the PHC free running timestamp. > + * parameter sts: If not NULL, it holds a pair of > + * timestamps from the system clock. The first reading is > + * made right before reading the lowest bits of the PHC > + * free running timestamp and the second reading > + * immediately follows that. > + * > + * @getfreeruncrosststamp: Reads the current time from the free running > + * hardware clock and system clock simultaneously. > + * parameter cts: Contains timestamp (device,system) > + * pair, where device time is the free running time > + * also used for @getfreeruntimex64 and system time is > + * realtime and monotonic. > + * > * @enable: Request driver to enable or disable an ancillary feature. > * parameter request: Desired resource to enable or disable. > * parameter on: Caller passes one to enable or zero to disable. > @@ -155,6 +177,11 @@ struct ptp_clock_info { > int (*getcrosststamp)(struct ptp_clock_info *ptp, > struct system_device_crosststamp *cts); > int (*settime64)(struct ptp_clock_info *p, const struct timespec64 *ts); > + int (*getfreeruntimex64)(struct ptp_clock_info *ptp, > + struct timespec64 *ts, > + struct ptp_system_timestamp *sts); > + int (*getfreeruncrosststamp)(struct ptp_clock_info *ptp, > + struct system_device_crosststamp *cts); Wow, that is really hard to read. Suggest freerun_gettimex64 and freerun_crosststamp instead. > int (*enable)(struct ptp_clock_info *ptp, > struct ptp_clock_request *request, int on); > int (*verify)(struct ptp_clock_info *ptp, unsigned int pin, > -- > 2.20.1 > Thanks, Richard
Powered by blists - more mailing lists