Commit 830e0dd9 authored by Jacob Keller's avatar Jacob Keller Committed by Jeff Kirsher

i40e: avoid overflow in i40e_ptp_adjfreq()

When operating at 1GbE, the base incval for the PTP clock is so large
that multiplying it by numbers close to the max_adj can overflow the
u64.

Rather than attempting to limit the max_adj to a value small enough to
avoid overflow, instead calculate the incvalue adjustment based on the
40GbE incvalue, and then multiply that by the scaling factor for the
link speed.

This sacrifices a small amount of precision in the adjustment but we
avoid erratic behavior of the clock due to the overflow caused if ppb is
very near the maximum adjustment.
Signed-off-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Tested-by: default avatarAndrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: default avatarJeff Kirsher <jeffrey.t.kirsher@intel.com>
parent 5305d0fe
...@@ -586,7 +586,7 @@ struct i40e_pf { ...@@ -586,7 +586,7 @@ struct i40e_pf {
unsigned long ptp_tx_start; unsigned long ptp_tx_start;
struct hwtstamp_config tstamp_config; struct hwtstamp_config tstamp_config;
struct mutex tmreg_lock; /* Used to protect the SYSTIME registers. */ struct mutex tmreg_lock; /* Used to protect the SYSTIME registers. */
u64 ptp_base_adj; u32 ptp_adj_mult;
u32 tx_hwtstamp_timeouts; u32 tx_hwtstamp_timeouts;
u32 tx_hwtstamp_skipped; u32 tx_hwtstamp_skipped;
u32 rx_hwtstamp_cleared; u32 rx_hwtstamp_cleared;
......
...@@ -16,9 +16,9 @@ ...@@ -16,9 +16,9 @@
* At 1Gb link, the period is multiplied by 20. (32ns) * At 1Gb link, the period is multiplied by 20. (32ns)
* 1588 functionality is not supported at 100Mbps. * 1588 functionality is not supported at 100Mbps.
*/ */
#define I40E_PTP_40GB_INCVAL 0x0199999999ULL #define I40E_PTP_40GB_INCVAL 0x0199999999ULL
#define I40E_PTP_10GB_INCVAL 0x0333333333ULL #define I40E_PTP_10GB_INCVAL_MULT 2
#define I40E_PTP_1GB_INCVAL 0x2000000000ULL #define I40E_PTP_1GB_INCVAL_MULT 20
#define I40E_PRTTSYN_CTL1_TSYNTYPE_V1 BIT(I40E_PRTTSYN_CTL1_TSYNTYPE_SHIFT) #define I40E_PRTTSYN_CTL1_TSYNTYPE_V1 BIT(I40E_PRTTSYN_CTL1_TSYNTYPE_SHIFT)
#define I40E_PRTTSYN_CTL1_TSYNTYPE_V2 (2 << \ #define I40E_PRTTSYN_CTL1_TSYNTYPE_V2 (2 << \
...@@ -106,17 +106,24 @@ static int i40e_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb) ...@@ -106,17 +106,24 @@ static int i40e_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
ppb = -ppb; ppb = -ppb;
} }
smp_mb(); /* Force any pending update before accessing. */ freq = I40E_PTP_40GB_INCVAL;
adj = READ_ONCE(pf->ptp_base_adj);
freq = adj;
freq *= ppb; freq *= ppb;
diff = div_u64(freq, 1000000000ULL); diff = div_u64(freq, 1000000000ULL);
if (neg_adj) if (neg_adj)
adj -= diff; adj = I40E_PTP_40GB_INCVAL - diff;
else else
adj += diff; adj = I40E_PTP_40GB_INCVAL + diff;
/* At some link speeds, the base incval is so large that directly
* multiplying by ppb would result in arithmetic overflow even when
* using a u64. Avoid this by instead calculating the new incval
* always in terms of the 40GbE clock rate and then multiplying by the
* link speed factor afterwards. This does result in slightly lower
* precision at lower link speeds, but it is fairly minor.
*/
smp_mb(); /* Force any pending update before accessing. */
adj *= READ_ONCE(pf->ptp_adj_mult);
wr32(hw, I40E_PRTTSYN_INC_L, adj & 0xFFFFFFFF); wr32(hw, I40E_PRTTSYN_INC_L, adj & 0xFFFFFFFF);
wr32(hw, I40E_PRTTSYN_INC_H, adj >> 32); wr32(hw, I40E_PRTTSYN_INC_H, adj >> 32);
...@@ -438,6 +445,7 @@ void i40e_ptp_set_increment(struct i40e_pf *pf) ...@@ -438,6 +445,7 @@ void i40e_ptp_set_increment(struct i40e_pf *pf)
struct i40e_link_status *hw_link_info; struct i40e_link_status *hw_link_info;
struct i40e_hw *hw = &pf->hw; struct i40e_hw *hw = &pf->hw;
u64 incval; u64 incval;
u32 mult;
hw_link_info = &hw->phy.link_info; hw_link_info = &hw->phy.link_info;
...@@ -445,10 +453,10 @@ void i40e_ptp_set_increment(struct i40e_pf *pf) ...@@ -445,10 +453,10 @@ void i40e_ptp_set_increment(struct i40e_pf *pf)
switch (hw_link_info->link_speed) { switch (hw_link_info->link_speed) {
case I40E_LINK_SPEED_10GB: case I40E_LINK_SPEED_10GB:
incval = I40E_PTP_10GB_INCVAL; mult = I40E_PTP_10GB_INCVAL_MULT;
break; break;
case I40E_LINK_SPEED_1GB: case I40E_LINK_SPEED_1GB:
incval = I40E_PTP_1GB_INCVAL; mult = I40E_PTP_1GB_INCVAL_MULT;
break; break;
case I40E_LINK_SPEED_100MB: case I40E_LINK_SPEED_100MB:
{ {
...@@ -459,15 +467,20 @@ void i40e_ptp_set_increment(struct i40e_pf *pf) ...@@ -459,15 +467,20 @@ void i40e_ptp_set_increment(struct i40e_pf *pf)
"1588 functionality is not supported at 100 Mbps. Stopping the PHC.\n"); "1588 functionality is not supported at 100 Mbps. Stopping the PHC.\n");
warn_once++; warn_once++;
} }
incval = 0; mult = 0;
break; break;
} }
case I40E_LINK_SPEED_40GB: case I40E_LINK_SPEED_40GB:
default: default:
incval = I40E_PTP_40GB_INCVAL; mult = 1;
break; break;
} }
/* The increment value is calculated by taking the base 40GbE incvalue
* and multiplying it by a factor based on the link speed.
*/
incval = I40E_PTP_40GB_INCVAL * mult;
/* Write the new increment value into the increment register. The /* Write the new increment value into the increment register. The
* hardware will not update the clock until both registers have been * hardware will not update the clock until both registers have been
* written. * written.
...@@ -476,7 +489,7 @@ void i40e_ptp_set_increment(struct i40e_pf *pf) ...@@ -476,7 +489,7 @@ void i40e_ptp_set_increment(struct i40e_pf *pf)
wr32(hw, I40E_PRTTSYN_INC_H, incval >> 32); wr32(hw, I40E_PRTTSYN_INC_H, incval >> 32);
/* Update the base adjustement value. */ /* Update the base adjustement value. */
WRITE_ONCE(pf->ptp_base_adj, incval); WRITE_ONCE(pf->ptp_adj_mult, mult);
smp_mb(); /* Force the above update. */ smp_mb(); /* Force the above update. */
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment