netdev - Re: Aquantia PHY in OCSGMII mode?

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: <20250801130420.m3fbqlvtzbdo5e5d@skbuf>
Date: Fri, 1 Aug 2025 16:04:20 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Alexander Wilhelm <alexander.wilhelm@...termo.com>,
	Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: Aquantia PHY in OCSGMII mode?

On Fri, Aug 01, 2025 at 01:23:44PM +0100, Russell King (Oracle) wrote:
> It looks like memac_select_pcs() and memac_prepare() fail to
> handle 2500BASEX despite memac_initialization() suggesting the
> SGMII PCS supports 2500BASEX.

Thanks for pointing this out, it seems to be a regression introduced by
commit 5d93cfcf7360 ("net: dpaa: Convert to phylink").

If there are no other volunteers, I can offer to submit a patch if
Alexander confirms this fixes his setup.

> It would also be good if the driver can also use
> pcs->supported_interfaces which states which modes the PCS layer
> supports as well.

The current algorithm in lynx_pcs_create() is too optimistic and
advertises host interfaces which the PCS may not actually support.

static const phy_interface_t lynx_interfaces[] = {
	PHY_INTERFACE_MODE_SGMII,
	PHY_INTERFACE_MODE_QSGMII,
	PHY_INTERFACE_MODE_1000BASEX,
	PHY_INTERFACE_MODE_2500BASEX,
	PHY_INTERFACE_MODE_10GBASER,
	PHY_INTERFACE_MODE_USXGMII,
};

	for (i = 0; i < ARRAY_SIZE(lynx_interfaces); i++)
		__set_bit(lynx_interfaces[i], lynx->pcs.supported_interfaces);

I am concerned that if we add logic to the MAC driver which does:

		phy_interface_or(config->supported_interfaces,
				 config->supported_interfaces,
				 pcs->supported_interfaces);

then we depart from the physical reality of the board and may end up
accepting a host interface which we should have rejected.

There is downstream code which refines lynx_pcs_create() to this:

	/* In case we have access to the SerDes phy/lane, then ask the SerDes
	 * driver what interfaces are supported based on the current PLL
	 * configuration.
	 */
	for (int i = 0; i < ARRAY_SIZE(lynx_interfaces); i++) {
		phy_interface_t iface = lynx_interfaces[i];

		err = phy_validate(lynx->serdes[PRIMARY_LANE],
				   PHY_MODE_ETHERNET, iface, NULL);
		if (err)
			continue;

		__set_bit(iface, supported_interfaces);
	}

but the infrastructure (the SerDes driver) is currently lacking upstream.