| 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