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
| ||
|
Message-ID: <20201031105721.cc5g3he66ku6rm5b@skbuf> Date: Sat, 31 Oct 2020 12:57:21 +0200 From: Ioana Ciornei <ciorneiioana@...il.com> To: Heiner Kallweit <hkallweit1@...il.com> Cc: Andrew Lunn <andrew@...n.ch>, Ioana Ciornei <ciorneiioana@...il.com>, Russell King <linux@...linux.org.uk>, Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, Ioana Ciornei <ioana.ciornei@....com>, Alexandru Ardelean <alexandru.ardelean@...log.com>, Andre Edich <andre.edich@...rochip.com>, Antoine Tenart <atenart@...nel.org>, Baruch Siach <baruch@...s.co.il>, Christophe Leroy <christophe.leroy@....fr>, Dan Murphy <dmurphy@...com>, Divya Koppera <Divya.Koppera@...rochip.com>, Florian Fainelli <f.fainelli@...il.com>, Hauke Mehrtens <hauke@...ke-m.de>, Jerome Brunet <jbrunet@...libre.com>, Kavya Sree Kotagiri <kavyasree.kotagiri@...rochip.com>, Linus Walleij <linus.walleij@...aro.org>, Marco Felsch <m.felsch@...gutronix.de>, Marek Vasut <marex@...x.de>, Martin Blumenstingl <martin.blumenstingl@...glemail.com>, Mathias Kresin <dev@...sin.me>, Maxim Kochetkov <fido_max@...ox.ru>, Michael Walle <michael@...le.cc>, Neil Armstrong <narmstrong@...libre.com>, Nisar Sayed <Nisar.Sayed@...rochip.com>, Oleksij Rempel <o.rempel@...gutronix.de>, Philippe Schenker <philippe.schenker@...adex.com>, Willy Liu <willy.liu@...ltek.com>, Yuiko Oshino <yuiko.oshino@...rochip.com> Subject: Re: [PATCH net-next 00/19] net: phy: add support for shared interrupts (part 1) On Sat, Oct 31, 2020 at 11:18:18AM +0100, Heiner Kallweit wrote: > On 31.10.2020 00:36, Andrew Lunn wrote: > >>> - Every PHY driver gains a .handle_interrupt() implementation that, for > >>> the most part, would look like below: > >>> > >>> irq_status = phy_read(phydev, INTR_STATUS); > >>> if (irq_status < 0) { > >>> phy_error(phydev); > >>> return IRQ_NONE; > >>> } > >>> > >>> if (irq_status == 0) > >> > >> Here I have a concern, bits may be set even if the respective interrupt > >> source isn't enabled. Therefore we may falsely blame a device to have > >> triggered the interrupt. irq_status should be masked with the actually > >> enabled irq source bits. > > > > Hi Heiner > > > Hi Andrew, > > > I would say that is a driver implementation detail, for each driver to > > handle how it needs to handle it. I've seen some hardware where the > > interrupt status is already masked with the interrupt enabled > > bits. I've soon other hardware where it is not. > > > Sure, I just wanted to add the comment before others simply copy and > paste this (pseudo) code. And in patch 9 (aquantia) and 18 (realtek) > it is used as is. And IIRC at least the Aquantia PHY doesn't mask > the interrupt status. > Hi Heiner, If I understand correctly what you are suggesting, the .handle_interrupt() for the aquantia would look like this: static irqreturn_t aqr_handle_interrupt(struct phy_device *phydev) { int irq_status; irq_status = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_TX_VEND_INT_STATUS2); if (irq_status < 0) { phy_error(phydev); return IRQ_NONE; } if (!(irq_status & MDIO_AN_TX_VEND_INT_STATUS2_MASK)) return IRQ_NONE; phy_trigger_machine(phydev); return IRQ_HANDLED; } So only return IRQ_HANDLED when one of the bits from INT_STATUS corresponding with the enabled interrupts are set, not if any other link status bit is set. Ok, I'll send a v2 with these kind of changes. Ioana
Powered by blists - more mailing lists