Skip to content

Commit

Permalink
i40e: avoid overflow in i40e_ptp_adjfreq()
Browse files Browse the repository at this point in the history
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: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
  • Loading branch information
Jacob Keller authored and Jeff Kirsher committed Apr 30, 2018
1 parent 5305d0f commit 830e0dd
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 15 deletions.
2 changes: 1 addition & 1 deletion drivers/net/ethernet/intel/i40e/i40e.h
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ struct i40e_pf {
unsigned long ptp_tx_start;
struct hwtstamp_config tstamp_config;
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_skipped;
u32 rx_hwtstamp_cleared;
Expand Down
41 changes: 27 additions & 14 deletions drivers/net/ethernet/intel/i40e/i40e_ptp.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
* At 1Gb link, the period is multiplied by 20. (32ns)
* 1588 functionality is not supported at 100Mbps.
*/
#define I40E_PTP_40GB_INCVAL 0x0199999999ULL
#define I40E_PTP_10GB_INCVAL 0x0333333333ULL
#define I40E_PTP_1GB_INCVAL 0x2000000000ULL
#define I40E_PTP_40GB_INCVAL 0x0199999999ULL
#define I40E_PTP_10GB_INCVAL_MULT 2
#define I40E_PTP_1GB_INCVAL_MULT 20

#define I40E_PRTTSYN_CTL1_TSYNTYPE_V1 BIT(I40E_PRTTSYN_CTL1_TSYNTYPE_SHIFT)
#define I40E_PRTTSYN_CTL1_TSYNTYPE_V2 (2 << \
Expand Down Expand Up @@ -106,17 +106,24 @@ static int i40e_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
ppb = -ppb;
}

smp_mb(); /* Force any pending update before accessing. */
adj = READ_ONCE(pf->ptp_base_adj);

freq = adj;
freq = I40E_PTP_40GB_INCVAL;
freq *= ppb;
diff = div_u64(freq, 1000000000ULL);

if (neg_adj)
adj -= diff;
adj = I40E_PTP_40GB_INCVAL - diff;
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_H, adj >> 32);
Expand Down Expand Up @@ -438,17 +445,18 @@ void i40e_ptp_set_increment(struct i40e_pf *pf)
struct i40e_link_status *hw_link_info;
struct i40e_hw *hw = &pf->hw;
u64 incval;
u32 mult;

hw_link_info = &hw->phy.link_info;

i40e_aq_get_link_info(&pf->hw, true, NULL, NULL);

switch (hw_link_info->link_speed) {
case I40E_LINK_SPEED_10GB:
incval = I40E_PTP_10GB_INCVAL;
mult = I40E_PTP_10GB_INCVAL_MULT;
break;
case I40E_LINK_SPEED_1GB:
incval = I40E_PTP_1GB_INCVAL;
mult = I40E_PTP_1GB_INCVAL_MULT;
break;
case I40E_LINK_SPEED_100MB:
{
Expand All @@ -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");
warn_once++;
}
incval = 0;
mult = 0;
break;
}
case I40E_LINK_SPEED_40GB:
default:
incval = I40E_PTP_40GB_INCVAL;
mult = 1;
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
* hardware will not update the clock until both registers have been
* written.
Expand All @@ -476,7 +489,7 @@ void i40e_ptp_set_increment(struct i40e_pf *pf)
wr32(hw, I40E_PRTTSYN_INC_H, incval >> 32);

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

Expand Down

0 comments on commit 830e0dd

Please sign in to comment.