* [PATCH net] net: stmmac: ensure PTP time register reads are consistent
@ 2022-02-03 16:00 ` Yannick Vignon
0 siblings, 0 replies; 10+ messages in thread
From: Yannick Vignon @ 2022-02-03 16:00 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S. Miller, Jakub Kicinski, Maxime Coquelin,
Rayagond Kokatanur, netdev, linux-stm32, linux-arm-kernel,
linux-kernel, sebastien.laveze, Vladimir Oltean, Xiaoliang Yang,
mingkai.hu, Joakim Zhang
Cc: Yannick Vignon
From: Yannick Vignon <yannick.vignon@nxp.com>
Even if protected from preemption and interrupts, a small time window
remains when the 2 register reads could return inconsistent values,
each time the "seconds" register changes. This could lead to an about
1-second error in the reported time.
Add logic to ensure the "seconds" and "nanoseconds" values are consistent.
Fixes: 92ba6888510c ("stmmac: add the support for PTP hw clock driver")
Signed-off-by: Yannick Vignon <yannick.vignon@nxp.com>
---
.../ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
index 074e2cdfb0fa..a7ec9f4d46ce 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
@@ -145,15 +145,20 @@ static int adjust_systime(void __iomem *ioaddr, u32 sec, u32 nsec,
static void get_systime(void __iomem *ioaddr, u64 *systime)
{
- u64 ns;
-
- /* Get the TSSS value */
- ns = readl(ioaddr + PTP_STNSR);
- /* Get the TSS and convert sec time value to nanosecond */
- ns += readl(ioaddr + PTP_STSR) * 1000000000ULL;
+ u64 ns, sec0, sec1;
+
+ /* Get the TSS value */
+ sec1 = readl_relaxed(ioaddr + PTP_STSR);
+ do {
+ sec0 = sec1;
+ /* Get the TSSS value */
+ ns = readl_relaxed(ioaddr + PTP_STNSR);
+ /* Get the TSS value */
+ sec1 = readl_relaxed(ioaddr + PTP_STSR);
+ } while (sec0 != sec1);
if (systime)
- *systime = ns;
+ *systime = ns + (sec1 * 1000000000ULL);
}
static void get_ptptime(void __iomem *ptpaddr, u64 *ptp_time)
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net] net: stmmac: ensure PTP time register reads are consistent
@ 2022-02-03 16:00 ` Yannick Vignon
0 siblings, 0 replies; 10+ messages in thread
From: Yannick Vignon @ 2022-02-03 16:00 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S. Miller, Jakub Kicinski, Maxime Coquelin,
Rayagond Kokatanur, netdev, linux-stm32, linux-arm-kernel,
linux-kernel, sebastien.laveze, Vladimir Oltean, Xiaoliang Yang,
mingkai.hu, Joakim Zhang
Cc: Yannick Vignon
From: Yannick Vignon <yannick.vignon@nxp.com>
Even if protected from preemption and interrupts, a small time window
remains when the 2 register reads could return inconsistent values,
each time the "seconds" register changes. This could lead to an about
1-second error in the reported time.
Add logic to ensure the "seconds" and "nanoseconds" values are consistent.
Fixes: 92ba6888510c ("stmmac: add the support for PTP hw clock driver")
Signed-off-by: Yannick Vignon <yannick.vignon@nxp.com>
---
.../ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
index 074e2cdfb0fa..a7ec9f4d46ce 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
@@ -145,15 +145,20 @@ static int adjust_systime(void __iomem *ioaddr, u32 sec, u32 nsec,
static void get_systime(void __iomem *ioaddr, u64 *systime)
{
- u64 ns;
-
- /* Get the TSSS value */
- ns = readl(ioaddr + PTP_STNSR);
- /* Get the TSS and convert sec time value to nanosecond */
- ns += readl(ioaddr + PTP_STSR) * 1000000000ULL;
+ u64 ns, sec0, sec1;
+
+ /* Get the TSS value */
+ sec1 = readl_relaxed(ioaddr + PTP_STSR);
+ do {
+ sec0 = sec1;
+ /* Get the TSSS value */
+ ns = readl_relaxed(ioaddr + PTP_STNSR);
+ /* Get the TSS value */
+ sec1 = readl_relaxed(ioaddr + PTP_STSR);
+ } while (sec0 != sec1);
if (systime)
- *systime = ns;
+ *systime = ns + (sec1 * 1000000000ULL);
}
static void get_ptptime(void __iomem *ptpaddr, u64 *ptp_time)
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net] net: stmmac: ensure PTP time register reads are consistent
2022-02-03 16:00 ` Yannick Vignon
@ 2022-02-03 16:28 ` Russell King (Oracle)
-1 siblings, 0 replies; 10+ messages in thread
From: Russell King (Oracle) @ 2022-02-03 16:28 UTC (permalink / raw)
To: Yannick Vignon
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S. Miller, Jakub Kicinski, Maxime Coquelin,
Rayagond Kokatanur, netdev, linux-stm32, linux-arm-kernel,
linux-kernel, sebastien.laveze, Vladimir Oltean, Xiaoliang Yang,
mingkai.hu, Joakim Zhang, Yannick Vignon
On Thu, Feb 03, 2022 at 05:00:25PM +0100, Yannick Vignon wrote:
> From: Yannick Vignon <yannick.vignon@nxp.com>
>
> Even if protected from preemption and interrupts, a small time window
> remains when the 2 register reads could return inconsistent values,
> each time the "seconds" register changes. This could lead to an about
> 1-second error in the reported time.
Have you checked whether the hardware protects against this (i.o.w. the
hardware latches the PTP_STSR value when PTP_STNSR is read, or vice
versa? Several PTP devices I've looked at do this to allow consistent
reading.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net] net: stmmac: ensure PTP time register reads are consistent
@ 2022-02-03 16:28 ` Russell King (Oracle)
0 siblings, 0 replies; 10+ messages in thread
From: Russell King (Oracle) @ 2022-02-03 16:28 UTC (permalink / raw)
To: Yannick Vignon
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S. Miller, Jakub Kicinski, Maxime Coquelin,
Rayagond Kokatanur, netdev, linux-stm32, linux-arm-kernel,
linux-kernel, sebastien.laveze, Vladimir Oltean, Xiaoliang Yang,
mingkai.hu, Joakim Zhang, Yannick Vignon
On Thu, Feb 03, 2022 at 05:00:25PM +0100, Yannick Vignon wrote:
> From: Yannick Vignon <yannick.vignon@nxp.com>
>
> Even if protected from preemption and interrupts, a small time window
> remains when the 2 register reads could return inconsistent values,
> each time the "seconds" register changes. This could lead to an about
> 1-second error in the reported time.
Have you checked whether the hardware protects against this (i.o.w. the
hardware latches the PTP_STSR value when PTP_STNSR is read, or vice
versa? Several PTP devices I've looked at do this to allow consistent
reading.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net] net: stmmac: ensure PTP time register reads are consistent
2022-02-03 16:28 ` Russell King (Oracle)
@ 2022-02-03 16:38 ` Yannick Vignon
-1 siblings, 0 replies; 10+ messages in thread
From: Yannick Vignon @ 2022-02-03 16:38 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S. Miller, Jakub Kicinski, Maxime Coquelin,
Rayagond Kokatanur, netdev, linux-stm32, linux-arm-kernel,
linux-kernel, sebastien.laveze, Vladimir Oltean, Xiaoliang Yang,
mingkai.hu, Joakim Zhang, Yannick Vignon
On 2/3/2022 5:28 PM, Russell King (Oracle) wrote:
> On Thu, Feb 03, 2022 at 05:00:25PM +0100, Yannick Vignon wrote:
>> From: Yannick Vignon <yannick.vignon@nxp.com>
>>
>> Even if protected from preemption and interrupts, a small time window
>> remains when the 2 register reads could return inconsistent values,
>> each time the "seconds" register changes. This could lead to an about
>> 1-second error in the reported time.
>
> Have you checked whether the hardware protects against this (i.o.w. the
> hardware latches the PTP_STSR value when PTP_STNSR is read, or vice
> versa? Several PTP devices I've looked at do this to allow consistent
> reading.
>
It doesn't. I was able to observe inconsistent values doing reads in
either order, and we had already observed the issue with that same IP on
another device (Cortex-M based, not running Linux). It's not easy to
reproduce, the time window is small, but it's there.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net] net: stmmac: ensure PTP time register reads are consistent
@ 2022-02-03 16:38 ` Yannick Vignon
0 siblings, 0 replies; 10+ messages in thread
From: Yannick Vignon @ 2022-02-03 16:38 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S. Miller, Jakub Kicinski, Maxime Coquelin,
Rayagond Kokatanur, netdev, linux-stm32, linux-arm-kernel,
linux-kernel, sebastien.laveze, Vladimir Oltean, Xiaoliang Yang,
mingkai.hu, Joakim Zhang, Yannick Vignon
On 2/3/2022 5:28 PM, Russell King (Oracle) wrote:
> On Thu, Feb 03, 2022 at 05:00:25PM +0100, Yannick Vignon wrote:
>> From: Yannick Vignon <yannick.vignon@nxp.com>
>>
>> Even if protected from preemption and interrupts, a small time window
>> remains when the 2 register reads could return inconsistent values,
>> each time the "seconds" register changes. This could lead to an about
>> 1-second error in the reported time.
>
> Have you checked whether the hardware protects against this (i.o.w. the
> hardware latches the PTP_STSR value when PTP_STNSR is read, or vice
> versa? Several PTP devices I've looked at do this to allow consistent
> reading.
>
It doesn't. I was able to observe inconsistent values doing reads in
either order, and we had already observed the issue with that same IP on
another device (Cortex-M based, not running Linux). It's not easy to
reproduce, the time window is small, but it's there.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net] net: stmmac: ensure PTP time register reads are consistent
2022-02-03 16:38 ` Yannick Vignon
@ 2022-02-03 16:44 ` Russell King (Oracle)
-1 siblings, 0 replies; 10+ messages in thread
From: Russell King (Oracle) @ 2022-02-03 16:44 UTC (permalink / raw)
To: Yannick Vignon
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S. Miller, Jakub Kicinski, Maxime Coquelin,
Rayagond Kokatanur, netdev, linux-stm32, linux-arm-kernel,
linux-kernel, sebastien.laveze, Vladimir Oltean, Xiaoliang Yang,
mingkai.hu, Joakim Zhang, Yannick Vignon
On Thu, Feb 03, 2022 at 05:38:10PM +0100, Yannick Vignon wrote:
> On 2/3/2022 5:28 PM, Russell King (Oracle) wrote:
> > On Thu, Feb 03, 2022 at 05:00:25PM +0100, Yannick Vignon wrote:
> > > From: Yannick Vignon <yannick.vignon@nxp.com>
> > >
> > > Even if protected from preemption and interrupts, a small time window
> > > remains when the 2 register reads could return inconsistent values,
> > > each time the "seconds" register changes. This could lead to an about
> > > 1-second error in the reported time.
> >
> > Have you checked whether the hardware protects against this (i.o.w. the
> > hardware latches the PTP_STSR value when PTP_STNSR is read, or vice
> > versa? Several PTP devices I've looked at do this to allow consistent
> > reading.
> >
>
> It doesn't. I was able to observe inconsistent values doing reads in either
> order, and we had already observed the issue with that same IP on another
> device (Cortex-M based, not running Linux). It's not easy to reproduce, the
> time window is small, but it's there.
Okay, thanks.
Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net] net: stmmac: ensure PTP time register reads are consistent
@ 2022-02-03 16:44 ` Russell King (Oracle)
0 siblings, 0 replies; 10+ messages in thread
From: Russell King (Oracle) @ 2022-02-03 16:44 UTC (permalink / raw)
To: Yannick Vignon
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S. Miller, Jakub Kicinski, Maxime Coquelin,
Rayagond Kokatanur, netdev, linux-stm32, linux-arm-kernel,
linux-kernel, sebastien.laveze, Vladimir Oltean, Xiaoliang Yang,
mingkai.hu, Joakim Zhang, Yannick Vignon
On Thu, Feb 03, 2022 at 05:38:10PM +0100, Yannick Vignon wrote:
> On 2/3/2022 5:28 PM, Russell King (Oracle) wrote:
> > On Thu, Feb 03, 2022 at 05:00:25PM +0100, Yannick Vignon wrote:
> > > From: Yannick Vignon <yannick.vignon@nxp.com>
> > >
> > > Even if protected from preemption and interrupts, a small time window
> > > remains when the 2 register reads could return inconsistent values,
> > > each time the "seconds" register changes. This could lead to an about
> > > 1-second error in the reported time.
> >
> > Have you checked whether the hardware protects against this (i.o.w. the
> > hardware latches the PTP_STSR value when PTP_STNSR is read, or vice
> > versa? Several PTP devices I've looked at do this to allow consistent
> > reading.
> >
>
> It doesn't. I was able to observe inconsistent values doing reads in either
> order, and we had already observed the issue with that same IP on another
> device (Cortex-M based, not running Linux). It's not easy to reproduce, the
> time window is small, but it's there.
Okay, thanks.
Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net] net: stmmac: ensure PTP time register reads are consistent
2022-02-03 16:00 ` Yannick Vignon
@ 2022-02-03 22:10 ` patchwork-bot+netdevbpf
-1 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-02-03 22:10 UTC (permalink / raw)
To: Yannick Vignon
Cc: peppe.cavallaro, alexandre.torgue, joabreu, davem, kuba,
mcoquelin.stm32, rayagond, netdev, linux-stm32, linux-arm-kernel,
linux-kernel, sebastien.laveze, olteanv, xiaoliang.yang_1,
mingkai.hu, qiangqing.zhang, yannick.vignon
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 3 Feb 2022 17:00:25 +0100 you wrote:
> From: Yannick Vignon <yannick.vignon@nxp.com>
>
> Even if protected from preemption and interrupts, a small time window
> remains when the 2 register reads could return inconsistent values,
> each time the "seconds" register changes. This could lead to an about
> 1-second error in the reported time.
>
> [...]
Here is the summary with links:
- [net] net: stmmac: ensure PTP time register reads are consistent
https://git.kernel.org/netdev/net/c/80d4609008e6
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net] net: stmmac: ensure PTP time register reads are consistent
@ 2022-02-03 22:10 ` patchwork-bot+netdevbpf
0 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-02-03 22:10 UTC (permalink / raw)
To: Yannick Vignon
Cc: peppe.cavallaro, alexandre.torgue, joabreu, davem, kuba,
mcoquelin.stm32, rayagond, netdev, linux-stm32, linux-arm-kernel,
linux-kernel, sebastien.laveze, olteanv, xiaoliang.yang_1,
mingkai.hu, qiangqing.zhang, yannick.vignon
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 3 Feb 2022 17:00:25 +0100 you wrote:
> From: Yannick Vignon <yannick.vignon@nxp.com>
>
> Even if protected from preemption and interrupts, a small time window
> remains when the 2 register reads could return inconsistent values,
> each time the "seconds" register changes. This could lead to an about
> 1-second error in the reported time.
>
> [...]
Here is the summary with links:
- [net] net: stmmac: ensure PTP time register reads are consistent
https://git.kernel.org/netdev/net/c/80d4609008e6
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-02-03 22:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-03 16:00 [PATCH net] net: stmmac: ensure PTP time register reads are consistent Yannick Vignon
2022-02-03 16:00 ` Yannick Vignon
2022-02-03 16:28 ` Russell King (Oracle)
2022-02-03 16:28 ` Russell King (Oracle)
2022-02-03 16:38 ` Yannick Vignon
2022-02-03 16:38 ` Yannick Vignon
2022-02-03 16:44 ` Russell King (Oracle)
2022-02-03 16:44 ` Russell King (Oracle)
2022-02-03 22:10 ` patchwork-bot+netdevbpf
2022-02-03 22:10 ` patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.