lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 17 Nov 2014 23:22:02 +0000 From: "Keller, Jacob E" <jacob.e.keller@...el.com> To: "richardcochran@...il.com" <richardcochran@...il.com> CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "davem@...emloft.net" <davem@...emloft.net>, "Allan, Bruce W" <bruce.w.allan@...el.com>, "Ronciak, John" <john.ronciak@...el.com>, "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>, "Vick, Matthew" <matthew.vick@...el.com> Subject: Re: [PATCH net-next 2/4] igb: do not clobber the TSAUXC bits on reset. On Tue, 2014-11-18 at 00:06 +0100, Richard Cochran wrote: > The TSAUXC register has a number of different bits, one of which disables > the main clock function. Previously, the clock was re-enabled by clearing > the entire register. This patch changes the code to preserve the values > of the other bits in that register. > This is a step in the right direction, but won't fully work due to this being called after a MAC reset. > Signed-off-by: Richard Cochran <richardcochran@...il.com> > --- > drivers/net/ethernet/intel/igb/igb_ptp.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c > index 794c139..ce57128 100644 > --- a/drivers/net/ethernet/intel/igb/igb_ptp.c > +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c > @@ -905,6 +905,8 @@ void igb_ptp_stop(struct igb_adapter *adapter) > void igb_ptp_reset(struct igb_adapter *adapter) > { > struct e1000_hw *hw = &adapter->hw; > + unsigned long flags; > + u32 tsauxc; > > if (!(adapter->flags & IGB_FLAG_PTP)) > return; > @@ -923,7 +925,11 @@ void igb_ptp_reset(struct igb_adapter *adapter) > case e1000_i210: > case e1000_i211: > /* Enable the timer functions and interrupts. */ > - wr32(E1000_TSAUXC, 0x0); > + spin_lock_irqsave(&adapter->tmreg_lock, flags); > + tsauxc = rd32(E1000_TSAUXC); > + tsauxc &= ~TSAUXC_DISABLE; > + wr32(E1000_TSAUXC, tsauxc); > + spin_unlock_irqrestore(&adapter->tmreg_lock, flags); This won't work. We call igb_ptp_reset directly after a HW reset (MAC reset). TSAUXC data is not maintained. We probably need to store driver state of the TSAUXC bits, and then re-enable them in this reset path. This code today is fine, because there was no other state we cared about in the path. That being said, I don't think this change breaks anything. I just think it will not solve the problem. Thanks, Jake > wr32(E1000_TSIM, TSYNC_INTERRUPTS); > wr32(E1000_IMS, E1000_IMS_TS); > break;
Powered by blists - more mailing lists