linux-kernel - Re: [PATCH RFC net-next 4/5] net: phy: introduce is_c45_over

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: <YkWVku32gbaBAdEH@shell.armlinux.org.uk>
Date:   Thu, 31 Mar 2022 12:50:42 +0100
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Michael Walle <michael@...le.cc>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        "David S . Miller" <davem@...emloft.net>,
        Xu Liang <lxu@...linear.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC net-next 4/5] net: phy: introduce is_c45_over_c22 flag

On Thu, Mar 24, 2022 at 07:55:28PM +0100, Andrew Lunn wrote:
> > The
> > > only valid case i can think of is for a very oddball PHY which has C45
> > > register space, but cannot actually do C45 transfers, and so C45 over
> > > C22 is the only option.
> > 
> > And how would you know that the PHY has the needed registers in c22
> > space? Or do we assume that every C45 PHY has these registers?
> 
> I think it is a reasonable assumption at the moment. We have around
> 170 MDIO bus masters in Linux. All but one can do C22.

I don't think that is correct. I'm aware of the Marvell XMDIO driver
that is C45 only, and also xgene's non-rgmii "xfi" variant which is
also C45 only. Note that the xfi variant doesn't reject C22 and makes
no distinction between a C22 and C45 access (so a C22 access to
phy_id = 0 reg = 0 hits C45 phy_id = 0 mmd 0 reg 0.

MDIO drivers are IMHO an utter mess and are in dire need of fixing...
and I'm coming to the conclusion that the bodge of passing both C22
and C45 accesses through the same read/write functions is a huge
mistake, one that is crying out for fixing to prevent more prolification
of this kind of mess.

Yes, it's a lot of work, but I think it needs to be done. Retrofitting
the MDIO drivers with checks etc sounds nice, but if we assume that
patches will continue to be applied to net-next with little review,
we have a losing battle - it would be better to have interfaces designed
to make this kind of mistake impossible.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!