netdev - Re: [PATCH net-next 0/3] net: stmmac: phylink PCS conversion part 3 (dodgy stuff)

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: <aQjgGBGY4T6xbjWr@shell.armlinux.org.uk>
Date: Mon, 3 Nov 2025 17:02:16 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Mohd Ayaan Anwar <mohd.anwar@....qualcomm.com>,
	Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Alexis Lothoré <alexis.lothore@...tlin.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	Boon Khai Ng <boon.khai.ng@...era.com>,
	Daniel Machon <daniel.machon@...rochip.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Furong Xu <0x1207@...il.com>,
	Jacob Keller <jacob.e.keller@...el.com>,
	Jakub Kicinski <kuba@...nel.org>,
	"Jan Petrous (OSS)" <jan.petrous@....nxp.com>,
	linux-arm-kernel@...ts.infradead.org,
	linux-stm32@...md-mailman.stormreply.com,
	Maxime Chevallier <maxime.chevallier@...tlin.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>, netdev@...r.kernel.org,
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
	Yu-Chun Lin <eleanor15x@...il.com>
Subject: Re: [PATCH net-next 0/3] net: stmmac: phylink PCS conversion part 3
 (dodgy stuff)

On Mon, Nov 03, 2025 at 02:13:53PM +0200, Vladimir Oltean wrote:
> > stmmac unfortunately doesn't give access to the raw Cisco SGMII
> > in-band control word. However, reading register 0xf8 bits 31:16 for
> > dwmac4, or register 0xd8 bits 15:0 for dwmac1000 will give this
> > information. In that bitfield, bits 2:1 give the speed. 2 = 1G,
> > 1 = 100M, 0 = 10M.
> 
> It might be Linux who is forcing the AQR115C into the nonsensical
> behaviour of advertising 10M in the SGMII control word while
> simultanously forcing the PHY MII to operate at 1G with flow control
> for the 10M media speed.

There's another factor in play here: the dwmac-qcom-ethqos glue driver
forces the configuration of the stmmac integrated PCS irrespective of
what phylink wants (whether or not patch 1 is being used.) That is,
for 1G, 100M and 10M, the SGMII in-band control word must be sent by
the PHY, and it must accurately indicate the symbol replication via
the speed bits. If this is not the case, then the stmmac PCS block will
not behave correctly.

So, if the PHY says in the SGMII configuration word that its operating
at 10M, but is internally rate-adapting to 1G and sending the packet
with no symbol replication, that is outside what the stmmac PCS block
will cope with.

> We don't control the latter, but we do control the former:
> aqr_gen2_config_inband(), if given modes == LINK_INBAND_ENABLE, will
> enable in-band for all media speeds that use PHY_INTERFACE_MODE_SGMII.
> Regardless of how the PHY was provisioned for each media speed, and
> especially regardless of rate matching settings, this function will
> uniformly set the same in-band enabled/disabled setting for all media
> speeds using the same host interface.
> 
> If dwmac_integrated_pcs_inband_caps(), as per Russell's patch 1/3,
> reports LINK_INBAND_ENABLE | LINK_INBAND_DISABLE, and if
> aqr_gen2_inband_caps() also reports LINK_INBAND_ENABLE | LINK_INBAND_DISABLE,
> then we're giving phylink_pcs_neg_mode() all the tools it needs to shoot
> itself in the foot, and select LINK_INBAND_ENABLE.
> 
> The judgement call in the Aquantia PHY driver was mine, as documented in
> commit 5d59109d47c0 ("net: phy: aquantia: report and configure in-band
> autoneg capabilities"). The idea being that the configuration would have
> been unsupportable anyway given the question that the framework asks:
> "does the PHY use in-band for SGMII, or does it not?"

>From the kernel messages provided, the PCS is reporting no
capabilities, but as you say, the PHY is reporting
LINK_INBAND_ENABLE | LINK_INBAND_DISABLE. The mode that phylink is
using is "phy/outband/sgmii" - so what _should_ be no in-band
signalling. However, note what I said above - for 1G, 100M and 10M,
qcom-ethqos does this:

	ethqos_pcs_set_inband(priv, true);

in its fix_mac_speed() hook, which translates to:

	stmmac_pcs_ctrl_ane(priv, true, 0);

which eventually gets through to:

	dwmac_ctrl_ane(priv, offset, true, false);

This sets GMAC_AN_CTRL_ANE | GMAC_AN_CTRL_RAN but not
GMAC_AN_CTRL_SGMRAL. This means "autoneg enable" "restart autoneg" and
!SGMRAL means that the SGMII rate adaption block operates according to
the link speed received from the PHY.

At this point, I'm not sure whether I should just rip out the
stmmac_pcs_ctrl_ane() stuff from dwmac-qcom-ethqos.c - I've been
operating on the assumption that this is working code. Maybe that's
a false assumption, and its been broken since a818bd12538c ("net:
stmmac: dwmac-qcom-ethqos: Add support for 2.5G SGMII").

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