Hi,
sob., 20 cze 2020 o 11:21 Russell King rmk+kernel@armlinux.org.uk napisał(a):
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 rmk+kernel@armlinux.org.uk
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index 7653277d03b7..313f5a60a605 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -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
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).
Best regards, Marcin
Hi Greg and Sasha,
pt., 9 paź 2020 o 05:43 Marcin Wojtas mw@semihalf.com napisał(a):
Hi,
sob., 20 cze 2020 o 11:21 Russell King rmk+kernel@armlinux.org.uk napisał(a):
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 rmk+kernel@armlinux.org.uk
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index 7653277d03b7..313f5a60a605 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -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
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).
Any chances to backport this patch to relevant v5.3+ stable branches?
Best regards, Marcin
On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
Hi Greg and Sasha,
pt., 9 paź 2020 o 05:43 Marcin Wojtas mw@semihalf.com napisał(a):
Hi,
sob., 20 cze 2020 o 11:21 Russell King rmk+kernel@armlinux.org.uk napisał(a):
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 rmk+kernel@armlinux.org.uk
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index 7653277d03b7..313f5a60a605 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -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
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).
Any chances to backport this patch to relevant v5.3+ stable branches?
What patch? What git commit id needs to be backported?
confused,
greg k-h
Hi Greg,
Apologies for delayed response:.
pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
Hi Greg and Sasha,
pt., 9 paź 2020 o 05:43 Marcin Wojtas mw@semihalf.com napisał(a):
Hi,
sob., 20 cze 2020 o 11:21 Russell King rmk+kernel@armlinux.org.uk napisał(a):
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 rmk+kernel@armlinux.org.uk
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index 7653277d03b7..313f5a60a605 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -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
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).
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/dr...
Do you think it would be possible to get it merged to v5.3+ stable branches?
Best regards, Marcin
On Tue, Dec 08, 2020 at 01:03:38PM +0100, Marcin Wojtas wrote:
Hi Greg,
Apologies for delayed response:.
pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
Hi Greg and Sasha,
pt., 9 paź 2020 o 05:43 Marcin Wojtas mw@semihalf.com napisał(a):
Hi,
sob., 20 cze 2020 o 11:21 Russell King rmk+kernel@armlinux.org.uk napisał(a):
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 rmk+kernel@armlinux.org.uk
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index 7653277d03b7..313f5a60a605 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -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
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).
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/dr...
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 sashal@kernel.org napisał(a):
On Tue, Dec 08, 2020 at 01:03:38PM +0100, Marcin Wojtas wrote:
Hi Greg,
Apologies for delayed response:.
pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
Hi Greg and Sasha,
pt., 9 paź 2020 o 05:43 Marcin Wojtas mw@semihalf.com napisał(a):
Hi,
sob., 20 cze 2020 o 11:21 Russell King rmk+kernel@armlinux.org.uk napisał(a):
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 rmk+kernel@armlinux.org.uk
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index 7653277d03b7..313f5a60a605 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -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
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).
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/dr...
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.
What is the best way to handle that?
Thanks, Marcin
On Tue, Dec 08, 2020 at 04:02:50PM +0100, Marcin Wojtas wrote:
Hi Sasha,
wt., 8 gru 2020 o 14:35 Sasha Levin sashal@kernel.org napisał(a):
On Tue, Dec 08, 2020 at 01:03:38PM +0100, Marcin Wojtas wrote:
Hi Greg,
Apologies for delayed response:.
pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
Hi Greg and Sasha,
pt., 9 paź 2020 o 05:43 Marcin Wojtas mw@semihalf.com napisał(a):
Hi,
sob., 20 cze 2020 o 11:21 Russell King rmk+kernel@armlinux.org.uk napisał(a): > > 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 rmk+kernel@armlinux.org.uk > --- > .../net/ethernet/marvell/mvpp2/mvpp2_main.c | 29 +++++++++---------- > 1 file changed, 14 insertions(+), 15 deletions(-) > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > index 7653277d03b7..313f5a60a605 100644 > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > @@ -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 >
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).
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/dr...
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.
What part fixes the issue? I can't see it...
thanks,
greg k-h
Hi Greg,
śr., 9 gru 2020 o 11:59 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Tue, Dec 08, 2020 at 04:02:50PM +0100, Marcin Wojtas wrote:
Hi Sasha,
wt., 8 gru 2020 o 14:35 Sasha Levin sashal@kernel.org napisał(a):
On Tue, Dec 08, 2020 at 01:03:38PM +0100, Marcin Wojtas wrote:
Hi Greg,
Apologies for delayed response:.
pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
Hi Greg and Sasha,
pt., 9 paź 2020 o 05:43 Marcin Wojtas mw@semihalf.com napisał(a): > > Hi, > > sob., 20 cze 2020 o 11:21 Russell King rmk+kernel@armlinux.org.uk napisał(a): > > > > 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 rmk+kernel@armlinux.org.uk > > --- > > .../net/ethernet/marvell/mvpp2/mvpp2_main.c | 29 +++++++++---------- > > 1 file changed, 14 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > > index 7653277d03b7..313f5a60a605 100644 > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > > @@ -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 > > > > 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). >
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/dr...
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.
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: diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index e98be8372780..9d71a4fe1750 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -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) @@ -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)) { @@ -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,
On Thu, Dec 10, 2020 at 03:35:29PM +0100, Marcin Wojtas wrote:
Hi Greg,
śr., 9 gru 2020 o 11:59 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
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: diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index e98be8372780..9d71a4fe1750 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -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) @@ -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)) {@@ -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,
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.
Hi Russell,
czw., 10 gru 2020 o 16:46 Russell King - ARM Linux admin linux@armlinux.org.uk napisał(a):
On Thu, Dec 10, 2020 at 03:35:29PM +0100, Marcin Wojtas wrote:
Hi Greg,
śr., 9 gru 2020 o 11:59 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
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: diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index e98be8372780..9d71a4fe1750 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -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) @@ -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)) {@@ -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,
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.
Correct. Hopefully the MDIO+ACPI patchset will land and I'll be able to get rid of this hack and do not maintain dual handling paths any longer.
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.
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).
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.
Totally agree. As soon as I got noticed directly it took me no time to find the regression root cause.
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.
Thank you Russell for your input.
Best regards, Marcin
On Thu, Dec 10, 2020 at 06:43:50PM +0100, Marcin Wojtas wrote:
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.
czw., 10 gru 2020 o 18:56 Russell King - ARM Linux admin linux@armlinux.org.uk napisał(a):
On Thu, Dec 10, 2020 at 06:43:50PM +0100, Marcin Wojtas wrote:
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.
Right. I think I can volunteer to be one (my name is in the driver code anyway :) ) and spend some time on reviewing/testing the patches, unless there are no objections. In such case, are the drivers/net Mainteners who decide/accept it?
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.
Best regards, Marcin
czw., 10 gru 2020 o 21:26 Andrew Lunn andrew@lunn.ch napisał(a):
+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.
I saw the results of the upcoming next revision from NXP, so I'm rather optimistic.
Best regards, Marcin
On Thu, Dec 10, 2020 at 9:27 PM Andrew Lunn andrew@lunn.ch wrote:
+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.
-Jon
Andrew
On Fri, Dec 11, 2020 at 06:03:57AM +0100, Jon Nettleton wrote:
On Thu, Dec 10, 2020 at 9:27 PM Andrew Lunn andrew@lunn.ch wrote:
+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 Jon
Cool, i just hope you get buy in from the ACPI maintainers this time.
Andrew
Hi Jakub,
czw., 10 gru 2020 o 15:35 Marcin Wojtas mw@semihalf.com napisał(a):
Hi Greg,
śr., 9 gru 2020 o 11:59 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Tue, Dec 08, 2020 at 04:02:50PM +0100, Marcin Wojtas wrote:
Hi Sasha,
wt., 8 gru 2020 o 14:35 Sasha Levin sashal@kernel.org napisał(a):
On Tue, Dec 08, 2020 at 01:03:38PM +0100, Marcin Wojtas wrote:
Hi Greg,
Apologies for delayed response:.
pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote: > Hi Greg and Sasha, > > pt., 9 paź 2020 o 05:43 Marcin Wojtas mw@semihalf.com napisał(a): > > > > Hi, > > > > sob., 20 cze 2020 o 11:21 Russell King rmk+kernel@armlinux.org.uk napisał(a): > > > > > > 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 rmk+kernel@armlinux.org.uk > > > --- > > > .../net/ethernet/marvell/mvpp2/mvpp2_main.c | 29 +++++++++---------- > > > 1 file changed, 14 insertions(+), 15 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > > > index 7653277d03b7..313f5a60a605 100644 > > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > > > @@ -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 > > > > > > > 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). > > > > 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/dr...
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.
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: diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index e98be8372780..9d71a4fe1750 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -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) @@ -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)) {@@ -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?
Do you think there is a chance to get the above fix merged to stable (v5.3+)?
Best regards, Marcin
On Sun, 20 Dec 2020 18:08:19 +0100 Marcin Wojtas wrote:
> > > 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). > > > > > > > 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/dr...
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.
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: diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index e98be8372780..9d71a4fe1750 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -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) @@ -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)) {@@ -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?
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?
On Mon, Dec 21, 2020 at 10:25:39AM -0800, Jakub Kicinski wrote:
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?
On Mon, 21 Dec 2020 18:30:32 +0000 Russell King - ARM Linux admin wrote:
On Mon, Dec 21, 2020 at 10:25:39AM -0800, Jakub Kicinski wrote:
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?
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")
to 5.4? Quoting Russell:
The problem is that mvpp2_acpi_start() passes an un-initialised (zeroed) port->phylink_config, as phylink is not used in ACPI setups.
Crash 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.
Folks here are willing to provide a more cut down fix if necessary.
Thanks!
On Mon, Dec 21, 2020 at 10:47:57AM -0800, Jakub Kicinski wrote:
On Mon, 21 Dec 2020 18:30:32 +0000 Russell King - ARM Linux admin wrote:
On Mon, Dec 21, 2020 at 10:25:39AM -0800, Jakub Kicinski wrote:
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?
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")
I've queued it for 5.4, thanks!
pon., 21 gru 2020 o 20:07 Sasha Levin sashal@kernel.org napisał(a):
On Mon, Dec 21, 2020 at 10:47:57AM -0800, Jakub Kicinski wrote:
On Mon, 21 Dec 2020 18:30:32 +0000 Russell King - ARM Linux admin wrote:
On Mon, Dec 21, 2020 at 10:25:39AM -0800, Jakub Kicinski wrote:
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?
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")
I've queued it for 5.4, thanks!
Thank you!
Best regards, Marcin
On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
Hi Greg and Sasha,
pt., 9 paź 2020 o 05:43 Marcin Wojtas mw@semihalf.com napisał(a):
Hi,
sob., 20 cze 2020 o 11:21 Russell King rmk+kernel@armlinux.org.uk napisał(a):
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 rmk+kernel@armlinux.org.uk
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index 7653277d03b7..313f5a60a605 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -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
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).
Any chances to backport this patch to relevant v5.3+ stable branches?
Who are you asking to do the backport? I guess you're asking me to backport it, but I don't generally follow the stable kernels, and I don't use ACPI on ARM systems, so I wouldn't be in a position to test that a backported version fixes the problem you are reporting.
linux-stable-mirror@lists.linaro.org
-
Andrew Lunn -
Greg Kroah-Hartman -
Jakub Kicinski -
Jon Nettleton -
Marcin Wojtas -
Russell King - ARM Linux admin -
Sasha Levin