linux-kernel - 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: <20251007140819.7s5zfy4zv7w3ffy5@skbuf>
Date: Tue, 7 Oct 2025 17:08:19 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: Alexander Wilhelm <alexander.wilhelm@...termo.com>
Cc: "Russell King (Oracle)" <linux@...linux.org.uk>,
	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?

Hi Alexander,

On Thu, Oct 02, 2025 at 07:54:48AM +0200, Alexander Wilhelm wrote:
> Hi Vladimir,
> 
> Thanks your for the hint with the IF_MODE register, I finally found the
> root cause of my issue. Unfortunately, my U-Boot implementation was setting
> the `IF_MODE_SGMII_EN` and `IF_MODE_USE_SGMII_AN` bits. This caused a 10x
> symbol replication when operating at 100M speed. At the same time, the
> `pcs-lynx` driver never modified these bits when 2500Base-X was configured.
> 
> I was able to fix this in U-Boot. Additionally, I explicitly cleared these
> bits in the Lynx driver whenever 2500Base-X is configured (see patch
> below). Iâ€™d like to hear your expertise on this: do you think this patch is
> necessary, or could there be scenarios where these flags should remain set
> for 2500Base-X?
> 
> 
> Best regards
> Alexander Wilhelm
> ---
>  drivers/net/pcs/pcs-lynx.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c
> index 23b40e9eacbb..2774c62fb0db 100644
> --- a/drivers/net/pcs/pcs-lynx.c
> +++ b/drivers/net/pcs/pcs-lynx.c
> @@ -169,6 +169,25 @@ static int lynx_pcs_config_giga(struct mdio_device *pcs,
>  					  neg_mode);
>  }
>  
> +static int lynx_pcs_config_2500basex(struct mdio_device *pcs,
> +				     unsigned int neg_mode)
> +{
> +	int err;
> +
> +	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) {
> +		dev_err(&pcs->dev,
> +			"AN not supported on 3.125GHz SerDes lane\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	err = mdiodev_modify(pcs, IF_MODE,
> +			     IF_MODE_SGMII_EN | IF_MODE_USE_SGMII_AN, 0);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
>  static int lynx_pcs_config_usxgmii(struct mdio_device *pcs,
>  				   const unsigned long *advertising,
>  				   unsigned int neg_mode)
> @@ -201,12 +220,7 @@ static int lynx_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
>  		return lynx_pcs_config_giga(lynx->mdio, ifmode, advertising,
>  					    neg_mode);
>  	case PHY_INTERFACE_MODE_2500BASEX:
> -		if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) {
> -			dev_err(&lynx->mdio->dev,
> -				"AN not supported on 3.125GHz SerDes lane\n");
> -			return -EOPNOTSUPP;
> -		}
> -		break;
> +		return lynx_pcs_config_2500basex(lynx->mdio, neg_mode);
>  	case PHY_INTERFACE_MODE_USXGMII:
>  		return lynx_pcs_config_usxgmii(lynx->mdio, advertising,
>  					       neg_mode);
> 
> base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
> -- 
> 2.43.0

Sorry for the delay. What you have found are undoubtebly two major bugs,
causing the Lynx PCS to operate in undefined behaviour territory.
Nonetheless, while your finding has helped me discover many previously
unknown facts about the hardware IP, I still cannot replicate exactly
your reported behaviour. In order to fully process things, I would like
to ask a few more clarification questions.

Is your U-Boot implementation based on NXP's dtsec_configure_serdes()?
https://github.com/u-boot/u-boot/blob/master/drivers/net/fm/eth.c#L57
Why would U-Boot set IF_MODE_SGMII_EN | IF_MODE_USE_SGMII_AN only when
the AQR115 resolves only to 100M, but not in the other cases (which do
not have this problem)? Or does it do it irrespective of resolved media
side link speed? Simply put: what did the code that you fixed up look like?

With the U-Boot fix reverted, could you please replicate the broken
setup with AQR115 linking at 100Mbps, and add the following function in
Linux drivers/pcs-lynx.c?

static void lynx_pcs_debug(struct mdio_device *pcs)
{
	int bmsr = mdiodev_read(pcs, MII_BMSR);
	int bmcr = mdiodev_read(pcs, MII_BMCR);
	int adv = mdiodev_read(pcs, MII_ADVERTISE);
	int lpa = mdiodev_read(pcs, MII_LPA);
	int if_mode = mdiodev_read(pcs, IF_MODE);

	dev_info(&pcs->dev, "BMSR 0x%x, BMCR 0x%x, ADV 0x%x, LPA 0x%x, IF_MODE 0x%x\n", bmsr, bmcr, adv, lpa, if_mode);
}

and call it from:

static void lynx_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
			       struct phylink_link_state *state)
{
	struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);

	lynx_pcs_debug(lynx->mdio); // <- here

	switch (state->interface) {
	...

With this, I would like to know:
(a) what is the IF_MODE register content outside of the IF_MODE_SGMII_EN
    and IF_MODE_USE_SGMII_AN bits.
(b) what is the SGMII code word advertised by the AQR115 in OCSGMII mode.

Then if you could replicate this test for 1Gbps medium link speed, it
would be great.