netdev - Re: [PATCH net-next 0/5] net: phy: C45-over-C22 access

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: <Y8834YKbZAoLyMQJ@lunn.ch>
Date:   Tue, 24 Jan 2023 02:44:01 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Michael Walle <michael@...le.cc>
Cc:     "Russell King (Oracle)" <linux@...linux.org.uk>,
        Yisen Zhuang <yisen.zhuang@...wei.com>,
        Salil Mehta <salil.mehta@...wei.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@...adcom.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Marek Behún <kabel@...nel.org>,
        Xu Liang <lxu@...linear.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 0/5] net: phy: C45-over-C22 access

On Tue, Jan 24, 2023 at 01:35:48AM +0100, Michael Walle wrote:
> >       - const: ethernet-phy-ieee802.3-c45
> >         description: PHYs that implement IEEE802.3 clause 45
> > 
> > But it is not clear what that actually means. Does it mean it has c45
> > registers, or does it mean it supports C45 bus transactions?
> 
> PHYs which support C45 access but don't have C45 registers aren't
> a thing I presume - or doesn't make any sense, right?
> 
> PHYs which have C45 registers but don't support C45 access exists.
> e.g. the EEE registers of C22 PHYs. But they are C22 PHYs.

I wonder if there are any C22 PHYs which only allow access to EEE via
C45 transactions. That would be pretty broken...

To some extent, i'm still looking at everything and trying to decide
if the current definitions/documentation make it clear if means C45
transfers or registers. And the documentation in DT is ambiguous, but
as you point out, it probably means registers, not transactions.

> So I'd say if you have compatible = "ethernet-phy-ieee802.3-c45",
> it is a PHY with C45 registers _and_ which are accessible by
> C45 transactions. But they might or might not support C22 access.
> But I think thats pretty irrelevant because you always do C45 if
> you can. You cannot do C45 if:
>  (1) the mdio bus doesn't support C45
>  (2) you have a broken C22 phy on the mdio bus
> 
> In both cases, if the PHY doesn't support C22-over-C45 you are
> screwed. Therefore, if you have either (1) or (2) we should always
> fall back to C22-over-C45.
> 
> > If we have that compatible, we could probe first using C45 and if that
> > fails, or is not supported by the bus master, probe using C45 over
> > C22. That seems safe. For Michael use case, the results of
> > mdiobus_prevent_c45_scan(bus) needs keeping as a property of bus, so
> > we know not to perform the C45 scan, and go direct to C45 over C22.
> 
> So you are talking about DT, in which case there is no auto probing.
> See phy_mask in the dt code. Only PHYs in the device tree are probed.
> (unless of course there is no reg property.. then there is some
> obscure auto scanning). So if you want a C45 PHY you'd have to
> have that compatible in any case.
> 
> Btw. I still don't know how you can get a C45 PHY instance without
> a device tree, except if there is a C45 only bus or the PHY doesn't
> respond on C22 ids. Maybe I'm missing something here..

In the DT case, you are probably correct. But there are a number of
MDIO busses which don't come from DT. Those are typically PCIe or USB
devices. Those do get scanned, and my recent changes should mean they
first get scanned using C22 and then C45. DSA switches also typically
don't have a MDIO node in DT, it is assumed there is a 1:1 mapping
between port number and address on the MDIO bus. But as you said, it
would require that they don't respond to C22, or the bus master does
not support C22, which does actually exist from Marvell at least.

In the none DT case, we probably cannot easily do anything about
C22-over-C45, because as Russell pointed out, it is potentially a
destructive process doing a scan. We want some indication we do expect
a PHY to be there. And "ethernet-phy-ieee802.3-c45" would do that.

	Andrew