[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251127134927.2133-1-chwee.lin.choong@intel.com>
Date: Thu, 27 Nov 2025 21:49:27 +0800
From: Chwee-Lin Choong <chwee.lin.choong@...el.com>
To: Tony Nguyen <anthony.l.nguyen@...el.com>,
Przemek Kitszel <przemyslaw.kitszel@...el.com>,
Andrew Lunn <andrew+netdev@...n.ch>,
"David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Richard Cochran <richardcochran@...il.com>,
Vinicius Costa Gomes <vinicius.gomes@...el.com>
Cc: intel-wired-lan@...ts.osuosl.org,
netdev@...r.kernel.org,
linux-kernel@...r.kernel.org,
Avi Shalev <avi.shalev@...el.com>,
Song Yoong Siang <yoong.siang.song@...el.com>,
Chwee-Lin Choong <chwee.lin.choong@...el.com>
Subject: [PATCH iwl-net v2] igc: fix race condition in TX timestamp read for register 0
The current HW bug workaround checks the TXTT_0 ready bit first,
then reads TXSTMPL_0 twice (before and after reading TXSTMPH_0)
to detect whether a new timestamp was captured by timestamp
register 0 during the workaround.
This sequence has a race: if a new timestamp is captured after
checking the TXTT_0 bit but before the first TXSTMPL_0 read, the
detection fails because both the “old” and “new” values come from
the same timestamp.
Fix by reading TXSTMPL_0 first to establish a baseline, then
checking the TXTT_0 bit. This ensures any timestamp captured
during the race window will be detected.
Old sequence:
1. Check TXTT_0 ready bit
2. Read TXSTMPL_0 (baseline)
3. Read TXSTMPH_0 (interrupt workaround)
4. Read TXSTMPL_0 (detect changes vs baseline)
New sequence:
1. Read TXSTMPL_0 (baseline)
2. Check TXTT_0 ready bit
3. Read TXSTMPH_0 (interrupt workaround)
4. Read TXSTMPL_0 (detect changes vs baseline)
Fixes: c789ad7cbebc ("igc: Work around HW bug causing missing timestamps")
Suggested-by: Avi Shalev <avi.shalev@...el.com>
Signed-off-by: Song Yoong Siang <yoong.siang.song@...el.com>
Signed-off-by: Chwee-Lin Choong <chwee.lin.choong@...el.com>
---
v2: Added detailed comments explaining the hardware bug workaround and race
detection mechanism
---
drivers/net/ethernet/intel/igc/igc_ptp.c | 43 ++++++++++++++----------
export | 0
2 files changed, 25 insertions(+), 18 deletions(-)
create mode 100644 export
diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index b7b46d863bee..7aae83c108fd 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -774,36 +774,43 @@ static void igc_ptp_tx_reg_to_stamp(struct igc_adapter *adapter,
static void igc_ptp_tx_hwtstamp(struct igc_adapter *adapter)
{
struct igc_hw *hw = &adapter->hw;
+ u32 txstmpl_old;
u64 regval;
u32 mask;
int i;
+ /* Establish baseline of TXSTMPL_0 before checking TXTT_0.
+ * This baseline is used to detect if a new timestamp arrives in
+ * register 0 during the hardware bug workaround below.
+ */
+ txstmpl_old = rd32(IGC_TXSTMPL);
+
mask = rd32(IGC_TSYNCTXCTL) & IGC_TSYNCTXCTL_TXTT_ANY;
if (mask & IGC_TSYNCTXCTL_TXTT_0) {
regval = rd32(IGC_TXSTMPL);
regval |= (u64)rd32(IGC_TXSTMPH) << 32;
} else {
- /* There's a bug in the hardware that could cause
- * missing interrupts for TX timestamping. The issue
- * is that for new interrupts to be triggered, the
- * IGC_TXSTMPH_0 register must be read.
+ /* TXTT_0 not set - register 0 has no new timestamp initially.
+ *
+ * Hardware bug: Future timestamp interrupts won't fire unless
+ * TXSTMPH_0 is read, even if the timestamp was captured in
+ * registers 1-3.
*
- * To avoid discarding a valid timestamp that just
- * happened at the "wrong" time, we need to confirm
- * that there was no timestamp captured, we do that by
- * assuming that no two timestamps in sequence have
- * the same nanosecond value.
+ * Workaround: Read TXSTMPH_0 here to enable future interrupts.
+ * However, this read clears TXTT_0. If a timestamp arrives in
+ * register 0 after checking TXTT_0 but before this read, it
+ * would be lost.
*
- * So, we read the "low" register, read the "high"
- * register (to latch a new timestamp) and read the
- * "low" register again, if "old" and "new" versions
- * of the "low" register are different, a valid
- * timestamp was captured, we can read the "high"
- * register again.
+ * To detect this race: We saved a baseline read of TXSTMPL_0
+ * before TXTT_0 check. After performing the workaround read of
+ * TXSTMPH_0, we read TXSTMPL_0 again. Since consecutive
+ * timestamps never share the same nanosecond value, a change
+ * between the baseline and new TXSTMPL_0 indicates a timestamp
+ * arrived during the race window. If so, read the complete
+ * timestamp.
*/
- u32 txstmpl_old, txstmpl_new;
+ u32 txstmpl_new;
- txstmpl_old = rd32(IGC_TXSTMPL);
rd32(IGC_TXSTMPH);
txstmpl_new = rd32(IGC_TXSTMPL);
@@ -818,7 +825,7 @@ static void igc_ptp_tx_hwtstamp(struct igc_adapter *adapter)
done:
/* Now that the problematic first register was handled, we can
- * use retrieve the timestamps from the other registers
+ * retrieve the timestamps from the other registers
* (starting from '1') with less complications.
*/
for (i = 1; i < IGC_MAX_TX_TSTAMP_REGS; i++) {
diff --git a/export b/export
new file mode 100644
index 000000000000..e69de29bb2d1
--
2.43.0
Powered by blists - more mailing lists