netdev - Re: mv88e6240 configuration broken for B850v3

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]

Date:   Mon, 6 Dec 2021 20:51:09 +0000
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Martyn Welch <martyn.welch@...labora.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        netdev@...r.kernel.org, kernel@...labora.com
Subject: Re: mv88e6240 configuration broken for B850v3

On Mon, Dec 06, 2021 at 10:23:08PM +0200, Vladimir Oltean wrote:
> On Mon, Dec 06, 2021 at 08:07:45PM +0000, Russell King (Oracle) wrote:
> > My conclusion from having read this thread is the CPU port is using PPU
> > polling, meaning that in mac_link_up():
> > 
> >         if ((!mv88e6xxx_phy_is_internal(ds, port) &&
> >              !mv88e6xxx_port_ppu_updates(chip, port)) ||
> >             mode == MLO_AN_FIXED) {
> > 
> > is false - because mv88e6xxx_port_ppu_updates() returns true, and
> > consequently we never undo this force-down.
> 
> We know that
> 1. A == mv88e6xxx_phy_is_internal(ds, port), B == mv88e6xxx_port_ppu_updates(chip, port), C == mode == MLO_AN_FIXED
> 2. (!A && !B) || C == false. This is due to the effect we observe: link is not forced up
> 2. C == false. This is due to the device tree.
> 3. !A && !B == false. This is due to statement (2), plus the rule that if X || Y == false and Y == false, then X must also be false.
> 4. We know that A is true, again due to device tree: port 4 < .num_internal_phys for MV88E6240 which is 5.
> 5. !A is false, due to 4.
> 
> So we have:
> 
> false && !B == false.
> 
> Therefore "!B" is "don't care". In other words we don't know whether
> mv88e6xxx_port_ppu_updates() is true or not.

With a bit of knowledge of how Marvell DSA switches work...

The "ppu" is the PHY polling unit. When the switch comes out of reset,
the PPU probes the MDIO bus, and sets the bit in the port status
register depending on whether it detects a PHY at the port address by
way of the PHY ID values. This bit is used to enable polling of the
PHY and is what mv88e6xxx_port_ppu_updates() reports. This bit will be
set for all internal PHYs unless we explicitly turn it off (we don't.)
Therefore, this is a reasonable assumption to make.

So, given that mv88e6xxx_port_ppu_updates() is most likely true as
I stated, it is also true that mv88e6xxx_phy_is_internal() is
"don't care".

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