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: 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>

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 70d369e9139c6448a7a08cb596f719f9bad5d68f..1d59ab6ca90fcff589f08de0dab885a10f7748c5 100644 (file)
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -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;

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index 43d7c44d6d9fe5792261201153e770f9e24cb342..aa3daec2049d986a962bbac0712b80b6e928fa79 100644 (file)
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -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 << \
@@ -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);
@@ -438,6 +445,7 @@ 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;
 
@@ -445,10 +453,10 @@ void i40e_ptp_set_increment(struct i40e_pf *pf)
 
        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:
        {
@@ -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.
@@ -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. */
 }