Re: [PATCH v8 net-next 14/15] net: dsa: netc: add support for the standardized counters

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Torn reads of 64-bit hardware statistics counters on 32-bit architectures
--

commit 9be701cf095ad465b1f7a0429d694425cf05049f
Author: Wei Fang <wei.fang@nxp.com>

net: dsa: netc: add support for the standardized counters

This commit adds support for reading RMON, MAC, pause, and control
statistics for the eMAC and pMAC on NXP NETC switch ports. The 64-bit
hardware counters are exposed through standard ethtool APIs.

> diff --git a/include/linux/fsl/netc_global.h b/include/linux/fsl/netc_global.h
> index fdecca8c90f0c..5b8ff528d3699 100644
> --- a/include/linux/fsl/netc_global.h
> +++ b/include/linux/fsl/netc_global.h
> @@ -5,6 +5,7 @@
>  #define __NETC_GLOBAL_H
>  
>  #include <linux/io.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>

[ ... ]

> +static inline u64 netc_read64(void __iomem *reg)
> +{
> +	return ioread64(reg);
> +}

Can this cause torn reads on 32-bit architectures?

Since linux/io-64-nonatomic-lo-hi.h is included, ioread64() will fall back
to ioread64_lo_hi() on 32-bit systems, which performs two independent 32-bit
reads (lower half, then upper half).

Because the MAC statistics are free-running hardware counters, if the lower
32 bits overflow between the two reads, the upper read will see the newly
incremented value. This could result in a spurious 4GB spike in the
aggregated value.

Should this implement a software retry loop to guarantee atomic reads of the
64-bit counters, similar to the pattern used in the ENETC driver?

do {
    high = ...;
    low = ...;
    tmp = ...;
} while (high != tmp);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518082506.1318236-1-wei.fang@nxp.com?part=14