Marvell mvpp2 improvements

This series primarily cleans up mvpp2, but also fixes a left-over from 91a208f2185a ("net: phylink: propagate resolved link config via mac_link_up()").

Patch 1 introduces some port helpers:
mvpp2_port_supports_xlg() - does the port support the XLG MAC mvpp2_port_supports_rgmii() - does the port support RGMII modes

Patch 2 introduces mvpp2_phylink_to_port(), rather than having repeated
open coding of container_of().

Patch 3 introduces mvpp2_modify(), which reads-modifies-writes a
register - I've converted the phylink specific code to use this helper.

Patch 4 moves the hardware control of the pause modes from
mvpp2_xlg_config() (which is called via the phylink_config method) to mvpp2_mac_link_up() - a change that was missed in the above referenced commit.

v2: remove "inline" in patch 2.

Add a helper to convert the struct phylink_config pointer passed in from phylink to the drivers internal struct mvpp2_port.

Signed-off-by Russell King <[email protected]>

---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
 1 file changed, 14 insertions(+), 15 deletions(-)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +14 -16

index 7653277d03b7..313f5a60a605 100644
@@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,
                                   struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
        /* Invalid combinations */

@@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
 static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
                                            struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 
        if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
                u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);

@@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
 
 static void mvpp2_mac_an_restart(struct phylink_config *config)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 
        writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,

@@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }
 

@@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (mvpp2_is_xlg(interface)) {

@@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 
 static void mvpp2_mac_link_down(struct phylink_config *config,
                                unsigned int mode, phy_interface_t interface)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (!phylink_autoneg_inband(mode)) {

@@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
                }
        }
 
-       netif_tx_stop_all_queues(dev);
+       netif_tx_stop_all_queues(port->dev);
        mvpp2_egress_disable(port);
        mvpp2_ingress_disable(port);
 
--

2.20.1

Hi,

sob., 20 cze 2020 o 11:21 Russell King <[email protected]> napisał(a):

Quoted 105 lines

Add a helper to convert the struct phylink_config pointer passed in from phylink to the drivers internal struct mvpp2_port.

Signed-off-by Russell King <[email protected]>

---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
 1 file changed, 14 insertions(+), 15 deletions(-)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +14 -16

index 7653277d03b7..313f5a60a605 100644
@@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,
                                   struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
        /* Invalid combinations */

@@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
 static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
                                            struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 
        if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
                u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);

@@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
 
 static void mvpp2_mac_an_restart(struct phylink_config *config)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 
        writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,

@@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }
 

@@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (mvpp2_is_xlg(interface)) {

@@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 
 static void mvpp2_mac_link_down(struct phylink_config *config,
                                unsigned int mode, phy_interface_t interface)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (!phylink_autoneg_inband(mode)) {

@@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
                }
        }
 
-       netif_tx_stop_all_queues(dev);
+       netif_tx_stop_all_queues(port->dev);
        mvpp2_egress_disable(port);
        mvpp2_ingress_disable(port);
 
--

Quoted 2 lines

2.20.1

This patch fixes a regression that was introduced in v5.3:
Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")

Above results in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be problematic especially for the distros using LTSv5.4 and above (the issue was reported on Fedora 32).

Please help with backporting to the stable v5.3+ branches (it applies smoothly on v5.4/v5.6/v5.8).

Hi Greg and Sasha,

pt., 9 paź 2020 o 05:43 Marcin Wojtas <[email protected]> napisał(a):

Quoted 120 lines

Hi,

sob., 20 cze 2020 o 11:21 Russell King <[email protected]> napisał(a):

Quoted 105 lines

Add a helper to convert the struct phylink_config pointer passed in from phylink to the drivers internal struct mvpp2_port.

Signed-off-by Russell King <[email protected]>

---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
 1 file changed, 14 insertions(+), 15 deletions(-)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +14 -16

index 7653277d03b7..313f5a60a605 100644
@@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,
                                   struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
        /* Invalid combinations */

@@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
 static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
                                            struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 
        if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
                u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);

@@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
 
 static void mvpp2_mac_an_restart(struct phylink_config *config)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 
        writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,

@@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }
 

@@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (mvpp2_is_xlg(interface)) {

@@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 
 static void mvpp2_mac_link_down(struct phylink_config *config,
                                unsigned int mode, phy_interface_t interface)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (!phylink_autoneg_inband(mode)) {

@@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
                }
        }
 
-       netif_tx_stop_all_queues(dev);
+       netif_tx_stop_all_queues(port->dev);
        mvpp2_egress_disable(port);
        mvpp2_ingress_disable(port);
 
--

Quoted 2 lines

2.20.1

This patch fixes a regression that was introduced in v5.3:
Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")

Above results in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be problematic especially for the distros using LTSv5.4 and above (the issue was reported on Fedora 32).

Quoted 3 lines

Please help with backporting to the stable v5.3+ branches (it applies smoothly on v5.4/v5.6/v5.8).

Any chances to backport this patch to relevant v5.3+ stable branches?

Hi Greg and Sasha,

pt., 9 paź 2020 o 05:43 Marcin Wojtas <[email protected]> napisał(a):

Quoted 120 lines

Hi,

sob., 20 cze 2020 o 11:21 Russell King <[email protected]> napisał(a):

Quoted 105 lines

Add a helper to convert the struct phylink_config pointer passed in from phylink to the drivers internal struct mvpp2_port.

Signed-off-by Russell King <[email protected]>

---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
 1 file changed, 14 insertions(+), 15 deletions(-)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +14 -16

index 7653277d03b7..313f5a60a605 100644
@@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,
                                   struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
        /* Invalid combinations */

@@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
 static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
                                            struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 
        if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
                u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);

@@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
 
 static void mvpp2_mac_an_restart(struct phylink_config *config)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 
        writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,

@@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }
 

@@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (mvpp2_is_xlg(interface)) {

@@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 
 static void mvpp2_mac_link_down(struct phylink_config *config,
                                unsigned int mode, phy_interface_t interface)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (!phylink_autoneg_inband(mode)) {

@@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
                }
        }
 
-       netif_tx_stop_all_queues(dev);
+       netif_tx_stop_all_queues(port->dev);
        mvpp2_egress_disable(port);
        mvpp2_ingress_disable(port);
 
--

Quoted 2 lines

2.20.1

This patch fixes a regression that was introduced in v5.3:
Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")

Above results in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be problematic especially for the distros using LTSv5.4 and above (the issue was reported on Fedora 32).

Quoted 2 lines

Please help with backporting to the stable v5.3+ branches (it applies smoothly on v5.4/v5.6/v5.8).

Any chances to backport this patch to relevant v5.3+ stable branches?

Quoted 123 lines

Hi Greg and Sasha,

pt., 9 paź 2020 o 05:43 Marcin Wojtas <[email protected]> napisał(a):

Quoted 120 lines

Hi,

sob., 20 cze 2020 o 11:21 Russell King <[email protected]> napisał(a):

Quoted 105 lines

Add a helper to convert the struct phylink_config pointer passed in from phylink to the drivers internal struct mvpp2_port.

Signed-off-by Russell King <[email protected]>

---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
 1 file changed, 14 insertions(+), 15 deletions(-)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +14 -16

index 7653277d03b7..313f5a60a605 100644
@@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,
                                   struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
        /* Invalid combinations */

@@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
 static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
                                            struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 
        if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
                u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);

@@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
 
 static void mvpp2_mac_an_restart(struct phylink_config *config)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 
        writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,

@@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }
 

@@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (mvpp2_is_xlg(interface)) {

@@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 
 static void mvpp2_mac_link_down(struct phylink_config *config,
                                unsigned int mode, phy_interface_t interface)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (!phylink_autoneg_inband(mode)) {

@@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
                }
        }
 
-       netif_tx_stop_all_queues(dev);
+       netif_tx_stop_all_queues(port->dev);
        mvpp2_egress_disable(port);
        mvpp2_ingress_disable(port);
 
--

Quoted 2 lines

2.20.1

This patch fixes a regression that was introduced in v5.3:
Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")

Above results in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be problematic especially for the distros using LTSv5.4 and above (the issue was reported on Fedora 32).

Quoted 5 lines

Quoted 3 lines

Please help with backporting to the stable v5.3+ branches (it applies smoothly on v5.4/v5.6/v5.8).

Any chances to backport this patch to relevant v5.3+ stable branches?

What patch? What git commit id needs to be backported?

Hi Greg,

Apologies for delayed response:.

pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman
<[email protected]> napisał(a):

Quoted 125 lines · Marcin Wojtas

Quoted 123 lines

Hi Greg and Sasha,

pt., 9 paź 2020 o 05:43 Marcin Wojtas <[email protected]> napisał(a):

Quoted 120 lines

Hi,

sob., 20 cze 2020 o 11:21 Russell King <[email protected]> napisał(a):

Quoted 105 lines

Add a helper to convert the struct phylink_config pointer passed in from phylink to the drivers internal struct mvpp2_port.

Signed-off-by Russell King <[email protected]>

---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
 1 file changed, 14 insertions(+), 15 deletions(-)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +14 -16

index 7653277d03b7..313f5a60a605 100644
@@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,
                                   struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
        /* Invalid combinations */

@@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
 static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
                                            struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 
        if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
                u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);

@@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
 
 static void mvpp2_mac_an_restart(struct phylink_config *config)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 
        writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,

@@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }
 

@@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (mvpp2_is_xlg(interface)) {

@@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 
 static void mvpp2_mac_link_down(struct phylink_config *config,
                                unsigned int mode, phy_interface_t interface)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (!phylink_autoneg_inband(mode)) {

@@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
                }
        }
 
-       netif_tx_stop_all_queues(dev);
+       netif_tx_stop_all_queues(port->dev);
        mvpp2_egress_disable(port);
        mvpp2_ingress_disable(port);
 
--

Quoted 2 lines

2.20.1

This patch fixes a regression that was introduced in v5.3:
Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")

Above results in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be problematic especially for the distros using LTSv5.4 and above (the issue was reported on Fedora 32).

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

Please help with backporting to the stable v5.3+ branches (it applies smoothly on v5.4/v5.6/v5.8).

Any chances to backport this patch to relevant v5.3+ stable branches?

What patch? What git commit id needs to be backported?

The actual patch is:
Commit 6c2b49eb9671 ("net: mvpp2: add mvpp2_phylink_to_port() helper").

URL for reference:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/mvpp2?h=v5.10-rc7&id=6c2b49eb96716e91f202756bfbd3f5fea3b2b172

Do you think it would be possible to get it merged to v5.3+ stable branches?

Quoted 144 lines

Hi Greg,

Apologies for delayed response:.

pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman
<[email protected]> napisał(a):

Quoted 125 lines · Marcin Wojtas

Quoted 123 lines

Hi Greg and Sasha,

pt., 9 paź 2020 o 05:43 Marcin Wojtas <[email protected]> napisał(a):

Quoted 120 lines

Hi,

sob., 20 cze 2020 o 11:21 Russell King <[email protected]> napisał(a):

Quoted 105 lines

Add a helper to convert the struct phylink_config pointer passed in from phylink to the drivers internal struct mvpp2_port.

Signed-off-by Russell King <[email protected]>

---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
 1 file changed, 14 insertions(+), 15 deletions(-)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +14 -16

index 7653277d03b7..313f5a60a605 100644
@@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,
                                   struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
        /* Invalid combinations */

@@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
 static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
                                            struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 
        if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
                u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);

@@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
 
 static void mvpp2_mac_an_restart(struct phylink_config *config)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 
        writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,

@@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }
 

@@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (mvpp2_is_xlg(interface)) {

@@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 
 static void mvpp2_mac_link_down(struct phylink_config *config,
                                unsigned int mode, phy_interface_t interface)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (!phylink_autoneg_inband(mode)) {

@@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
                }
        }
 
-       netif_tx_stop_all_queues(dev);
+       netif_tx_stop_all_queues(port->dev);
        mvpp2_egress_disable(port);
        mvpp2_ingress_disable(port);
 
--

Quoted 2 lines

2.20.1

This patch fixes a regression that was introduced in v5.3:
Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")

Above results in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be problematic especially for the distros using LTSv5.4 and above (the issue was reported on Fedora 32).

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

Please help with backporting to the stable v5.3+ branches (it applies smoothly on v5.4/v5.6/v5.8).

Any chances to backport this patch to relevant v5.3+ stable branches?

What patch? What git commit id needs to be backported?

The actual patch is:
Commit 6c2b49eb9671 ("net: mvpp2: add mvpp2_phylink_to_port() helper").

Quoted 4 lines

URL for reference:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/mvpp2?h=v5.10-rc7&id=6c2b49eb96716e91f202756bfbd3f5fea3b2b172

Do you think it would be possible to get it merged to v5.3+ stable branches?

Could you explain how that patch fixes anything? It reads like a cleanup.

Hi Sasha,

wt., 8 gru 2020 o 14:35 Sasha Levin <[email protected]> napisał(a):

Quoted 146 lines · Marcin Wojtas

Quoted 144 lines

Hi Greg,

Apologies for delayed response:.

pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman
<[email protected]> napisał(a):

Quoted 125 lines · Marcin Wojtas

Quoted 123 lines

Hi Greg and Sasha,

pt., 9 paź 2020 o 05:43 Marcin Wojtas <[email protected]> napisał(a):

Quoted 120 lines

Hi,

sob., 20 cze 2020 o 11:21 Russell King <[email protected]> napisał(a):

Quoted 105 lines

Add a helper to convert the struct phylink_config pointer passed in from phylink to the drivers internal struct mvpp2_port.

Signed-off-by Russell King <[email protected]>

---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
 1 file changed, 14 insertions(+), 15 deletions(-)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +14 -16

index 7653277d03b7..313f5a60a605 100644
@@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,
                                   struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
        /* Invalid combinations */

@@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
 static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
                                            struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 
        if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
                u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);

@@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
 
 static void mvpp2_mac_an_restart(struct phylink_config *config)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 
        writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,

@@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }
 

@@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (mvpp2_is_xlg(interface)) {

@@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 
 static void mvpp2_mac_link_down(struct phylink_config *config,
                                unsigned int mode, phy_interface_t interface)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (!phylink_autoneg_inband(mode)) {

@@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
                }
        }
 
-       netif_tx_stop_all_queues(dev);
+       netif_tx_stop_all_queues(port->dev);
        mvpp2_egress_disable(port);
        mvpp2_ingress_disable(port);
 
--

Quoted 2 lines

2.20.1

This patch fixes a regression that was introduced in v5.3:
Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")

Above results in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be problematic especially for the distros using LTSv5.4 and above (the issue was reported on Fedora 32).

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

Please help with backporting to the stable v5.3+ branches (it applies smoothly on v5.4/v5.6/v5.8).

Any chances to backport this patch to relevant v5.3+ stable branches?

What patch? What git commit id needs to be backported?

The actual patch is:
Commit 6c2b49eb9671 ("net: mvpp2: add mvpp2_phylink_to_port() helper").

Quoted 8 lines

Quoted 4 lines

URL for reference:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/mvpp2?h=v5.10-rc7&id=6c2b49eb96716e91f202756bfbd3f5fea3b2b172

Do you think it would be possible to get it merged to v5.3+ stable branches?

Could you explain how that patch fixes anything? It reads like a cleanup.

Indeed, I am aware of it, but I'm not sure about the best way to fix it. In fact the mentioned patch is an unintentional fix. Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API") reworked an argument list of mvpp2_mac_config() routine in a way that resulted in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8. Part of Russell's patch resolves this issue.

Hi Greg,

śr., 9 gru 2020 o 11:59 Greg Kroah-Hartman
<[email protected]> napisał(a):

Quoted 1 line

What part fixes the issue? I can't see it...

I re-checked in my setup and here's the smallest part of the original patch, that fixes previously described issue:

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +5 -0

index e98be8372780..9d71a4fe1750 100644
@@ -4767,6 +4767,11 @@ static void mvpp2_port_copy_mac_addr(struct
 net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,

struct phylink_link_state *state)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +3 -5

@@ -5105,13 +5110,12 @@ static void mvpp2_gmac_config(struct
 mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }

@@ -5151,8 +5155,7 @@ static void mvpp2_mac_link_up(struct
 phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 

        if (mvpp2_is_xlg(interface)) {

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +1 -1

@@ -5199,7 +5202,7 @@ static void mvpp2_mac_link_up(struct
 phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 

static void mvpp2_mac_link_down(struct phylink_config *config,

Quoted 158 lines

Hi Sasha,

wt., 8 gru 2020 o 14:35 Sasha Levin <[email protected]> napisał(a):

Quoted 146 lines · Marcin Wojtas

Quoted 144 lines

Hi Greg,

Apologies for delayed response:.

pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman
<[email protected]> napisał(a):

Quoted 125 lines · Marcin Wojtas

Quoted 123 lines

Hi Greg and Sasha,

pt., 9 paź 2020 o 05:43 Marcin Wojtas <[email protected]> napisał(a):

Quoted 120 lines

Hi,

sob., 20 cze 2020 o 11:21 Russell King <[email protected]> napisał(a):

Quoted 105 lines

Add a helper to convert the struct phylink_config pointer passed in from phylink to the drivers internal struct mvpp2_port.

Signed-off-by Russell King <[email protected]>

---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
 1 file changed, 14 insertions(+), 15 deletions(-)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +14 -16

index 7653277d03b7..313f5a60a605 100644
@@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,
                                   struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
        /* Invalid combinations */

@@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
 static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
                                            struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 
        if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
                u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);

@@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
 
 static void mvpp2_mac_an_restart(struct phylink_config *config)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 
        writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,

@@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }
 

@@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (mvpp2_is_xlg(interface)) {

@@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 
 static void mvpp2_mac_link_down(struct phylink_config *config,
                                unsigned int mode, phy_interface_t interface)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (!phylink_autoneg_inband(mode)) {

@@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
                }
        }
 
-       netif_tx_stop_all_queues(dev);
+       netif_tx_stop_all_queues(port->dev);
        mvpp2_egress_disable(port);
        mvpp2_ingress_disable(port);
 
--

Quoted 2 lines

2.20.1

This patch fixes a regression that was introduced in v5.3:
Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")

Above results in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be problematic especially for the distros using LTSv5.4 and above (the issue was reported on Fedora 32).

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

Please help with backporting to the stable v5.3+ branches (it applies smoothly on v5.4/v5.6/v5.8).

Any chances to backport this patch to relevant v5.3+ stable branches?

What patch? What git commit id needs to be backported?

The actual patch is:
Commit 6c2b49eb9671 ("net: mvpp2: add mvpp2_phylink_to_port() helper").

Quoted 8 lines

Quoted 4 lines

URL for reference:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/mvpp2?h=v5.10-rc7&id=6c2b49eb96716e91f202756bfbd3f5fea3b2b172

Do you think it would be possible to get it merged to v5.3+ stable branches?

Could you explain how that patch fixes anything? It reads like a cleanup.

Quoted 7 lines

Indeed, I am aware of it, but I'm not sure about the best way to fix it. In fact the mentioned patch is an unintentional fix. Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API") reworked an argument list of mvpp2_mac_config() routine in a way that resulted in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8. Part of Russell's patch resolves this issue.

What part fixes the issue? I can't see it...

Quoted 63 lines

Hi Greg,

śr., 9 gru 2020 o 11:59 Greg Kroah-Hartman
<[email protected]> napisał(a):

Quoted 1 line

What part fixes the issue? I can't see it...

I re-checked in my setup and here's the smallest part of the original patch, that fixes previously described issue:

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +5 -0

index e98be8372780..9d71a4fe1750 100644
@@ -4767,6 +4767,11 @@ static void mvpp2_port_copy_mac_addr(struct
 net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,

struct phylink_link_state *state)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +3 -5

@@ -5105,13 +5110,12 @@ static void mvpp2_gmac_config(struct
 mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }

@@ -5151,8 +5155,7 @@ static void mvpp2_mac_link_up(struct
 phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 

        if (mvpp2_is_xlg(interface)) {

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +1 -1

@@ -5199,7 +5202,7 @@ static void mvpp2_mac_link_up(struct
 phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 

Quoted 1 line

static void mvpp2_mac_link_down(struct phylink_config *config,

The problem is caused by this hack:

                /* Phylink isn't used as of now for ACPI, so the MAC has to be
                 * configured manually when the interface is started. This will
                 * be removed as soon as the phylink ACPI support lands in.
                 */
                struct phylink_link_state state = {
                        .interface = port->phy_interface,
                };
                mvpp2_mac_config(&port->phylink_config, MLO_AN_INBAND, &state);
                mvpp2_mac_link_up(&port->phylink_config, MLO_AN_INBAND,
                                  port->phy_interface, NULL);

which passes an un-initialised (zeroed) port->phylink_config, as phylink is not used in ACPI setups.

The problem occurs because port->phylink_config.dev (which is a NULL pointer in this instance) is passed to to_net_dev():

#define to_net_dev(d) container_of(d, struct net_device, dev)

Which then means netdev_priv(dev) attempts to dereference a not-quite NULL pointer, leading to an oops.

The problem here is that the bug was not noticed; it seems hardly anyone bothers to run mainline kernels with ACPI on Marvell platforms, or if they do, they don't bother reporting to mainline communities when they have problems. Likely, there's posts on some random web-based bulletin board or mailing list that kernel developers don't read somewhere complaining that there's an oops.

Like...

https://lists.einval.com/pipermail/macchiato/2020-January/000309.html
https://gist.github.com/AdrianKoshka/ff9862da2183a2d8e26d47baf8dc04b9

This kind of segmentation is very disappointing; it means potentially lots of bugs go by unnoticed by kernel developers, and bugs only get fixed by chance. Had it been reported to somewhere known earlier this year, it is likely that a proper fix patch would have been created.

How this gets handled is ultimately up to the stable developers to decide what they wish to accept. Do they wish to accept a back-ported full version of my commit 6c2b49eb9671 ("net: mvpp2: add mvpp2_phylink_to_port() helper") that unintentionally fixed this unknown issue, or do they want a more minimal fix such as the cut-down version of that commit that Marcin has supplied.

Until something changes in the way bugs get reported, I suspect this won't be the only instance of bug-fixing-by-accident happening.

I must admit that due to other duties I did not follow the mainline mvpp2 for a couple revisions (and I am not maintainer of it). However recently I got reached-out directly by different developers - the trigger was different distros upgrading the kernel above v5.4+ and for some reasons the DT path is not chosen there (and the ACPI will be chosen more and more in the SystemReady world).

Quoted 6 lines

I must admit that due to other duties I did not follow the mainline mvpp2 for a couple revisions (and I am not maintainer of it). However recently I got reached-out directly by different developers - the trigger was different distros upgrading the kernel above v5.4+ and for some reasons the DT path is not chosen there (and the ACPI will be chosen more and more in the SystemReady world).

Please note that there is no active maintainer for mvpp2.

It will be good to get rid of the ACPI hack here, as that means we'll be using the same code paths for both ACPI and DT, meaning hopefully less bugs like this go unnoticed.

+1. As soon as the MDIO+ACPI lands, I plan to do the rework.

Quoted 2 lines

+1. As soon as the MDIO+ACPI lands, I plan to do the rework.

Don't hold you breath. It has gone very quiet about ACPI in net devices.

Quoted 2 lines

+1. As soon as the MDIO+ACPI lands, I plan to do the rework.

Don't hold you breath. It has gone very quiet about ACPI in net devices.

Andrew

Quoted 5 lines · Andrew Lunn

Quoted 2 lines

+1. As soon as the MDIO+ACPI lands, I plan to do the rework.

Don't hold you breath. It has gone very quiet about ACPI in net devices.

NXP resources were re-allocated for their next internal BSP release. I have been working with Calvin over the past week and a half and the new patchset will be submitted early next week most likely.

Hi Greg,

śr., 9 gru 2020 o 11:59 Greg Kroah-Hartman
<[email protected]> napisał(a):

Quoted 168 lines · Marcin Wojtas

Quoted 158 lines

Hi Sasha,

wt., 8 gru 2020 o 14:35 Sasha Levin <[email protected]> napisał(a):

Quoted 146 lines · Marcin Wojtas

Quoted 144 lines

Hi Greg,

Apologies for delayed response:.

pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman
<[email protected]> napisał(a):

Quoted 125 lines · Marcin Wojtas

Quoted 123 lines

Hi Greg and Sasha,

pt., 9 paź 2020 o 05:43 Marcin Wojtas <[email protected]> napisał(a):

Quoted 120 lines

Hi,

sob., 20 cze 2020 o 11:21 Russell King <[email protected]> napisał(a):

Quoted 105 lines

Add a helper to convert the struct phylink_config pointer passed in from phylink to the drivers internal struct mvpp2_port.

Signed-off-by Russell King <[email protected]>

---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
 1 file changed, 14 insertions(+), 15 deletions(-)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +14 -16

index 7653277d03b7..313f5a60a605 100644
@@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,
                                   struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
        /* Invalid combinations */

@@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
 static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
                                            struct phylink_link_state *state)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 
        if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
                u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);

@@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
 
 static void mvpp2_mac_an_restart(struct phylink_config *config)
 {
-       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-                                              phylink_config);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 
        writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,

@@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }
 

@@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (mvpp2_is_xlg(interface)) {

@@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 
 static void mvpp2_mac_link_down(struct phylink_config *config,
                                unsigned int mode, phy_interface_t interface)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 
        if (!phylink_autoneg_inband(mode)) {

@@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
                }
        }
 
-       netif_tx_stop_all_queues(dev);
+       netif_tx_stop_all_queues(port->dev);
        mvpp2_egress_disable(port);
        mvpp2_ingress_disable(port);
 
--

Quoted 2 lines

2.20.1

This patch fixes a regression that was introduced in v5.3:
Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")

Above results in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be problematic especially for the distros using LTSv5.4 and above (the issue was reported on Fedora 32).

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

Please help with backporting to the stable v5.3+ branches (it applies smoothly on v5.4/v5.6/v5.8).

Any chances to backport this patch to relevant v5.3+ stable branches?

What patch? What git commit id needs to be backported?

The actual patch is:
Commit 6c2b49eb9671 ("net: mvpp2: add mvpp2_phylink_to_port() helper").

Quoted 8 lines

Quoted 4 lines

URL for reference:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/mvpp2?h=v5.10-rc7&id=6c2b49eb96716e91f202756bfbd3f5fea3b2b172

Do you think it would be possible to get it merged to v5.3+ stable branches?

Could you explain how that patch fixes anything? It reads like a cleanup.

Quoted 7 lines

Indeed, I am aware of it, but I'm not sure about the best way to fix it. In fact the mentioned patch is an unintentional fix. Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API") reworked an argument list of mvpp2_mac_config() routine in a way that resulted in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8. Part of Russell's patch resolves this issue.

Quoted 2 lines

What part fixes the issue? I can't see it...

I re-checked in my setup and here's the smallest part of the original patch, that fixes previously described issue:

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +5 -0

index e98be8372780..9d71a4fe1750 100644
@@ -4767,6 +4767,11 @@ static void mvpp2_port_copy_mac_addr(struct
 net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,

struct phylink_link_state *state)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +3 -5

@@ -5105,13 +5110,12 @@ static void mvpp2_gmac_config(struct
 mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }

@@ -5151,8 +5155,7 @@ static void mvpp2_mac_link_up(struct
 phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 

        if (mvpp2_is_xlg(interface)) {

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +1 -1

@@ -5199,7 +5202,7 @@ static void mvpp2_mac_link_up(struct
 phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 

static void mvpp2_mac_link_down(struct phylink_config *config,
--

Do you think there is a point of slicing the original patch or rather cherry-pick as-is?

Quoted 97 lines

Quoted 37 lines

Quoted 29 lines

Quoted 20 lines

Quoted 20 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

This patch fixes a regression that was introduced in v5.3:
Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")

Above results in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be problematic especially for the distros using LTSv5.4 and above (the issue was reported on Fedora 32).

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

Please help with backporting to the stable v5.3+ branches (it applies smoothly on v5.4/v5.6/v5.8).

Any chances to backport this patch to relevant v5.3+ stable branches?

What patch? What git commit id needs to be backported?

The actual patch is:
Commit 6c2b49eb9671 ("net: mvpp2: add mvpp2_phylink_to_port() helper").

Quoted 8 lines

Quoted 4 lines

URL for reference:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/mvpp2?h=v5.10-rc7&id=6c2b49eb96716e91f202756bfbd3f5fea3b2b172

Do you think it would be possible to get it merged to v5.3+ stable branches?

Could you explain how that patch fixes anything? It reads like a cleanup.

Quoted 7 lines

Indeed, I am aware of it, but I'm not sure about the best way to fix it. In fact the mentioned patch is an unintentional fix. Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API") reworked an argument list of mvpp2_mac_config() routine in a way that resulted in a NULL pointer dereference when booting the Armada7k8k/CN913x with ACPI between 5.3 and 5.8. Part of Russell's patch resolves this issue.

Quoted 2 lines

What part fixes the issue? I can't see it...

I re-checked in my setup and here's the smallest part of the original patch, that fixes previously described issue:

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +5 -0

index e98be8372780..9d71a4fe1750 100644
@@ -4767,6 +4767,11 @@ static void mvpp2_port_copy_mac_addr(struct
 net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,

struct phylink_link_state *state)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +3 -5

@@ -5105,13 +5110,12 @@ static void mvpp2_gmac_config(struct
 mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;
 
        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }

@@ -5151,8 +5155,7 @@ static void mvpp2_mac_link_up(struct
 phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;
 

        if (mvpp2_is_xlg(interface)) {

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +1 -1

@@ -5199,7 +5202,7 @@ static void mvpp2_mac_link_up(struct
 phylink_config *config,
 
        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }
 

Quoted 6 lines

static void mvpp2_mac_link_down(struct phylink_config *config,
--

Do you think there is a point of slicing the original patch or rather cherry-pick as-is?

Do you think there is a chance to get the above fix merged to stable (v5.3+)?

We need to work with stable maintainers on this, let's see..

Greg asked for a clear description of what happens, from your previous response it sounds like a null-deref in mvpp2_mac_config(). Is the netdev -> config -> netdev linking not ready by the time mvpp2_mac_config() is called?

Quoted 6 lines · Jakub Kicinski

We need to work with stable maintainers on this, let's see..

Greg asked for a clear description of what happens, from your previous response it sounds like a null-deref in mvpp2_mac_config(). Is the netdev -> config -> netdev linking not ready by the time mvpp2_mac_config() is called?

We are going round in circles, so nothing is going to happen.

I stated in detail in one of my emails on the 10th December why the problem occurs. So, Greg has the description already. There is no need to repeat it.

Can we please move forward with this?

Quoted 8 lines · On Mon, 21 Dec 2020 18:30:32 +0000 Russell King - ARM Linux admin wrote:

Quoted 6 lines · Jakub Kicinski

We need to work with stable maintainers on this, let's see..

Greg asked for a clear description of what happens, from your previous response it sounds like a null-deref in mvpp2_mac_config(). Is the netdev -> config -> netdev linking not ready by the time mvpp2_mac_config() is called?

Quoted 7 lines

We are going round in circles, so nothing is going to happen.

I stated in detail in one of my emails on the 10th December why the problem occurs. So, Greg has the description already. There is no need to repeat it.

Can we please move forward with this?

Well, the fact it wasn't quoted in Marcin's reply and that I didn't spot it when scanning the 30 email thread should be a clear enough indication whether pinging threads is a good strategy..

A clear, fresh backport request would had been much more successful and easier for Greg to process. If you still don't see a reply in 2 weeks, please just do that.

In case Greg is in fact reading this:

Greg, can we backport:

6c2b49eb9671 ("net: mvpp2: add mvpp2_phylink_to_port() helper")

Quoted 25 lines · On Mon, 21 Dec 2020 18:30:32 +0000 Russell King - ARM Linux admin wrote:

Quoted 8 lines · Jakub Kicinski

Quoted 6 lines

We need to work with stable maintainers on this, let's see..

Greg asked for a clear description of what happens, from your previous response it sounds like a null-deref in mvpp2_mac_config(). Is the netdev -> config -> netdev linking not ready by the time mvpp2_mac_config() is called?

Quoted 7 lines

We are going round in circles, so nothing is going to happen.

I stated in detail in one of my emails on the 10th December why the problem occurs. So, Greg has the description already. There is no need to repeat it.

Can we please move forward with this?

Well, the fact it wasn't quoted in Marcin's reply and that I didn't spot it when scanning the 30 email thread should be a clear enough indication whether pinging threads is a good strategy..

A clear, fresh backport request would had been much more successful and easier for Greg to process. If you still don't see a reply in 2 weeks, please just do that.

Quoted 6 lines

In case Greg is in fact reading this:

Greg, can we backport:

6c2b49eb9671 ("net: mvpp2: add mvpp2_phylink_to_port() helper")

I've queued it for 5.4, thanks!