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]

Message-ID: <Ya6FcTIbyO+zXj7V@shell.armlinux.org.uk>
Date:   Mon, 6 Dec 2021 21:49:37 +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 09:27:33PM +0000, Russell King (Oracle) wrote:
> We used to just rely on the PPU bit for making the decision, but when
> I introduced that helper, I forgot that the PPU bit doesn't exist on
> the 6250 family, which resulted in commit 4a3e0aeddf09. Looking at
> 4a3e0aeddf09, I now believe the fix there to be wrong. It should
> have made mv88e6xxx_port_ppu_updates() follow
> mv88e6xxx_phy_is_internal() for internal ports only for the 6250 family
> that has the link status bit in that position, especially as one can
> disable the PPU bit in DSA switches such as 6390, which for some ports
> stops the PHY being used and switches the port to serdes mode.
> "Internal" ports aren't always internal on these switches.

Here's the situation I'm concerned about. The 88E6390X has two serdes
each with four lanes. Let's just think about one serdes. Lane 0 is
assigned to port 9 and lane 1 to port 4. We don't need to consider
any others.

If the PHY_DETECT bit (effectively PPU poll enable) is set for port 4,
which is an "internal" port, then the port is in auto-media mode, and
the PPU will poll the internal PHY and the serdes, and configure
according to which has link.

If the PPU bit is clear, then the port is forced to serdes mode.
However, in this configuration, we end up with:

	mv88e6xxx_phy_is_internal(ds, port) = true
	mv88e6xxx_port_ppu_updates(chip, port) = false

which results in:

        if ((!mv88e6xxx_phy_is_internal(ds, port) &&
             !mv88e6xxx_port_ppu_updates(chip, port)) ||
            mode == MLO_AN_FIXED) {

being false since we have (!true && !false) || false. So, in actual
fact, when we have a PHY_DETECT bit, we _do_ need to take note of it
whether the port is "internal" or not. Essentially, that means that
for DSA switches that are not part of the 6250, we should be using
the PHY_DETECT bit.

For the 6250 family, the problem is that there's no PHY_DETECT bit,
and that's the link status. So I've started a separate discussion
with Maarten to find out which Marvell switch is being used and
whether an alterative approach would work for him.

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