Netdev List
 help / color / mirror / Atom feed
From: Jacob Keller <jacob.e.keller@intel.com>
To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
Cc: netdev@vger.kernel.org,
	 Maciej Machnikowski <maciej.machnikowski@intel.com>,
	 Anthony Nguyen <anthony.l.nguyen@intel.com>,
	 Przemyslaw Korba <przemyslaw.korba@intel.com>,
	 Grzegorz Nitka <grzegorz.nitka@intel.com>,
	Petr Oros <poros@redhat.com>,
	 alexander.nowlin@intel.com, kevin.bross@intel.com,
	ranjit.cavatur@intel.com,
	 Jacob Keller <jacob.e.keller@intel.com>,
	 Maciek Machnikowski <maciej.machnikowski@intel.com>,
	 Karol Kolacinski <karol.kolacinski@intel.com>,
	 Aleksandr Loktionov <aleksandr.loktionov@intel.com>,
	 Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>,
	 Przemyslaw Korba <przemyslaw.korba@intel.com>
Subject: [PATCH iwl-net v2 00/14] ice: E82x: timestamp processing logic fixes
Date: Tue, 25 Aug 2026 15:53:24 -0700	[thread overview]
Message-ID: <20260825-jk-e825c-minimized-fixes-v2-0-8223f95d26e3@intel.com> (raw)

This series contains several related fixes for the ice driver PTP logic
relating to timestamp handling and device (re)initialization.

Of particular note is some changes around the handling of timestamps that
are requested near device state changes such as administrative up/down
cycles and link change events.

The E825 device logic in the PHY has an internal counter which is used as
part of the "threshold" logic which determines when the device will trigger
an interrupt signal from a given PHY port to the MAC. This internal counter
requires some precise handling to ensure that the internal device state
remains in sync with software expectations. Otherwise, the device can be
finagled into a state where the PHY stops producing new timestamp interrupt
notifications to the MAC indefinitely. This in turn degrades the timestamp
processing latency and results in application failures for common
timestamping applications such as ptp4l.

There are four major categories of problem resolved by this series:

 * Timestamp requests made while the PHY_REG_TX_OFFSET_READY bit is cleared
   will increment the internal counter, but leave their valid bit set to 0.
   Upon read, the counter is not decremented. This leads to a desync of the
   counter and blocks the PHY interrupt.

 * Software logic for tracking timestamps incorrectly cleared in-use bits
   without waiting for completion in certain cases. If the timestamp *does*
   later complete, it leaves an "orphaned" ready bit which is not
   tracked by software. This results in the internal counter becoming
   desynced if that index is re-used.

 * The PHY timestamp memory region lacks pull-down zero-initialization at
   power on, resulting in uninitialized random data in the memory region.
   If software reads these values, an entry with its valid bit set to 1 can
   trigger a counter decrement and cause an underflow which results in the
   counter becoming desynced.

 * Timestamp request which complete near the beginning of the PHY losing
   link can become "stuck" such that the hardware logic triggered by a
   timestamp read does not activate. The memory status bit and the valid
   bit in the the timestamp index are not cleared. This window where this
   may occur begins *before* the firmware notifies the driver of link loss.
   When this occurs, the driver may accidentally re-use a stale timestamp,
   and the IRQ re-trigger logic triggers a repeated IRQ "storm" that can
   consume significant excess CPU time.

The series' primary focus is towards preventing driver flows that can
trigger the above sequences. It is based on work from Przemyslaw Korba which
was previously posted at [1]. During that series development, Petr from
RedHat reported the 3rd issue mentioned above. While attempting to root
cause that issue, several other issues were uncovered and those fixes have
also been included in this series.

First, the locking around the PTP ports list in the adapter structure is
converted to use RCU primitives and a spinlock, resolving a couple of
reports from Petr about places where the original list was accessed without
lock protection. Note that an older version of this fix used an xarray
instead of the list. The xarray has more overhead and results in an
increase of ~25 microseconds to the average latency for processing Tx
timestamps. The list is simpler and avoids this overhead.

Next, the PTP reset flow is fixed to stop tearing down the Tx tracker
during a CORE or GLOBAL reset. This avoids causing Tx timestamps to break
permanently after such a reset. This issue was found by Sashiko during
review of a previous version of this series.

Next, the ice_ptp_request_ts() function is updated to sequence the marking
of the in_use bitmap in order to work properly with the lockless reader in
the IRQ thread. This issue was reported by Sashiko during review of a
previous version of this series.

Next, come two fixes for E822 hardware that were originally posted as part
of Przemyslaw Korba's work [1]. The E822-only "vernier" offset validation
work task is properly canceled during device reset, and new timestamp
requests are kept disabled until the validation task completes.

Next, Arkadiusz modifies the driver to stop pretending that the link has
gone down during ice_down(). This removes "virtual" PTP link changes that
occurred on several flows including MTU change, Eswitch setup, and others.
Now, the driver only triggers a PTP PHY timer reinitialization when the
physical PHY link has changed instead of during many other actions.

Next, the driver is modified to stop clearing the PHY_REG_TX_OFFSET_READY
bit. This bits only purpose is to tell hardware to mark any captured
timestamps as invalid. Since this also disables the necessary side effects
on read it is problematic to have cleared. According to hardware engineers,
keeping it enabled should not have any other side effects.

Next, the driver is modified to clear the PHY_REG_TX_MEMORY_STATUS by
reading each index *prior* to the PHY soft reset. This ensures that any
stale or invalid data left in the memory array is cleared, followed by the
counter being reset via the PHY soft reset procedure.

Next, the E825 timer start procedure is modified to first include a soft
reset. This ensures that upon link up the device is reconfigured from a
known-good state with its internal counter reset and everything cleared.

Next, Petr modifies the ice_ptp_flush_tx_tracker() function to wait a little
bit for any outstanding timestamps before flushing.

Next, Petr modifies the ice_ptp_process_tx_tstamp() function to avoid
releasing any index from software unless either a) it is actually completed
by hardware or b) it is timed out waiting for a full two seconds. This
closes the final gap from the second issue mentioned above. Instead of
immediately releasing the index, the software now waits until hardware has
completed it or the driver has waited long enough to be sufficiently sure
that no such timestamp will be done.

Next, the driver is modified to no longer mark timestamps as "stale" during
a clock adjust event. This avoids marking timestamps as stale unnecessarily.

Finally, the ice_ptp_process_tx_tstamp() function is modified to verify
that hardware actually cleared the ready bitmap. This ensures that we do
not report false timestamps near a link down event.

Link: [1] https://lore.kernel.org/intel-wired-lan/20260720120151.2675206-1-przemyslaw.korba@intel.com/
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Changes in v2:
- Two new fixes based on feedback from Sashiko review, including a reset
  issue surfaced by a CORE or GLOBAL reset, as well as a subtle issue with
  sequencing of the in_use bitmap.
- Fix the sequencing of kref_put when iterating the port list.
- Add kref_get/kref_put on ice_ptp_link_change to ensure that a link change
  happening concurrently with a teardown won't trigger use-after-free
  accesses.
- Avoid re-enabling Tx timestamps for E822 devices when the
  ice_start_phy_timer_e82x() function fails.
- Always trigger a link state refresh in ice_rebuild() instead of only
  conditionally, ensuring that after a reset the device is always
  reconfigured appropriately.
- Return the error if ice_clear_ptp_tstamp_eth56g() fails to read the PHY
  register, instead of silently ignoring it.
- Mark timestamps stale if we fail to read the ice_get_phy_tx_tstamp_ready
  bitmap, in addition to if the bitmap fails to clear.
- Check the return value of ice_check_phy_tx_tstamp_ready() to avoid
  triggering an IRQ if we fail to access the PHY.
- Link to v1: https://patch.msgid.link/20260821-jk-e825c-minimized-fixes-v1-0-9d0731eb4858@intel.com

---
Arkadiusz Kubalewski (1):
      ice: call PTP link change only from link events

Jacob Keller (9):
      ice: use reference counting and RCU for PTP port access
      ice: fix removal of PTP timestamp tracker during reset
      ice: set in_use only after preparing Tx timestamp index
      ice: E825: stop clearing PHY_REG_TX_OFFSET_READY
      ice: E825: clear PHY_REG_TX_MEMORY_STATUS prior to soft reset
      ice: E825: perform a soft reset when starting the PHY timer
      ice: remove unnecessary discarding of timestamps after clock adjust
      ice: skip reading Tx ready bitmap on ports with no timestamps
      ice: don't clear in_use until HW clears ready bitmap

Karol Kolacinski (2):
      ice: E822: keep Tx timestamps disabled during offset calibration
      ice: E822: cancel offset verification work during reset preparation

Petr Oros (2):
      ice: wait for in-flight Tx timestamps before flushing the tracker
      ice: keep Tx timestamp slots tracked until completion or timeout

 drivers/net/ethernet/intel/ice/ice_adapter.h |   6 +-
 drivers/net/ethernet/intel/ice/ice_ptp.h     |  12 +-
 drivers/net/ethernet/intel/ice/ice_adapter.c |   7 +-
 drivers/net/ethernet/intel/ice/ice_main.c    |  12 +-
 drivers/net/ethernet/intel/ice/ice_ptp.c     | 363 ++++++++++++++++++---------
 drivers/net/ethernet/intel/ice/ice_ptp_hw.c  | 104 ++++----
 6 files changed, 328 insertions(+), 176 deletions(-)
---
base-commit: dc4b95b8fee95113587e93ca116356032d271371
change-id: 20260819-jk-e825c-minimized-fixes-8559addec172

Best regards,
--  
Jacob Keller <jacob.e.keller@intel.com>


             reply	other threads:[~2026-08-25 22:55 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-25 22:53 Jacob Keller [this message]
2026-08-25 22:53 ` [PATCH iwl-net v2 01/14] ice: use reference counting and RCU for PTP port access Jacob Keller
2026-08-25 22:53 ` [PATCH iwl-net v2 02/14] ice: fix removal of PTP timestamp tracker during reset Jacob Keller
2026-08-25 22:53 ` [PATCH iwl-net v2 03/14] ice: set in_use only after preparing Tx timestamp index Jacob Keller
2026-08-25 22:53 ` [PATCH iwl-net v2 04/14] ice: E822: keep Tx timestamps disabled during offset calibration Jacob Keller
2026-08-25 22:53 ` [PATCH iwl-net v2 05/14] ice: E822: cancel offset verification work during reset preparation Jacob Keller
2026-08-25 22:53 ` [PATCH iwl-net v2 06/14] ice: call PTP link change only from link events Jacob Keller
2026-08-25 22:53 ` [PATCH iwl-net v2 07/14] ice: E825: stop clearing PHY_REG_TX_OFFSET_READY Jacob Keller
2026-08-25 22:53 ` [PATCH iwl-net v2 08/14] ice: E825: clear PHY_REG_TX_MEMORY_STATUS prior to soft reset Jacob Keller
2026-08-25 22:53 ` [PATCH iwl-net v2 09/14] ice: E825: perform a soft reset when starting the PHY timer Jacob Keller
2026-08-25 22:53 ` [PATCH iwl-net v2 10/14] ice: wait for in-flight Tx timestamps before flushing the tracker Jacob Keller
2026-08-25 22:53 ` [PATCH iwl-net v2 11/14] ice: keep Tx timestamp slots tracked until completion or timeout Jacob Keller
2026-08-25 22:53 ` [PATCH iwl-net v2 12/14] ice: remove unnecessary discarding of timestamps after clock adjust Jacob Keller
2026-08-25 22:53 ` [PATCH iwl-net v2 13/14] ice: skip reading Tx ready bitmap on ports with no timestamps Jacob Keller
2026-08-25 22:53 ` [PATCH iwl-net v2 14/14] ice: don't clear in_use until HW clears ready bitmap Jacob Keller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260825-jk-e825c-minimized-fixes-v2-0-8223f95d26e3@intel.com \
    --to=jacob.e.keller@intel.com \
    --cc=aleksandr.loktionov@intel.com \
    --cc=alexander.nowlin@intel.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=arkadiusz.kubalewski@intel.com \
    --cc=grzegorz.nitka@intel.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=karol.kolacinski@intel.com \
    --cc=kevin.bross@intel.com \
    --cc=maciej.machnikowski@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=poros@redhat.com \
    --cc=przemyslaw.korba@intel.com \
    --cc=ranjit.cavatur@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox