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

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 2401 bytes --]

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.

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


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]