netdev - Re: igb: acknowledging time sync interrupts

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CDCA1885.1E850%matthew.vick@intel.com>
Date:	Tue, 28 May 2013 15:20:10 +0000
From:	"Vick, Matthew" <matthew.vick@...el.com>
To:	Richard Cochran <richardcochran@...il.com>
CC:	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: igb: acknowledging time sync interrupts

On 5/26/13 3:39 AM, "Richard Cochran" <richardcochran@...il.com> wrote:

>Matt,
>
>In igb_main.c you have ISR code like:
>
>	if (icr & E1000_ICR_TS) {
>		u32 tsicr = rd32(E1000_TSICR);
>
>		if (tsicr & E1000_TSICR_TXTS) {
>			/* acknowledge the interrupt */
>			wr32(E1000_TSICR, E1000_TSICR_TXTS);
>			/* retrieve hardware timestamp */
>			schedule_work(&adapter->ptp_tx_work);
>		}
>	}
>
>In the datasheet for the 82580 and the i210, for TSICR it says,
>
>   Note: Once ICR.Time_Sync is set, the internal value of this
>         register should be cleared by writing 1b to all bits
>         or cleared by a read to enable receiving an additional
>         ^^^^^^^^^^^^^^^^^^^^
>         ICR.Time_Sync interrupt.
>
>and that implies that your write to acknowledge the interrupt is
>superfluous, since you already read the TSICR.
>
>Is this an error in the datasheets, or is the code doing extra,
>unneeded work?
>
>Thanks,
>Richard

Richard,

Good catch--and you're correct--but I had issues with the read of TSICR
not clearing like it should on the 82580 (but it would work fine on the
I350 and I210). I decided the cleaner implementation would be to
explicitly acknowledge the interrupt across the board. I haven't had the
time to follow up with the hardware team, but my suspicion is that it's an
errata with the 82580.

I'll start some internal discussion to see if I can get an official answer
on the 82580.

Cheers,
Matthew

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html