On 2026-07-27 10:03:28+01:00, David Woodhouse wrote: > On Fri, 2026-07-17 at 06:59 +0000, Arthur Kiyanovski wrote: > > > Introduce two new ioctls that extend existing PTP timestamp interfaces > > with clock quality information: > > > > - PTP_SYS_OFFSET_EXTENDED_ATTRS: Extends PTP_SYS_OFFSET_EXTENDED > > - PTP_SYS_OFFSET_PRECISE_ATTRS: Extends PTP_SYS_OFFSET_PRECISE > > > > These ioctls provide quality attributes alongside timestamps: > > > > 1. error_bound: Maximum deviation from true time (nanoseconds), based > > on device's internal clock state > > 2. clock_status: Synchronization state (unknown, initializing, > > synchronized, free-running, unreliable) > > 3. timescale: Time reference (TAI, UTC, etc.) > > 4. counter_value: Raw system counter (e.g. TSC ticks) captured by the > > timekeeping core alongside each system timestamp > > 5. counter_id: Identifies the counter source (e.g. TSC, ARM arch counter) > > > > This supports three use cases: > > > > 1. Managed PHC devices (e.g., ENA, vmclock) that maintain their own > > synchronization and can report quality metrics directly to userspace > > without requiring ptp4l > > > > 2. Applications that need complete time quality information in a single > > call, regardless of how the PHC is synchronized > > > > 3. VMMs that need raw system counter values paired > > with PTP timestamps for feed-forward clock calibration, avoiding the > > feedback loop inherent in NTP-style synchronization > > > > Timescale definitions use a Continuity/Discipline framework to describe > > timeline properties and steering behavior consistently across all > > entries. > > > > This implementation is based on the original RFC and the UAPI design > > discussion linked below. > > > > Link: > > https://lore.kernel.org/netdev/[email protected]/ > > Link: https://lore.kernel.org/all/87se7ht25o.ffs@tglx/ > > Signed-off-by: Amit Bernstein <[email protected]> > > Signed-off-by: Arthur Kiyanovski <[email protected]> > > ... > > > +static long ptp_sys_offset_extended_attrs(struct ptp_clock *ptp, void > > __user *arg) > > +{ > > + struct ptp_sys_offset_attrs *data __free(kfree) = NULL; > > + struct ptp_attrs_request request; > > + struct ptp_system_timestamp sts; > > + unsigned int n_samples; > > + int err; > > + > > + if (copy_from_user(&request, arg, sizeof(request))) > > + return -EFAULT; > > + > > + if (request.valid || > > + request.num_samples > PTP_MAX_SAMPLES || > > + request.num_samples == 0) > > + return -EINVAL; > > + > > + err = ptp_validate_sys_offset_clockid(request.clock_id); > > + if (err) > > + return err; > > + > > + n_samples = request.num_samples; > > + sts.clockid = request.clock_id; > > + > > + data = kzalloc(struct_size(data, timestamps, n_samples), GFP_KERNEL); > > + if (!data) > > + return -ENOMEM; > > + > > + data->request.num_samples = n_samples; > > + > > + for (unsigned int i = 0; i < n_samples; i++) { > > + struct ptp_clock_attrs att = {}; > > + struct timespec64 ts; > > + > > + if (ptp->info->gettimexattrs64) > > + err = ptp->info->gettimexattrs64(ptp->info, &ts, > > + &sts, &att); > > + else if (ptp->info->gettimex64) > > + err = ptp->info->gettimex64(ptp->info, &ts, &sts); > > + else > > + return -EOPNOTSUPP; > > + > > + if (err) > > + return err; > > + > > + /* Filter out disabled or unavailable clocks */ > > + if (!sts.pre_sts.valid || !sts.post_sts.valid) > > + return -EINVAL; > > + > > + data->timestamps[i].pre_systime.sys_time = > > + ktime_to_ns(sts.pre_sts.systime); > > + data->timestamps[i].pre_systime.sys_rawtime = > > + ktime_to_ns(sts.pre_sts.monoraw); > > + data->timestamps[i].pre_systime.sys_counter = > > + sts.pre_sts.cycles; > > I hate all this line wrapping, btw. I'll defer to the net coding style > if they insist, but my preference would just be just to have longer > lines. Especially when it's a block of assignments like this, the > wrapped form is *much* harder to read. >
Agreed — I'll reduce the wrapping in v6 (a local temp var + a small helper make that block read much better). Thanks. > > + data->timestamps[i].pre_systime.sys_counter_id = > > + sts.pre_sts.cs_id; > > You added PTP_COUNTER_* constants as we discussed¹... but didn't you > forget to *map* to them here? > > ¹ https://lore.kernel.org/all/877boqtg3g.ffs@tglx/ > Good catch — you're right, that's a bug. I'll add the csid→PTP_COUNTER_* mapping in v6. > > Arguably, if the timestamps have a hw_csid then you should be returning > that in place of the main one which will be kvmclock or hyperv > nonsense. You can do that here for extended_attrs but precise_attrs > will need a bit more work (as system_device_crosststamp doesn't have > the hw_* fields). I won't randomise you further by asking you to handle > that though. I'll do it in a follow-up. > Thanks — I'll leave the hw_csid part for your follow-up as you offered. This series exposes the base cs_id and leaves room to add it without an ABI change. > You can either map KVMCLOCK to unknown or give it a > PTP_COUNTER_KVMCLOCK if you prefer. I suggest the former, as nobody who > cares about time should be using kvmclock. And it means my promised > follow-up will be providing counter data which previously wasn't > provided, rather than *changing* what's provided. Will do the former in v6 — map KVMCLOCK to UNKNOWN — so your follow-up adds new counter data rather than changing existing values. Thanks.
