[Intel-wired-lan] [PATCH] e1000e: don't modify SYSTIM registers during SIOCSHWTSTAMP ioctl

[Intel-wired-lan] [PATCH] e1000e: don't modify SYSTIM registers during SIOCSHWTSTAMP ioctl

[Jacob Keller]

The e1000e_config_hwtstamp function was incorrectly resetting the SYSTIM
registers every time the ioctl was being run. If you happened to be
running ptp4l and lost the PTP connect (removing cable, or blocking the
UDP traffic for example), then ptp4l will eventually perform a restart
which involves re-requesting timestamp settings. In e1000e this has the
unfortunate and incorrect result of resetting SYSTIME to the kernel
time. Since kernel time is usually in UTC, and PTP time is in TAI, this
results in the leap second being re-applied.

Fix this by extracting the SYSTIME reset out into its own function,
e1000e_ptp_reset, which we call during reset to restore the hardware
registers. This function will (a) restart the timecounter based on the
new system time, (b) restore the previous PPB setting, and (c) restore
the previous hwtstamp settings.

In order to perform (b), I had to modify the adjfreq ptp function
pointer to store the old delta each time it is called. This also has the
side effect of restoring the correct base timinca register correctly.
The driver does not need to explicitly zero the ptp_delta variable since
the entire adapter structure comes zero-initialized.

Reported-by: Brian Walsh <brian@walsh.ws>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---

Notes:
    This patch conflicts with Brian's "e1000e fix ptp time reset on newtork
    interruption", and that patch MUST be removed from the queue before applying
    this one.


    Jeff, this patch WILL NOT apply unless you drop Brian's 2nd patch in
    his series.

 drivers/net/ethernet/intel/e1000e/e1000.h  |  1 +
 drivers/net/ethernet/intel/e1000e/netdev.c | 42 +++++++++++++++++++++---------
 drivers/net/ethernet/intel/e1000e/ptp.c    |  2 ++
 3 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 010e6d61c855..ef96cd11d6d2 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -347,6 +347,7 @@ struct e1000_adapter {
 	struct ptp_clock *ptp_clock;
 	struct ptp_clock_info ptp_clock_info;
 	struct pm_qos_request pm_qos_req;
+	s32 ptp_delta;
 
 	u16 eee_advert;
 };
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 50d2684ac146..4fbf424b1c33 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -3580,7 +3580,6 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter,
 	bool is_l4 = false;
 	bool is_l2 = false;
 	u32 regval;
-	s32 ret_val;
 
 	if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP))
 		return -EINVAL;
@@ -3719,16 +3718,6 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter,
 	er32(RXSTMPH);
 	er32(TXSTMPH);
 
-	/* Get and set the System Time Register SYSTIM base frequency */
-	ret_val = e1000e_get_base_timinca(adapter, &regval);
-	if (ret_val)
-		return ret_val;
-	ew32(TIMINCA, regval);
-
-	/* reset the ns time counter */
-	timecounter_init(&adapter->tc, &adapter->cc,
-			 ktime_to_ns(ktime_get_real()));
-
 	return 0;
 }
 
@@ -3885,6 +3874,33 @@ static void e1000_flush_desc_rings(struct e1000_adapter *adapter)
 }
 
 /**
+ * e1000e_ptp_reset - reset the timesync registers after a hardware reset
+ * @adapter: board private structure
+ *
+ * When the MAC is reset, all hardware bits for timesync will be reset to the
+ * default values. This function will restore the settings last in place.
+ * Since the clock SYSTIME registers are reset, we will simply restore the
+ * cyclecounter to the kernel real clock time.
+ **/
+static void e1000e_ptp_reset(struct e1000_adapter *adapter)
+{
+	unsigned long flags;
+
+	/* restore the previous ptp frequency delta */
+	adapter->ptp_clock_info.adjfreq(&adapter->ptp_clock_info,
+					adapter->ptp_delta);
+
+	/* reset the systim ns time counter */
+	spin_lock_irqsave(&adapter->systim_lock, flags);
+	timecounter_init(&adapter->tc, &adapter->cc,
+			 ktime_to_ns(ktime_get_real()));
+	spin_unlock_irqrestore(&adapter->systim_lock, flags);
+
+	/* restore the previous hwtstamp configuration settings */
+	e1000e_config_hwtstamp(adapter, &adapter->hwtstamp_config);
+}
+
+/**
  * e1000e_reset - bring the hardware into a known good state
  *
  * This function boots the hardware and enables some settings that
@@ -4063,8 +4079,8 @@ void e1000e_reset(struct e1000_adapter *adapter)
 
 	e1000e_reset_adaptive(hw);
 
-	/* initialize systim and reset the ns time counter */
-	e1000e_config_hwtstamp(adapter, &adapter->hwtstamp_config);
+	/* restore systim and hwtstamp settings */
+	e1000e_ptp_reset(adapter);
 
 	/* Set EEE advertisement as appropriate */
 	if (adapter->flags2 & FLAG2_HAS_EEE) {
diff --git a/drivers/net/ethernet/intel/e1000e/ptp.c b/drivers/net/ethernet/intel/e1000e/ptp.c
index e2ff3ef75d5d..2e1b17ad52a3 100644
--- a/drivers/net/ethernet/intel/e1000e/ptp.c
+++ b/drivers/net/ethernet/intel/e1000e/ptp.c
@@ -79,6 +79,8 @@ static int e1000e_phc_adjfreq(struct ptp_clock_info *ptp, s32 delta)
 
 	ew32(TIMINCA, timinca);
 
+	adapter->ptp_delta = delta;
+
 	spin_unlock_irqrestore(&adapter->systim_lock, flags);
 
 	return 0;
-- 
2.8.1.102.ga49ec4a

[Intel-wired-lan] [PATCH] e1000e: don't modify SYSTIM registers during SIOCSHWTSTAMP ioctl

[Keller, Jacob E]

On Thu, 2016-04-14 at 16:29 -0700, Jacob Keller wrote:
> The e1000e_config_hwtstamp function was incorrectly resetting the
> SYSTIM
> registers every time the ioctl was being run. If you happened to be
> running ptp4l and lost the PTP connect (removing cable, or blocking
> the
> UDP traffic for example), then ptp4l will eventually perform a
> restart
> which involves re-requesting timestamp settings. In e1000e this has
> the
> unfortunate and incorrect result of resetting SYSTIME to the kernel
> time. Since kernel time is usually in UTC, and PTP time is in TAI,
> this
> results in the leap second being re-applied.
> 
> Fix this by extracting the SYSTIME reset out into its own function,
> e1000e_ptp_reset, which we call during reset to restore the hardware
> registers. This function will (a) restart the timecounter based on
> the
> new system time, (b) restore the previous PPB setting, and (c)
> restore
> the previous hwtstamp settings.
> 
> In order to perform (b), I had to modify the adjfreq ptp function
> pointer to store the old delta each time it is called. This also has
> the
> side effect of restoring the correct base timinca register correctly.
> The driver does not need to explicitly zero the ptp_delta variable
> since
> the entire adapter structure comes zero-initialized.
> 
> Reported-by: Brian Walsh <brian@walsh.ws>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> 

@Jeff,

This patch replaces the currently queued and proposed patch by Brian,
you will need to drop 2/2 of his series (but please keep 1/2!).

@Brian,

Can you test this locally and see if it resolves your issue?

Thanks,
Jake


> Notes:
> ????This patch conflicts with Brian's "e1000e fix ptp time reset on
> newtork
> ????interruption", and that patch MUST be removed from the queue
> before applying
> ????this one.
> 
> 
> ????Jeff, this patch WILL NOT apply unless you drop Brian's 2nd patch
> in
> ????his series.
> 
> ?drivers/net/ethernet/intel/e1000e/e1000.h??|??1 +
> ?drivers/net/ethernet/intel/e1000e/netdev.c | 42
> +++++++++++++++++++++---------
> ?drivers/net/ethernet/intel/e1000e/ptp.c????|??2 ++
> ?3 files changed, 32 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h
> b/drivers/net/ethernet/intel/e1000e/e1000.h
> index 010e6d61c855..ef96cd11d6d2 100644
> --- a/drivers/net/ethernet/intel/e1000e/e1000.h
> +++ b/drivers/net/ethernet/intel/e1000e/e1000.h
> @@ -347,6 +347,7 @@ struct e1000_adapter {
> ?	struct ptp_clock *ptp_clock;
> ?	struct ptp_clock_info ptp_clock_info;
> ?	struct pm_qos_request pm_qos_req;
> +	s32 ptp_delta;
> ?
> ?	u16 eee_advert;
> ?};
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
> b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 50d2684ac146..4fbf424b1c33 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -3580,7 +3580,6 @@ static int e1000e_config_hwtstamp(struct
> e1000_adapter *adapter,
> ?	bool is_l4 = false;
> ?	bool is_l2 = false;
> ?	u32 regval;
> -	s32 ret_val;
> ?
> ?	if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP))
> ?		return -EINVAL;
> @@ -3719,16 +3718,6 @@ static int e1000e_config_hwtstamp(struct
> e1000_adapter *adapter,
> ?	er32(RXSTMPH);
> ?	er32(TXSTMPH);
> ?
> -	/* Get and set the System Time Register SYSTIM base
> frequency */
> -	ret_val = e1000e_get_base_timinca(adapter, &regval);
> -	if (ret_val)
> -		return ret_val;
> -	ew32(TIMINCA, regval);
> -
> -	/* reset the ns time counter */
> -	timecounter_init(&adapter->tc, &adapter->cc,
> -			?ktime_to_ns(ktime_get_real()));
> -
> ?	return 0;
> ?}
> ?
> @@ -3885,6 +3874,33 @@ static void e1000_flush_desc_rings(struct
> e1000_adapter *adapter)
> ?}
> ?
> ?/**
> + * e1000e_ptp_reset - reset the timesync registers after a hardware
> reset
> + * @adapter: board private structure
> + *
> + * When the MAC is reset, all hardware bits for timesync will be
> reset to the
> + * default values. This function will restore the settings last in
> place.
> + * Since the clock SYSTIME registers are reset, we will simply
> restore the
> + * cyclecounter to the kernel real clock time.
> + **/
> +static void e1000e_ptp_reset(struct e1000_adapter *adapter)
> +{
> +	unsigned long flags;
> +
> +	/* restore the previous ptp frequency delta */
> +	adapter->ptp_clock_info.adjfreq(&adapter->ptp_clock_info,
> +					adapter->ptp_delta);
> +
> +	/* reset the systim ns time counter */
> +	spin_lock_irqsave(&adapter->systim_lock, flags);
> +	timecounter_init(&adapter->tc, &adapter->cc,
> +			?ktime_to_ns(ktime_get_real()));
> +	spin_unlock_irqrestore(&adapter->systim_lock, flags);
> +
> +	/* restore the previous hwtstamp configuration settings */
> +	e1000e_config_hwtstamp(adapter, &adapter->hwtstamp_config);
> +}
> +
> +/**
> ? * e1000e_reset - bring the hardware into a known good state
> ? *
> ? * This function boots the hardware and enables some settings that
> @@ -4063,8 +4079,8 @@ void e1000e_reset(struct e1000_adapter
> *adapter)
> ?
> ?	e1000e_reset_adaptive(hw);
> ?
> -	/* initialize systim and reset the ns time counter */
> -	e1000e_config_hwtstamp(adapter, &adapter->hwtstamp_config);
> +	/* restore systim and hwtstamp settings */
> +	e1000e_ptp_reset(adapter);
> ?
> ?	/* Set EEE advertisement as appropriate */
> ?	if (adapter->flags2 & FLAG2_HAS_EEE) {
> diff --git a/drivers/net/ethernet/intel/e1000e/ptp.c
> b/drivers/net/ethernet/intel/e1000e/ptp.c
> index e2ff3ef75d5d..2e1b17ad52a3 100644
> --- a/drivers/net/ethernet/intel/e1000e/ptp.c
> +++ b/drivers/net/ethernet/intel/e1000e/ptp.c
> @@ -79,6 +79,8 @@ static int e1000e_phc_adjfreq(struct ptp_clock_info
> *ptp, s32 delta)
> ?
> ?	ew32(TIMINCA, timinca);
> ?
> +	adapter->ptp_delta = delta;
> +
> ?	spin_unlock_irqrestore(&adapter->systim_lock, flags);
> ?
> ?	return 0;

[Intel-wired-lan] [PATCH] e1000e: don't modify SYSTIM registers during SIOCSHWTSTAMP ioctl

[Brian Walsh]

On Thu, Apr 14, 2016 at 04:29:24PM -0700, Jacob Keller wrote:
> The e1000e_config_hwtstamp function was incorrectly resetting the SYSTIM
> registers every time the ioctl was being run. If you happened to be
> running ptp4l and lost the PTP connect (removing cable, or blocking the
> UDP traffic for example), then ptp4l will eventually perform a restart
> which involves re-requesting timestamp settings. In e1000e this has the
> unfortunate and incorrect result of resetting SYSTIME to the kernel
> time. Since kernel time is usually in UTC, and PTP time is in TAI, this
> results in the leap second being re-applied.
> 
> Fix this by extracting the SYSTIME reset out into its own function,
> e1000e_ptp_reset, which we call during reset to restore the hardware
> registers. This function will (a) restart the timecounter based on the
> new system time, (b) restore the previous PPB setting, and (c) restore
> the previous hwtstamp settings.
> 
> In order to perform (b), I had to modify the adjfreq ptp function
> pointer to store the old delta each time it is called. This also has the
> side effect of restoring the correct base timinca register correctly.
> The driver does not need to explicitly zero the ptp_delta variable since
> the entire adapter structure comes zero-initialized.
> 
> Reported-by: Brian Walsh <brian@walsh.ws>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---

My machine will not boot with this patch. It is failing in the device
initialization.

Brian

[   12.938544] e1000e: Intel(R) PRO/1000 Network Driver - 2.3.2-k
[   12.938695] e1000e: Copyright(c) 1999 - 2014 Intel Corporation.
[   12.948646] e1000e 0000:07:00.0: Interrupt Throttling Rate (ints/sec) set to dynamic conservative mode
[   13.061455] BUG: unable to handle kernel NULL pointer dereference at (null)
[   13.061696] IP: [<  (null)>]   (null)
[   13.061875] *pde = 00000000
[   13.062017] Oops: 0000 [#1] PREEMPT SMP
[   13.062017] Modules linked in: e1000e(+)
[   13.062017] CPU: 1 PID: 1134 Comm: modprobe Not tainted 4.1.7 #2
[   13.062017] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./microETXexpress-DC    , BIOS UNP1R112 04/12/2010
[   13.062017] task: f5ebe200 ti: f5694000 task.ti: f5694000
[   13.062017] EIP: 0060:[<00000000>] EFLAGS: 00010282 CPU: 1
[   13.062017] EIP is at 0x0
[   13.063225] EAX: f57d3144 EBX: f57d04c0 ECX: 00000000 EDX: 00000000
[   13.063225] ESI: f57d091c EDI: f57d3104 EBP: f5695d7c ESP: f5695d58
[   13.063225]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[   13.063225] CR0: 8005003b CR2: 00000000 CR3: 356cd000 CR4: 000006c0
[   13.063225] Stack:
[   13.063225]  f81a030a 00007a0e 00007333 f8195fac 00048000 f57d04c0 f57d04c0 00000000
[   13.063225]  00000000 f5695db8 f81a186a 00000001 00020000 f5f04464 f5f04400 f57d091c
[   13.063225]  f57d0000 c13b7680 f5695db8 c13b8768 4d010000 f5695dd8 f5f04400 f5f04464
[   13.063225] Call Trace:
[   13.063225]  [<f81a030a>] ? e1000e_reset+0x313/0x447 [e1000e]
[   13.063225]  [<f8195fac>] ? e1000e_read_nvm_eerd+0x50/0x77 [e1000e]
[   13.063225]  [<f81a186a>] e1000_probe+0x8a3/0xa6d [e1000e]
[   13.063225]  [<c13b7680>] ? spin_unlock_irqrestore+0x8/0xa
[   13.063225]  [<c13b8768>] ? __pm_runtime_resume+0x3b/0x43
[   13.063225]  [<c1297746>] local_pci_probe+0x30/0x69
[   13.063225]  [<c129792f>] pci_device_probe+0x42/0x61
[   13.063225]  [<c13b1683>] driver_probe_device+0xae/0x1bc
[   13.063225]  [<c13b17cd>] __driver_attach+0x3c/0x54
[   13.063225]  [<c13b0548>] bus_for_each_dev+0x38/0x61
[   13.063225]  [<c13b14fe>] driver_attach+0x14/0x16
[   13.063225]  [<c13b1791>] ? driver_probe_device+0x1bc/0x1bc
[   13.063225]  [<c13b10dc>] bus_add_driver+0x9b/0x177
[   13.063225]  [<f8077000>] ? 0xf8077000
[   13.063225]  [<f8077000>] ? 0xf8077000
[   13.063225]  [<c13b1ca3>] driver_register+0x70/0xa2
[   13.063225]  [<c12979d0>] __pci_register_driver+0x2e/0x31
[   13.063225]  [<f8077030>] e1000_init_module+0x30/0x32 [e1000e]
[   13.063225]  [<c1000401>] do_one_initcall+0xc9/0x146
[   13.063225]  [<c108ac9d>] ? do_init_module+0x21/0x18a
[   13.063225]  [<c10f7db6>] ? kmem_cache_alloc_trace+0xa1/0xab
[   13.063225]  [<c108ac9d>] ? do_init_module+0x21/0x18a
[   13.063225]  [<c108accc>] do_init_module+0x50/0x18a
[   13.063225]  [<c108c68e>] load_module+0xeb8/0x1084
[   13.063225]  [<c108c998>] SyS_init_module+0xcf/0xd3
[   13.063225]  [<c16302ac>] sysenter_do_call+0x12/0x12
[   13.063225] Code:  Bad EIP value.
[   13.063225] EIP: [<00000000>] 0x0 SS:ESP 0068:f5695d58
[   13.063225] CR2: 0000000000000000
[   13.092775] ---[ end trace 0d302018b434ce8a ]---

[Intel-wired-lan] [PATCH] e1000e: don't modify SYSTIM registers during SIOCSHWTSTAMP ioctl

[Brian Walsh]

On Thu, Apr 14, 2016 at 04:29:24PM -0700, Jacob Keller wrote:
> The e1000e_config_hwtstamp function was incorrectly resetting the SYSTIM
> registers every time the ioctl was being run. If you happened to be
> running ptp4l and lost the PTP connect (removing cable, or blocking the
> UDP traffic for example), then ptp4l will eventually perform a restart
> which involves re-requesting timestamp settings. In e1000e this has the
> unfortunate and incorrect result of resetting SYSTIME to the kernel
> time. Since kernel time is usually in UTC, and PTP time is in TAI, this
> results in the leap second being re-applied.
> 
> Fix this by extracting the SYSTIME reset out into its own function,
> e1000e_ptp_reset, which we call during reset to restore the hardware
> registers. This function will (a) restart the timecounter based on the
> new system time, (b) restore the previous PPB setting, and (c) restore
> the previous hwtstamp settings.
> 
> In order to perform (b), I had to modify the adjfreq ptp function
> pointer to store the old delta each time it is called. This also has the
> side effect of restoring the correct base timinca register correctly.
> The driver does not need to explicitly zero the ptp_delta variable since
> the entire adapter structure comes zero-initialized.
> 
> Reported-by: Brian Walsh <brian@walsh.ws>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---

My last attempt was on an older 4.1 kernel. I am seeing the same failure
on the latest 4.6 rc3.

[   14.029654] e1000e: Intel(R) PRO/1000 Network Driver - 3.2.6-k
[   14.088821] e1000e: Copyright(c) 1999 - 2015 Intel Corporation.
[   14.113831] e1000e 0000:07:00.0: Interrupt Throttling Rate (ints/sec) set to dynamic conservative mode
[   14.255644] BUG: unable to handle kernel NULL pointer dereference at (null)
[   14.260555] IP: [<  (null)>]   (null)
[   14.268130] *pde = 00000000
[   14.273156] Oops: 0000 [#1] PREEMPT SMP
[   14.276980] Modules linked in: e1000e(+)
[   14.281386] CPU: 0 PID: 1227 Comm: modprobe Tainted: G        W 4.6.0-rc3+ #2
[   14.281386] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./microETXexpress-DC    , BIOS UNP1R112 04/12/2010
[   14.281386] task: f5ea5100 ti: f57c4000 task.ti: f57c4000
[   14.281386] EIP: 0060:[<00000000>] EFLAGS: 00010282 CPU: 0
[   14.281386] EIP is at 0x0
[   14.281386] EAX: f57d7184 EBX: f57d4500 ECX: 00000000 EDX: 00000000
[   14.281386] ESI: f57d495c EDI: f57d7144 EBP: f57c5ce8 ESP: f57c5cc4
[   14.281386]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[   14.281386] CR0: 80050033 CR2: 00000000 CR3: 3575f000 CR4: 000006c0
[   14.281386] Stack:
[   14.281386]  f81935b6 00007a0e 00007333 f81890f4 00048000 f57d4500 f57d4500 00000000
[   14.281386]  00000000 f57c5d3c f8194f9e 00000001 00020000 f5e78864 f5e78800 f57d495c
[   14.365891]  f57d4000 f57d7138 c10593b2 f5e788c8 00000000 f57d70c0 f57d70b0 f57d70a0
[   14.365891] Call Trace:
[   14.365891]  [<f81935b6>] ? e1000e_reset+0x323/0x49f [e1000e]
[   14.386984]  [<f81890f4>] ? e1000e_read_nvm_eerd+0x50/0x77 [e1000e]
[   14.391710]  [<f8194f9e>] e1000_probe+0x8bb/0xa88 [e1000e]
[   14.391710]  [<c10593b2>] ? preempt_count_add+0x78/0x95
[   14.391710]  [<c12a851c>] local_pci_probe+0x30/0x69
[   14.391710]  [<c12a8721>] pci_device_probe+0x50/0x77
[   14.391710]  [<c13df028>] driver_probe_device+0xe2/0x1f3
[   14.391710]  [<c13df189>] __driver_attach+0x50/0x6c
[   14.391710]  [<c13ddc59>] bus_for_each_dev+0x38/0x61
[   14.391710]  [<c13ded57>] driver_attach+0x14/0x16
[   14.391710]  [<c13df139>] ? driver_probe_device+0x1f3/0x1f3
[   14.391710]  [<c13de82b>] bus_add_driver+0xb4/0x191
[   14.391710]  [<f8069000>] ? 0xf8069000
[   14.391710]  [<f8069000>] ? 0xf8069000
[   14.391710]  [<c13df71f>] driver_register+0x70/0xa2
[   14.391710]  [<c12a87d8>] __pci_register_driver+0x35/0x39
[   14.391710]  [<f8069030>] e1000_init_module+0x30/0x32 [e1000e]
[   14.391710]  [<c1000409>] do_one_initcall+0xc9/0x146
[   14.391710]  [<c110ccf3>] ? kmem_cache_alloc_trace+0xca/0xd4
[   14.391710]  [<c109562d>] ? do_init_module+0x24/0x195
[   14.391710]  [<c109565c>] do_init_module+0x53/0x195
[   14.391710]  [<c10971fe>] load_module+0x1000/0x11d7
[   14.391710]  [<c10ff926>] ? __vmalloc_node+0x3d/0x44
[   14.391710]  [<c10974c1>] ? SyS_init_module+0x63/0xdd
[   14.391710]  [<c1097539>] SyS_init_module+0xdb/0xdd
[   14.391710]  [<c1001472>] do_fast_syscall_32+0x80/0xbf
[   14.391710]  [<c16763fb>] sysenter_past_esp+0x40/0x6a
[   14.391710] Code:  Bad EIP value.
[   14.391710] EIP: [<00000000>] 0x0 SS:ESP 0068:f57c5cc4
[   14.391710] CR2: 0000000000000000
[   14.513519] ---[ end trace 0004a75013c7c407 ]---

[Intel-wired-lan] [PATCH] e1000e: don't modify SYSTIM registers during SIOCSHWTSTAMP ioctl

[Keller, Jacob E]

On Fri, 2016-04-15 at 16:16 -0400, Brian Walsh wrote:
> On Thu, Apr 14, 2016 at 04:29:24PM -0700, Jacob Keller wrote:
> > 
> > The e1000e_config_hwtstamp function was incorrectly resetting the
> > SYSTIM
> > registers every time the ioctl was being run. If you happened to be
> > running ptp4l and lost the PTP connect (removing cable, or blocking
> > the
> > UDP traffic for example), then ptp4l will eventually perform a
> > restart
> > which involves re-requesting timestamp settings. In e1000e this has
> > the
> > unfortunate and incorrect result of resetting SYSTIME to the kernel
> > time. Since kernel time is usually in UTC, and PTP time is in TAI,
> > this
> > results in the leap second being re-applied.
> > 
> > Fix this by extracting the SYSTIME reset out into its own function,
> > e1000e_ptp_reset, which we call during reset to restore the
> > hardware
> > registers. This function will (a) restart the timecounter based on
> > the
> > new system time, (b) restore the previous PPB setting, and (c)
> > restore
> > the previous hwtstamp settings.
> > 
> > In order to perform (b), I had to modify the adjfreq ptp function
> > pointer to store the old delta each time it is called. This also
> > has the
> > side effect of restoring the correct base timinca register
> > correctly.
> > The driver does not need to explicitly zero the ptp_delta variable
> > since
> > the entire adapter structure comes zero-initialized.
> > 
> > Reported-by: Brian Walsh <brian@walsh.ws>
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > ---
> My last attempt was on an older 4.1 kernel. I am seeing the same
> failure
> on the latest 4.6 rc3.

I've sent a v2 that should address that, I didn't realize e1000e_reset
was called prior to e1000e_ptp_init.

Thanks,
Jake