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.

> +             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/


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.

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.

Attachment: smime.p7s
Description: S/MIME cryptographic signature

Reply via email to