The timesync register space is not accessible through a secondary NAC.
ice_get_primary_hw() currently falls back to the caller's hardware when
the control PF is absent, causing secondary PFs to access the wrong
register space after the control PF is removed.
Return NULL instead and propagate the missing control PF through PHC
reads and timer setup paths. This leaves secondary PFs operational while
PTP requests fail safely until a control PF is available again.
A failed PHC read invalidates the cached PHC time by publishing zero.
ice_ptp_get_rx_hwts() already treats a zero cache as "no timestamp";
teach ice_ptp_extend_40b_ts() to do the same so that Tx timestamps are
discarded rather than extended against a value that no longer tracks
the clock.
Fixes: e2193f9f9ec9 ("ice: enable timesync operation on 2xNAC E825 devices")
Signed-off-by: Sergey Temerkhanov <[email protected]>
Reviewed-by: Aleksandr Loktionov <[email protected]>
---
drivers/net/ethernet/intel/ice/ice.h | 9 +-
drivers/net/ethernet/intel/ice/ice_ptp.c | 85 ++++++++++++++-----
drivers/net/ethernet/intel/ice/ice_ptp.h | 11 +--
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 24 +++++-
.../net/ethernet/intel/ice/virt/virtchnl.c | 5 +-
5 files changed, 98 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice.h
b/drivers/net/ethernet/intel/ice/ice.h
index b88c89db9387..aa81b6db7502 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -1147,8 +1147,8 @@ static inline bool ice_pf_src_tmr_owned(struct ice_pf *pf)
* while holding adapter->ctrl_pf_lock.
* hw is embedded in struct ice_pf, so either mechanism protects its lifetime.
*
- * Return: A pointer to ice_hw structure with access to timesync
- * register space.
+ * Return: A pointer to ice_hw structure with access to timesync register
+ * space, or NULL if the control PF is not available.
*/
static inline struct ice_hw *ice_get_primary_hw(struct ice_pf *pf)
{
@@ -1157,10 +1157,7 @@ static inline struct ice_hw *ice_get_primary_hw(struct
ice_pf *pf)
ctrl_pf = rcu_dereference_check(pf->adapter->ctrl_pf,
lockdep_is_held(&pf->adapter->ctrl_pf_lock));
- if (!ctrl_pf)
- return &pf->hw;
- else
- return &ctrl_pf->hw;
+ return ctrl_pf ? &ctrl_pf->hw : NULL;
}
/**
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
b/drivers/net/ethernet/intel/ice/ice_ptp.c
index ece2e2ba2602..471fc39f379b 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -215,17 +215,23 @@ void ice_ptp_restore_timestamp_mode(struct ice_pf *pf)
* @pf: Board private structure
* @sts: Optional parameter for holding a pair of system timestamps from
* the system clock. Will be ignored if NULL is given.
+ * @time: Storage for the source clock time
+ *
+ * Return: 0 on success, negative error code otherwise.
*/
-u64 ice_ptp_read_src_clk_reg(struct ice_pf *pf,
- struct ptp_system_timestamp *sts)
+int ice_ptp_read_src_clk_reg(struct ice_pf *pf,
+ struct ptp_system_timestamp *sts, u64 *time)
{
struct ice_hw *hw = &pf->hw;
u32 hi, lo, lo2;
u8 tmr_idx;
scoped_guard(rcu) {
- if (!ice_is_primary(hw))
+ if (!ice_is_primary(hw)) {
hw = ice_get_primary_hw(pf);
+ if (!hw)
+ return -ENODEV;
+ }
tmr_idx = ice_get_ptp_src_clock_index(hw);
guard(spinlock)(&pf->adapter->ptp_gltsyn_time_lock);
@@ -233,12 +239,12 @@ u64 ice_ptp_read_src_clk_reg(struct ice_pf *pf,
ptp_read_system_prets(sts);
if (hw->mac_type == ICE_MAC_E830) {
- u64 clk_time = rd64(hw, E830_GLTSYN_TIME_L(tmr_idx));
+ *time = rd64(hw, E830_GLTSYN_TIME_L(tmr_idx));
/* Read the system timestamp post PHC read */
ptp_read_system_postts(sts);
- return clk_time;
+ return 0;
}
lo = rd32(hw, GLTSYN_TIME_L(tmr_idx));
@@ -261,7 +267,9 @@ u64 ice_ptp_read_src_clk_reg(struct ice_pf *pf,
}
- return ((u64)hi << 32) | lo;
+ *time = ((u64)hi << 32) | lo;
+
+ return 0;
}
/**
@@ -361,6 +369,7 @@ static u64 ice_ptp_extend_40b_ts(struct ice_pf *pf, u64
in_tstamp)
{
const u64 mask = GENMASK_ULL(31, 0);
unsigned long discard_time;
+ u64 cached_time;
/* Discard the hardware timestamp if the cached PHC time is too old */
discard_time = pf->ptp.cached_phc_jiffies + msecs_to_jiffies(2000);
@@ -369,8 +378,17 @@ static u64 ice_ptp_extend_40b_ts(struct ice_pf *pf, u64
in_tstamp)
return 0;
}
- return ice_ptp_extend_32b_ts(READ_ONCE(pf->ptp.cached_phc_time),
- (in_tstamp >> 8) & mask);
+ /* Discard it as well if there is no cached PHC time at all, which is
+ * how ice_ptp_update_cached_phctime() reports that the source clock
+ * has become unreachable.
+ */
+ cached_time = READ_ONCE(pf->ptp.cached_phc_time);
+ if (!cached_time) {
+ pf->ptp.tx_hwtstamp_discarded++;
+ return 0;
+ }
+
+ return ice_ptp_extend_32b_ts(cached_time, (in_tstamp >> 8) & mask);
}
/**
@@ -1035,13 +1053,15 @@ static int ice_ptp_init_tx(struct ice_pf *pf, struct
ice_ptp_tx *tx, u8 port)
* Return:
* * 0 - OK, successfully updated
* * -EAGAIN - PF was busy, need to reschedule the update
+ * * -ENODEV - no control PF owns the source clock register space
+ * * -EOPNOTSUPP - PTP support is not compiled in
*/
static int ice_ptp_update_cached_phctime(struct ice_pf *pf)
{
struct device *dev = ice_pf_to_dev(pf);
unsigned long update_before;
u64 systime;
- int i;
+ int err, i;
update_before = pf->ptp.cached_phc_jiffies + msecs_to_jiffies(2000);
if (pf->ptp.cached_phc_time &&
@@ -1054,11 +1074,22 @@ static int ice_ptp_update_cached_phctime(struct ice_pf
*pf)
}
/* Read the current PHC time */
- systime = ice_ptp_read_src_clk_reg(pf, NULL);
-
- /* Update the cached PHC time stored in the PF structure */
- WRITE_ONCE(pf->ptp.cached_phc_time, systime);
- WRITE_ONCE(pf->ptp.cached_phc_jiffies, jiffies);
+ err = ice_ptp_read_src_clk_reg(pf, NULL, &systime);
+ if (err) {
+ /* The source clock is gone, so the cache can no longer be
+ * refreshed. Publish zero instead of leaving the last good
+ * value behind: neither ice_ptp_get_rx_hwts() nor
+ * ice_ptp_extend_40b_ts() may extend against a value that no
+ * longer tracks the PHC, and both treat a zero cache as
+ * "no timestamp".
+ */
+ systime = 0;
+ WRITE_ONCE(pf->ptp.cached_phc_time, 0);
+ } else {
+ /* Update the cached PHC time stored in the PF structure */
+ WRITE_ONCE(pf->ptp.cached_phc_time, systime);
+ WRITE_ONCE(pf->ptp.cached_phc_jiffies, jiffies);
+ }
if (test_and_set_bit(ICE_CFG_BUSY, pf->state))
return -EAGAIN;
@@ -1081,7 +1112,7 @@ static int ice_ptp_update_cached_phctime(struct ice_pf
*pf)
}
clear_bit(ICE_CFG_BUSY, pf->state);
- return 0;
+ return err;
}
/**
@@ -1111,8 +1142,8 @@ static void ice_ptp_reset_cached_phctime(struct ice_pf
*pf)
* properly reset them here. This could lead to reporting of
* invalid timestamps, but there isn't much we can do.
*/
- dev_warn(dev, "%s: ICE_CFG_BUSY, unable to immediately update
cached PHC time\n",
- __func__);
+ dev_warn(dev, "%s: unable to immediately update cached PHC
time, err %d\n",
+ __func__, err);
/* Queue the work item to update the Rx rings when possible */
kthread_queue_delayed_work(pf->ptp.kworker, &pf->ptp.work,
@@ -1877,6 +1908,7 @@ static int ice_ptp_cfg_perout(struct ice_pf *pf, struct
ptp_perout_request *rq,
u64 clk, period, start, phase;
struct ice_hw *hw = &pf->hw;
int pin_desc_idx;
+ int err;
pin_desc_idx = ice_ptp_find_pin_idx(pf, PTP_PF_PEROUT, rq->index);
if (pin_desc_idx < 0)
@@ -1915,7 +1947,10 @@ static int ice_ptp_cfg_perout(struct ice_pf *pf, struct
ptp_perout_request *rq,
* at the next multiple of period, maintaining phase at least 0.5 second
* from now, so we have time to write it to HW.
*/
- clk = ice_ptp_read_src_clk_reg(pf, NULL) + NSEC_PER_MSEC * 500;
+ err = ice_ptp_read_src_clk_reg(pf, NULL, &clk);
+ if (err)
+ return err;
+ clk += NSEC_PER_MSEC * 500;
if (rq->flags & PTP_PEROUT_PHASE || start <= clk - prop_delay_ns)
start = div64_u64(clk + period - 1, period) * period + phase;
@@ -2058,8 +2093,12 @@ ice_ptp_gettimex64(struct ptp_clock_info *info, struct
timespec64 *ts,
{
struct ice_pf *pf = ptp_info_to_pf(info);
u64 time_ns;
+ int err;
+
+ err = ice_ptp_read_src_clk_reg(pf, sts, &time_ns);
+ if (err)
+ return err;
- time_ns = ice_ptp_read_src_clk_reg(pf, sts);
*ts = ns_to_timespec64(time_ns);
return 0;
}
@@ -3112,9 +3151,13 @@ static void ice_ptp_periodic_work(struct kthread_work
*work)
ice_ptp_maybe_trigger_tx_interrupt(pf);
- /* Run twice a second or reschedule if phc update failed */
+ /* Run twice a second, or retry soon if another thread was busy
+ * updating the Rx rings. Other errors, such as a missing control PF,
+ * do not clear up on their own and must not turn the periodic worker
+ * into a busy loop.
+ */
kthread_queue_delayed_work(ptp->kworker, &ptp->work,
- msecs_to_jiffies(err ? 10 : 500));
+ msecs_to_jiffies(err == -EAGAIN ? 10 : 500));
}
/**
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h
b/drivers/net/ethernet/intel/ice/ice_ptp.h
index faf7b6dc4847..21c5434f77f1 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -329,8 +329,8 @@ void ice_ptp_complete_tx_single_tstamp(struct ice_ptp_tx
*tx);
void ice_ptp_process_ts(struct ice_pf *pf);
irqreturn_t ice_ptp_ts_irq(struct ice_pf *pf);
bool ice_ptp_tx_tstamps_pending(struct ice_pf *pf, bool in_irq);
-u64 ice_ptp_read_src_clk_reg(struct ice_pf *pf,
- struct ptp_system_timestamp *sts);
+int ice_ptp_read_src_clk_reg(struct ice_pf *pf,
+ struct ptp_system_timestamp *sts, u64 *time);
u64 ice_ptp_get_rx_hwts(const union ice_32b_rx_flex_desc *rx_desc,
const struct ice_pkt_ctx *pkt_ctx);
@@ -382,10 +382,11 @@ ice_ptp_tx_tstamps_pending(struct ice_pf *pf, bool in_irq)
return false;
}
-static inline u64 ice_ptp_read_src_clk_reg(struct ice_pf *pf,
- struct ptp_system_timestamp *sts)
+static inline int ice_ptp_read_src_clk_reg(struct ice_pf *pf,
+ struct ptp_system_timestamp *sts,
+ u64 *time)
{
- return 0;
+ return -EOPNOTSUPP;
}
static inline u64
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index c84adc096027..dde6bddd4ecf 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -355,8 +355,11 @@ void ice_ptp_src_cmd(struct ice_hw *hw, enum
ice_ptp_tmr_cmd cmd)
guard(rcu)();
- if (!ice_is_primary(hw))
+ if (!ice_is_primary(hw)) {
hw = ice_get_primary_hw(pf);
+ if (!hw)
+ return;
+ }
wr32(hw, GLTSYN_CMD, cmd_val);
}
@@ -375,8 +378,11 @@ static void ice_ptp_exec_tmr_cmd(struct ice_hw *hw)
guard(rcu)();
- if (!ice_is_primary(hw))
+ if (!ice_is_primary(hw)) {
hw = ice_get_primary_hw(pf);
+ if (!hw)
+ return;
+ }
guard(spinlock)(&pf->adapter->ptp_gltsyn_time_lock);
wr32(hw, GLTSYN_CMD_SYNC, SYNC_EXEC_CMD);
@@ -2039,6 +2045,8 @@ static int ice_read_phy_and_phc_time_eth56g(struct ice_hw
*hw, u8 port,
guard(rcu)();
pri_hw = ice_get_primary_hw(pf);
+ if (!pri_hw)
+ return -ENODEV;
zo = rd32(pri_hw, GLTSYN_SHTIME_0(tmr_idx));
lo = rd32(pri_hw, GLTSYN_SHTIME_L(tmr_idx));
@@ -2222,6 +2230,11 @@ int ice_start_phy_timer_eth56g(struct ice_hw *hw, u8
port)
*/
rcu_read_lock();
pri_hw = ice_get_primary_hw(pf);
+ if (!pri_hw) {
+ rcu_read_unlock();
+ err = -ENODEV;
+ goto err_ptp_unlock;
+ }
lo = rd32(pri_hw, GLTSYN_INCVAL_L(tmr_idx));
hi = rd32(pri_hw, GLTSYN_INCVAL_H(tmr_idx));
@@ -5367,8 +5380,13 @@ bool ice_ptp_lock(struct ice_hw *hw)
down_read(&pf->adapter->ctrl_pf_lock);
- if (!ice_is_primary(hw))
+ if (!ice_is_primary(hw)) {
hw = ice_get_primary_hw(pf);
+ if (!hw) {
+ up_read(&pf->adapter->ctrl_pf_lock);
+ return false;
+ }
+ }
#define MAX_TRIES 15
diff --git a/drivers/net/ethernet/intel/ice/virt/virtchnl.c
b/drivers/net/ethernet/intel/ice/virt/virtchnl.c
index ca8018e3dd42..66332d02c5ba 100644
--- a/drivers/net/ethernet/intel/ice/virt/virtchnl.c
+++ b/drivers/net/ethernet/intel/ice/virt/virtchnl.c
@@ -2485,7 +2485,10 @@ static int ice_vc_get_phc_time(struct ice_vf *vf)
len = sizeof(*phc_time);
- phc_time->time = ice_ptp_read_src_clk_reg(pf, NULL);
+ if (ice_ptp_read_src_clk_reg(pf, NULL, &phc_time->time)) {
+ v_ret = VIRTCHNL_STATUS_ERR_NOT_SUPPORTED;
+ len = 0;
+ }
err:
/* send the response back to the VF */
--
2.53.0
