netdev - Re: [PATCH RFC net-next 0/4] net: xpcs: cleanups and partial support for KSZ9477

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: <20250331143113.r6t5we52wp77qqjr@skbuf>
Date: Mon, 31 Mar 2025 17:31:13 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: Tristram.Ha@...rochip.com
Cc: linux@...linux.org.uk, andrew@...n.ch, davem@...emloft.net,
	edumazet@...gle.com, hkallweit1@...il.com, kuba@...nel.org,
	netdev@...r.kernel.org, pabeni@...hat.com,
	UNGLinuxDriver@...rochip.com, Woojung.Huh@...rochip.com
Subject: Re: [PATCH RFC net-next 0/4] net: xpcs: cleanups and partial support
 for KSZ9477

Hi Tristram,

On Tue, Mar 18, 2025 at 07:59:07PM +0000, Tristram.Ha@...rochip.com wrote:
> Sorry for the long delay.  After discussing with the Microchip design
> team we come up with this explanation for setting the
> DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII (bit 3) of DW_VR_MII_AN_CTRL (0x8001)
> register in 1000Base-X mode to make it work with AN on.
> 
> KSZ9477 has the older version of 1000Base-X Synopsys IP which works in
> 1000Base-X mode without AN.

I was unaware of the existence of such Synopsys IP. In which relevant
way is it "older"? Does it specifically claim it supports 1000Base-X,
but only without AN? Or if not, is it an SGMII-only base IP, then? The
two are compatible when AN is disabled and the SGMII IP is configured
for 1Gbps, and can be mistaken for one another.

You're making it sound as if "1000Base-X" support was patched by the
Microchip integration over a base PCS IP which did not support it.

> When AN is on the port does not pass traffic because it does not
> detect a link.

Which port does not detect a link? The KSZ9477 port or its link partner?

> Setting DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII allows the link to be
> turned on by either setting DW_VR_MII_SGMII_LINK_STS (bit 4) or
> DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL (bit 0) in DW_VR_MII_DIG_CTRL1
> (0x8000) register.  After that the port can pass traffic.

Can you comment on whether Microchip has given these bits some
integration-specific meaning, or whether their meaning from the base IP,
summarized by me in this table taken from the XPCS data book, has been
preserved unchanged?
https://lore.kernel.org/netdev/20250128152324.3p2ccnxoz5xta7ct@skbuf/

So far, the only noted fact is that they take effect also for
PCS_MODE=0b00 (1000Base-X), and not just for PCS_MODE=0b10 (SGMII),
as originally intended in the base IP. But we don't exactly know what
they do. Just that the Microchip IP wants them set to one.

If their meaning is otherwise the same, all data available to me points
to the conclusion that the "1000Base-X with autoneg on" mode in KSZ9477
is actually SGMII with a config word customized by software, and with
some conditions commented out from the base IP, to allow the code word
to be customizable even in PCS_MODE=0b00.

If the above conclusion is true, I think we need to pay more attention
at whether the transmitted config word is truly 1000Base-X compatible,
as Russell pointed out here:
https://lore.kernel.org/netdev/Z5iiXWkhm2OvbjOx@shell.armlinux.org.uk/#t
The discussion there got pretty involved and branched out into other
topics, but I couldn't find a response from you on that specific second
paragraph.

> This is still a specific KSZ9477 problem so it may be safer to put this
> code under "if (xpcs->info.pma == MICROCHIP_KSZ9477_PMD_ID)" check.

If the meaning of these fields was not kept 100% intact by Microchip
during their integration, including the context in which they are valid,
then I 100% agree. But I would still like an answer to the questions above.