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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 25 Nov 2022 17:35:55 +0200 From: Vladimir Oltean <vladimir.oltean@....com> To: "Russell King (Oracle)" <linux@...linux.org.uk> Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Heiner Kallweit <hkallweit1@...il.com>, Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, UNGLinuxDriver@...rochip.com, bcm-kernel-feedback-list@...adcom.com, Madalin Bucur <madalin.bucur@....nxp.com>, Camelia Groza <camelia.groza@....com>, Claudiu Manoil <claudiu.manoil@....com>, Ioana Ciornei <ioana.ciornei@....com>, Maxim Kochetkov <fido_max@...ox.ru>, Sean Anderson <sean.anderson@...o.com>, Antoine Tenart <atenart@...nel.org>, Michael Walle <michael@...le.cc>, Raag Jadav <raagjadav@...il.com>, Siddharth Vadapalli <s-vadapalli@...com>, Ong Boon Leong <boon.leong.ong@...el.com>, Colin Foster <colin.foster@...advantage.com>, Marek Behun <marek.behun@....cz> Subject: Re: [PATCH v4 net-next 3/8] net: phy: bcm84881: move the in-band capability check where it belongs On Fri, Nov 25, 2022 at 01:43:34PM +0000, Russell King (Oracle) wrote: > Hi Vladimir, > > On Fri, Nov 25, 2022 at 02:30:42PM +0200, Vladimir Oltean wrote: > > Hi Russell, > > > > Sorry for the delay. Had to do something else yesterday and the day before. > > I think there was some kind of celebration going on in the US for at > least one of those days... Yeah, but I don't celebrate that. I had to write some documentation. > > So the default EXT_SR is being changed by the PHY driver from 0x9088 to > > 0x9084 (MII_M1111_HWCFG_MODE_COPPER_1000X_AN -> MII_M1111_HWCFG_MODE_SGMII_NO_CLK). > > > > I don't know if it's possible to force these modules to operate in > > 1000BASE-X mode. If you're interested in the results there, please give > > me some guidance. > > The value of "EXT_SR before" is 1000base-X, so if you change sfp-bus.c:: > sfp_select_interface() to use 1000BASEX instead of SGMII then you'll be > using 1000BASEX instead (and it should work, although at fixed 1G > speeds). The only reason the module is working in SGMII mode is because, > as you've noticed above, we switch it to SGMII mode in > m88e1111_config_init_sgmii(). > Which is an interesting thing, because m88e1111_config_init_1000basex() does not change the HWCFG_MODE_MASK to something with 1000X in it. Anyway, with sfp_select_interface() hacked, I can confirm it works in 1000BASE-X with AN enabled too. [ 69.746643] fsl_dpaa2_eth dpni.1 dpmac7: configuring for inband/sgmii link mode [ 69.845784] fsl_dpaa2_eth dpni.1 dpmac7: PHY driver reported AN inband 0x4 // PHY_AN_INBAND_ON_TIMEOUT [ 69.852764] fsl_dpaa2_eth dpni.1 dpmac7: switched to inband/1000base-x link mode // MLO_AN_INBAND [ 69.934191] Marvell 88E1111 i2c:sfp-0:16: m88e1111_config_init_1000basex: EXT_SR before 0x9088 after 0x9088, fiber page BMCR before 0x1140 after 0x1140 [ 70.015735] fsl_dpaa2_eth dpni.1 dpmac7: PHY [i2c:sfp-0:16] driver [Marvell 88E1111] (irq=POLL) ping 192.168.100.2 PING 192.168.100.2 (192.168.100.2) 56(84) bytes of data. 64 bytes from 192.168.100.2: icmp_seq=1 ttl=64 time=0.874 ms 64 bytes from 192.168.100.2: icmp_seq=2 ttl=64 time=0.225 ms 64 bytes from 192.168.100.2: icmp_seq=3 ttl=64 time=0.216 ms ^C --- 192.168.100.2 ping statistics --- 3 packets transmitted, 3 received, 0% packet loss, time 2019ms rtt min/avg/max/mdev = 0.216/0.438/0.874/0.308 ms printed with code: static int m88e1111_config_init_1000basex(struct phy_device *phydev) { int extsr = phy_read(phydev, MII_M1111_PHY_EXT_SR); int fiber_bmcr_before, fiber_bmcr_after; int ext_sr_after; int err, mode; if (extsr < 0) return extsr; fiber_bmcr_before = phy_read_paged(phydev, MII_MARVELL_FIBER_PAGE, MII_BMCR); if (fiber_bmcr_before < 0) return fiber_bmcr_before; /* If using copper mode, ensure 1000BaseX auto-negotiation is enabled. * FIXME: does this actually enable 1000BaseX auto-negotiation if it * was previously disabled in the Fiber BMCR? 2.3.1.6 suggests not! */ mode = extsr & MII_M1111_HWCFG_MODE_MASK; if (mode == MII_M1111_HWCFG_MODE_COPPER_1000X_NOAN) { err = phy_modify(phydev, MII_M1111_PHY_EXT_SR, MII_M1111_HWCFG_MODE_MASK | MII_M1111_HWCFG_SERIAL_AN_BYPASS, MII_M1111_HWCFG_MODE_COPPER_1000X_AN | MII_M1111_HWCFG_SERIAL_AN_BYPASS); if (err < 0) return err; } ext_sr_after = phy_read(phydev, MII_M1111_PHY_EXT_SR); if (ext_sr_after < 0) return ext_sr_after; fiber_bmcr_after = phy_read_paged(phydev, MII_MARVELL_FIBER_PAGE, MII_BMCR); if (fiber_bmcr_after < 0) return fiber_bmcr_after; phydev_err(phydev, "%s: EXT_SR before 0x%x after 0x%x, fiber page BMCR before 0x%x after 0x%x\n", __func__, extsr, ext_sr_after, fiber_bmcr_before, fiber_bmcr_after); return 0; } Furthermore, I can confirm that if the fiber page BMCR has BMCR_ANENABLE disabled, the link with my Lynx PCS in MLO_AN_INBAND is broken (and that the write to EXT_SR doesn't change the value of the BMCR). But there's actually a problem (or maybe two problems). First is that if I make phylink treat the ON_TIMEOUT capability by using MLO_AN_PHY (basically like this): phylink_sfp_config_phy(): /* Select whether to operate in in-band mode or not, based on the * capability of the PHY in the current link mode. */ ret = phy_validate_an_inband(phy, iface); phylink_err(pl, "PHY driver reported AN inband 0x%x\n", ret); if (ret == PHY_AN_INBAND_UNKNOWN) { mode = MLO_AN_INBAND; phylink_dbg(pl, "PHY driver does not report in-band autoneg capability, assuming true\n"); // } else if (ret & (PHY_AN_INBAND_ON | PHY_AN_INBAND_ON_TIMEOUT)) { } else if (ret & PHY_AN_INBAND_ON) { mode = MLO_AN_INBAND; } else { mode = MLO_AN_PHY; } [ 30.059923] fsl_dpaa2_eth dpni.1 dpmac7: PHY driver reported AN inband 0x4 // PHY_AN_INBAND_ON_TIMEOUT [ 30.066867] fsl_dpaa2_eth dpni.1 dpmac7: switched to phy/1000base-x link mode // MLO_AN_PHY [ 30.153350] Marvell 88E1111 i2c:sfp-0:16: m88e1111_config_init_1000basex: EXT_SR before 0x9088 after 0x9088, fiber page BMCR before 0x1140 after 0x1140 [ 30.238970] fsl_dpaa2_eth dpni.1 dpmac7: PHY [i2c:sfp-0:16] driver [Marvell 88E1111] (irq=POLL) then pinging is broken with mismatched in-band AN settings ("TIMEOUT" in PHY, "OFF" in PCS). I triple-checked this. ping 192.168.100.2 PING 192.168.100.2 (192.168.100.2) 56(84) bytes of data. >From 192.168.100.1 icmp_seq=1 Destination Host Unreachable >From 192.168.100.1 icmp_seq=2 Destination Host Unreachable >From 192.168.100.1 icmp_seq=3 Destination Host Unreachable >From 192.168.100.1 icmp_seq=4 Destination Host Unreachable >From 192.168.100.1 icmp_seq=5 Destination Host Unreachable >From 192.168.100.1 icmp_seq=6 Destination Host Unreachable ^C --- 192.168.100.2 ping statistics --- 9 packets transmitted, 0 received, +6 errors, 100% packet loss, time 8170ms However, if using the same phylink code (to force a mismatch), I unhack sfp_select_interface() and use SGMII mode, the timeout feature does actually work: [ 30.262979] fsl_dpaa2_eth dpni.1 dpmac7: PHY driver reported AN inband 0x4 // PHY_AN_INBAND_ON_TIMEOUT [ 30.270349] fsl_dpaa2_eth dpni.1 dpmac7: switched to phy/sgmii link mode // MLO_AN_PHY [ 30.351066] Marvell 88E1111 i2c:sfp-0:16: m88e1111_config_init_sgmii: EXT_SR before 0x9088 after 0x9084, fiber page BMCR before 0x1140 after 0x1140 [ 30.433236] fsl_dpaa2_eth dpni.1 dpmac7: PHY [i2c:sfp-0:16] driver [Marvell 88E1111] (irq=POLL) this is a functional link despite the mismatched settings. ping 192.168.100.2 PING 192.168.100.2 (192.168.100.2) 56(84) bytes of data. 64 bytes from 192.168.100.2: icmp_seq=1 ttl=64 time=0.885 ms 64 bytes from 192.168.100.2: icmp_seq=2 ttl=64 time=0.221 ms 64 bytes from 192.168.100.2: icmp_seq=3 ttl=64 time=0.216 ms 64 bytes from 192.168.100.2: icmp_seq=4 ttl=64 time=0.217 ms 64 bytes from 192.168.100.2: icmp_seq=5 ttl=64 time=0.238 ms ^C --- 192.168.100.2 ping statistics --- 5 packets transmitted, 5 received, 0% packet loss, time 4062ms rtt min/avg/max/mdev = 0.216/0.355/0.885/0.264 ms The second problem is that not even *matched* settings work if I turn off BMCR_ANENABLE in the PHY fiber page. [ 30.809869] fsl_dpaa2_eth dpni.1 dpmac7: configuring for inband/sgmii link mode [ 30.817936] mdio_bus 0x0000000008c1f000:00: MII_BMCR 0x1140 MII_BMSR 0x9 MII_ADVERTISE 0x1 MII_LPA 0x0 IF_MODE 0x3 // PCS registers at the end of lynx_pcs_config_giga() [ 30.917651] fsl_dpaa2_eth dpni.1 dpmac7: PHY driver reported AN inband 0x4 // ignore; m88e1111_validate_an_inband() is hardcoded for this and does not detect BMCR for BASE-X [ 30.924571] fsl_dpaa2_eth dpni.1 dpmac7: switched to phy/1000base-x link mode [ 30.932441] mdio_bus 0x0000000008c1f000:00: MII_BMCR 0x140 MII_BMSR 0xd MII_ADVERTISE 0x1 MII_LPA 0x0 IF_MODE 0x1 [ 31.032547] Marvell 88E1111 i2c:sfp-0:16: m88e1111_config_init_1000basex: EXT_SR before 0x9088 after 0x9088, fiber page BMCR before 0x140 after 0x140 [ 31.117668] fsl_dpaa2_eth dpni.1 dpmac7: PHY [i2c:sfp-0:16] driver [Marvell 88E1111] (irq=POLL) ping 192.168.100.2 PING 192.168.100.2 (192.168.100.2) 56(84) bytes of data. ^C --- 192.168.100.2 ping statistics --- 4 packets transmitted, 0 received, 100% packet loss, time 3058ms What's common is that if in-band autoneg is turned off (either forced off or via timeout), 1000BASE-X between the Lynx PCS and the 88E1111 simply doesn't work. > > I was curious if the fiber page BMCR has an effect for in-band autoneg, > > and at least for SGMII it surely does. If I add this to > > m88e1111_config_init_sgmii(): > > > > phy_modify_paged(phydev, MII_MARVELL_FIBER_PAGE, MII_BMCR, > > BMCR_ANENABLE, 0); > > > > (and force an intentional mismatch) then I am able to break the link > > with my Lynx PCS. > > Yes, the fiber page is re-used for the host side of the link when > operating in SGMII and 1000baseX modes, so changes there have the > expected effect. > > > If my hunch is correct that this also holds true for 1000BASE-X, then > > you are also correct that m88e1111_config_init_1000basex() only allows > > AN bypass but does not seem to enable in-band AN in itself, if it wasn't > > enabled. > > > > The implication here is that maybe we should test for the fiber page > > BMCR in both SGMII and 1000BASE-X modes? > > I think a more comprehensive test would be to write the fiber page > BMCR with 0x140 before changing the mode from 1000baseX to SGMII and > see whether the BMCR changes value. My suspicion is it won't, and > the hwcfg_mode only has an effect on the settings in the fiber page > under hardware reset conditions, and mode changes have no effect on > the fiber page. Confirmed that changes to the EXT_SR register don't cause changes to the MII_BMCR register: [ 28.587838] Marvell 88E1111 i2c:sfp-0:16: m88e1111_config_init_sgmii: EXT_SR before 0x9088 after 0x9084, fiber page BMCR before 0x140 after 0x140 generated by: static int m88e1111_config_init_sgmii(struct phy_device *phydev) { int fiber_bmcr_before, fiber_bmcr_after; int ext_sr_before, ext_sr_after; int err; ext_sr_before = phy_read(phydev, MII_M1111_PHY_EXT_SR); if (ext_sr_before < 0) return ext_sr_before; err = phy_modify_paged(phydev, MII_MARVELL_FIBER_PAGE, MII_BMCR, BMCR_ANENABLE, 0); if (err < 0) return err; fiber_bmcr_before = phy_read_paged(phydev, MII_MARVELL_FIBER_PAGE, MII_BMCR); if (fiber_bmcr_before < 0) return fiber_bmcr_before; err = m88e1111_config_init_hwcfg_mode( phydev, MII_M1111_HWCFG_MODE_SGMII_NO_CLK, MII_M1111_HWCFG_FIBER_COPPER_AUTO); if (err < 0) return err; ext_sr_after = phy_read(phydev, MII_M1111_PHY_EXT_SR); if (ext_sr_after < 0) return ext_sr_after; fiber_bmcr_after = phy_read_paged(phydev, MII_MARVELL_FIBER_PAGE, MII_BMCR); if (fiber_bmcr_after < 0) return fiber_bmcr_after; phydev_err(phydev, "%s: EXT_SR before 0x%x after 0x%x, fiber page BMCR before 0x%x after 0x%x\n", __func__, ext_sr_before, ext_sr_after, fiber_bmcr_before, fiber_bmcr_after); /* make sure copper is selected */ return marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); }
Powered by blists - more mailing lists