[PATCH iwl-next v3 0/2] idpf: add direct access for PHC control

[PATCH iwl-next v3 0/2] idpf: add direct access for PHC control

[Przemyslaw Korba]

IDPF allows to access the clock through virtchnl messages, or directly,
through PCI BAR registers. Registers offsets are negotiated with the
Control Plane during driver initialization process.

This series adds support for the direct mode, in which the main timer is
disciplined and the Tx timestamps are read straight from the BAR
registers, instead of sending a virtchnl message to the Control Plane.

Patch 1 adds the direct access to discipline the main timer.

Patch 2 adds the direct readout of the Tx timestamps. The latches are
collected by a timer that runs for as long as the Tx timestamping is
enabled, so no register access is added to the Tx and the Tx completion
paths. Latches whose timestamp does not show up within 500 ms are
reclaimed and counted as lost in the HW timestamping statistics.

v3:
* Cancel the Tx timestamp timer before starting it again, so that it is
  never re-armed while its callback is running (patch 2).
* Keep the readiness bit set when the latch holds no valid timestamp yet,
  so that the latch is read again on the next tick (patch 2).

v2:
* Use explicit spin_lock()/spin_unlock() instead of guard() (patches 1, 2).
* Do not drop the mapped cmd_sync address when the cross timestamp one
  fails to map, so direct mode never writes to a NULL register (patch 1).
* Check the Tx timestamping state under the latches lock when handing out
  a latch, to close the race with disabling it (patch 2).
* Stop the timer and flush the pending latches when the Tx timestamping is
  disabled or the vport is stopped, restart it on open (patch 2).

Przemyslaw Korba (2):
  idpf: add direct access to discipline the main timer
  idpf: add direct Tx timestamp readout

 drivers/net/ethernet/intel/idpf/idpf.h        |   6 +
 drivers/net/ethernet/intel/idpf/idpf_dev.c    |   9 +-
 .../net/ethernet/intel/idpf/idpf_ethtool.c    |   8 +-
 .../ethernet/intel/idpf/idpf_lan_pf_regs.h    |   3 +
 drivers/net/ethernet/intel/idpf/idpf_lib.c    |   2 +
 drivers/net/ethernet/intel/idpf/idpf_ptp.c    | 673 ++++++++++++++++--
 drivers/net/ethernet/intel/idpf/idpf_ptp.h    | 130 +++-
 drivers/net/ethernet/intel/idpf/idpf_txrx.c   |  23 +-
 .../net/ethernet/intel/idpf/idpf_virtchnl.c   |  12 +-
 .../ethernet/intel/idpf/idpf_virtchnl_ptp.c   | 201 +++++-
 include/linux/net/intel/virtchnl2.h           |   6 +-
 11 files changed, 935 insertions(+), 138 deletions(-)


base-commit: ff2563fbe0e6d28f561d58012efb9b47595fd933
-- 
2.47.3

[PATCH iwl-next v3 1/2] idpf: add direct access to discipline the main timer

[Przemyslaw Korba]

IDPF allows to access the clock through virtchnl messages, or directly,
through PCI BAR registers. Registers offsets are negotiated with the
Control Plane during driver initialization process.
Add support for direct operations to modify the clock.

Co-developed-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
Reviewed-by: Naman Gulati <namangulati@google.com>
Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf_dev.c    |   9 +-
 .../ethernet/intel/idpf/idpf_lan_pf_regs.h    |   3 +
 drivers/net/ethernet/intel/idpf/idpf_ptp.c    | 270 +++++++++++++++---
 drivers/net/ethernet/intel/idpf/idpf_ptp.h    |  78 +++--
 .../ethernet/intel/idpf/idpf_virtchnl_ptp.c   | 115 ++++++--
 5 files changed, 388 insertions(+), 87 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_dev.c b/drivers/net/ethernet/intel/idpf/idpf_dev.c
index 61f5e8e4d20b..1518555d680c 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_dev.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_dev.c
@@ -150,8 +150,13 @@ static void idpf_trigger_reset(struct idpf_adapter *adapter,
  */
 static void idpf_ptp_reg_init(const struct idpf_adapter *adapter)
 {
-	adapter->ptp->cmd.shtime_enable_mask = PF_GLTSYN_CMD_SYNC_SHTIME_EN_M;
-	adapter->ptp->cmd.exec_cmd_mask = PF_GLTSYN_CMD_SYNC_EXEC_CMD_M;
+	adapter->ptp->cmd = (struct idpf_ptp_cmd_mask) {
+		.shtime_enable	= PF_GLTSYN_CMD_SYNC_SHTIME_EN_M,
+		.exec_cmd	= PF_GLTSYN_CMD_SYNC_EXEC_CMD_M,
+		.init_time	= PF_GLTSYN_CMD_INIT_TIME_M,
+		.init_incval	= PF_GLTSYN_CMD_INIT_INCVAL_M,
+		.adj_time	= PF_GLTSYN_CMD_ADJ_TIME_M,
+	};
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lan_pf_regs.h b/drivers/net/ethernet/intel/idpf/idpf_lan_pf_regs.h
index cc9aa2b6a14a..b2d786584dbd 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lan_pf_regs.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_lan_pf_regs.h
@@ -56,6 +56,9 @@
 /* Timesync registers */
 #define PF_GLTSYN_CMD_SYNC_EXEC_CMD_M	GENMASK(1, 0)
 #define PF_GLTSYN_CMD_SYNC_SHTIME_EN_M	BIT(2)
+#define PF_GLTSYN_CMD_INIT_TIME_M	BIT(0)
+#define PF_GLTSYN_CMD_INIT_INCVAL_M	BIT(1)
+#define PF_GLTSYN_CMD_ADJ_TIME_M	BIT(2)
 
 /* Interrupts */
 #define PF_GLINT_BASE			0x08900000
diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
index 9b15973d2f7e..df2da430ff28 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
@@ -23,6 +23,40 @@ idpf_ptp_get_access(const struct idpf_adapter *adapter, u32 direct, u32 mailbox)
 		return IDPF_PTP_NONE;
 }
 
+/**
+ * idpf_ptp_check_features_access - Validate dependent PTP access modes
+ * @adapter: Driver specific private structure
+ *
+ * Some operations reuse the register set of another one, so they cannot use
+ * the direct access on their own:
+ *  - adjusting the device clock and the cross timestamping both read the
+ *    device clock registers,
+ *  - setting the device clock time additionally drives the same shadow
+ *    registers and command interface as the clock adjustment.
+ * Downgrade such an operation to the mailbox access, or disable it, when the
+ * operation it depends on does not use the direct access.
+ */
+void idpf_ptp_check_features_access(const struct idpf_adapter *adapter)
+{
+	struct idpf_ptp *ptp = adapter->ptp;
+
+	if (ptp->get_dev_clk_time_access != IDPF_PTP_DIRECT) {
+		ptp->adj_dev_clk_time_access =
+			idpf_ptp_mb_access(ptp,
+					   VIRTCHNL2_CAP_PTP_ADJ_DEVICE_CLK_MB);
+		ptp->get_cross_tstamp_access =
+			idpf_ptp_mb_access(ptp,
+					   VIRTCHNL2_CAP_PTP_GET_CROSS_TIME_MB);
+	}
+
+	if (ptp->adj_dev_clk_time_access == IDPF_PTP_DIRECT)
+		return;
+
+	ptp->set_dev_clk_time_access =
+		idpf_ptp_mb_access(ptp,
+				   VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME_MB);
+}
+
 /**
  * idpf_ptp_get_features_access - Determine the access type of PTP features
  * @adapter: Driver specific private structure
@@ -69,6 +103,8 @@ void idpf_ptp_get_features_access(const struct idpf_adapter *adapter)
 	ptp->tx_tstamp_access = idpf_ptp_get_access(adapter,
 						    direct,
 						    mailbox);
+
+	idpf_ptp_check_features_access(adapter);
 }
 
 /**
@@ -80,14 +116,39 @@ static void idpf_ptp_enable_shtime(struct idpf_adapter *adapter)
 	u32 shtime_enable, exec_cmd;
 
 	/* Get offsets */
-	shtime_enable = adapter->ptp->cmd.shtime_enable_mask;
-	exec_cmd = adapter->ptp->cmd.exec_cmd_mask;
+	shtime_enable = adapter->ptp->cmd.shtime_enable;
+	exec_cmd = adapter->ptp->cmd.exec_cmd;
 
 	/* Set the shtime en and the sync field */
 	writel(shtime_enable, adapter->ptp->dev_clk_regs.cmd_sync);
 	writel(exec_cmd | shtime_enable, adapter->ptp->dev_clk_regs.cmd_sync);
 }
 
+/**
+ * idpf_ptp_tmr_cmd - Prepare and trigger a timer sync command
+ * @adapter: Driver specific private structure
+ * @cmd: Command to be executed
+ *
+ * This function must be serialized with idpf_ptp_enable_shtime() which also
+ * manipulates cmd_sync register. The read_dev_clk_lock provides this
+ * serialization.
+ */
+static void idpf_ptp_tmr_cmd(struct idpf_adapter *adapter, u32 cmd)
+{
+	struct idpf_ptp *ptp = adapter->ptp;
+
+	/* Lock is acquired by the caller to serialize shadow register setup
+	 * with the command trigger.
+	 */
+	lockdep_assert_held(&ptp->read_dev_clk_lock);
+
+	writel(cmd, ptp->dev_clk_regs.cmd);
+	writel(cmd, ptp->dev_clk_regs.phy_cmd);
+	writel(ptp->cmd.exec_cmd, ptp->dev_clk_regs.cmd_sync);
+	writel(0, ptp->dev_clk_regs.cmd);
+	writel(0, ptp->dev_clk_regs.phy_cmd);
+}
+
 /**
  * idpf_ptp_read_src_clk_reg_direct - Read directly the main timer value
  * @adapter: Driver specific private structure
@@ -139,7 +200,7 @@ static int idpf_ptp_read_src_clk_reg_mailbox(struct idpf_adapter *adapter,
 	/* Read the system timestamp pre PHC read */
 	ptp_read_system_prets(sts);
 
-	err = idpf_ptp_get_dev_clk_time(adapter, &clk_time);
+	err = idpf_ptp_get_dev_clk_time_mb(adapter, &clk_time);
 	if (err)
 		return err;
 
@@ -223,7 +284,7 @@ static int idpf_ptp_get_sync_device_time_mailbox(struct idpf_adapter *adapter,
 	struct idpf_ptp_dev_timers cross_time;
 	int err;
 
-	err = idpf_ptp_get_cross_time(adapter, &cross_time);
+	err = idpf_ptp_get_cross_time_mb(adapter, &cross_time);
 	if (err)
 		return err;
 
@@ -413,6 +474,35 @@ static int idpf_ptp_update_cached_phctime(struct idpf_adapter *adapter)
 	return 0;
 }
 
+/**
+ * idpf_ptp_set_dev_clk_time_direct- Set the time of the clock directly through
+ *				     BAR registers.
+ * @adapter: Driver specific private structure
+ * @dev_clk_time: Value expressed in nanoseconds to set
+ *
+ * Set the time of the device clock to provided value directly through BAR
+ * registers received during PTP capabilities negotiation.
+ */
+static void idpf_ptp_set_dev_clk_time_direct(struct idpf_adapter *adapter,
+					     u64 dev_clk_time)
+{
+	struct idpf_ptp *ptp = adapter->ptp;
+	u32 dev_clk_time_l = lower_32_bits(dev_clk_time);
+	u32 dev_clk_time_h = upper_32_bits(dev_clk_time);
+
+	spin_lock(&ptp->read_dev_clk_lock);
+
+	writel(dev_clk_time_l, ptp->dev_clk_regs.dev_clk_ns_l);
+	writel(dev_clk_time_h, ptp->dev_clk_regs.dev_clk_ns_h);
+
+	writel(dev_clk_time_l, ptp->dev_clk_regs.phy_clk_ns_l);
+	writel(dev_clk_time_h, ptp->dev_clk_regs.phy_clk_ns_h);
+
+	idpf_ptp_tmr_cmd(adapter, ptp->cmd.init_time);
+
+	spin_unlock(&ptp->read_dev_clk_lock);
+}
+
 /**
  * idpf_ptp_settime64 - Set the time of the clock
  * @info: the driver's PTP info structure
@@ -432,16 +522,23 @@ static int idpf_ptp_settime64(struct ptp_clock_info *info,
 	u64 ns;
 
 	access = adapter->ptp->set_dev_clk_time_access;
-	if (access != IDPF_PTP_MAILBOX)
-		return -EOPNOTSUPP;
-
 	ns = timespec64_to_ns(ts);
 
-	err = idpf_ptp_set_dev_clk_time(adapter, ns);
-	if (err) {
-		pci_err(adapter->pdev, "Failed to set the time, err: %pe\n",
-			ERR_PTR(err));
-		return err;
+	switch (access) {
+	case IDPF_PTP_MAILBOX:
+		err = idpf_ptp_set_dev_clk_time_mb(adapter, ns);
+		if (err) {
+			pci_err(adapter->pdev,
+				"Failed to set the time: %pe\n", ERR_PTR(err));
+			return err;
+		}
+		break;
+	case IDPF_PTP_DIRECT:
+		idpf_ptp_set_dev_clk_time_direct(adapter, ns);
+		break;
+	case IDPF_PTP_NONE:
+	default:
+		return -EOPNOTSUPP;
 	}
 
 	err = idpf_ptp_update_cached_phctime(adapter);
@@ -474,6 +571,38 @@ static int idpf_ptp_adjtime_nonatomic(struct ptp_clock_info *info, s64 delta)
 	return idpf_ptp_settime64(info, &now);
 }
 
+/**
+ * idpf_ptp_adj_dev_clk_time_direct - Adjust the time of the clock directly
+ *				      through BAR registers.
+ * @adapter: Driver specific private structure
+ * @delta: Offset in nanoseconds to adjust the time by
+ *
+ * Adjust the time of the clock directly through BAR registers received during
+ * PTP capabilities negotiation.
+ */
+static void idpf_ptp_adj_dev_clk_time_direct(struct idpf_adapter *adapter,
+					     s64 delta)
+{
+	struct idpf_ptp *ptp = adapter->ptp;
+	/* The caller (idpf_ptp_adjtime) already verified delta fits in s32. */
+	u32 delta_ns = (s32)delta;
+
+	spin_lock(&ptp->read_dev_clk_lock);
+
+	/* For ADJ_TIME the SHADJ pair is a fixed point value: _L holds the
+	 * sub-nanosecond fraction, _H the 2's complement nanosecond part.
+	 */
+	writel(0, ptp->dev_clk_regs.shadj_l);
+	writel(delta_ns, ptp->dev_clk_regs.shadj_h);
+
+	writel(0, ptp->dev_clk_regs.phy_shadj_l);
+	writel(delta_ns, ptp->dev_clk_regs.phy_shadj_h);
+
+	idpf_ptp_tmr_cmd(adapter, ptp->cmd.adj_time);
+
+	spin_unlock(&ptp->read_dev_clk_lock);
+}
+
 /**
  * idpf_ptp_adjtime - Adjust the time of the clock by the indicated delta
  * @info: the driver's PTP info structure
@@ -488,7 +617,7 @@ static int idpf_ptp_adjtime(struct ptp_clock_info *info, s64 delta)
 	int err;
 
 	access = adapter->ptp->adj_dev_clk_time_access;
-	if (access != IDPF_PTP_MAILBOX)
+	if (access == IDPF_PTP_NONE)
 		return -EOPNOTSUPP;
 
 	/* Hardware only supports atomic adjustments using signed 32-bit
@@ -498,11 +627,22 @@ static int idpf_ptp_adjtime(struct ptp_clock_info *info, s64 delta)
 	if (delta > S32_MAX || delta < S32_MIN)
 		return idpf_ptp_adjtime_nonatomic(info, delta);
 
-	err = idpf_ptp_adj_dev_clk_time(adapter, delta);
-	if (err) {
-		pci_err(adapter->pdev, "Failed to adjust the clock with delta %lld err: %pe\n",
-			delta, ERR_PTR(err));
-		return err;
+	switch (access) {
+	case IDPF_PTP_MAILBOX:
+		err = idpf_ptp_adj_dev_clk_time_mb(adapter, delta);
+		if (err) {
+			pci_err(adapter->pdev,
+				"Failed to adjust the clock with delta %lld err: %pe\n",
+				delta, ERR_PTR(err));
+			return err;
+		}
+		break;
+	case IDPF_PTP_DIRECT:
+		idpf_ptp_adj_dev_clk_time_direct(adapter, delta);
+		break;
+	case IDPF_PTP_NONE:
+	default:
+		return -EOPNOTSUPP;
 	}
 
 	err = idpf_ptp_update_cached_phctime(adapter);
@@ -513,6 +653,35 @@ static int idpf_ptp_adjtime(struct ptp_clock_info *info, s64 delta)
 	return 0;
 }
 
+/**
+ * idpf_ptp_adj_dev_clk_fine_direct - Adjust clock increment rate directly
+ *				      through BAR registers.
+ * @adapter: Driver specific private structure
+ * @incval: Source timer increment value per clock cycle
+ *
+ * Adjust clock increment rate directly through BAR registers received during
+ * PTP capabilities negotiation.
+ */
+static void idpf_ptp_adj_dev_clk_fine_direct(struct idpf_adapter *adapter,
+					     u64 incval)
+{
+	struct idpf_ptp *ptp = adapter->ptp;
+	u32 incval_l = lower_32_bits(incval);
+	u32 incval_h = upper_32_bits(incval);
+
+	spin_lock(&ptp->read_dev_clk_lock);
+
+	writel(incval_l, ptp->dev_clk_regs.shadj_l);
+	writel(incval_h, ptp->dev_clk_regs.shadj_h);
+
+	writel(incval_l, ptp->dev_clk_regs.phy_shadj_l);
+	writel(incval_h, ptp->dev_clk_regs.phy_shadj_h);
+
+	idpf_ptp_tmr_cmd(adapter, ptp->cmd.init_incval);
+
+	spin_unlock(&ptp->read_dev_clk_lock);
+}
+
 /**
  * idpf_ptp_adjfine - Adjust clock increment rate
  * @info: the driver's PTP info structure
@@ -531,16 +700,25 @@ static int idpf_ptp_adjfine(struct ptp_clock_info *info, long scaled_ppm)
 	int err;
 
 	access = adapter->ptp->adj_dev_clk_time_access;
-	if (access != IDPF_PTP_MAILBOX)
-		return -EOPNOTSUPP;
-
 	incval = adapter->ptp->base_incval;
-
 	diff = adjust_by_scaled_ppm(incval, scaled_ppm);
-	err = idpf_ptp_adj_dev_clk_fine(adapter, diff);
-	if (err)
-		pci_err(adapter->pdev, "Failed to adjust clock increment rate for scaled ppm %ld %pe\n",
-			scaled_ppm, ERR_PTR(err));
+
+	switch (access) {
+	case IDPF_PTP_MAILBOX:
+		err = idpf_ptp_adj_dev_clk_fine_mb(adapter, diff);
+		if (err) {
+			pci_err(adapter->pdev,
+				"Failed to adjust clock increment rate\n");
+			return err;
+		}
+		break;
+	case IDPF_PTP_DIRECT:
+		idpf_ptp_adj_dev_clk_fine_direct(adapter, diff);
+		break;
+	case IDPF_PTP_NONE:
+	default:
+		return -EOPNOTSUPP;
+	}
 
 	return 0;
 }
@@ -767,7 +945,7 @@ void idpf_tstamp_task(struct work_struct *work)
 
 	vport = container_of(work, struct idpf_vport, tstamp_task);
 
-	idpf_ptp_get_tx_tstamp(vport);
+	idpf_ptp_get_tx_tstamp_mb(vport);
 }
 
 /**
@@ -938,6 +1116,7 @@ bool idpf_ptp_get_txq_tstamp_capability(struct idpf_tx_queue *txq)
  */
 int idpf_ptp_init(struct idpf_adapter *adapter)
 {
+	struct idpf_ptp *ptp;
 	struct timespec64 ts;
 	int err;
 
@@ -950,8 +1129,10 @@ int idpf_ptp_init(struct idpf_adapter *adapter)
 	if (!adapter->ptp)
 		return -ENOMEM;
 
+	ptp = adapter->ptp;
+
 	/* add a back pointer to adapter */
-	adapter->ptp->adapter = adapter;
+	ptp->adapter = adapter;
 
 	if (adapter->dev_ops.reg_ops.ptp_reg_init)
 		adapter->dev_ops.reg_ops.ptp_reg_init(adapter);
@@ -962,29 +1143,34 @@ int idpf_ptp_init(struct idpf_adapter *adapter)
 		goto free_ptp;
 	}
 
-	spin_lock_init(&adapter->ptp->read_dev_clk_lock);
+	/* Do not initialize the PTP if the device clock time cannot be read. */
+	if (ptp->get_dev_clk_time_access == IDPF_PTP_NONE) {
+		err = -EIO;
+		goto free_ptp;
+	}
+
+	spin_lock_init(&ptp->read_dev_clk_lock);
 
 	err = idpf_ptp_create_clock(adapter);
 	if (err)
 		goto free_ptp;
-
-	if (adapter->ptp->get_dev_clk_time_access != IDPF_PTP_NONE)
-		ptp_schedule_worker(adapter->ptp->clock, 0);
+	ptp_schedule_worker(ptp->clock, 0);
 
 	/* Write the default increment time value if the clock adjustments
 	 * are enabled.
 	 */
-	if (adapter->ptp->adj_dev_clk_time_access != IDPF_PTP_NONE) {
-		err = idpf_ptp_adj_dev_clk_fine(adapter,
-						adapter->ptp->base_incval);
+	if (ptp->adj_dev_clk_time_access == IDPF_PTP_MAILBOX) {
+		err = idpf_ptp_adj_dev_clk_fine_mb(adapter, ptp->base_incval);
 		if (err)
 			goto remove_clock;
+	} else if (ptp->adj_dev_clk_time_access == IDPF_PTP_DIRECT) {
+		idpf_ptp_adj_dev_clk_fine_direct(adapter, ptp->base_incval);
 	}
 
 	/* Write the initial time value if the set time operation is enabled */
-	if (adapter->ptp->set_dev_clk_time_access != IDPF_PTP_NONE) {
+	if (ptp->set_dev_clk_time_access != IDPF_PTP_NONE) {
 		ts = ktime_to_timespec64(ktime_get_real());
-		err = idpf_ptp_settime64(&adapter->ptp->info, &ts);
+		err = idpf_ptp_settime64(&ptp->info, &ts);
 		if (err)
 			goto remove_clock;
 	}
@@ -994,14 +1180,14 @@ int idpf_ptp_init(struct idpf_adapter *adapter)
 	return 0;
 
 remove_clock:
-	if (adapter->ptp->get_dev_clk_time_access != IDPF_PTP_NONE)
-		ptp_cancel_worker_sync(adapter->ptp->clock);
+	if (ptp->get_dev_clk_time_access != IDPF_PTP_NONE)
+		ptp_cancel_worker_sync(ptp->clock);
 
-	ptp_clock_unregister(adapter->ptp->clock);
-	adapter->ptp->clock = NULL;
+	ptp_clock_unregister(ptp->clock);
+	ptp->clock = NULL;
 
 free_ptp:
-	kfree(adapter->ptp);
+	kfree(ptp);
 	adapter->ptp = NULL;
 
 	return err;
diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.h b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
index 785da03e4cf5..0a2f08b17033 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_ptp.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
@@ -7,13 +7,19 @@
 #include <linux/ptp_clock_kernel.h>
 
 /**
- * struct idpf_ptp_cmd - PTP command masks
- * @exec_cmd_mask: mask to trigger command execution
- * @shtime_enable_mask: mask to enable shadow time
+ * struct idpf_ptp_cmd_mask - PTP command masks
+ * @exec_cmd: mask to trigger command execution
+ * @shtime_enable: mask to enable shadow time
+ * @init_time: initialize the device clock timer
+ * @init_incval: initialize increment value
+ * @adj_time: adjust the device clock timer
  */
-struct idpf_ptp_cmd {
-	u32 exec_cmd_mask;
-	u32 shtime_enable_mask;
+struct idpf_ptp_cmd_mask {
+	u32 exec_cmd;
+	u32 shtime_enable;
+	u32 init_time;
+	u32 init_incval;
+	u32 adj_time;
 };
 
 /* struct idpf_ptp_dev_clk_regs - PTP device registers
@@ -183,7 +189,7 @@ struct idpf_ptp {
 	struct idpf_adapter *adapter;
 	u64 base_incval;
 	u64 max_adj;
-	struct idpf_ptp_cmd cmd;
+	struct idpf_ptp_cmd_mask cmd;
 	u64 cached_phc_time;
 	unsigned long cached_phc_jiffies;
 	struct idpf_ptp_dev_clk_regs dev_clk_regs;
@@ -212,6 +218,22 @@ idpf_ptp_info_to_adapter(const struct ptp_clock_info *info)
 	return ptp->adapter;
 }
 
+/**
+ * idpf_ptp_mb_access - Get the mailbox access type of the PTP feature
+ * @ptp: PTP specific structure
+ * @mailbox: Capability that indicates the mailbox access
+ *
+ * Used to downgrade a feature that cannot use the direct access.
+ *
+ * Return: mailbox access when the Control Plane advertises it, no access
+ * otherwise.
+ */
+static inline enum idpf_ptp_access
+idpf_ptp_mb_access(const struct idpf_ptp *ptp, u32 mailbox)
+{
+	return ptp->caps & mailbox ? IDPF_PTP_MAILBOX : IDPF_PTP_NONE;
+}
+
 /**
  * struct idpf_ptp_dev_timers - System time and device time values
  * @sys_time_ns: system time value expressed in nanoseconds
@@ -269,16 +291,17 @@ int idpf_ptp_init(struct idpf_adapter *adapter);
 void idpf_ptp_release(struct idpf_adapter *adapter);
 int idpf_ptp_get_caps(struct idpf_adapter *adapter);
 void idpf_ptp_get_features_access(const struct idpf_adapter *adapter);
+void idpf_ptp_check_features_access(const struct idpf_adapter *adapter);
 bool idpf_ptp_get_txq_tstamp_capability(struct idpf_tx_queue *txq);
-int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
-			      struct idpf_ptp_dev_timers *dev_clk_time);
-int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
-			    struct idpf_ptp_dev_timers *cross_time);
-int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter, u64 time);
-int idpf_ptp_adj_dev_clk_fine(struct idpf_adapter *adapter, u64 incval);
-int idpf_ptp_adj_dev_clk_time(struct idpf_adapter *adapter, s64 delta);
+int idpf_ptp_get_cross_time_mb(struct idpf_adapter *adapter,
+			       struct idpf_ptp_dev_timers *cross_time);
+int idpf_ptp_get_dev_clk_time_mb(struct idpf_adapter *adapter,
+				 struct idpf_ptp_dev_timers *dev_clk_time);
+int idpf_ptp_set_dev_clk_time_mb(struct idpf_adapter *adapter, u64 time);
+int idpf_ptp_adj_dev_clk_fine_mb(struct idpf_adapter *adapter, u64 incval);
+int idpf_ptp_adj_dev_clk_time_mb(struct idpf_adapter *adapter, s64 delta);
 int idpf_ptp_get_vport_tstamps_caps(struct idpf_vport *vport);
-int idpf_ptp_get_tx_tstamp(struct idpf_vport *vport);
+int idpf_ptp_get_tx_tstamp_mb(struct idpf_vport *vport);
 int idpf_ptp_set_timestamp_mode(struct idpf_vport *vport,
 				struct kernel_hwtstamp_config *config);
 u64 idpf_ptp_extend_ts(struct idpf_vport *vport, u64 in_tstamp);
@@ -302,6 +325,9 @@ static inline int idpf_ptp_get_caps(struct idpf_adapter *adapter)
 static inline void
 idpf_ptp_get_features_access(const struct idpf_adapter *adapter) { }
 
+static inline void
+idpf_ptp_check_features_access(const struct idpf_adapter *adapter) { }
+
 static inline bool
 idpf_ptp_get_txq_tstamp_capability(struct idpf_tx_queue *txq)
 {
@@ -309,33 +335,33 @@ idpf_ptp_get_txq_tstamp_capability(struct idpf_tx_queue *txq)
 }
 
 static inline int
-idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
-			  struct idpf_ptp_dev_timers *dev_clk_time)
+idpf_ptp_get_dev_clk_time_mb(struct idpf_adapter *adapter,
+			     struct idpf_ptp_dev_timers *dev_clk_time)
 {
 	return -EOPNOTSUPP;
 }
 
 static inline int
-idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
-			struct idpf_ptp_dev_timers *cross_time)
+idpf_ptp_get_cross_time_mb(struct idpf_adapter *adapter,
+			   struct idpf_ptp_dev_timers *cross_time)
 {
 	return -EOPNOTSUPP;
 }
 
-static inline int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter,
-					    u64 time)
+static inline int idpf_ptp_set_dev_clk_time_mb(struct idpf_adapter *adapter,
+					       u64 time)
 {
 	return -EOPNOTSUPP;
 }
 
-static inline int idpf_ptp_adj_dev_clk_fine(struct idpf_adapter *adapter,
-					    u64 incval)
+static inline int idpf_ptp_adj_dev_clk_fine_mb(struct idpf_adapter *adapter,
+					       u64 incval)
 {
 	return -EOPNOTSUPP;
 }
 
-static inline int idpf_ptp_adj_dev_clk_time(struct idpf_adapter *adapter,
-					    s64 delta)
+static inline int idpf_ptp_adj_dev_clk_time_mb(struct idpf_adapter *adapter,
+					       s64 delta)
 {
 	return -EOPNOTSUPP;
 }
@@ -345,7 +371,7 @@ static inline int idpf_ptp_get_vport_tstamps_caps(struct idpf_vport *vport)
 	return -EOPNOTSUPP;
 }
 
-static inline int idpf_ptp_get_tx_tstamp(struct idpf_vport *vport)
+static inline int idpf_ptp_get_tx_tstamp_mb(struct idpf_vport *vport)
 {
 	return -EOPNOTSUPP;
 }
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
index 1422b30d09a3..ef71df2d51ef 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
@@ -5,6 +5,30 @@
 #include "idpf_ptp.h"
 #include "idpf_virtchnl.h"
 
+/**
+ * idpf_ptp_regs_access - Get the access type usable with the mapped registers
+ * @ptp: PTP specific structure
+ * @regs: registers the direct access path needs
+ * @num_regs: number of entries in @regs
+ * @mailbox: capability that indicates the mailbox access
+ *
+ * Direct access is only usable when every offset received from the Control
+ * Plane was successfully translated to a BAR address.
+ *
+ * Return: direct access when all registers are mapped, mailbox access when
+ * advertised, no access otherwise.
+ */
+static enum idpf_ptp_access
+idpf_ptp_regs_access(const struct idpf_ptp *ptp, void __iomem * const *regs,
+		     u32 num_regs, u32 mailbox)
+{
+	for (u32 i = 0; i < num_regs; i++)
+		if (!regs[i])
+			return idpf_ptp_mb_access(ptp, mailbox);
+
+	return IDPF_PTP_DIRECT;
+}
+
 /**
  * idpf_ptp_get_caps - Send virtchnl get ptp capabilities message
  * @adapter: Driver specific private structure
@@ -19,7 +43,10 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
 		.caps = cpu_to_le32(VIRTCHNL2_CAP_PTP_GET_DEVICE_CLK_TIME |
 				    VIRTCHNL2_CAP_PTP_GET_DEVICE_CLK_TIME_MB |
 				    VIRTCHNL2_CAP_PTP_GET_CROSS_TIME |
+				    VIRTCHNL2_CAP_PTP_GET_CROSS_TIME_MB |
+				    VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME |
 				    VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME_MB |
+				    VIRTCHNL2_CAP_PTP_ADJ_DEVICE_CLK |
 				    VIRTCHNL2_CAP_PTP_ADJ_DEVICE_CLK_MB |
 				    VIRTCHNL2_CAP_PTP_TX_TSTAMPS_MB)
 	};
@@ -35,6 +62,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
 	struct idpf_ptp_secondary_mbx *scnd_mbx;
 	struct idpf_ptp *ptp = adapter->ptp;
 	enum idpf_ptp_access access_type;
+	void __iomem *cross_cmd_sync;
 	u32 temp_offset;
 	size_t reply_sz;
 	int err;
@@ -90,6 +118,19 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
 	ptp->dev_clk_regs.cmd_sync =
 		libie_pci_get_mmio_addr(mmio, temp_offset);
 
+	/* Direct access is only usable when every register was mapped. */
+	void __iomem * const clk_regs[] = {
+		ptp->dev_clk_regs.dev_clk_ns_l,
+		ptp->dev_clk_regs.dev_clk_ns_h,
+		ptp->dev_clk_regs.phy_clk_ns_l,
+		ptp->dev_clk_regs.phy_clk_ns_h,
+		ptp->dev_clk_regs.cmd_sync,
+	};
+
+	ptp->get_dev_clk_time_access =
+		idpf_ptp_regs_access(ptp, clk_regs, ARRAY_SIZE(clk_regs),
+				     VIRTCHNL2_CAP_PTP_GET_DEVICE_CLK_TIME_MB);
+
 cross_tstamp:
 	access_type = ptp->get_cross_tstamp_access;
 	if (access_type != IDPF_PTP_DIRECT)
@@ -104,13 +145,31 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
 	ptp->dev_clk_regs.sys_time_ns_h =
 		libie_pci_get_mmio_addr(mmio, temp_offset);
 	temp_offset = le32_to_cpu(cross_tstamp_offsets.cmd_sync_trigger);
-	ptp->dev_clk_regs.cmd_sync =
-		libie_pci_get_mmio_addr(mmio, temp_offset);
+	cross_cmd_sync = libie_pci_get_mmio_addr(mmio, temp_offset);
+
+	/* The register is shared with the device clock access, so a failed
+	 * translation must not replace the address mapped above.
+	 */
+	if (cross_cmd_sync)
+		ptp->dev_clk_regs.cmd_sync = cross_cmd_sync;
+
+	/* Cross timestamping also samples the device clock registers. */
+	void __iomem * const cross_regs[] = {
+		ptp->dev_clk_regs.dev_clk_ns_l,
+		ptp->dev_clk_regs.dev_clk_ns_h,
+		ptp->dev_clk_regs.sys_time_ns_l,
+		ptp->dev_clk_regs.sys_time_ns_h,
+		cross_cmd_sync,
+	};
+
+	ptp->get_cross_tstamp_access =
+		idpf_ptp_regs_access(ptp, cross_regs, ARRAY_SIZE(cross_regs),
+				     VIRTCHNL2_CAP_PTP_GET_CROSS_TIME_MB);
 
 discipline_clock:
 	access_type = ptp->adj_dev_clk_time_access;
 	if (access_type != IDPF_PTP_DIRECT)
-		goto free_resp;
+		goto check_features_access;
 
 	clk_adj_offsets = recv_ptp_caps_msg->clk_adj_offsets;
 
@@ -143,13 +202,35 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
 	ptp->dev_clk_regs.phy_shadj_h =
 		libie_pci_get_mmio_addr(mmio, temp_offset);
 
+	/* Both the main and the PHY timer are disciplined together. */
+	void __iomem * const adj_regs[] = {
+		ptp->dev_clk_regs.cmd,
+		ptp->dev_clk_regs.incval_l,
+		ptp->dev_clk_regs.incval_h,
+		ptp->dev_clk_regs.shadj_l,
+		ptp->dev_clk_regs.shadj_h,
+		ptp->dev_clk_regs.phy_cmd,
+		ptp->dev_clk_regs.phy_incval_l,
+		ptp->dev_clk_regs.phy_incval_h,
+		ptp->dev_clk_regs.phy_shadj_l,
+		ptp->dev_clk_regs.phy_shadj_h,
+	};
+
+	ptp->adj_dev_clk_time_access =
+		idpf_ptp_regs_access(ptp, adj_regs, ARRAY_SIZE(adj_regs),
+				     VIRTCHNL2_CAP_PTP_ADJ_DEVICE_CLK_MB);
+
+	/* Reconcile dependencies after address validation changed access modes. */
+check_features_access:
+	idpf_ptp_check_features_access(adapter);
+
 free_resp:
 	libie_ctlq_release_rx_buf(&xn_params.recv_mem);
 	return err;
 }
 
 /**
- * idpf_ptp_get_dev_clk_time - Send virtchnl get device clk time message
+ * idpf_ptp_get_dev_clk_time_mb - Send virtchnl get device clk time message
  * @adapter: Driver specific private structure
  * @dev_clk_time: Pointer to the device clock structure where the value is set
  *
@@ -157,8 +238,8 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
  *
  * Return: 0 on success, -errno otherwise.
  */
-int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
-			      struct idpf_ptp_dev_timers *dev_clk_time)
+int idpf_ptp_get_dev_clk_time_mb(struct idpf_adapter *adapter,
+				 struct idpf_ptp_dev_timers *dev_clk_time)
 {
 	struct virtchnl2_ptp_get_dev_clk_time *get_dev_clk_time_resp;
 	struct virtchnl2_ptp_get_dev_clk_time get_dev_clk_time_msg;
@@ -191,7 +272,7 @@ int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
 }
 
 /**
- * idpf_ptp_get_cross_time - Send virtchnl get cross time message
+ * idpf_ptp_get_cross_time_mb - Send virtchnl get cross time message
  * @adapter: Driver specific private structure
  * @cross_time: Pointer to the device clock structure where the value is set
  *
@@ -200,8 +281,8 @@ int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
  *
  * Return: 0 on success, -errno otherwise.
  */
-int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
-			    struct idpf_ptp_dev_timers *cross_time)
+int idpf_ptp_get_cross_time_mb(struct idpf_adapter *adapter,
+			       struct idpf_ptp_dev_timers *cross_time)
 {
 	struct virtchnl2_ptp_get_cross_time cross_time_send, *cross_time_recv;
 	struct libie_ctlq_xn_send_params xn_params = {
@@ -229,7 +310,7 @@ int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
 }
 
 /**
- * idpf_ptp_set_dev_clk_time - Send virtchnl set device time message
+ * idpf_ptp_set_dev_clk_time_mb - Send virtchnl set device time message
  * @adapter: Driver specific private structure
  * @time: New time value
  *
@@ -237,7 +318,7 @@ int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
  *
  * Return: 0 on success, -errno otherwise.
  */
-int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter, u64 time)
+int idpf_ptp_set_dev_clk_time_mb(struct idpf_adapter *adapter, u64 time)
 {
 	struct virtchnl2_ptp_set_dev_clk_time set_dev_clk_time_msg = {
 		.dev_time_ns = cpu_to_le64(time),
@@ -257,7 +338,7 @@ int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter, u64 time)
 }
 
 /**
- * idpf_ptp_adj_dev_clk_time - Send virtchnl adj device clock time message
+ * idpf_ptp_adj_dev_clk_time_mb - Send virtchnl adj device clock time message
  * @adapter: Driver specific private structure
  * @delta: Offset in nanoseconds to adjust the time by
  *
@@ -265,7 +346,7 @@ int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter, u64 time)
  *
  * Return: 0 on success, -errno otherwise.
  */
-int idpf_ptp_adj_dev_clk_time(struct idpf_adapter *adapter, s64 delta)
+int idpf_ptp_adj_dev_clk_time_mb(struct idpf_adapter *adapter, s64 delta)
 {
 	struct virtchnl2_ptp_adj_dev_clk_time adj_dev_clk_time_msg = {
 		.delta = cpu_to_le64(delta),
@@ -285,7 +366,7 @@ int idpf_ptp_adj_dev_clk_time(struct idpf_adapter *adapter, s64 delta)
 }
 
 /**
- * idpf_ptp_adj_dev_clk_fine - Send virtchnl adj time message
+ * idpf_ptp_adj_dev_clk_fine_mb - Send virtchnl adj time message
  * @adapter: Driver specific private structure
  * @incval: Source timer increment value per clock cycle
  *
@@ -294,7 +375,7 @@ int idpf_ptp_adj_dev_clk_time(struct idpf_adapter *adapter, s64 delta)
  *
  * Return: 0 on success, -errno otherwise.
  */
-int idpf_ptp_adj_dev_clk_fine(struct idpf_adapter *adapter, u64 incval)
+int idpf_ptp_adj_dev_clk_fine_mb(struct idpf_adapter *adapter, u64 incval)
 {
 	struct virtchnl2_ptp_adj_dev_clk_fine adj_dev_clk_fine_msg = {
 		.incval = cpu_to_le64(incval),
@@ -612,7 +693,7 @@ idpf_ptp_get_tx_tstamp_async_handler(void *ctx, struct kvec *mem, int status)
 }
 
 /**
- * idpf_ptp_get_tx_tstamp - Send virtchnl get Tx timestamp latches message
+ * idpf_ptp_get_tx_tstamp_mb - Send virtchnl get Tx timestamp latches message
  * @vport: Virtual port structure
  *
  * Send virtchnl get Tx tstamp message to read the value of the HW timestamp.
@@ -620,7 +701,7 @@ idpf_ptp_get_tx_tstamp_async_handler(void *ctx, struct kvec *mem, int status)
  *
  * Return: 0 on success, -errno otherwise.
  */
-int idpf_ptp_get_tx_tstamp(struct idpf_vport *vport)
+int idpf_ptp_get_tx_tstamp_mb(struct idpf_vport *vport)
 {
 	struct virtchnl2_ptp_get_vport_tx_tstamp_latches *send_tx_tstamp_msg;
 	struct idpf_ptp_vport_tx_tstamp_caps *tx_tstamp_caps;
-- 
2.47.3

[PATCH iwl-next v3 2/2] idpf: add direct Tx timestamp readout

[Przemyslaw Korba]

Currently IDPF supports only mailbox access to PHC operations and
Tx timestamping, however the driver shall also be able to handle direct
access. The difference is that PHC/timestamps actions are performed
directly, through BAR registers, instead of sending a virtchnl message to
CP. Registers offsets are negotiated with the CP during the capabilities
negotiation.

In the direct mode the latches are collected by a timer that runs at a
fixed 100 us period for as long as the Tx timestamping is enabled. The
Tx path only hands out a latch index, and the Tx completion path does
not touch the latches at all, so no register access is added to either
of them.

Hand out a latch only while the Tx timestamping is enabled on the
netdev, which is checked under the latches lock. When the user turns it
off or the vport is stopped, stop the timer and flush the latches that
are still waiting, as nothing would collect them anymore. Start the
timer again when the vport is opened with the Tx timestamping enabled.

The latch readiness bits are write-1-to-clear and are not cleared by
reading the timestamp registers, so clear the bit of every latch the
timer consumes. Otherwise a bit set once stays set forever and every
subsequent packet reusing that index is reported with a stale timestamp.
Clear it for a latch given up on as well, as the hardware may report it
after the deadline has passed. Cross-check the per-latch valid bit too,
to never report a latch the device has not written. All of the clearing
is done by the collecting timer, so that the Tx path is left without any
register access.

Give up on a timestamp request after 500 ms. A packet dropped inside the
device never latches a timestamp, and without a deadline it would leak
both the skb and the latch index. Count such requests as lost in the HW
timestamping statistics.

Co-developed-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
Reviewed-by: Naman Gulati <namangulati@google.com>
Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf.h        |   6 +
 .../net/ethernet/intel/idpf/idpf_ethtool.c    |   8 +-
 drivers/net/ethernet/intel/idpf/idpf_lib.c    |   2 +
 drivers/net/ethernet/intel/idpf/idpf_ptp.c    | 403 +++++++++++++++++-
 drivers/net/ethernet/intel/idpf/idpf_ptp.h    |  52 ++-
 drivers/net/ethernet/intel/idpf/idpf_txrx.c   |  23 +-
 .../net/ethernet/intel/idpf/idpf_virtchnl.c   |  12 +-
 .../ethernet/intel/idpf/idpf_virtchnl_ptp.c   |  86 +++-
 include/linux/net/intel/virtchnl2.h           |   6 +-
 9 files changed, 547 insertions(+), 51 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
index 77729921b0a3..7978ca56d1d5 100644
--- a/drivers/net/ethernet/intel/idpf/idpf.h
+++ b/drivers/net/ethernet/intel/idpf/idpf.h
@@ -270,12 +270,16 @@ enum idpf_vport_flags {
  * @discarded: Number of Tx skbs discarded due to cached PHC
  *	       being too old to correctly extend timestamp
  * @flushed: Number of Tx skbs flushed due to interface closed
+ * @expired: Number of Tx skbs dropped because the hardware has not latched
+ *	     the timestamp in time, e.g. when the packet was dropped by the
+ *	     device after it has been transmitted by the driver
  */
 struct idpf_tstamp_stats {
 	struct u64_stats_sync stats_sync;
 	u64_stats_t packets;
 	u64_stats_t discarded;
 	u64_stats_t flushed;
+	u64_stats_t expired;
 };
 
 struct idpf_port_stats {
@@ -383,6 +387,7 @@ struct idpf_q_vec_rsrc {
  * @tx_tstamp_caps: Capabilities negotiated for Tx timestamping
  * @tstamp_config: The Tx tstamp config
  * @tstamp_task: Tx timestamping task
+ * @tx_tstamp_timer: timer that collects the Tx timestamps in the direct mode
  * @tstamp_stats: Tx timestamping statistics
  */
 struct idpf_vport {
@@ -419,6 +424,7 @@ struct idpf_vport {
 	struct idpf_ptp_vport_tx_tstamp_caps *tx_tstamp_caps;
 	struct kernel_hwtstamp_config tstamp_config;
 	struct work_struct tstamp_task;
+	struct hrtimer tx_tstamp_timer;
 	struct idpf_tstamp_stats tstamp_stats;
 };
 
diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
index 95c45f12b0f9..96d5c372f7cb 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
@@ -1739,8 +1739,9 @@ static int idpf_get_ts_info(struct net_device *netdev,
  * @ts_stats: HW timestamping stats structure
  *
  * Collect HW timestamping statistics including successfully timestamped
- * packets, discarded due to illegal values, flushed during releasing PTP and
- * skipped due to lack of the free index.
+ * packets, discarded due to illegal values, flushed during releasing PTP,
+ * expired because the hardware never latched a timestamp and skipped due to
+ * lack of the free index.
  */
 static void idpf_get_ts_stats(struct net_device *netdev,
 			      struct ethtool_ts_stats *ts_stats)
@@ -1755,7 +1756,8 @@ static void idpf_get_ts_stats(struct net_device *netdev,
 	do {
 		start = u64_stats_fetch_begin(&vport->tstamp_stats.stats_sync);
 		ts_stats->pkts = u64_stats_read(&vport->tstamp_stats.packets);
-		ts_stats->lost = u64_stats_read(&vport->tstamp_stats.flushed);
+		ts_stats->lost = u64_stats_read(&vport->tstamp_stats.flushed) +
+				 u64_stats_read(&vport->tstamp_stats.expired);
 		ts_stats->err = u64_stats_read(&vport->tstamp_stats.discarded);
 	} while (u64_stats_fetch_retry(&vport->tstamp_stats.stats_sync, start));
 
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 237f9ae9f1ca..10ee15108073 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -845,6 +845,7 @@ static void idpf_vport_stop(struct idpf_vport *vport, bool rtnl)
 
 	idpf_send_disable_vport_msg(adapter, vport_id);
 	idpf_send_disable_queues_msg(vport);
+	idpf_ptp_tx_tstamp_stop(vport);
 	idpf_send_map_unmap_queue_vector_msg(adapter, rsrc, vport_id, false);
 	/* Normally we ask for queues in create_vport, but if the number of
 	 * initially requested queues have changed, for example via ethtool
@@ -1450,6 +1451,7 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
 		goto disable_vport;
 	}
 
+	idpf_ptp_tx_tstamp_start(vport);
 	idpf_up_complete(vport);
 
 	if (rtnl)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
index df2da430ff28..5d745a53d686 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
@@ -4,6 +4,16 @@
 #include "idpf.h"
 #include "idpf_ptp.h"
 
+/* Period of the Tx timestamp collection. Way below the jiffy granularity, as
+ * a timestamp reported late skews the PTP error bounds.
+ */
+#define IDPF_PTP_TX_TSTAMP_PERIOD_NS		(100 * NSEC_PER_USEC)
+
+/* Time after which a Tx timestamp that the hardware has never latched is
+ * given up on, e.g. when the packet was dropped inside the device.
+ */
+#define IDPF_PTP_TX_TSTAMP_TIMEOUT		msecs_to_jiffies(500)
+
 /**
  * idpf_ptp_get_access - Determine the access type of the PTP features
  * @adapter: Driver specific private structure
@@ -820,6 +830,55 @@ u64 idpf_ptp_extend_ts(struct idpf_vport *vport, u64 in_tstamp)
 						 lower_32_bits(in_tstamp));
 }
 
+/**
+ * idpf_ptp_clear_tx_tstamp_ready - Clear the readiness bits of Tx latches
+ * @caps: Tx timestamp capabilities of the vport
+ * @map: bitmap of the latch indexes to clear
+ *
+ * The readiness bits are write-1-to-clear and are not cleared by reading the
+ * latch registers, so a bit left behind would make the next user of the same
+ * index report the timestamp of the previous packet. Only the bits set in
+ * @map are written, as the remaining ones may belong to latches that are
+ * still waiting to be read.
+ */
+void idpf_ptp_clear_tx_tstamp_ready(struct idpf_ptp_vport_tx_tstamp_caps *caps,
+				    u64 map)
+{
+	u32 map_l = lower_32_bits(map);
+	u32 map_h = upper_32_bits(map);
+
+	/* Flush the posted writes, the bits have to be cleared in the
+	 * hardware before the indexes are handed out again.
+	 */
+	if (map_l) {
+		writel(map_l, caps->readiness_addr_l);
+		readl(caps->readiness_addr_l);
+	}
+
+	if (map_h) {
+		writel(map_h, caps->readiness_addr_h);
+		readl(caps->readiness_addr_h);
+	}
+}
+
+/**
+ * idpf_ptp_read_tx_tstamp_readiness - Read the Tx timestamp readiness bitmap
+ * @caps: Tx timestamp capabilities of the vport
+ *
+ * Return: bitmap of the latch indexes for which the hardware has reported
+ * a timestamp.
+ */
+static u64
+idpf_ptp_read_tx_tstamp_readiness(struct idpf_ptp_vport_tx_tstamp_caps *caps)
+{
+	u32 readiness_l, readiness_h;
+
+	readiness_l = readl(caps->readiness_addr_l);
+	readiness_h = readl(caps->readiness_addr_h);
+
+	return ((u64)readiness_h << 32) | readiness_l;
+}
+
 /**
  * idpf_ptp_request_ts - Request an available Tx timestamp index
  * @tx_q: Transmit queue on which the Tx timestamp is requested
@@ -835,15 +894,30 @@ u64 idpf_ptp_extend_ts(struct idpf_vport *vport, u64 in_tstamp)
 int idpf_ptp_request_ts(struct idpf_tx_queue *tx_q, struct sk_buff *skb,
 			u32 *idx)
 {
+	struct idpf_ptp_vport_tx_tstamp_caps *tstamp_caps;
 	struct idpf_ptp_tx_tstamp *ptp_tx_tstamp;
+	struct idpf_vport *vport;
 	struct list_head *head;
 
-	/* Get the index from the free latches list */
-	spin_lock(&tx_q->cached_tstamp_caps->latches_lock);
+	tstamp_caps = tx_q->cached_tstamp_caps;
+	vport = idpf_netdev_to_vport(tx_q->netdev);
+
+	spin_lock(&tstamp_caps->latches_lock);
 
-	head = &tx_q->cached_tstamp_caps->latches_free;
+	/* Disabling the Tx timestamping flushes the latches under this lock,
+	 * so one taken here is always collected or flushed.
+	 */
+	if (READ_ONCE(vport->tstamp_config.tx_type) != HWTSTAMP_TX_ON) {
+		spin_unlock(&tstamp_caps->latches_lock);
+
+		return -EPERM;
+	}
+
+	/* Get the index from the free latches list */
+	head = &tstamp_caps->latches_free;
 	if (list_empty(head)) {
-		spin_unlock(&tx_q->cached_tstamp_caps->latches_lock);
+		spin_unlock(&tstamp_caps->latches_lock);
+
 		return -ENOBUFS;
 	}
 
@@ -851,19 +925,296 @@ int idpf_ptp_request_ts(struct idpf_tx_queue *tx_q, struct sk_buff *skb,
 					 list_member);
 	list_del(&ptp_tx_tstamp->list_member);
 
+	ptp_tx_tstamp->deadline = jiffies + IDPF_PTP_TX_TSTAMP_TIMEOUT;
 	ptp_tx_tstamp->skb = skb_get(skb);
 	skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
 
 	/* Move the element to the used latches list */
-	list_add(&ptp_tx_tstamp->list_member,
-		 &tx_q->cached_tstamp_caps->latches_in_use);
-	spin_unlock(&tx_q->cached_tstamp_caps->latches_lock);
+	list_add(&ptp_tx_tstamp->list_member, &tstamp_caps->latches_in_use);
 
 	*idx = ptp_tx_tstamp->idx;
 
+	spin_unlock(&tstamp_caps->latches_lock);
+
 	return 0;
 }
 
+/**
+ * idpf_ptp_read_tx_tstamp - Read and extract timestamp from latch registers
+ * @tstamp_caps: Tx timestamp capabilities of the vport
+ * @tx_tstamp: Tx timestamp structure with latch register addresses
+ *
+ * Read the timestamp from the latch registers, whose layout the IDPF
+ * specification defines as:
+ *   low  bit[0]   : valid flag
+ *   low  bit[31:1]: timestamp bits [30:0]
+ *   high bit[7:0] : timestamp bits [38:31]
+ * Together they form a 39-bit timestamp, which is then shifted right by
+ * tstamp_ns_lo_bit to remove sub-nanosecond precision bits.
+ *
+ * The caller has already found the latch index set in the readiness bitmap,
+ * which is preferred over the per-latch valid flag because it covers every
+ * latch in a single pair of reads. The valid flag is still checked here, as
+ * the hardware may overwrite a latch whose readiness bit has been left set.
+ *
+ * @latches_lock must be held across the readiness check, this call and the
+ * subsequent clearing of the readiness bit, to keep the index from being
+ * recycled in between.
+ *
+ * Return: extracted timestamp value in nanoseconds, 0 if the latch does not
+ * hold a valid timestamp.
+ */
+static u64
+idpf_ptp_read_tx_tstamp(struct idpf_ptp_vport_tx_tstamp_caps *tstamp_caps,
+			struct idpf_ptp_tx_tstamp *tx_tstamp)
+{
+	u32 tstamp_l, tstamp_h;
+	u64 tstamp;
+
+	lockdep_assert_held(&tstamp_caps->latches_lock);
+
+	tstamp_l = readl(tx_tstamp->tx_latch_reg_addr_l);
+	tstamp_h = readl(tx_tstamp->tx_latch_reg_addr_h);
+
+	if (unlikely(!(tstamp_l & IDPF_PTP_TX_TSTAMP_L_VALID_M)))
+		return 0;
+
+	tstamp = FIELD_GET(IDPF_PTP_TX_TSTAMP_L_TSTAMP_M, tstamp_l);
+	tstamp |= FIELD_PREP(IDPF_PTP_TX_TSTAMP_H_POS_M,
+			     FIELD_GET(IDPF_PTP_TX_TSTAMP_H_TSTAMP_M, tstamp_h));
+
+	return tstamp >> tstamp_caps->tstamp_ns_lo_bit;
+}
+
+/**
+ * idpf_ptp_report_tx_tstamp - Report timestamp to stack and update stats
+ * @vport: Virtual port structure
+ * @skb: Socket buffer to report timestamp for
+ * @tstamp: Raw timestamp value to extend and report
+ *
+ * Extend the 39-bit hardware timestamp to 64-bit, report to stack via
+ * skb_tstamp_tx(), and update packet statistics.
+ */
+static void idpf_ptp_report_tx_tstamp(struct idpf_vport *vport,
+				      struct sk_buff *skb, u64 tstamp)
+{
+	struct skb_shared_hwtstamps shhwtstamps;
+	u64 extended_tstamp;
+
+	extended_tstamp = idpf_ptp_extend_ts(vport, tstamp);
+	if (!extended_tstamp)
+		goto consume;
+
+	shhwtstamps = (struct skb_shared_hwtstamps){
+		.hwtstamp	= ns_to_ktime(extended_tstamp),
+	};
+
+	skb_tstamp_tx(skb, &shhwtstamps);
+
+	u64_stats_update_begin(&vport->tstamp_stats.stats_sync);
+	u64_stats_inc(&vport->tstamp_stats.packets);
+	u64_stats_update_end(&vport->tstamp_stats.stats_sync);
+
+consume:
+	consume_skb(skb);
+}
+
+/**
+ * idpf_ptp_get_tx_tstamp_direct - Read timestamps directly from HW registers
+ * @vport: Virtual port structure
+ *
+ * Collect the latches for which the hardware has reported a timestamp, as
+ * well as the ones that have not been reported before their deadline, and
+ * hand them over to the stack. A packet dropped inside the device never
+ * latches a timestamp, so without the deadline its latch would never be
+ * reclaimed.
+ *
+ * The latches are detached from the in-use list before the lock is dropped,
+ * so that their skbs can be reported without the index being recycled in the
+ * meantime.
+ */
+static void idpf_ptp_get_tx_tstamp_direct(struct idpf_vport *vport)
+{
+	struct idpf_ptp_vport_tx_tstamp_caps *tstamp_caps;
+	struct idpf_ptp_tx_tstamp *ptp_tx_tstamp, *tmp;
+	u64 readiness_bitmap, clear_map = 0;
+	LIST_HEAD(done);
+
+	tstamp_caps = vport->tx_tstamp_caps;
+
+	spin_lock(&tstamp_caps->latches_lock);
+
+	if (list_empty(&tstamp_caps->latches_in_use)) {
+		spin_unlock(&tstamp_caps->latches_lock);
+		return;
+	}
+
+	readiness_bitmap = idpf_ptp_read_tx_tstamp_readiness(tstamp_caps);
+
+	list_for_each_entry_safe(ptp_tx_tstamp, tmp,
+				 &tstamp_caps->latches_in_use, list_member) {
+		u32 idx = ptp_tx_tstamp->idx;
+
+		ptp_tx_tstamp->tstamp = 0;
+
+		if (readiness_bitmap & BIT_ULL(idx)) {
+			ptp_tx_tstamp->tstamp =
+				idpf_ptp_read_tx_tstamp(tstamp_caps,
+							ptp_tx_tstamp);
+			/* Keep the bit set if the latch holds no valid
+			 * timestamp yet, so that it is read again.
+			 */
+			if (ptp_tx_tstamp->tstamp)
+				clear_map |= BIT_ULL(idx);
+		}
+
+		/* A latch with no timestamp is kept in use until its deadline
+		 * passes, as the hardware may still be about to report it.
+		 * Once it does, drop its readiness bit as well, so that the
+		 * next user of the index is not given a stale timestamp if
+		 * the hardware reports one after the latch has been reclaimed.
+		 */
+		if (!ptp_tx_tstamp->tstamp) {
+			if (time_is_after_jiffies(ptp_tx_tstamp->deadline))
+				continue;
+
+			clear_map |= BIT_ULL(idx);
+		}
+
+		list_move(&ptp_tx_tstamp->list_member, &done);
+	}
+
+	/* The indexes are not recycled before the splice below, so the bits
+	 * of every latch read above can be dropped in a single write.
+	 */
+	idpf_ptp_clear_tx_tstamp_ready(tstamp_caps, clear_map);
+
+	spin_unlock(&tstamp_caps->latches_lock);
+
+	list_for_each_entry(ptp_tx_tstamp, &done, list_member) {
+		if (ptp_tx_tstamp->tstamp) {
+			idpf_ptp_report_tx_tstamp(vport, ptp_tx_tstamp->skb,
+						  ptp_tx_tstamp->tstamp);
+		} else {
+			u64_stats_update_begin(&vport->tstamp_stats.stats_sync);
+			u64_stats_inc(&vport->tstamp_stats.expired);
+			u64_stats_update_end(&vport->tstamp_stats.stats_sync);
+
+			net_warn_ratelimited("%s: Tx timestamp for latch %u never arrived\n",
+					     vport->netdev->name,
+					     ptp_tx_tstamp->idx);
+
+			dev_kfree_skb_any(ptp_tx_tstamp->skb);
+		}
+
+		ptp_tx_tstamp->skb = NULL;
+	}
+
+	spin_lock(&tstamp_caps->latches_lock);
+	list_splice(&done, &tstamp_caps->latches_free);
+	spin_unlock(&tstamp_caps->latches_lock);
+}
+
+/**
+ * idpf_ptp_tx_tstamp_timer_cb - Periodically collect the Tx timestamps
+ * @timer: Tx timestamp collection timer
+ *
+ * Return: HRTIMER_RESTART to keep collecting at a fixed period.
+ */
+static enum hrtimer_restart idpf_ptp_tx_tstamp_timer_cb(struct hrtimer *timer)
+{
+	struct idpf_vport *vport;
+
+	vport = container_of(timer, struct idpf_vport, tx_tstamp_timer);
+
+	idpf_ptp_get_tx_tstamp_direct(vport);
+	hrtimer_forward_now(timer, IDPF_PTP_TX_TSTAMP_PERIOD_NS);
+
+	return HRTIMER_RESTART;
+}
+
+/**
+ * idpf_ptp_tx_tstamp_timer_init - Set up the Tx timestamp collection timer
+ * @vport: Virtual port structure
+ */
+void idpf_ptp_tx_tstamp_timer_init(struct idpf_vport *vport)
+{
+	hrtimer_setup(&vport->tx_tstamp_timer, idpf_ptp_tx_tstamp_timer_cb,
+		      CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
+}
+
+/**
+ * idpf_ptp_tx_tstamp_start - Start collecting the Tx timestamps
+ * @vport: Virtual port structure
+ *
+ * In the direct mode nothing in the Tx path reads the latches, so the
+ * collection is driven by a timer that runs for as long as the Tx
+ * timestamping is enabled and the vport is up.
+ */
+void idpf_ptp_tx_tstamp_start(struct idpf_vport *vport)
+{
+	if (!idpf_ptp_is_vport_tx_tstamp_ena(vport) ||
+	    vport->tx_tstamp_caps->access != IDPF_PTP_DIRECT)
+		return;
+
+	/* The callback re-arms itself, so it must be finished before the timer
+	 * is started again, e.g. when only the Rx filter is being changed.
+	 */
+	if (READ_ONCE(vport->tstamp_config.tx_type) == HWTSTAMP_TX_ON) {
+		hrtimer_cancel(&vport->tx_tstamp_timer);
+		hrtimer_start(&vport->tx_tstamp_timer,
+			      IDPF_PTP_TX_TSTAMP_PERIOD_NS,
+			      HRTIMER_MODE_REL_SOFT);
+	}
+}
+
+/**
+ * idpf_ptp_tx_tstamp_stop - Stop collecting the Tx timestamps
+ * @vport: Virtual port structure
+ *
+ * Stop the collection timer and flush the latches that are still waiting for
+ * a timestamp, as nothing would collect them anymore. No new latch is handed
+ * out afterwards, either because the Tx timestamping has been disabled, which
+ * idpf_ptp_request_ts() checks under the latches lock, or because the Tx path
+ * is stopped.
+ */
+void idpf_ptp_tx_tstamp_stop(struct idpf_vport *vport)
+{
+	struct idpf_ptp_vport_tx_tstamp_caps *tstamp_caps;
+	struct idpf_ptp_tx_tstamp *ptp_tx_tstamp, *tmp;
+	u64 clear_map = 0;
+
+	if (!idpf_ptp_is_vport_tx_tstamp_ena(vport) ||
+	    vport->tx_tstamp_caps->access != IDPF_PTP_DIRECT)
+		return;
+
+	tstamp_caps = vport->tx_tstamp_caps;
+
+	hrtimer_cancel(&vport->tx_tstamp_timer);
+
+	spin_lock_bh(&tstamp_caps->latches_lock);
+
+	u64_stats_update_begin(&vport->tstamp_stats.stats_sync);
+	list_for_each_entry_safe(ptp_tx_tstamp, tmp,
+				 &tstamp_caps->latches_in_use, list_member) {
+		u64_stats_inc(&vport->tstamp_stats.flushed);
+
+		clear_map |= BIT_ULL(ptp_tx_tstamp->idx);
+
+		if (ptp_tx_tstamp->skb)
+			consume_skb(ptp_tx_tstamp->skb);
+
+		ptp_tx_tstamp->skb = NULL;
+		list_move(&ptp_tx_tstamp->list_member,
+			  &tstamp_caps->latches_free);
+	}
+	u64_stats_update_end(&vport->tstamp_stats.stats_sync);
+
+	idpf_ptp_clear_tx_tstamp_ready(tstamp_caps, clear_map);
+
+	spin_unlock_bh(&tstamp_caps->latches_lock);
+}
+
 /**
  * idpf_ptp_set_rx_tstamp - Enable or disable Rx timestamping
  * @vport: Virtual port structure
@@ -923,12 +1274,21 @@ int idpf_ptp_set_timestamp_mode(struct idpf_vport *vport,
 	case HWTSTAMP_TX_ON:
 		if (!idpf_ptp_is_vport_tx_tstamp_ena(vport))
 			return -EINVAL;
+
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	vport->tstamp_config.tx_type = config->tx_type;
+	/* Publish the new mode first, so that the Tx path stops handing out
+	 * latches, then flush the ones that are still waiting.
+	 */
+	WRITE_ONCE(vport->tstamp_config.tx_type, config->tx_type);
+	if (config->tx_type == HWTSTAMP_TX_ON)
+		idpf_ptp_tx_tstamp_start(vport);
+	else
+		idpf_ptp_tx_tstamp_stop(vport);
+
 	idpf_ptp_set_rx_tstamp(vport, config->rx_filter);
 	*config = vport->tstamp_config;
 
@@ -1035,8 +1395,19 @@ static void idpf_ptp_release_vport_tstamp(struct idpf_vport *vport)
 {
 	struct idpf_ptp_tx_tstamp *ptp_tx_tstamp, *tmp;
 	struct list_head *head;
+	u64 clear_map = 0;
 
-	cancel_work_sync(&vport->tstamp_task);
+	switch (vport->tx_tstamp_caps->access) {
+	case IDPF_PTP_MAILBOX:
+		cancel_work_sync(&vport->tstamp_task);
+		break;
+	case IDPF_PTP_DIRECT:
+		hrtimer_cancel(&vport->tx_tstamp_timer);
+		break;
+	case IDPF_PTP_NONE:
+	default:
+		break;
+	}
 
 	/* Remove list with free latches */
 	spin_lock_bh(&vport->tx_tstamp_caps->latches_lock);
@@ -1053,6 +1424,10 @@ static void idpf_ptp_release_vport_tstamp(struct idpf_vport *vport)
 	list_for_each_entry_safe(ptp_tx_tstamp, tmp, head, list_member) {
 		u64_stats_inc(&vport->tstamp_stats.flushed);
 
+		/* The index is only range-checked in the direct mode */
+		if (vport->tx_tstamp_caps->access == IDPF_PTP_DIRECT)
+			clear_map |= BIT_ULL(ptp_tx_tstamp->idx);
+
 		list_del(&ptp_tx_tstamp->list_member);
 		if (ptp_tx_tstamp->skb)
 			consume_skb(ptp_tx_tstamp->skb);
@@ -1061,6 +1436,10 @@ static void idpf_ptp_release_vport_tstamp(struct idpf_vport *vport)
 	}
 	u64_stats_update_end(&vport->tstamp_stats.stats_sync);
 
+	if (vport->tx_tstamp_caps->access == IDPF_PTP_DIRECT)
+		idpf_ptp_clear_tx_tstamp_ready(vport->tx_tstamp_caps,
+					       clear_map);
+
 	spin_unlock_bh(&vport->tx_tstamp_caps->latches_lock);
 
 	kfree(vport->tx_tstamp_caps);
@@ -1098,10 +1477,8 @@ bool idpf_ptp_get_txq_tstamp_capability(struct idpf_tx_queue *txq)
 {
 	if (!txq || !txq->cached_tstamp_caps)
 		return false;
-	else if (txq->cached_tstamp_caps->access)
-		return true;
-	else
-		return false;
+
+	return txq->cached_tstamp_caps->access != IDPF_PTP_NONE;
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.h b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
index 0a2f08b17033..3b4ecf63e176 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_ptp.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
@@ -6,6 +6,17 @@
 
 #include <linux/ptp_clock_kernel.h>
 
+/* Layout of the Tx timestamp latch registers, mandated by the IDPF
+ * specification. Only the register offsets are negotiated with the Control
+ * Plane, the field placement within them is fixed:
+ * Low register:  bit[0] = valid, bit[31:1] = timestamp bits [30:0]
+ * High register: bit[7:0] = timestamp bits [38:31]
+ */
+#define IDPF_PTP_TX_TSTAMP_L_VALID_M	BIT(0)
+#define IDPF_PTP_TX_TSTAMP_L_TSTAMP_M	GENMASK(31, 1)
+#define IDPF_PTP_TX_TSTAMP_H_TSTAMP_M	GENMASK(7, 0)
+#define IDPF_PTP_TX_TSTAMP_H_POS_M	GENMASK_ULL(38, 31)
+
 /**
  * struct idpf_ptp_cmd_mask - PTP command masks
  * @exec_cmd: mask to trigger command execution
@@ -122,40 +133,48 @@ struct idpf_ptp_tx_tstamp_status {
 /**
  * struct idpf_ptp_tx_tstamp - Parameters for Tx timestamping
  * @list_member: the list member structure
- * @tx_latch_reg_offset_l: Tx tstamp latch low register offset
- * @tx_latch_reg_offset_h: Tx tstamp latch high register offset
+ * @tx_latch_reg_addr_l: Tx tstamp latch low register address
+ * @tx_latch_reg_addr_h: Tx tstamp latch high register address
  * @skb: the pointer to the SKB for this timestamp request
  * @tstamp: the Tx tstamp value
+ * @deadline: time after which the latch is reclaimed even if the hardware has
+ *	      not reported the timestamp
  * @idx: the index of the Tx tstamp
  */
 struct idpf_ptp_tx_tstamp {
 	struct list_head list_member;
-	u32 tx_latch_reg_offset_l;
-	u32 tx_latch_reg_offset_h;
+	void __iomem *tx_latch_reg_addr_l;
+	void __iomem *tx_latch_reg_addr_h;
 	struct sk_buff *skb;
 	u64 tstamp;
+	unsigned long deadline;
 	u32 idx;
 };
 
 /**
  * struct idpf_ptp_vport_tx_tstamp_caps - Tx timestamp capabilities
- * @vport_id: the vport id
  * @num_entries: the number of negotiated Tx timestamp entries
  * @tstamp_ns_lo_bit: first bit for nanosecond part of the timestamp
- * @latches_lock: the lock to the lists of free/used timestamp indexes
- * @status_lock: the lock to the status tracker
- * @access: indicates an access to Tx timestamp
+ * @access: the type of the supported access to Tx timestamp
+ * @latches_lock: the lock to the lists of free/used timestamp indexes. Taken
+ *		  from the Tx path and from the timestamp collection timer, so
+ *		  process context users must disable bottom halves
+ * @status_lock: the lock to the status tracker. Mailbox access only, also
+ *		 taken from the NAPI completion path
+ * @readiness_addr_l: Tx tstamp readiness bitmap low register address
+ * @readiness_addr_h: Tx tstamp readiness bitmap high register address
  * @latches_free: the list of the free Tx timestamps latches
  * @latches_in_use: the list of the used Tx timestamps latches
  * @tx_tstamp_status: Tx tstamp status tracker
  */
 struct idpf_ptp_vport_tx_tstamp_caps {
-	u32 vport_id;
 	u16 num_entries;
 	u16 tstamp_ns_lo_bit;
+	enum idpf_ptp_access access;
 	spinlock_t latches_lock;
 	spinlock_t status_lock;
-	bool access:1;
+	void __iomem *readiness_addr_l;
+	void __iomem *readiness_addr_h;
 	struct list_head latches_free;
 	struct list_head latches_in_use;
 	struct idpf_ptp_tx_tstamp_status tx_tstamp_status[];
@@ -302,12 +321,18 @@ int idpf_ptp_adj_dev_clk_fine_mb(struct idpf_adapter *adapter, u64 incval);
 int idpf_ptp_adj_dev_clk_time_mb(struct idpf_adapter *adapter, s64 delta);
 int idpf_ptp_get_vport_tstamps_caps(struct idpf_vport *vport);
 int idpf_ptp_get_tx_tstamp_mb(struct idpf_vport *vport);
+void
+idpf_ptp_clear_tx_tstamp_ready(struct idpf_ptp_vport_tx_tstamp_caps *caps,
+			       u64 map);
 int idpf_ptp_set_timestamp_mode(struct idpf_vport *vport,
 				struct kernel_hwtstamp_config *config);
 u64 idpf_ptp_extend_ts(struct idpf_vport *vport, u64 in_tstamp);
 u64 idpf_ptp_tstamp_extend_32b_to_64b(u64 cached_phc_time, u32 in_timestamp);
 int idpf_ptp_request_ts(struct idpf_tx_queue *tx_q, struct sk_buff *skb,
 			u32 *idx);
+void idpf_ptp_tx_tstamp_timer_init(struct idpf_vport *vport);
+void idpf_ptp_tx_tstamp_start(struct idpf_vport *vport);
+void idpf_ptp_tx_tstamp_stop(struct idpf_vport *vport);
 void idpf_tstamp_task(struct work_struct *work);
 #else /* CONFIG_PTP_1588_CLOCK */
 static inline int idpf_ptp_init(struct idpf_adapter *adapter)
@@ -400,6 +425,13 @@ static inline int idpf_ptp_request_ts(struct idpf_tx_queue *tx_q,
 	return -EOPNOTSUPP;
 }
 
+static inline void idpf_ptp_tx_tstamp_timer_init(struct idpf_vport *vport) { }
+
+static inline void idpf_ptp_tx_tstamp_start(struct idpf_vport *vport) { }
+
+static inline void idpf_ptp_tx_tstamp_stop(struct idpf_vport *vport) { }
+
 static inline void idpf_tstamp_task(struct work_struct *work) { }
+
 #endif /* CONFIG_PTP_1588_CLOCK */
 #endif /* _IDPF_PTP_H */
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
index 8239c1e59620..d687b7c240e4 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
@@ -1459,9 +1459,13 @@ static int idpf_vport_init_fast_path_txqs(struct idpf_vport *vport,
 					  struct idpf_q_vec_rsrc *rsrc)
 {
 	struct idpf_ptp_vport_tx_tstamp_caps *caps = vport->tx_tstamp_caps;
-	struct work_struct *tstamp_task = &vport->tstamp_task;
+	struct work_struct *tstamp_task = NULL;
 	int k = 0;
 
+	if (vport->adapter->ptp &&
+	    vport->adapter->ptp->tx_tstamp_access != IDPF_PTP_NONE)
+		tstamp_task = &vport->tstamp_task;
+
 	vport->txqs = kzalloc_objs(*vport->txqs, rsrc->num_txq);
 	if (!vport->txqs)
 		return -ENOMEM;
@@ -2030,7 +2034,8 @@ int idpf_vport_queues_alloc(struct idpf_vport *vport,
  * @skb: socket buffer to provide Tx timestamp value
  *
  * Schedule a work to read Tx timestamp value generated once the packet is
- * transmitted.
+ * transmitted. In the direct access mode the timestamps are collected by a
+ * periodic timer instead, so that no register access is needed here.
  */
 static void idpf_tx_read_tstamp(struct idpf_tx_queue *txq, struct sk_buff *skb)
 {
@@ -2038,7 +2043,10 @@ static void idpf_tx_read_tstamp(struct idpf_tx_queue *txq, struct sk_buff *skb)
 	struct idpf_ptp_tx_tstamp_status *tx_tstamp_status;
 
 	tx_tstamp_caps = txq->cached_tstamp_caps;
-	spin_lock_bh(&tx_tstamp_caps->status_lock);
+	if (!tx_tstamp_caps || tx_tstamp_caps->access != IDPF_PTP_MAILBOX)
+		return;
+
+	spin_lock(&tx_tstamp_caps->status_lock);
 
 	for (u32 i = 0; i < tx_tstamp_caps->num_entries; i++) {
 		tx_tstamp_status = &tx_tstamp_caps->tx_tstamp_status[i];
@@ -2055,7 +2063,7 @@ static void idpf_tx_read_tstamp(struct idpf_tx_queue *txq, struct sk_buff *skb)
 		break;
 	}
 
-	spin_unlock_bh(&tx_tstamp_caps->status_lock);
+	spin_unlock(&tx_tstamp_caps->status_lock);
 }
 
 #define idpf_tx_splitq_clean_bump_ntc(txq, ntc, desc, buf)	\
@@ -2981,6 +2989,13 @@ static int idpf_tx_tstamp(struct idpf_tx_queue *tx_q, struct sk_buff *skb,
 	if (likely(!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)))
 		return -1;
 
+	/* Cheap check to skip the latches lock, the authoritative one is done
+	 * under it by idpf_ptp_request_ts().
+	 */
+	if (READ_ONCE(idpf_netdev_to_vport(tx_q->netdev)->tstamp_config.tx_type) !=
+	    HWTSTAMP_TX_ON)
+		return -1;
+
 	if (!idpf_ptp_get_txq_tstamp_capability(tx_q))
 		return -1;
 
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index 1a77bcbe48d8..04236f0ff20e 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -3364,7 +3364,17 @@ int idpf_vport_init(struct idpf_vport *vport, struct idpf_vport_max_q *max_q)
 		return 0;
 	}
 
-	INIT_WORK(&vport->tstamp_task, idpf_tstamp_task);
+	switch (vport->adapter->ptp->tx_tstamp_access) {
+	case IDPF_PTP_MAILBOX:
+		INIT_WORK(&vport->tstamp_task, idpf_tstamp_task);
+		break;
+	case IDPF_PTP_DIRECT:
+		idpf_ptp_tx_tstamp_timer_init(vport);
+		break;
+	case IDPF_PTP_NONE:
+	default:
+		break;
+	}
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
index ef71df2d51ef..5cf441708ed3 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
@@ -48,6 +48,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
 				    VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME_MB |
 				    VIRTCHNL2_CAP_PTP_ADJ_DEVICE_CLK |
 				    VIRTCHNL2_CAP_PTP_ADJ_DEVICE_CLK_MB |
+				    VIRTCHNL2_CAP_PTP_TX_TSTAMPS |
 				    VIRTCHNL2_CAP_PTP_TX_TSTAMPS_MB)
 	};
 	struct libie_ctlq_xn_send_params xn_params = {
@@ -405,22 +406,24 @@ int idpf_ptp_adj_dev_clk_fine_mb(struct idpf_adapter *adapter, u64 incval)
  */
 int idpf_ptp_get_vport_tstamps_caps(struct idpf_vport *vport)
 {
+	struct libie_mmio_info *mmio = &vport->adapter->ctlq_ctx.mmio_info;
 	struct virtchnl2_ptp_get_vport_tx_tstamp_caps send_tx_tstamp_caps;
 	struct virtchnl2_ptp_get_vport_tx_tstamp_caps *rcv_tx_tstamp_caps;
 	struct virtchnl2_ptp_tx_tstamp_latch_caps tx_tstamp_latch_caps;
+	enum idpf_ptp_access tstamp_access, get_dev_clk_access;
 	struct idpf_ptp_vport_tx_tstamp_caps *tstamp_caps;
 	struct idpf_ptp_tx_tstamp *ptp_tx_tstamp, *tmp;
 	struct libie_ctlq_xn_send_params xn_params = {
 		.chnl_opcode = VIRTCHNL2_OP_PTP_GET_VPORT_TX_TSTAMP_CAPS,
 		.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
 	};
-	enum idpf_ptp_access tstamp_access, get_dev_clk_access;
 	struct idpf_ptp *ptp = vport->adapter->ptp;
 	struct list_head *head;
+	u64 clear_map = 0;
 	size_t reply_sz;
 	u16 num_latches;
+	u32 size, temp;
 	int err = 0;
-	u32 size;
 
 	if (!ptp)
 		return -EOPNOTSUPP;
@@ -458,7 +461,7 @@ int idpf_ptp_get_vport_tstamps_caps(struct idpf_vport *vport)
 		goto get_tstamp_caps_out;
 	}
 
-	tstamp_caps->access = true;
+	tstamp_caps->access = tstamp_access;
 	tstamp_caps->num_entries = num_latches;
 
 	INIT_LIST_HEAD(&tstamp_caps->latches_in_use);
@@ -469,9 +472,22 @@ int idpf_ptp_get_vport_tstamps_caps(struct idpf_vport *vport)
 
 	tstamp_caps->tstamp_ns_lo_bit = rcv_tx_tstamp_caps->tstamp_ns_lo_bit;
 
-	for (u16 i = 0; i < tstamp_caps->num_entries; i++) {
-		__le32 offset_l, offset_h;
+	if (tstamp_access == IDPF_PTP_DIRECT) {
+		temp = le32_to_cpu(rcv_tx_tstamp_caps->readiness_offset_l);
+		tstamp_caps->readiness_addr_l =
+			libie_pci_get_mmio_addr(mmio, temp);
+		temp = le32_to_cpu(rcv_tx_tstamp_caps->readiness_offset_h);
+		tstamp_caps->readiness_addr_h =
+			libie_pci_get_mmio_addr(mmio, temp);
+
+		if (!tstamp_caps->readiness_addr_l ||
+		    !tstamp_caps->readiness_addr_h) {
+			err = -EINVAL;
+			goto err_free_tstamp_caps;
+		}
+	}
 
+	for (u16 i = 0; i < num_latches; i++) {
 		ptp_tx_tstamp = kzalloc_obj(*ptp_tx_tstamp);
 		if (!ptp_tx_tstamp) {
 			err = -ENOMEM;
@@ -483,25 +499,59 @@ int idpf_ptp_get_vport_tstamps_caps(struct idpf_vport *vport)
 		if (tstamp_access != IDPF_PTP_DIRECT)
 			goto skip_offsets;
 
-		offset_l = tx_tstamp_latch_caps.tx_latch_reg_offset_l;
-		offset_h = tx_tstamp_latch_caps.tx_latch_reg_offset_h;
-		ptp_tx_tstamp->tx_latch_reg_offset_l = le32_to_cpu(offset_l);
-		ptp_tx_tstamp->tx_latch_reg_offset_h = le32_to_cpu(offset_h);
+		temp = le32_to_cpu(tx_tstamp_latch_caps.tx_latch_reg_offset_l);
+		ptp_tx_tstamp->tx_latch_reg_addr_l =
+			libie_pci_get_mmio_addr(mmio, temp);
+		temp = le32_to_cpu(tx_tstamp_latch_caps.tx_latch_reg_offset_h);
+		ptp_tx_tstamp->tx_latch_reg_addr_h =
+			libie_pci_get_mmio_addr(mmio, temp);
+
+		if (!ptp_tx_tstamp->tx_latch_reg_addr_l ||
+		    !ptp_tx_tstamp->tx_latch_reg_addr_h) {
+			err = -EINVAL;
+			goto err_free_ptp_tx_tstamp;
+		}
 
 skip_offsets:
+		/* Direct mode uses a 64-bit readiness bitmap, so an index
+		 * outside of its range means the Control Plane sent a
+		 * malformed message and nothing it reported can be trusted.
+		 */
+		if (tstamp_access == IDPF_PTP_DIRECT &&
+		    tx_tstamp_latch_caps.index >= BITS_PER_TYPE(u64)) {
+			pci_err(vport->adapter->pdev,
+				"Tx timestamp latch index %u out of the readiness bitmap range\n",
+				tx_tstamp_latch_caps.index);
+			err = -EINVAL;
+			goto err_free_ptp_tx_tstamp;
+		}
+
 		ptp_tx_tstamp->idx = tx_tstamp_latch_caps.index;
 
+		/* The readiness bits are write-1-to-clear, so collect the ones
+		 * that a previous owner of the latch may have left set. The
+		 * index is only range-checked in the direct mode.
+		 */
+		if (tstamp_access == IDPF_PTP_DIRECT)
+			clear_map |= BIT_ULL(ptp_tx_tstamp->idx);
+
 		list_add(&ptp_tx_tstamp->list_member,
 			 &tstamp_caps->latches_free);
 
 		tstamp_caps->tx_tstamp_status[i].state = IDPF_PTP_FREE;
 	}
 
+	/* No traffic is flowing yet, so a single write drops every stale bit */
+	if (tstamp_access == IDPF_PTP_DIRECT)
+		idpf_ptp_clear_tx_tstamp_ready(tstamp_caps, clear_map);
+
 	vport->tx_tstamp_caps = tstamp_caps;
 	libie_ctlq_release_rx_buf(&xn_params.recv_mem);
 
 	return 0;
 
+err_free_ptp_tx_tstamp:
+	kfree(ptp_tx_tstamp);
 err_free_ptp_tx_stamp_list:
 	head = &tstamp_caps->latches_free;
 	list_for_each_entry_safe(ptp_tx_tstamp, tmp, head, list_member) {
@@ -509,6 +559,7 @@ int idpf_ptp_get_vport_tstamps_caps(struct idpf_vport *vport)
 		kfree(ptp_tx_tstamp);
 	}
 
+err_free_tstamp_caps:
 	kfree(tstamp_caps);
 get_tstamp_caps_out:
 	libie_ctlq_release_rx_buf(&xn_params.recv_mem);
@@ -521,8 +572,8 @@ int idpf_ptp_get_vport_tstamps_caps(struct idpf_vport *vport)
  *				    the skb compatibility.
  * @caps: Tx timestamp capabilities that monitor the latch status
  * @skb: skb for which the tstamp value is returned through virtchnl message
- * @current_state: Current state of the Tx timestamp latch
- * @expected_state: Expected state of the Tx timestamp latch
+ * @state_before: Current state of the Tx timestamp latch
+ * @state_after: Expected state of the Tx timestamp latch
  *
  * Find a proper skb tracker for which the Tx timestamp is received and change
  * the state to expected value.
@@ -532,24 +583,23 @@ int idpf_ptp_get_vport_tstamps_caps(struct idpf_vport *vport)
 static bool
 idpf_ptp_update_tstamp_tracker(struct idpf_ptp_vport_tx_tstamp_caps *caps,
 			       struct sk_buff *skb,
-			       enum idpf_ptp_tx_tstamp_state current_state,
-			       enum idpf_ptp_tx_tstamp_state expected_state)
+			       enum idpf_ptp_tx_tstamp_state state_before,
+			       enum idpf_ptp_tx_tstamp_state state_after)
 {
 	bool updated = false;
 
-	spin_lock(&caps->status_lock);
+	spin_lock_bh(&caps->status_lock);
 	for (u16 i = 0; i < caps->num_entries; i++) {
 		struct idpf_ptp_tx_tstamp_status *status;
 
 		status = &caps->tx_tstamp_status[i];
-
-		if (skb == status->skb && status->state == current_state) {
-			status->state = expected_state;
+		if (skb == status->skb && status->state == state_before) {
+			status->state = state_after;
 			updated = true;
 			break;
 		}
 	}
-	spin_unlock(&caps->status_lock);
+	spin_unlock_bh(&caps->status_lock);
 
 	return updated;
 }
diff --git a/include/linux/net/intel/virtchnl2.h b/include/linux/net/intel/virtchnl2.h
index 39fea65c075c..80787197e7e2 100644
--- a/include/linux/net/intel/virtchnl2.h
+++ b/include/linux/net/intel/virtchnl2.h
@@ -1464,7 +1464,8 @@ VIRTCHNL2_CHECK_STRUCT_LEN(16, virtchnl2_ptp_tx_tstamp_latch_caps);
  * @num_latches: Total number of latches
  * @tstamp_ns_lo_bit: First bit for nanosecond part of the timestamp
  * @tstamp_ns_hi_bit: Last bit for nanosecond part of the timestamp
- * @pad: Padding for future tstamp granularity extensions
+ * @readiness_offset_l: Tx tstamp readiness bitmap low register offset
+ * @readiness_offset_h: Tx tstamp readiness bitmap high register offset
  * @tstamp_latches: Capabilities of Tx timestamp entries
  *
  * PF/VF sends this message to negotiate the Tx timestamp latches for each
@@ -1477,7 +1478,8 @@ struct virtchnl2_ptp_get_vport_tx_tstamp_caps {
 	__le16 num_latches;
 	u8 tstamp_ns_lo_bit;
 	u8 tstamp_ns_hi_bit;
-	u8 pad[8];
+	__le32 readiness_offset_l;
+	__le32 readiness_offset_h;
 
 	struct virtchnl2_ptp_tx_tstamp_latch_caps tstamp_latches[]
 						  __counted_by_le(num_latches);
-- 
2.47.3

Re: [PATCH iwl-next v3 2/2] idpf: add direct Tx timestamp readout

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] idpf: premature idpf_ptp_release() before vport stop in
  idpf_vc_core_deinit()
- [Medium] idpf: stale W1C readiness bits left set after latch timeout
  or flush
- [Medium] idpf: missing IDPF_VPORT_UP check in
  idpf_ptp_tx_tstamp_start() and mode set
--

commit 4103e67711694e72d8e2828d99514ccf51cd2f07
Author: Przemyslaw Korba <przemyslaw.korba@intel.com>

idpf: add direct Tx timestamp readout

This commit adds direct BAR register access for reading Tx timestamps via a
periodic 100 us hrtimer, along with write-1-to-clear readiness bitmap
handling and a 500 ms expiration timeout for unlatched requests.

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> index df2da430ff28f..5d745a53d6865 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c

[ ... ]

> @@ -851,19 +925,296 @@ int idpf_ptp_request_ts(struct idpf_tx_queue *tx_q, struct sk_buff *skb,

[ ... ]

> +		/* A latch with no timestamp is kept in use until its deadline
> +		 * passes, as the hardware may still be about to report it.
> +		 * Once it does, drop its readiness bit as well, so that the
> +		 * next user of the index is not given a stale timestamp if
> +		 * the hardware reports one after the latch has been reclaimed.
> +		 */
> +		if (!ptp_tx_tstamp->tstamp) {
> +			if (time_is_after_jiffies(ptp_tx_tstamp->deadline))
> +				continue;
> +
> +			clear_map |= BIT_ULL(idx);
> +		}
> +
> +		list_move(&ptp_tx_tstamp->list_member, &done);
> +	}
> +
> +	/* The indexes are not recycled before the splice below, so the bits
> +	 * of every latch read above can be dropped in a single write.
> +	 */
> +	idpf_ptp_clear_tx_tstamp_ready(tstamp_caps, clear_map);

[Severity: Medium]
Can a packet that latches in hardware after its deadline expires (or after
idpf_ptp_tx_tstamp_stop() flushes latches_in_use) leave its write-1-to-clear
readiness bit set while the latch sits on latches_free?

Because readiness_addr_l and readiness_addr_h are write-1-to-clear, calling
idpf_ptp_clear_tx_tstamp_ready() when the deadline expires (or when
HWTSTAMP_TX_OFF flushes latches_in_use on a live vport) is a no-op for any
in-flight packet whose readiness bit is still 0 in hardware.  If that queued
packet finishes transmission afterwards (for example, after SO_TXTIME pacing
or flow control), the hardware sets both the readiness bit and
IDPF_PTP_TX_TSTAMP_L_VALID_M while the latch is on latches_free, so the next
packet allocated that index will report the previous packet's stale
timestamp.

Clearing stale readiness bits in idpf_ptp_get_tx_tstamp_direct() for bits
not currently in latches_in_use (and in idpf_ptp_tx_tstamp_start()) would
avoid handing out a latch with a pending hardware timestamp.

[ ... ]

> +void idpf_ptp_tx_tstamp_start(struct idpf_vport *vport)
> +{
> +	if (!idpf_ptp_is_vport_tx_tstamp_ena(vport) ||
> +	    vport->tx_tstamp_caps->access != IDPF_PTP_DIRECT)
> +		return;
> +
> +	/* The callback re-arms itself, so it must be finished before the timer
> +	 * is started again, e.g. when only the Rx filter is being changed.
> +	 */
> +	if (READ_ONCE(vport->tstamp_config.tx_type) == HWTSTAMP_TX_ON) {
> +		hrtimer_cancel(&vport->tx_tstamp_timer);
> +		hrtimer_start(&vport->tx_tstamp_timer,
> +			      IDPF_PTP_TX_TSTAMP_PERIOD_NS,
> +			      HRTIMER_MODE_REL_SOFT);
> +	}
> +}

[ ... ]

> @@ -923,12 +1274,21 @@ int idpf_ptp_set_timestamp_mode(struct idpf_vport *vport,
>  	case HWTSTAMP_TX_ON:
>  		if (!idpf_ptp_is_vport_tx_tstamp_ena(vport))
>  			return -EINVAL;
> +
>  		break;
>  	default:
>  		return -EINVAL;
>  	}
>  
> -	vport->tstamp_config.tx_type = config->tx_type;
> +	/* Publish the new mode first, so that the Tx path stops handing out
> +	 * latches, then flush the ones that are still waiting.
> +	 */
> +	WRITE_ONCE(vport->tstamp_config.tx_type, config->tx_type);
> +	if (config->tx_type == HWTSTAMP_TX_ON)
> +		idpf_ptp_tx_tstamp_start(vport);
> +	else
> +		idpf_ptp_tx_tstamp_stop(vport);
> +
>  	idpf_ptp_set_rx_tstamp(vport, config->rx_filter);
>  	*config = vport->tstamp_config;

[Severity: Medium]
Can idpf_hwtstamp_set() -> idpf_ptp_set_timestamp_mode() start
vport->tx_tstamp_timer and dereference a NULL rsrc->rxq_grps when the vport
is down?

idpf_hwtstamp_set() only checks vport->link_up, which
idpf_handle_event_link() sets even while IDPF_VPORT_UP is clear and queues
are not allocated.  Calling idpf_ptp_set_timestamp_mode() in that state arms
tx_tstamp_timer on a stopped vport and then crashes in
idpf_ptp_set_rx_tstamp() when indexing rsrc->rxq_grps.

Also, when a vport is reopened, idpf_vport_open() calls
idpf_ptp_tx_tstamp_start(vport) before idpf_up_complete() sets IDPF_VPORT_UP,
and never calls idpf_ptp_set_rx_tstamp() to restore the PTP flag on the
newly allocated Rx queues.

> @@ -1035,8 +1395,19 @@ static void idpf_ptp_release_vport_tstamp(struct idpf_vport *vport)
>  {
>  	struct idpf_ptp_tx_tstamp *ptp_tx_tstamp, *tmp;
>  	struct list_head *head;
> +	u64 clear_map = 0;
>  
> -	cancel_work_sync(&vport->tstamp_task);
> +	switch (vport->tx_tstamp_caps->access) {
> +	case IDPF_PTP_MAILBOX:
> +		cancel_work_sync(&vport->tstamp_task);
> +		break;
> +	case IDPF_PTP_DIRECT:
> +		hrtimer_cancel(&vport->tx_tstamp_timer);
> +		break;
> +	case IDPF_PTP_NONE:
> +	default:
> +		break;
> +	}
>  
>  	/* Remove list with free latches */
>  	spin_lock_bh(&vport->tx_tstamp_caps->latches_lock);

[ ... ]

> @@ -1061,6 +1436,10 @@ static void idpf_ptp_release_vport_tstamp(struct idpf_vport *vport)
>  	}
>  	u64_stats_update_end(&vport->tstamp_stats.stats_sync);
>  
> +	if (vport->tx_tstamp_caps->access == IDPF_PTP_DIRECT)
> +		idpf_ptp_clear_tx_tstamp_ready(vport->tx_tstamp_caps,
> +					       clear_map);
> +
>  	spin_unlock_bh(&vport->tx_tstamp_caps->latches_lock);
>  
>  	kfree(vport->tx_tstamp_caps);

[Severity: High]
Can idpf_vc_core_deinit() free vport->tx_tstamp_caps while Tx queues and
ndo_hwtstamp_set are still active?

drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:idpf_vc_core_deinit() {
    ...
	idpf_ptp_release(adapter);
	idpf_deinit_task(adapter);
    ...
}

During idpf_remove() and idpf_shutdown(), idpf_ptp_release() runs before
idpf_deinit_task() stops the vports and unregisters the netdevs, and without
holding vport_ctrl_lock:

CPU0                                  CPU1
idpf_vc_core_deinit()
  idpf_ptp_release()
    idpf_ptp_release_vport_tstamp()
      hrtimer_cancel(&tx_tstamp_timer)
                                      idpf_hwtstamp_set()
                                        ...
                                          idpf_ptp_tx_tstamp_start()
                                            hrtimer_start(&tx_tstamp_timer)
      kfree(vport->tx_tstamp_caps)
      vport->tx_tstamp_caps = NULL
                                      <softirq>
                                        idpf_ptp_tx_tstamp_timer_cb()
                                          idpf_ptp_get_tx_tstamp_direct()
                                            spin_lock(&tstamp_caps->...)
                                            <-- NULL pointer dereference

In addition, because vport->tstamp_config.tx_type remains HWTSTAMP_TX_ON and
each queue's tx_q->cached_tstamp_caps still points to the freed structure
until idpf_deinit_task() -> idpf_vport_stop() disables Tx, any packet
transmitted or completed in the meantime dereferences freed
tx_q->cached_tstamp_caps in idpf_ptp_request_ts() or idpf_tx_read_tstamp().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112123503.234508-1-przemyslaw.korba@intel.com?part=2

RE: [PATCH iwl-next v3 2/2] idpf: add direct Tx timestamp readout

[Korba, Przemyslaw]

> -----Original Message-----
> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Thursday, November 12, 2026 2:23 PM
> To: Korba, Przemyslaw <przemyslaw.korba@intel.com>; intel-wired-lan@lists.osuosl.org
> Subject: Re: [PATCH iwl-next v3 2/2] idpf: add direct Tx timestamp readout
> 
> Thank you for your contribution! Sashiko AI review found 3 potential
> issues to consider:
> - [High] idpf: premature idpf_ptp_release() before vport stop in
>   idpf_vc_core_deinit()
> - [Medium] idpf: stale W1C readiness bits left set after latch timeout
>   or flush
> - [Medium] idpf: missing IDPF_VPORT_UP check in
>   idpf_ptp_tx_tstamp_start() and mode set
> --
> 
> commit 4103e67711694e72d8e2828d99514ccf51cd2f07
> Author: Przemyslaw Korba <przemyslaw.korba@intel.com>
> 
> idpf: add direct Tx timestamp readout
> 
> This commit adds direct BAR register access for reading Tx timestamps via a
> periodic 100 us hrtimer, along with write-1-to-clear readiness bitmap
> handling and a 500 ms expiration timeout for unlatched requests.
> 
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> > index df2da430ff28f..5d745a53d6865 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> 
> [ ... ]
> 
> > @@ -851,19 +925,296 @@ int idpf_ptp_request_ts(struct idpf_tx_queue *tx_q, struct sk_buff *skb,
> 
> [ ... ]
> 
> > +		/* A latch with no timestamp is kept in use until its deadline
> > +		 * passes, as the hardware may still be about to report it.
> > +		 * Once it does, drop its readiness bit as well, so that the
> > +		 * next user of the index is not given a stale timestamp if
> > +		 * the hardware reports one after the latch has been reclaimed.
> > +		 */
> > +		if (!ptp_tx_tstamp->tstamp) {
> > +			if (time_is_after_jiffies(ptp_tx_tstamp->deadline))
> > +				continue;
> > +
> > +			clear_map |= BIT_ULL(idx);
> > +		}
> > +
> > +		list_move(&ptp_tx_tstamp->list_member, &done);
> > +	}
> > +
> > +	/* The indexes are not recycled before the splice below, so the bits
> > +	 * of every latch read above can be dropped in a single write.
> > +	 */
> > +	idpf_ptp_clear_tx_tstamp_ready(tstamp_caps, clear_map);
> 
> [Severity: Medium]
> Can a packet that latches in hardware after its deadline expires (or after
> idpf_ptp_tx_tstamp_stop() flushes latches_in_use) leave its write-1-to-clear
> readiness bit set while the latch sits on latches_free?
> 
> Because readiness_addr_l and readiness_addr_h are write-1-to-clear, calling
> idpf_ptp_clear_tx_tstamp_ready() when the deadline expires (or when
> HWTSTAMP_TX_OFF flushes latches_in_use on a live vport) is a no-op for any
> in-flight packet whose readiness bit is still 0 in hardware.  If that queued
> packet finishes transmission afterwards (for example, after SO_TXTIME pacing
> or flow control), the hardware sets both the readiness bit and
> IDPF_PTP_TX_TSTAMP_L_VALID_M while the latch is on latches_free, so the next
> packet allocated that index will report the previous packet's stale
> timestamp.
> 
> Clearing stale readiness bits in idpf_ptp_get_tx_tstamp_direct() for bits
> not currently in latches_in_use (and in idpf_ptp_tx_tstamp_start()) would
> avoid handing out a latch with a pending hardware timestamp.
> 

We already clear the bit on both paths, the timeout path is for packets dropped in the device, and the flush race is a microsecond-scale window the hardware gives us no way to close.
Won't fix in this series

> [ ... ]
> 
> > +void idpf_ptp_tx_tstamp_start(struct idpf_vport *vport)
> > +{
> > +	if (!idpf_ptp_is_vport_tx_tstamp_ena(vport) ||
> > +	    vport->tx_tstamp_caps->access != IDPF_PTP_DIRECT)
> > +		return;
> > +
> > +	/* The callback re-arms itself, so it must be finished before the timer
> > +	 * is started again, e.g. when only the Rx filter is being changed.
> > +	 */
> > +	if (READ_ONCE(vport->tstamp_config.tx_type) == HWTSTAMP_TX_ON) {
> > +		hrtimer_cancel(&vport->tx_tstamp_timer);
> > +		hrtimer_start(&vport->tx_tstamp_timer,
> > +			      IDPF_PTP_TX_TSTAMP_PERIOD_NS,
> > +			      HRTIMER_MODE_REL_SOFT);
> > +	}
> > +}
> 
> [ ... ]
> 
> > @@ -923,12 +1274,21 @@ int idpf_ptp_set_timestamp_mode(struct idpf_vport *vport,
> >  	case HWTSTAMP_TX_ON:
> >  		if (!idpf_ptp_is_vport_tx_tstamp_ena(vport))
> >  			return -EINVAL;
> > +
> >  		break;
> >  	default:
> >  		return -EINVAL;
> >  	}
> >
> > -	vport->tstamp_config.tx_type = config->tx_type;
> > +	/* Publish the new mode first, so that the Tx path stops handing out
> > +	 * latches, then flush the ones that are still waiting.
> > +	 */
> > +	WRITE_ONCE(vport->tstamp_config.tx_type, config->tx_type);
> > +	if (config->tx_type == HWTSTAMP_TX_ON)
> > +		idpf_ptp_tx_tstamp_start(vport);
> > +	else
> > +		idpf_ptp_tx_tstamp_stop(vport);
> > +
> >  	idpf_ptp_set_rx_tstamp(vport, config->rx_filter);
> >  	*config = vport->tstamp_config;
> 
> [Severity: Medium]
> Can idpf_hwtstamp_set() -> idpf_ptp_set_timestamp_mode() start
> vport->tx_tstamp_timer and dereference a NULL rsrc->rxq_grps when the vport
> is down?
> 
> idpf_hwtstamp_set() only checks vport->link_up, which
> idpf_handle_event_link() sets even while IDPF_VPORT_UP is clear and queues
> are not allocated.  Calling idpf_ptp_set_timestamp_mode() in that state arms
> tx_tstamp_timer on a stopped vport and then crashes in
> idpf_ptp_set_rx_tstamp() when indexing rsrc->rxq_grps.
> 
> Also, when a vport is reopened, idpf_vport_open() calls
> idpf_ptp_tx_tstamp_start(vport) before idpf_up_complete() sets IDPF_VPORT_UP,
> and never calls idpf_ptp_set_rx_tstamp() to restore the PTP flag on the
> newly allocated Rx queues.

The rxq_grps dereference and the Rx PTP flag not being restored on reopen are both in the 
existing Rx timestamp code, not in this patch, so they fit better as a separate fix.
Won't fix in this series
> 
> > @@ -1035,8 +1395,19 @@ static void idpf_ptp_release_vport_tstamp(struct idpf_vport *vport)
> >  {
> >  	struct idpf_ptp_tx_tstamp *ptp_tx_tstamp, *tmp;
> >  	struct list_head *head;
> > +	u64 clear_map = 0;
> >
> > -	cancel_work_sync(&vport->tstamp_task);
> > +	switch (vport->tx_tstamp_caps->access) {
> > +	case IDPF_PTP_MAILBOX:
> > +		cancel_work_sync(&vport->tstamp_task);
> > +		break;
> > +	case IDPF_PTP_DIRECT:
> > +		hrtimer_cancel(&vport->tx_tstamp_timer);
> > +		break;
> > +	case IDPF_PTP_NONE:
> > +	default:
> > +		break;
> > +	}
> >
> >  	/* Remove list with free latches */
> >  	spin_lock_bh(&vport->tx_tstamp_caps->latches_lock);
> 
> [ ... ]
> 
> > @@ -1061,6 +1436,10 @@ static void idpf_ptp_release_vport_tstamp(struct idpf_vport *vport)
> >  	}
> >  	u64_stats_update_end(&vport->tstamp_stats.stats_sync);
> >
> > +	if (vport->tx_tstamp_caps->access == IDPF_PTP_DIRECT)
> > +		idpf_ptp_clear_tx_tstamp_ready(vport->tx_tstamp_caps,
> > +					       clear_map);
> > +
> >  	spin_unlock_bh(&vport->tx_tstamp_caps->latches_lock);
> >
> >  	kfree(vport->tx_tstamp_caps);
> 
> [Severity: High]
> Can idpf_vc_core_deinit() free vport->tx_tstamp_caps while Tx queues and
> ndo_hwtstamp_set are still active?
> 
> drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:idpf_vc_core_deinit() {
>     ...
> 	idpf_ptp_release(adapter);
> 	idpf_deinit_task(adapter);
>     ...
> }
> 
> During idpf_remove() and idpf_shutdown(), idpf_ptp_release() runs before
> idpf_deinit_task() stops the vports and unregisters the netdevs, and without
> holding vport_ctrl_lock:
> 
> CPU0                                  CPU1
> idpf_vc_core_deinit()
>   idpf_ptp_release()
>     idpf_ptp_release_vport_tstamp()
>       hrtimer_cancel(&tx_tstamp_timer)
>                                       idpf_hwtstamp_set()
>                                         ...
>                                           idpf_ptp_tx_tstamp_start()
>                                             hrtimer_start(&tx_tstamp_timer)
>       kfree(vport->tx_tstamp_caps)
>       vport->tx_tstamp_caps = NULL
>                                       <softirq>
>                                         idpf_ptp_tx_tstamp_timer_cb()
>                                           idpf_ptp_get_tx_tstamp_direct()
>                                             spin_lock(&tstamp_caps->...)
>                                             <-- NULL pointer dereference
> 
> In addition, because vport->tstamp_config.tx_type remains HWTSTAMP_TX_ON and
> each queue's tx_q->cached_tstamp_caps still points to the freed structure
> until idpf_deinit_task() -> idpf_vport_stop() disables Tx, any packet
> transmitted or completed in the meantime dereferences freed
> tx_q->cached_tstamp_caps in idpf_ptp_request_ts() or idpf_tx_read_tstamp().
> 

The ordering exists, but it isn't introduced by this series. idpf_ptp_release() followed by idpf_deinit_task() has been in idpf_vc_core_deinit() for a long time.
Won't fix in this series

> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261112123503.234508-1-przemyslaw.korba@intel.com?part=2