netdev - Re: [net-next,05/14] net: stmmac: add stmmac core serdes support

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: <aXEN6B5_T9uFzVvu@shell.armlinux.org.uk>
Date: Wed, 21 Jan 2026 17:33:28 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Jakub Kicinski <kuba@...nel.org>, linux-phy@...ts.infradead.org,
	davem@...emloft.net, maxime.chevallier@...tlin.com,
	alexandre.torgue@...s.st.com, mohd.anwar@....qualcomm.com,
	neil.armstrong@...aro.org, hkallweit1@...il.com,
	mcoquelin.stm32@...il.com, netdev@...r.kernel.org,
	edumazet@...gle.com, linux-arm-msm@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, vkoul@...nel.org,
	andrew@...n.ch, pabeni@...hat.com, andrew+netdev@...n.ch,
	linux-stm32@...md-mailman.stormreply.com
Subject: Re: [net-next,05/14] net: stmmac: add stmmac core serdes support

On Wed, Jan 21, 2026 at 06:23:45PM +0200, Vladimir Oltean wrote:
> On Wed, Jan 21, 2026 at 02:46:42PM +0000, Russell King (Oracle) wrote:
> > On Tue, Jan 20, 2026 at 02:11:14PM +0200, Vladimir Oltean wrote:
> > > On Tue, Jan 20, 2026 at 10:12:46AM +0000, Russell King (Oracle) wrote:
> > > > First, I'll say I'm on a very short fuse today; no dinner last night,
> > > > at the hospital up until 5:30am, and a fucking cold caller rang the door
> > > > bell at 10am this morning. Just fucking our luck.
> > > 
> > > Sorry to hear that.
> > > 
> > > > On Tue, Jan 20, 2026 at 10:18:44AM +0200, Vladimir Oltean wrote:
> > > > > Isn't it sufficient to set pl->pcs to NULL when pcs_enable() fails and
> > > > > after calling pcs_disable(), though?
> > > >
> > > > No. We've already called mac_prepare(), pcs_pre_config(),
> > > > pcs_post_config() by this time, we're past the point of being able to
> > > > unwind.
> > > 
> > > I'm set out to resolve a much smaller problem.
> > > 
> > > Calling it a full "unwind" is perhaps a bit much, because pcs_pre_config()
> > > and pcs_post_config() don't have unwinding equivalents, unlike how
> > > pcs_enable() has pcs_disable(). I don't see what API convention would be
> > > violated if phylink decided to drop a PCS whose enable() returned an error.
> > 
> > While pcs_pre_config() and pcs_post_config() do not have unwinding
> > equivalents (what would they be?) the issue here is that these could
> > have changed any state that isn't simply undone by calling
> > pcs_disable().
> > 
> > For example, pcs_pre_config() could have reprogrammed signal routing,
> > clocking, or power supplies to blocks.
> > 
> > This already applies to Marvell DSA pcs-639x.c, where the pre/post
> > config hooks change the power state of the PCS block (for errata
> > handling), and the only way that gets undone is via a call to
> > pcs_disable() which explicitly disables IRQs and power for the PCS. Its
> > pcs_disable() isn't a strict reversal of pcs_enable(), it does more.
> > 
> > We already declare the interface to be dead on pcs_post_config()
> > failure, but we don't do that for pcs_enable() failure.
> > 
> > Maybe I need to explicitly state that pcs_disable() does not directly
> > balance pcs_enable(), but that _and_ the effects of pcs_pre_config()
> > and pcs_post_config(). However, that itself will add to the problems.
> > What if pcs_pre_config() and pcs_post_config() succeed but not
> > pcs_enable()? pcs-639x needs pcs_disable() to be called, but if we
> > require pcs_disable() to be balanced with a successful call to
> > pcs_enable(), that messes up that driver, and pretty much makes it
> > impossible to work around the errata.
> 
> What if we reordered phylink_major_config() such that phylink_pcs_enable()
> comes first, followed by phylink_pcs_pre_config() -> phylink_mac_config() ->
> phylink_pcs_post_config()? Superficially looking at pcs-639x, I don't
> think it would break.

I'm sorry, but I don't have time to continue this discussion today. I
woke late, we're trying to cram in the meals (in the middle of delayed
lunch-time dinner right now), work wants a quick call to discuss a
project that I missed the meeting for yesterday (which I haven't yet
had time for...)

Sorry, but while you may wish to get this sorted, for me this is a very
low priority issue that can be addressed later. Don't think I will have
time to review anything you send - and that's not a personal attack,
it's because I'm barely managing to hold everything together at my
end, and I don't have the time.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!