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: <20230821181350.akn5mir2woj2ioke@skbuf> Date: Mon, 21 Aug 2023 21:13:50 +0300 From: Vladimir Oltean <vladimir.oltean@....com> To: Sean Anderson <sean.anderson@...o.com> Cc: netdev@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org, "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>, Heiner Kallweit <hkallweit1@...il.com>, Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, Madalin Bucur <madalin.bucur@....com>, Ioana Ciornei <ioana.ciornei@....com>, Camelia Groza <camelia.groza@....com>, Li Yang <leoyang.li@....com>, Rob Herring <robh+dt@...nel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor@...nel.org>, Maxime Chevallier <maxime.chevallier@...tlin.com>, Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.org> Subject: Re: [RFC PATCH net-next 2/8] phy: introduce the PHY_MODE_ETHERNET_PHY mode for phy_set_mode_ext() Hi Sean, On Mon, Aug 21, 2023 at 01:30:46PM -0400, Sean Anderson wrote: > On 8/17/23 11:06, Vladimir Oltean wrote: > > As opposed to PHY_MODE_ETHERNET which takes a phy_interface_t as is > > expected to be used by an Ethernet MAC driver, PHY_MODE_ETHERNET takes > > an enum ethtool_link_mode_bit_indices and expects to be used by an > > Ethernet PHY driver. > > > > It is true that the phy_interface_t type also contains definitions for > > PHY_INTERFACE_MODE_10GKR and PHY_INTERFACE_MODE_1000BASEKX, but those > > were deemed to be mistakes, and shouldn't be used going forward, when > > 10GBase-KR and 1GBase-KX are really link modes. Thus, I believe that the > > distinction is necessary, rather than hacking more improper PHY modes. > > 10GBase-KR and 1000Base-KX are both electrically (e.g. link mode) and > functionally (e.g. phy mode) different from 10GBase-R and 1000Base-X due > to differing autonegotiation. So the phy modes are still relevant, and > should still be used to ensure the correct form of autonegotiation is > selected. > > That said, I do agree that from the phy's (serdes's) point of view, > there are only electrical differences between these modes. > > However, I'm not sure we need to have a separate mode here. I think this > would only be necessary if there were electrically-incompatible modes > which shared the same signalling. E.g. if 802.3 decided that they wanted > a "long range backplane ethernet" or somesuch with different > drive/equalization requirements from 1000BASE-KX et al. but with the > same signalling. Otherwise, we can infer the link mode from the phy > mode. > > --Sean Thanks for taking the time to look at this RFC. I will ask a clarification question. When you say "I'm not sure we need to have a separate mode here", what do you mean? The lynx-28g implementation (not shown here) will need to distinguish between 1000Base-X and 1000Base-KX, and between 10GBase-R and 10GBase-KR respectively, to configure the number of electrical equalization taps in the LNmTECR registers, and to allocate memory for the ("K"-specific) link training algorithm. Also, in the particular case of BaseX vs BaseKX, we need to modify the PCCR8 register depending on whether the C22 BaseX PCS or the C45 PCS + AN/LT blocks need to be available over MDIO. So, passing PHY_INTERFACE_MODE_1000BASEX when we intend 1000Base-KX is simply not possible, because the dpaa2-mac consumer already uses PHY_INTERFACE_MODE_1000BASEX to mean a very different (and legit) thing. Do you mean instead that we could use the PHY_INTERFACE_MODE_1000BASEKX that you've added to phy_interface_t? It's not clear that this is what you're suggesting, so feel free to stop reading here if it isn't. But mtip_backplane uses linkmode_c73_priority_resolution() (a function added by me, sure, but nonetheless, it operates in the linkmode namespace, as a PHY driver helper should) to figure out the proper argument to pass to phy_set_mode_ext(). That argument has the enum ethtool_link_mode_bit_indices. So, a translation between enum ethtool_link_mode_bit_indices and phy_interface_t would be needed. That would be more or less doable for 1000Base-KX and 10GBase-KR, but it needs more phy_interface_t additions for: static const enum ethtool_link_mode_bit_indices c73_linkmodes[] = { ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT, ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT, /* ETHTOOL_LINK_MODE_100000baseKP4_Full_BIT not supported */ /* ETHTOOL_LINK_MODE_100000baseCR10_Full_BIT not supported */ ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT, ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT, ETHTOOL_LINK_MODE_25000baseKR_Full_BIT, ETHTOOL_LINK_MODE_25000baseCR_Full_BIT, /* ETHTOOL_LINK_MODE_25000baseKRS_Full_BIT not supported */ /* ETHTOOL_LINK_MODE_25000baseCRS_Full_BIT not supported */ ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT, ETHTOOL_LINK_MODE_1000baseKX_Full_BIT, }; I guess that network PHY maintainers will need to chime in and say whether that's the path forward or not.
Powered by blists - more mailing lists