| Message ID | 20210323080229.28283-1-yangbo.lu@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | ptp_qoriq: fix overflow in ptp_qoriq_adjfine() u64 calcalation | expand |
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Tue, 23 Mar 2021 16:02:29 +0800 you wrote: > Current calculation for diff of TMR_ADD register value may have > 64-bit overflow in this code line, when long type scaled_ppm is > large. > > adj *= scaled_ppm; > > This patch is to resolve it by using mul_u64_u64_div_u64(). > > [...] Here is the summary with links: - ptp_qoriq: fix overflow in ptp_qoriq_adjfine() u64 calcalation https://git.kernel.org/netdev/net/c/f51d7bf1dbe5 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
On Tue, Mar 23, 2021 at 04:02:29PM +0800, Yangbo Lu wrote: > Current calculation for diff of TMR_ADD register value may have > 64-bit overflow in this code line, when long type scaled_ppm is > large. > > adj *= scaled_ppm; > > This patch is to resolve it by using mul_u64_u64_div_u64(). > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> Acked-by: Richard Cochran <richardcochran@gmail.com>
On Tue, Mar 23, 2021 at 04:02:29PM +0800, Yangbo Lu wrote: > Current calculation for diff of TMR_ADD register value may have > 64-bit overflow in this code line, when long type scaled_ppm is > large. > > adj *= scaled_ppm; > > This patch is to resolve it by using mul_u64_u64_div_u64(). > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > Acked-by: Richard Cochran <richardcochran@gmail.com> > --- > drivers/ptp/ptp_qoriq.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/ptp/ptp_qoriq.c b/drivers/ptp/ptp_qoriq.c > index 68beb1bd07c0..f7f220700cb5 100644 > --- a/drivers/ptp/ptp_qoriq.c > +++ b/drivers/ptp/ptp_qoriq.c > @@ -189,15 +189,16 @@ int ptp_qoriq_adjfine(struct ptp_clock_info *ptp, long scaled_ppm) > tmr_add = ptp_qoriq->tmr_add; > adj = tmr_add; > > - /* calculate diff as adj*(scaled_ppm/65536)/1000000 > - * and round() to the nearest integer > + /* > + * Calculate diff and round() to the nearest integer > + * > + * diff = adj * (ppb / 1000000000) > + * = adj * scaled_ppm / 65536000000 > */ > - adj *= scaled_ppm; > - diff = div_u64(adj, 8000000); > - diff = (diff >> 13) + ((diff >> 12) & 1); > + diff = mul_u64_u64_div_u64(adj, scaled_ppm, 32768000000); mul_u64_u64_div_u64() is not exported. As result, every build with CONFIG_PTP_1588_CLOCK_QORIQ=m (ie every allmodconfig build) fails with: ERROR: modpost: "mul_u64_u64_div_u64" [drivers/ptp/ptp-qoriq.ko] undefined! or a similar error. Guenter
On Thu, Mar 25, 2021 at 03:23:07AM -0700, Guenter Roeck wrote: > On Tue, Mar 23, 2021 at 04:02:29PM +0800, Yangbo Lu wrote: > > Current calculation for diff of TMR_ADD register value may have > > 64-bit overflow in this code line, when long type scaled_ppm is > > large. > > > > adj *= scaled_ppm; > > > > This patch is to resolve it by using mul_u64_u64_div_u64(). > > > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > > Acked-by: Richard Cochran <richardcochran@gmail.com> > > --- > > drivers/ptp/ptp_qoriq.c | 13 +++++++------ > > 1 file changed, 7 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/ptp/ptp_qoriq.c b/drivers/ptp/ptp_qoriq.c > > index 68beb1bd07c0..f7f220700cb5 100644 > > --- a/drivers/ptp/ptp_qoriq.c > > +++ b/drivers/ptp/ptp_qoriq.c > > @@ -189,15 +189,16 @@ int ptp_qoriq_adjfine(struct ptp_clock_info *ptp, long scaled_ppm) > > tmr_add = ptp_qoriq->tmr_add; > > adj = tmr_add; > > > > - /* calculate diff as adj*(scaled_ppm/65536)/1000000 > > - * and round() to the nearest integer > > + /* > > + * Calculate diff and round() to the nearest integer > > + * > > + * diff = adj * (ppb / 1000000000) > > + * = adj * scaled_ppm / 65536000000 > > */ > > - adj *= scaled_ppm; > > - diff = div_u64(adj, 8000000); > > - diff = (diff >> 13) + ((diff >> 12) & 1); > > + diff = mul_u64_u64_div_u64(adj, scaled_ppm, 32768000000); > > mul_u64_u64_div_u64() is not exported. As result, every build with > CONFIG_PTP_1588_CLOCK_QORIQ=m (ie every allmodconfig build) fails with: > > ERROR: modpost: "mul_u64_u64_div_u64" [drivers/ptp/ptp-qoriq.ko] undefined! > > or a similar error. > Ah, never mind. I see this is fixed in mainline (export added). I see the problem in pending-fixes and in next-20210325. Sorry for the noise. Guenter
FYI, On Tue, 23 Mar 2021 at 13:26, Yangbo Lu <yangbo.lu@nxp.com> wrote: > > Current calculation for diff of TMR_ADD register value may have > 64-bit overflow in this code line, when long type scaled_ppm is > large. > > adj *= scaled_ppm; > > This patch is to resolve it by using mul_u64_u64_div_u64(). > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > --- > drivers/ptp/ptp_qoriq.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/ptp/ptp_qoriq.c b/drivers/ptp/ptp_qoriq.c > index 68beb1bd07c0..f7f220700cb5 100644 > --- a/drivers/ptp/ptp_qoriq.c > +++ b/drivers/ptp/ptp_qoriq.c > @@ -189,15 +189,16 @@ int ptp_qoriq_adjfine(struct ptp_clock_info *ptp, long scaled_ppm) > tmr_add = ptp_qoriq->tmr_add; > adj = tmr_add; > > - /* calculate diff as adj*(scaled_ppm/65536)/1000000 > - * and round() to the nearest integer > + /* > + * Calculate diff and round() to the nearest integer > + * > + * diff = adj * (ppb / 1000000000) > + * = adj * scaled_ppm / 65536000000 > */ > - adj *= scaled_ppm; > - diff = div_u64(adj, 8000000); > - diff = (diff >> 13) + ((diff >> 12) & 1); > + diff = mul_u64_u64_div_u64(adj, scaled_ppm, 32768000000); While building Linux next 20210325 tag for powerpc architecture ppc6xx_defconfig failed due to following warnings / errors. - powerpc (ppc6xx_defconfig) with gcc-8 - powerpc (ppc6xx_defconfig) with gcc-9 - powerpc (ppc6xx_defconfig) with gcc-10 make --silent --keep-going --jobs=8 O=/home/tuxbuild/.cache/tuxmake/builds/1/tmp ARCH=powerpc CROSS_COMPILE=powerpc64le-linux-gnu- 'CC=sccache powerpc64le-linux-gnu-gcc' 'HOSTCC=sccache gcc' INFO: Uncompressed kernel (size 0xd985ec) overlaps the address of the wrapper(0x400000) INFO: Fixing the link_address of wrapper to (0xe00000) INFO: Uncompressed kernel (size 0xd985ec) overlaps the address of the wrapper(0x400000) INFO: Fixing the link_address of wrapper to (0xe00000) INFO: Uncompressed kernel (size 0xd985ec) overlaps the address of the wrapper(0x400000) INFO: Fixing the link_address of wrapper to (0xe00000) ERROR: modpost: "mul_u64_u64_div_u64" [drivers/ptp/ptp-qoriq.ko] undefined! Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> Steps to reproduce: ------------------- # TuxMake is a command line tool and Python library that provides # portable and repeatable Linux kernel builds across a variety of # architectures, toolchains, kernel configurations, and make targets. # # TuxMake supports the concept of runtimes. # See https://docs.tuxmake.org/runtimes/, for that to work it requires # that you install podman or docker on your system. # # To install tuxmake on your system globally: # sudo pip3 install -U tuxmake # # See https://docs.tuxmake.org/ for complete documentation. tuxmake --runtime podman --target-arch powerpc --toolchain gcc-8 --kconfig ppc6xx_defconfig -- Linaro LKFT https://lkft.linaro.org
From: Guenter Roeck <linux@roeck-us.net> Date: Thu, 25 Mar 2021 03:23:07 -0700 > mul_u64_u64_div_u64() is not exported. As result, every build with > CONFIG_PTP_1588_CLOCK_QORIQ=m (ie every allmodconfig build) fails with: > > ERROR: modpost: "mul_u64_u64_div_u64" [drivers/ptp/ptp-qoriq.ko] undefined! > > or a similar error. I fixed this with a follow-up commit to export the symbol.
diff --git a/drivers/ptp/ptp_qoriq.c b/drivers/ptp/ptp_qoriq.c index 68beb1bd07c0..f7f220700cb5 100644 --- a/drivers/ptp/ptp_qoriq.c +++ b/drivers/ptp/ptp_qoriq.c @@ -189,15 +189,16 @@ int ptp_qoriq_adjfine(struct ptp_clock_info *ptp, long scaled_ppm) tmr_add = ptp_qoriq->tmr_add; adj = tmr_add; - /* calculate diff as adj*(scaled_ppm/65536)/1000000 - * and round() to the nearest integer + /* + * Calculate diff and round() to the nearest integer + * + * diff = adj * (ppb / 1000000000) + * = adj * scaled_ppm / 65536000000 */ - adj *= scaled_ppm; - diff = div_u64(adj, 8000000); - diff = (diff >> 13) + ((diff >> 12) & 1); + diff = mul_u64_u64_div_u64(adj, scaled_ppm, 32768000000); + diff = DIV64_U64_ROUND_UP(diff, 2); tmr_add = neg_adj ? tmr_add - diff : tmr_add + diff; - ptp_qoriq->write(®s->ctrl_regs->tmr_add, tmr_add); return 0;
Current calculation for diff of TMR_ADD register value may have 64-bit overflow in this code line, when long type scaled_ppm is large. adj *= scaled_ppm; This patch is to resolve it by using mul_u64_u64_div_u64(). Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> --- drivers/ptp/ptp_qoriq.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)