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@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Co-developed-by: Song Yoong Siang <yoong.siang.song@intel.com>
Signed-off-by: Song Yoong Siang <yoong.siang.song@intel.com>
Signed-off-by: Chwee-Lin Choong <chwee.lin.choong@intel.com>
Tested-by: Avigail Dahan <avigailx.dahan@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>

diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index b7b46d863bee44ef99ac65ab7bd306501b1ef394..7aae83c108fd7611b00bf075592f93a902b83422 100644 (file)
--- 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++) {