netdev - RE: [PATCH v3] net: macb: add support for high speed interface

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:   Thu, 22 Oct 2020 10:29:43 +0000
From:   Parshuram Raju Thombare <pthombar@...ence.com>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
CC:     "andrew@...n.ch" <andrew@...n.ch>,
        "nicolas.ferre@...rochip.com" <nicolas.ferre@...rochip.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Milind Parab <mparab@...ence.com>
Subject: RE: [PATCH v3] net: macb: add support for high speed interface

>If you're going to support pcs_ops for the 10GBASE-R mode, can you
>also convert macb to use pcs_ops for the lower speed modes as well?

Ok, I will add pcs_ops for low speed as well. In fact, having single
pcs_ops and using check for interface type within each method of
pcs_ops to decide appropriate action will also solve the below mentioned
issue when phylink requests 10GBASE-R and subsequently selects a
different interface mode.

Since macb_mac_an_restart  is empty, macb_mac_pcs_get_state only
sets "state->link = 0" and there is nothing to be done in pcs_config, there will
be no changes in pcs_ops methods pcs_get_state, pcs_config and pcs_link_up,
except guarding current code in these methods by
interface == PHY_INTERFACE_MODE_10GBASER check.

pcs_config and pcs_link_up passes "interface" as an argument, and in
pcs_get_state call "state->interface" appeared to be populated just before
calling it and hence should be valid.

 528         state->interface = pl->link_config.interface;
 ...
 535
 536         if (pl->pcs_ops)
 537                 pl->pcs_ops->pcs_get_state(pl->pcs, state);
 538         else if (pl->mac_ops->mac_pcs_get_state)
 539                 pl->mac_ops->mac_pcs_get_state(pl->config, state);
 540         else
 541                 state->link = 0;

>> +	val = gem_readl(bp, NCFGR);
>> +	val = GEM_BIT(PCSSEL) | (~GEM_BIT(SGMIIEN) & val);
>> +	gem_writel(bp, NCFGR, val);
>
>This looks like it's configuring the MAC rather than the PCS - it
>should probably be in mac_prepare() or mac_config().

Ok

>What happens if phylink requests 10GBASE-R and subsequently selects
>a different interface mode? You end up with the PCS still registered
>and phylink will use it even when not in 10GBASE-R mode - so its
>functions will also be called.