netdev - Re: [PATCH net] net: phy: avoid clearing PHY interrupts twice in irq handler

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]

Message-ID: <fbf0a7739f5c8442c1d2b0aa9aba086d@walle.cc>
Date:   Wed, 04 Mar 2020 14:02:47 +0100
From:   Michael Walle <michael@...le.cc>
To:     Heiner Kallweit <hkallweit1@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH net] net: phy: avoid clearing PHY interrupts twice in irq
 handler

Am 2020-03-04 13:13, schrieb Heiner Kallweit:
> On 02.03.2020 00:20, Heiner Kallweit wrote:
>> On 01.03.2020 23:52, Michael Walle wrote:
>>> Am 2020-03-01 21:36, schrieb Heiner Kallweit:
>>>> On all PHY drivers that implement did_interrupt() reading the 
>>>> interrupt
>>>> status bits clears them. This means we may loose an interrupt that
>>>> is triggered between calling did_interrupt() and 
>>>> phy_clear_interrupt().
>>>> As part of the fix make it a requirement that did_interrupt() clears
>>>> the interrupt.
>>> 
>>> Looks good. But how would you use did_interrupt() and 
>>> handle_interrupt()
>>> together? I guess you can't. At least not if handle_interrupt() has
>>> to read the pending bits again. So you'd have to handle custom
>>> interrupts in did_interrupt(). Any idea how to solve that?
>>> 
>>> [I know, this is only about fixing the lost interrupts.]
>>> 
>> Right, this one is meant for stable to fix the issue with the 
>> potentially
>> lost interrupts. Based on it I will submit a patch for net-next that
>> tackles the issue that did_interrupt() has to read (and therefore 
>> clear)
>> irq status bits and therefore makes them unusable for 
>> handle_interrupt().
>> The basic idea is that did_interrupt() is called only if 
>> handle_interrupt()
>> isn't implemented. handle_interrupt() has to include the did_interrupt
>> functionality. It can read the irq status once and store it in a 
>> variable
>> for later use.
>> 
> In case you wait for this patch to base further own work on it:
> I'm waiting for next merge of net into net-next, because my patch will
> apply cleanly only after that. This merge should happen in the next 
> days.

Ok, thanks for the information. I have enough other things to do ;)

-michael