linux-kernel - Re: [PATCH net v2] net: dpaa2-mac: Get serdes only for backplane links

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: <20230323152724.fxcxysfe637bqxzt@LXL00007.wbi.nxp.com>
Date:   Thu, 23 Mar 2023 17:27:24 +0200
From:   Ioana Ciornei <ioana.ciornei@....com>
To:     Sean Anderson <sean.anderson@...o.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org,
        Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net v2] net: dpaa2-mac: Get serdes only for backplane
 links

On Mon, Mar 06, 2023 at 11:13:17AM -0500, Sean Anderson wrote:
> On 3/6/23 03:09, Ioana Ciornei wrote:
> > On Fri, Mar 03, 2023 at 07:31:59PM -0500, Sean Anderson wrote:
> >> When commenting on what would become commit 085f1776fa03 ("net: dpaa2-mac:
> >> add backplane link mode support"), Ioana Ciornei said [1]:
> >> 
> >> > ...DPMACs in TYPE_BACKPLANE can have both their PCS and SerDes managed
> >> > by Linux (since the firmware is not touching these). That being said,
> >> > DPMACs in TYPE_PHY (the type that is already supported in dpaa2-mac) can
> >> > also have their PCS managed by Linux (no interraction from the
> >> > firmware's part with the PCS, just the SerDes).
> >> 
> >> This implies that Linux only manages the SerDes when the link type is
> >> backplane. Modify the condition in dpaa2_mac_connect to reflect this,
> >> moving the existing conditions to more appropriate places.
> > 
> > I am not sure I understand why are you moving the conditions to
> > different places. Could you please explain?
> 
> This is not (just) a movement of conditions, but a changing of what they
> apply to.
> 
> There are two things which this patch changes: whether we manage the phy
> and whether we say we support alternate interfaces. According to your
> comment above (and roughly in-line with my testing), Linux manages the
> phy *exactly* when the link type is BACKPLANE. In all other cases, the
> firmware manages the phy. Similarly, alternate interfaces are supported
> *exactly* when the firmware supports PROTOCOL_CHANGE. However, currently
> the conditions do not match this.
> 
> > Why not just append the existing condition from dpaa2_mac_connect() with
> > "mac->attr.link_type == DPMAC_LINK_TYPE_BACKPLANE"?
> > 
> > This way, the serdes_phy is populated only if all the conditions pass
> > and you don't have to scatter them all around the driver.
> 
> If we have link type BACKPLANE, Linux manages the phy, even if the
> firmware doesn't support changing the interface. Therefore, we need to
> grab the phy, but not fill in alternate interfaces.
> 
> This does not scatter the conditions, but instead moves them to exactly
> where they are needed. Currently, they are in the wrong places.

Sorry for not making my position clear from the first time which is:
there is no point in having a SerDes driver or a reference to the
SerDes PHY if the firmware does not actually support changing of
interfaces.

Why I think that is because the SerDes is configured at boot time
anyway for the interface type defined in the RCW (Reset Configuration
Word). If the firmware does not support any protocol change then why
trouble the dpaa2-eth driver with anything SerDes related?

This is why I am ok with only extending the condition from
dpaa2_mac_connect() with an additional check but not the exact patch
that you sent.

Ioana