netdev - Re: [PATCH net] net/mlx5: Dynamic cyclecounter shift calculation for PTP free running clock

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <B74120A4-4468-4106-AD73-D9936B06BA6E@bridgetech.tv>
Date: Wed, 23 Aug 2023 23:31:26 +0000
From: Kenneth Klette Jonassen <kenneth.jonassen@...dgetech.tv>
To: Rahul Rameshbabu <rrameshbabu@...dia.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Saeed Mahameed
	<saeed@...nel.org>, Jakub Kicinski <kuba@...nel.org>, Richard Cochran
	<richardcochran@...il.com>, David Miller <davem@...emloft.net>, Paolo Abeni
	<pabeni@...hat.com>, Vadim Fedorenko <vadfed@...a.com>
Subject: Re: [PATCH net] net/mlx5: Dynamic cyclecounter shift calculation for
 PTP free running clock


> On 22 Aug 2023, at 01:05, Rahul Rameshbabu <rrameshbabu@...dia.com> wrote:

...

> +static u32 mlx5_ptp_shift_constant(u32 dev_freq_khz)
> +{
> + /* Optimal shift constant leads to corrections above just 1 scaled ppm.
> + *
> + * Two sets of equations are needed to derive the optimal shift
> + * constant for the cyclecounter.

This is easy to follow, so I’m not suggesting you change it. But out of
curiosity, have you considered using the more generic
clocks_calc_mult_shift() to find a suitable shift value?

> @@ -909,7 +931,7 @@ static void mlx5_timecounter_init(struct mlx5_core_dev *mdev)
> 
> dev_freq = MLX5_CAP_GEN(mdev, device_frequency_khz);
> timer->cycles.read = read_internal_timer;
> - timer->cycles.shift = MLX5_CYCLES_SHIFT;
> + timer->cycles.shift = mlx5_ptp_shift_constant(dev_freq);
> timer->cycles.mult = clocksource_khz2mult(dev_freq,
>  timer->cycles.shift);
> timer->nominal_c_mult = timer->cycles.mult;

The mask assignment one line below nominal_c_mult could need updating
to match the new shift, e.g.,

        timer->cycles.mask = CLOCKSOURCE_MASK(64 - timer->cycles.shift);

?

It’s currently fixed to 41, which (via 64-23 = 41) matches the
cyclecounter bits shifted out constant of 23 in kernels v6.3 and before.