| 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
| ||
|
Message-ID: <20250128182638.uz7xzdi6jnvtxi3m@skbuf>
Date: Tue, 28 Jan 2025 20:26:38 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Tristram.Ha@...rochip.com, Woojung Huh <woojung.huh@...rochip.com>,
Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>,
Maxime Chevallier <maxime.chevallier@...tlin.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
UNGLinuxDriver@...rochip.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC net-next 1/2] net: pcs: xpcs: Add special code to
operate in Microchip KSZ9477 switch
On Tue, Jan 28, 2025 at 04:32:07PM +0000, Russell King (Oracle) wrote:
> I note that in the KSZ9477 documentation, bit[0] of VR_MII_DIG_CTRL1
> is marked as "reserved" and takes the value zero, so in the case of
> PHY-side SGMII, (6) always applies to KSZ9477 and (5) never does.
Good point. In human language, this means that when configuring the
KSZ9477 XPCS for the SGMII PHY role, it always expects the contents of
the autoneg code word to come from registers (VR_MII_AN_CTRL,
SR_MII_AN_ADV and SR_MII_CTRL), and never from input wires coming
from blocks external to the XPCS IP (xpcs_sgmii_link_sts_i,
xpcs_sgmii_full_duplex_i, xpcs_sgmii_link_speed_i[1:0]).
With the very important note that said SGMII PHY mode is still
under-specified in Linux, and the discussion about it still needs
to be had.
> This also solves my concern about 2a22b7ae2fa3 ("net: pcs: xpcs: adapt
> Wangxun NICs for SGMII mode"), because there (5) would apply.
Ok, so Wangxun shimmied in an operating mode where the XPCS does act
like a PHY, but receives the info about how to populate the autoneg
code word from wires, not from registers, and that's why Tristram is
noticing that the driver does not write the registers.
Not great, but at least the info we're gathering seems consistent thus far.
> > So my understanding is that SR_MII_AN_ADV needs to be written only if
> > TX_CONFIG=1 (SJA1105 calls this AUTONEG_CONTROL[PHY_MODE]).
>
> Yes, agreed. Thankfully, SJA1105 sets PHY_MODE/TX_CONFIG=0 and leaves
> SGMII_PHY_MODE_CTRL unaltered. TXGBE sets TX_CONFIG=1 but sets
> SGMII_PHY_MODE_CTRL=1 which also avoids this requirement.
Correct, for SJA1105 I realized the mode where PHY_MODE/TX_CONFIG=1
isn't supported in phylink, and I didn't consider it important enough to
raise the issue, so it's simply not configurable that way. I was thinking,
just like we have phy-mode = "rev-mii" and "rev-rmii" for MII and RMII
in the role of a PHY, that something like "rev-sgmii" would be the way
to explore. But I really have no interest or use case to explore this myself.
> > That's quite
> > different, and that will make sense when you consider that there's also
> > a table with the places the autoneg code word gets its info from:
> >
> > Config_Reg Bits in the 1000BASE-X and SGMII Mode
> >
> > +----------------+-------------------+--------------------+--------------------------------------------+
> > | Config_Reg bit | 1000Base-X mode | SGMII mode value | SGMII mode value |
> > | | | when TX_CONFIG = 0 | when TX_CONFIG = 1 |
> > +----------------+-------------------+--------------------+--------------------------------------------+
> > | 15 | Next page support | 0 | Link up or down. |
> > | | | | If DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL == 0, |
> > | | | | this bit is derived from Bit 4 |
> > | | | | (SGMII_LINK_STS) of the VR_MII_AN_CTRL. |
> > | | | | If DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL == 1, |
> > | | | | this bit is derived from the input port |
> > | | | | 'xpcs_sgmii_link_sts_i' |
> > +----------------+-------------------+--------------------+--------------------------------------------+
> > | 14 | ACK | 1 | 1 |
> > +----------------+-------------------+--------------------+--------------------------------------------+
> > | 13 | RF[1] | 0 | 0 |
> > +----------------+-------------------+--------------------+--------------------------------------------+
> > | 12 | RF[0] | 0 | FULL_DUPLEX |
> > | | | | If DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL == 0, |
> > | | | | this bit is derived from Bit 5 (FD) of |
> > | | | | the SR_MII_AN_ADV. |
> > | | | | If DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL == 1, |
> > | | | | this bit is derived from the input port |
> > | | | | 'xpcs_sgmii_full_duplex_i' |
> > +----------------+-------------------+--------------------+--------------------------------------------+
> > | 11:10 | Reserved | 0 | SPEED |
> > | | | | If DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL == 0, |
> > | | | | these bits are derived from Bit 13 (SS13) |
> > | | | | and Bit 6 (SS6) of the SR_MII_CTRL. |
> > | | | | If DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL == 1, |
> > | | | | this bit is derived from the input port |
> > | | | | 'xpcs_sgmii_link_speed_i[1:0]' |
> > +----------------+-------------------+--------------------+--------------------------------------------+
> > | 9 | Reserved | 0 | 0 |
> > +----------------+-------------------+--------------------+--------------------------------------------+
> > | 8:7 | PAUSE[1:0] | 0 | 0 |
> > +----------------+-------------------+--------------------+--------------------------------------------+
> > | 6 | HALF_DUPLEX | 0 | 0 |
> > +----------------+-------------------+--------------------+--------------------------------------------+
> > | 5 | FULL_DUPLEX | 0 | 0 |
> > +----------------+-------------------+--------------------+--------------------------------------------+
> > | 4:1 | Reserved | 0 | 0 |
> > +----------------+-------------------+--------------------+--------------------------------------------+
> > | 0 | Reserved | 1 | 1 |
> > +----------------+-------------------+--------------------+--------------------------------------------+
> >
> > I haven't figured out either what might be going on with the KSZ9477
> > integration, I just made a quick refresher and I thought this might be
> > useful for you as well, Russell. I do notice Tristram does force
> > TX_CONFIG=1 (DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII), but I don't understand
> > what's truly behind that. Hopefully just a misunderstanding.
>
> If you want to peek at the KSZ9477 documentation, what I'm looking at is
> available from here:
> https://www.microchip.com/en-us/product/ksz9477#Documentation
Thanks.
> Interestingly, there are a number of errata:
>
> Module 7 - SGMII auto-negotiation does not set bit 0 in the
> auto-negotiation code word
> Basically requires MII_ADVERTISE to be written as 0x1a0, and is only
> needed after power-up or reset.
Ok, I guess 0x1a0 would otherwise be the SR_MII_AN_ADV default value -
Nothing looks special about it. The layout of this register is the
table above. Bits 8:7 (PAUSE) and 5 (FULL_DUPLEX) are set, the rest
(NEXT_PAGE, REMOTE_FAULT, HALF_DUPLEX) are unset.
It's odd that writing this register would fix anything, especially
seeing that it isn't documented anywhere that bit 0 of the autoneg code
word would ever originate from SR_MII_AN_ADV in the 2 SGMII mode columns,
or would be affected by a modification of that register. But ok, I can't
contradict that...
It is under-specified whether the erratum occurs when TX_CONFIG is 1 or 0.
Unless specified otherwise, I am going to assume both modes.
I believe this erratum workaround should be implemented by Tristram;
I'm not seeing it in this patch.
> Module 8 - SGMII port link details from the connected SGMII PHY are not
> passed properly to the port 7 GMAC
> Basically, AUTONEG_INTR_STATUS needs to be read, and the PCS
> manually programmed for the speed.
Ok, I think this is answering my question to Tristram:
xpcs_link_up_sgmii_1000basex() needs to force the speed in MII_BMCR even
for PHYLINK_PCS_NEG_INBAND_ENABLED, where that otherwise shouldn't be
needed. It means that VR_MII_DIG_CTRL1[MAC_AUTO_SW] doesn't work? Bit 9
of "SGMII DIGITAL CONTROL REGISTER" is also hidden, and marked as reserved (0).
A reference to the KSZ9477 errata sheet module 8 in the code would be
nice. To me that is sufficient.
> Module 15 - SGMII registers are not initialized by hardware reset
> Requires that bit 15 of BASIC_CONTROL is set to reset the registers.
Nothing actionable here, the xpcs driver already performs reset.
> All three are not scheduled to be fixed, apparently.
>
> There seems to be no information listed there regarding the requirement
> for SGMII PHY mode.
Not that I can find either.
> > Tristram, why do you set this field to 1? Linux only supports the
> > configuration where a MAC behaves like a MAC. There needs to be an
> > entire discussion if you want to configure a MAC to be a SGMII autoneg
> > master (like a PHY), how to model that.
>
> (Using SJA1105 register terminology...)
>
> Looking at the patch, Tristram is setting PHY_MODE=1 and SGMII_LINK=1
> not when configuring for SGMII, but when configuring for 1000base-X.
Yeah, you're right... (?!) Again, misunderstanding, I hope?
> This is reflected in the documentation for KSZ9477 - which states that
> both these bits need to be set in "SerDes" aka 1000base-X mode. The
> question is... where did that statement come from, and should we be
> doing that for 1000base-X mode anyway?
Here, because SJA1105 only supports SGMII and _not_ 1000Base-X, I don't
have practical experience. Thus I can only refer you to:
Programming Guidelines for Clause 37 Auto-Negotiation
The Clause 37 auto-negotiation is enabled only when Bit 12 of the
SR_MII_CTRL Register is set. For Backplane Ethernet configurations,
the default value of this bit is 0. For all other configurations, the
default value of this bit is 1. When this bit is enabled, the Clause 37
auto-negotiation is initiated on the following events:
- Power on Reset
- Soft Reset
The DWC_xpcs initiates the auto-negotiation on the following events:
- When the link partner requests auto-negotiation by transmitting
configuration code groups.
- When the receive data path loses code group synchronization for more
than 10 ms (in 1000BASE-X mode) or 1.6 ms (in SGMII mode).
- When an error condition is detected while receiving the /C/ or /I/
order sets.
- When the host or software requests auto-negotiation by setting Bit 9
in the SR_MII_CTRL Register.
The following list explains the auto-negotiation process:
1. The DWC_xpcs starts auto-negotiation by first transmitting the
configuration words with all zeroes for 10 ms (1.6 ms for the SGMII
interface).
2. The SR_MII_AN_ADV Register content is transmitted in the
configuration words in the 1000BASE-X mode. In the SGMII mode, the
values given in the "Config_Reg Bits in the 1000BASE-X and SGMII Mode"
table are transmitted in the configuration word. The auto-negotiation
is complete when both link partners have exchanged their base pages.
3. DWC_xpcs generates an interrupt on completion (sbd_intr_o) of
auto-negotiation when Bit 0 of VR_MII_AN_CTRL Register is set to 1.
4. The auto-negotiation completion is indicated in the VR_MII_AN_INTR_STS
register.
Note: In configurations with MDIO interface, you must read the
VR_MII_AN_INTR_STS register after the auto-negotiation is complete.
Auto-negotiation Status reads incorrect value when Status is not
read in the previous Auto-negotiation session and MDC clock is not
active when the Management is not accessing the PHY/PCS registers.
Because of this limitation, Management may get incorrect
information of Link partner and this can cause link failure.
However, this limitation is not visible if the programming
guidelines are followed as mentioned in the databook. This is
because the minimum time between two successive auto-negotiations
has at least 3.2ms and Management or Host is expected to read the
current status of the Auto-negotiation before the next auto-negotiation
is completed.
5. In the MAC attached to the DWC_xpcs, the Transmit and Receive Flow
Control mode is determined based on the capabilities of the partner
(given in SR_MII_LP_BABL) and the half-duplex or full-duplex
operating mode.
In SGMII auto-negotiation, the received link status such as speed,
duplex mode is also given in the VR_MII_AN_INTR_STS Register.
When Clause 37 auto-negotiation is complete, DWC_xpcs automatically
resolves the duplex mode by selecting the duplex mode of its link
partner. If auto-negotiation is disabled, the DWC_xpcs selects the
duplex mode defined in Bit 8 of SR_MII_CTRL Register.
Nothing, in my reading, suggests to me that DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII
would be considered by the XPCS when operating in 1000Base-X mode
(DW_VR_MII_PCS_MODE_MASK == DW_VR_MII_PCS_MODE_C37_1000BASEX).
Powered by blists - more mailing lists