netdev - Re: [Linux-stm32] [PATCH RFC net-next 6/7] net: stmmac: add helpers to indicate WoL enable status

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: <b88160a5-a0b8-4a1a-a489-867b8495a88e@lunn.ch>
Date: Tue, 29 Jul 2025 19:27:11 +0200
From: Andrew Lunn <andrew@...n.ch>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Gatien CHEVALLIER <gatien.chevallier@...s.st.com>,
	netdev@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
	Andrew Lunn <andrew+netdev@...n.ch>,
	Eric Dumazet <edumazet@...gle.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	"David S. Miller" <davem@...emloft.net>,
	linux-arm-kernel@...ts.infradead.org,
	Heiner Kallweit <hkallweit1@...il.com>
Subject: Re: [Linux-stm32] [PATCH RFC net-next 6/7] net: stmmac: add helpers
 to indicate WoL enable status

> stmmac gets this wrong right now, but (as I've written previously)
> this is going to be a *very* difficult problem to solve, because
> the PHY drivers are - to put it bluntly - "utter crap" when it
> comes to WoL.

Agreed.

> I'll take the rtl8211f again as an example - its get_wol()
> implementation is quite typical of many PHY drivers. Someone comes
> along and decides to implement WoL support at the PHY. They add the
> .get_wol() method, which unconditionally returns the PHY's hardware
> capabilities without regards for the rest of the system.
> 
> Consider the case where a PHY supports WoL, but the signalling for
> WoL to wake up the system is not wired. The .get_wol() method happily
> says that WoL is supported. Let's say that the PHY supports magic
> packet, and so does the MAC, and the MAC WoL is functional.
> 
> Now, with what Andrew said in his email, and consider what this means.
> .set_wol() is called, requesting magic packet. The PHY driver says "oh
> yes, the PHY hardware supports this, I'll program the PHY and return
> zero". At this point, the MAC thinks the PHY has accepted the WoL
> configuration.
> 
> The user suspends the system. The user sends the correct magic
> packet. The system does not wake up. The user is now confused.

There are some MAC drivers which simply trust the PHY. They pass
.get_wol() and .set_wol() direct to the PHY. They don't attempt to
perform MAC WoL, or the MAC driver does not have any hardware support
for it. Such systems are going to end up with a confused user when the
driver says WoL is enabled, but it does not wake.

So while i agree we cannot simply 'fix' stmmac, the issue of PHY
drivers not behaving properly is a bigger problem across a wide range
of MAC drivers.

I think we could quickly improve the situation to some degree by
reviewing the PHY drivers. e.g. the current code in mxl-gpy.c makes it
clear WoL is just another interrupt source. There is no special
pin. So get_wol() needs a call to phy_interrupt_is_valid(phydev) and
return not return any WoL modes if there is not a valid interrupt.

This will not work for all PHYs, e.g. the Marvell 1G PHYs can
repurposed LED2 for WoL indication.

motorcomm.c looks broken. The code suggests WoL is just another
interrupt source, but the driver lacks interrupt handling...

The broadcom code looks like it gets it correct.
bcm54xx_phy_can_wakeup() checks if there is an interrupt or a
dedicated GPIO, and return no wakeup modes if not. KUDOS to Florians
team.

dp83822.c appears to be missing a phy_interrupt_is_valid(phydev),
since WoL appears to be just another interrupt source.

Same for dp83867.c.

And i did notice that the Broadcom code is the only one doing anything
with enable_irq_wake()/disable_irq_wake(). We need to scatter these
into the drivers.

	Andrew