Commit 6990dc39 authored by Chwee-Lin Choong's avatar Chwee-Lin Choong Committed by Tony Nguyen
Browse files

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: c789ad7c ("igc: Work around HW bug causing missing timestamps")
Suggested-by: default avatarAvi Shalev <avi.shalev@intel.com>
Reviewed-by: default avatarAleksandr Loktionov <aleksandr.loktionov@intel.com>
Co-developed-by: default avatarSong Yoong Siang <yoong.siang.song@intel.com>
Signed-off-by: default avatarSong Yoong Siang <yoong.siang.song@intel.com>
Signed-off-by: default avatarChwee-Lin Choong <chwee.lin.choong@intel.com>
Tested-by: default avatarAvigail Dahan <avigailx.dahan@intel.com>
Signed-off-by: default avatarTony Nguyen <anthony.l.nguyen@intel.com>
parent 41a9a682

drivers/net/ethernet/intel/igc/igc_ptp.c

+25 −18
Original line number Diff line number Diff line
@@ -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.

		 *

		 * 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.

		 * Hardware bug: Future timestamp interrupts won't fire unless

		 * TXSTMPH_0 is read, even if the timestamp was captured in

		 * registers 1-3.

		 *

		 * 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.

		 * 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.

		 *

		 * 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++) {