Re: [PATCH net-next v4] ptp: add Alibaba CIPU PTP clock driver

[Wen Gu]

On 2025/8/16 02:43, David Woodhouse wrote:
> On Fri, 2025-08-15 at 11:38 -0700, Jakub Kicinski wrote:
> > On Tue, 12 Aug 2025 19:53:21 +0800 Wen Gu wrote:
> > > This adds a driver for Alibaba CIPU PTP clock. The CIPU, an underlying
> > > infrastructure of Alibaba Cloud, synchronizes time with reference clocks
> > > continuously and provides PTP clocks for VMs and bare metals in cloud.
> >
> > > +static struct attribute *ptp_cipu_attrs[] = {
> > > +	&dev_attr_reg_dev_feat.attr,
> > > +	&dev_attr_reg_gst_feat.attr,
> > > +	&dev_attr_reg_drv_ver.attr,
> > > +	&dev_attr_reg_env_ver.attr,
> > > +	&dev_attr_reg_dev_stat.attr,
> > > +	&dev_attr_reg_sync_stat.attr,
> > > +	&dev_attr_reg_tm_prec_ns.attr,
> > > +	&dev_attr_reg_epo_base_yr.attr,
> > > +	&dev_attr_reg_leap_sec.attr,
> > > +	&dev_attr_reg_max_lat_ns.attr,
> > > +	&dev_attr_reg_mt_tout_us.attr,
> > > +	&dev_attr_reg_thresh_us.attr,
> > > +
> > > +	&dev_attr_ptp_gettm.attr,
> > > +	&dev_attr_ptp_gettm_inval_err.attr,
> > > +	&dev_attr_ptp_gettm_tout_err.attr,
> > > +	&dev_attr_ptp_gettm_excd_thresh.attr,
> > > +
> > > +	&dev_attr_dev_clk_abn.attr,
> > > +	&dev_attr_dev_clk_abn_rec.attr,
> > > +	&dev_attr_dev_maint.attr,
> > > +	&dev_attr_dev_maint_rec.attr,
> > > +	&dev_attr_dev_maint_tout.attr,
> > > +	&dev_attr_dev_busy.attr,
> > > +	&dev_attr_dev_busy_rec.attr,
> > > +	&dev_attr_dev_err.attr,
> > > +	&dev_attr_dev_err_rec.attr,
> >
> > This driver is lacking documentation. You need to describe how the user
> > is expected to interact with the device and document all these sysfs
> > attributes.
> >
> > Maybe it's just me, but in general I really wish someone stepped up
> > and created a separate subsystem for all these cloud / vm clocks.
> > They have nothing to do with PTP. In my mind PTP clocks are simple HW
> > tickers on which we build all the time related stuff. While this driver
> > reports the base year for the epoch and leap second status via sysfs.
>
> None of it should exist in the cloud anyway. The *only* thing that
> makes sense for a VM is for the hypervisor to just *tell* the guest
> what the relationship is between the CPU's hardware counter (e.g. TSC)
> and real time. Which is what VMclock was invented for. Use that,
> instead of making *every* guest on the system duplicate the same work
> of synchronising the *same* underlying oscillator. Badly, with steal
> time in the mix.

Our design is similar, but based on Alibaba CIPU[1].

The CIPU in cloud synchronizes time with the atomic clock and get
high-precision time value. It then virtualizes PCI devices and passes
them to VMs or bare metals. So VMs and bare metals can use these PCI
devices's timestamp registers to obtain the high-precision time.
The driver here exposes these devices as PTP clocks for use by VMs and
bare metals (e.g. chrony).

[1] https://www.alibabacloud.com/blog/a-detailed-explanation-about-alibaba-cloud-cipu_599183

> Given PCIe PTM to synchronize counters, you could even implement
> vmclock over PCI for bare metal.

PCIe PTM is not supported by CIPU, so vmclock can't work for bare metals,
that's also one of the reasons why we choose current design.

Thanks.