netdev - Re: GPY215 PHY interrupt issue

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: <158870dd20a5e30cda9f17009aa0c6c8@walle.cc>
Date:   Thu, 01 Dec 2022 22:31:19 +0100
From:   Michael Walle <michael@...le.cc>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Horatiu Vultur <horatiu.vultur@...rochip.com>,
        netdev@...r.kernel.org, Xu Liang <lxu@...linear.com>
Subject: Re: GPY215 PHY interrupt issue

Am 2022-12-01 16:54, schrieb Andrew Lunn:
>> So, switching the line to GPIO input doesn't help here, which also
>> means the interrupt line will be stuck the whole time.
> 
> Sounds like they totally messed up the design somehow.
> 
> Since we are into horrible hack territory.....
> 
> I assume you are using the Link state change interrupt? LSTC?

Yes, but recently I've found it that it also happens with
the speed change interrupt (during link-up). By pure luck (or
bad luck really?) I discovered that when I reduce the MDIO
frequency I get a similar behavior for the interrupt line
at link-up with the LSPC interrupt. I don't think it has
something to do with the frequency but with changed timing.
Anyway, I need to do the workaround for LTSC and for LSPC...

At the moment I have the following, which works:

@@ -560,6 +610,8 @@ static int gpy_config_intr(struct phy_device 
*phydev)

  static irqreturn_t gpy_handle_interrupt(struct phy_device *phydev)
  {
+	bool needs_mdint_wa = phydev->drv->phy_id == PHY_ID_GPY215B ||
+			     phydev->drv->phy_id == PHY_ID_GPY215C;
  	int reg;

  	reg = phy_read(phydev, PHY_ISTAT);
@@ -571,6 +623,23 @@ static irqreturn_t gpy_handle_interrupt(struct 
phy_device *phydev)
  	if (!(reg & PHY_IMASK_MASK))
  		return IRQ_NONE;

+	/* The PHY might leave the interrupt line asserted even after 
PHY_ISTAT
+	* is read. To avoid interrupt storms, delay the interrupt handling as
+	* long as the PHY drives the interrupt line. An internal bus read will
+	* stall as long as the interrupt line is asserted, thus just read a
+	* random register here.
+	* Because we cannot access the internal bus at all while the interrupt
+	* is driven by the PHY, there is no way to make the interrupt line
+	* unstuck (e.g. by changing the pinmux to GPIO input) during that time
+	* frame. Therefore, polling is the best we can do and won't do any 
more
+	* harm.
+	* It was observed that this bug happens on link state and link speed
+	* changes on a GPY215B and GYP215C independent of the firmware version
+	* (which doesn't mean that this list is exhaustive).
+	*/
+	if (needs_mdint_wa && (reg & (PHY_IMASK_LSTC | PHY_IMASK_LSPC)))
+		gpy_mbox_read(phydev, REG_GPIO0_OUT);
+
  	phy_trigger_machine(phydev);

  	return IRQ_HANDLED;

> Maybe instead use Link speed change and Duplex mode change? And
> disallow 10/Half. Some PHYs change to 10/Half when they loose
> link. They might be enough to tell you the link has changed. You can
> then read the BMSR to find out what actually happened.
> 
> This is assuming that interrupts in general are not FUBAR.

It seems like at least these two are :/ So with the code above
we could avoid the interrupt storm but we can't do anything about
the blocked interrupt line. I'm unsure if that is acceptable or
if we'd have to disable interrupts on this PHY altogether and
fallback to polling.

-michael