netdev - Re: [PATCH] can: flexcan: fix irq flooding by clearing all interrupt sources

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] [day] [month] [year] [list]

Date:	Wed, 14 Dec 2011 14:21:18 +0100
From:	Wolfgang Grandegger <wg@...ndegger.com>
To:	Marc Kleine-Budde <mkl@...gutronix.de>
CC:	netdev@...r.kernel.org, linux-can@...r.kernel.org,
	Reuben Dowle <Reuben.Dowle@...ico.com>,
	Lothar Waßmann <LW@...O-electronics.de>
Subject: Re: [PATCH] can: flexcan: fix irq flooding by clearing all interrupt
 sources

Hi Marc,

On 12/13/2011 05:22 PM, Wolfgang Grandegger wrote:
> On 12/13/2011 01:59 PM, Marc Kleine-Budde wrote:
>> On 12/13/2011 01:53 PM, Wolfgang Grandegger wrote:
>>>> I got my MX35PDK board working and can confirm, that the patch works on
>>>> a i.mx35 as well. My testing also confirms, that the ESR TWRN_INT,
>>>> RWRN_INT, FLEXCAN_ESR_BOFF_INT do not function as documented. These
>>>> flags do show up once, together with ERR_INT, and then, after clearing,
>>>> never again. Obviously a bug in the Flexcan logic. From the feedback we
>>>> can say, that only the i.MX28 does behave differently (==correctly). All
>>>> other seem to work with the current code:
>>>>
>>>>  Flexcan on
>>>>  - i.mx25
>>>>  - i.mx35
>>>>  - i.mx53
>>>>  - P1010/P1020
>>>
>>> But unfortunately, state change reporting looks different with this patch :)
>>
>> Hmm - so not schedule for stable. What about your buf-off-handling, will
>> this change the reporting again?
> 
> Well, as it is a serious problem on i.MX28, I would schedule this patch
> for stable as well. The error and state change reporting is bogus on the
> Flexcan anyhow. Without this patch, I get "active->warning->passive" if
> I send a message with cable disconnect (no ack). With patch just
> "active->warning". That's the same behaviour
> as on the i.MX28, also with my new state and bus-off handling. See:

I now understand the difference. I "flexcan_irq" we have:

	/*
	 * schedule NAPI in case of:
	 * - rx IRQ
	 * - state change IRQ
	 * - bus error IRQ and bus error reporting is activated
	 */
	if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
	    (reg_esr & FLEXCAN_ESR_ERR_STATE) ||
	    flexcan_has_and_handle_berr(priv, reg_esr)) {

Without this patch, "reg_esr & FLEXCAN_ESR_ERR_STATE" is *always* true
because the RWRN_INT is never cleared. This means, *any* message is
scheduled. As a nice side effect, this patch fixes this bug a well.

Only because any message is scheduled, the state change to error passive
is recognized in flexcan_poll(). The state change to error passive is
not signaled by an extra interrupt and is therefore only visible
together with RX, TX done or bus-error events.

Wolfgang.

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