From 61ec9a8154a58420e3c913c5e62a9e00b10f1936 Mon Sep 17 00:00:00 2001 From: Mathew McBride Date: Fri, 2 Jun 2023 04:14:14 +0000 Subject: [PATCH] armvirt: add SFP support patches for NXP Layerscape DPAA2 platforms This series resolves a long term issue with Phylink and SFPs on DPAA2 hardware (LS1088,LS2088,LX2160) where a locking problem prevented the system from rebooting. Patches solving this issue existed prior to 6.2 but were not accepted upstream. See the original series on patchwork: https://patchwork.kernel.org/project/netdevbpf/cover/20221129141221.872653-1-vladimir.oltean@nxp.com/ And this thread on the Traverse forum: https://forum.traverse.com.au/t/reboot-command-regression-from-5-10-to-5-15-kernel/133/12 Signed-off-by: Mathew McBride --- ...a2-eth-don-t-use-ENOTSUPP-error-code.patch | 44 +++ ...e-dpaa2_mac_is_type_fixed-with-dpaa2.patch | 99 ++++++ ...sorb-phylink_start-call-into-dpaa2_m.patch | 88 +++++ ...move-defensive-check-in-dpaa2_mac_di.patch | 50 +++ ...sign-priv-mac-after-dpaa2_mac_connec.patch | 101 ++++++ ...-assign-port_priv-mac-after-dpaa2_ma.patch | 73 ++++ ...h-MAC-stringset-to-ethtool-S-even-if.patch | 111 ++++++ ...-replace-direct-MAC-access-with-dpaa.patch | 28 ++ ...nnect-to-MAC-before-requesting-the-e.patch | 93 +++++ ...rialize-changes-to-priv-mac-with-a-m.patch | 320 ++++++++++++++++++ ...h-serialize-changes-to-priv-mac-with.patch | 203 +++++++++++ ...c-move-rtnl_lock-only-around-phylink.patch | 113 +++++++ 12 files changed, 1323 insertions(+) create mode 100644 target/linux/armvirt/patches-6.1/701-v6.2-0001-net-dpaa2-eth-don-t-use-ENOTSUPP-error-code.patch create mode 100644 target/linux/armvirt/patches-6.1/701-v6.2-0002-net-dpaa2-replace-dpaa2_mac_is_type_fixed-with-dpaa2.patch create mode 100644 target/linux/armvirt/patches-6.1/701-v6.2-0003-net-dpaa2-mac-absorb-phylink_start-call-into-dpaa2_m.patch create mode 100644 target/linux/armvirt/patches-6.1/701-v6.2-0004-net-dpaa2-mac-remove-defensive-check-in-dpaa2_mac_di.patch create mode 100644 target/linux/armvirt/patches-6.1/701-v6.2-0005-net-dpaa2-eth-assign-priv-mac-after-dpaa2_mac_connec.patch create mode 100644 target/linux/armvirt/patches-6.1/701-v6.2-0006-net-dpaa2-switch-assign-port_priv-mac-after-dpaa2_ma.patch create mode 100644 target/linux/armvirt/patches-6.1/701-v6.2-0007-net-dpaa2-publish-MAC-stringset-to-ethtool-S-even-if.patch create mode 100644 target/linux/armvirt/patches-6.1/701-v6.2-0008-net-dpaa2-switch-replace-direct-MAC-access-with-dpaa.patch create mode 100644 target/linux/armvirt/patches-6.1/701-v6.2-0009-net-dpaa2-eth-connect-to-MAC-before-requesting-the-e.patch create mode 100644 target/linux/armvirt/patches-6.1/701-v6.2-0010-net-dpaa2-eth-serialize-changes-to-priv-mac-with-a-m.patch create mode 100644 target/linux/armvirt/patches-6.1/701-v6.2-0011-net-dpaa2-switch-serialize-changes-to-priv-mac-with.patch create mode 100644 target/linux/armvirt/patches-6.1/701-v6.2-0012-net-dpaa2-mac-move-rtnl_lock-only-around-phylink.patch diff --git a/target/linux/armvirt/patches-6.1/701-v6.2-0001-net-dpaa2-eth-don-t-use-ENOTSUPP-error-code.patch b/target/linux/armvirt/patches-6.1/701-v6.2-0001-net-dpaa2-eth-don-t-use-ENOTSUPP-error-code.patch new file mode 100644 index 00000000000..6238c0a9039 --- /dev/null +++ b/target/linux/armvirt/patches-6.1/701-v6.2-0001-net-dpaa2-eth-don-t-use-ENOTSUPP-error-code.patch @@ -0,0 +1,44 @@ +From f3763a0c1b07273218cbf5886bdf8df9df501111 Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean +Date: Tue, 29 Nov 2022 16:12:10 +0200 +Subject: [PATCH 03/14] net: dpaa2-eth: don't use -ENOTSUPP error code + +dpaa2_eth_setup_dpni() is called from the probe path and +dpaa2_eth_set_link_ksettings() is propagated to user space. + +include/linux/errno.h says that ENOTSUPP is "Defined for the NFSv3 +protocol". Conventional wisdom has it to not use it in networking +drivers. Replace it with -EOPNOTSUPP. + +Signed-off-by: Vladimir Oltean +Reviewed-by: Andrew Lunn +Reviewed-by: Ioana Ciornei +Tested-by: Ioana Ciornei +Signed-off-by: Paolo Abeni +--- + drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 2 +- + drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c | 2 +- + 2 files changed, 2 insertions(+), 2 deletions(-) + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +@@ -3613,7 +3613,7 @@ static int dpaa2_eth_setup_dpni(struct f + dev_err(dev, "DPNI version %u.%u not supported, need >= %u.%u\n", + priv->dpni_ver_major, priv->dpni_ver_minor, + DPNI_VER_MAJOR, DPNI_VER_MINOR); +- err = -ENOTSUPP; ++ err = -EOPNOTSUPP; + goto close; + } + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c +@@ -118,7 +118,7 @@ dpaa2_eth_set_link_ksettings(struct net_ + struct dpaa2_eth_priv *priv = netdev_priv(net_dev); + + if (!dpaa2_eth_is_type_phy(priv)) +- return -ENOTSUPP; ++ return -EOPNOTSUPP; + + return phylink_ethtool_ksettings_set(priv->mac->phylink, link_settings); + } diff --git a/target/linux/armvirt/patches-6.1/701-v6.2-0002-net-dpaa2-replace-dpaa2_mac_is_type_fixed-with-dpaa2.patch b/target/linux/armvirt/patches-6.1/701-v6.2-0002-net-dpaa2-replace-dpaa2_mac_is_type_fixed-with-dpaa2.patch new file mode 100644 index 00000000000..501eaf42ff3 --- /dev/null +++ b/target/linux/armvirt/patches-6.1/701-v6.2-0002-net-dpaa2-replace-dpaa2_mac_is_type_fixed-with-dpaa2.patch @@ -0,0 +1,99 @@ +From 022a11062261dc4703da846d3bf4d194ef6bebf5 Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean +Date: Tue, 29 Nov 2022 16:12:11 +0200 +Subject: [PATCH 04/14] net: dpaa2: replace dpaa2_mac_is_type_fixed() with + dpaa2_mac_is_type_phy() + +dpaa2_mac_is_type_fixed() is a header with no implementation and no +callers, which is referenced from the documentation though. It can be +deleted. + +On the other hand, it would be useful to reuse the code between +dpaa2_eth_is_type_phy() and dpaa2_switch_port_is_type_phy(). That common +code should be called dpaa2_mac_is_type_phy(), so let's create that. + +The removal and the addition are merged into the same patch because, +in fact, is_type_phy() is the logical opposite of is_type_fixed(). + +Signed-off-by: Vladimir Oltean +Reviewed-by: Andrew Lunn +Reviewed-by: Ioana Ciornei +Tested-by: Ioana Ciornei +Signed-off-by: Paolo Abeni +--- + .../ethernet/freescale/dpaa2/mac-phy-support.rst | 9 ++++++--- + drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h | 7 +------ + drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h | 10 ++++++++-- + drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.h | 7 +------ + 4 files changed, 16 insertions(+), 17 deletions(-) + +--- a/Documentation/networking/device_drivers/ethernet/freescale/dpaa2/mac-phy-support.rst ++++ b/Documentation/networking/device_drivers/ethernet/freescale/dpaa2/mac-phy-support.rst +@@ -181,10 +181,13 @@ when necessary using the below listed AP + - int dpaa2_mac_connect(struct dpaa2_mac *mac); + - void dpaa2_mac_disconnect(struct dpaa2_mac *mac); + +-A phylink integration is necessary only when the partner DPMAC is not of TYPE_FIXED. +-One can check for this condition using the below API:: ++A phylink integration is necessary only when the partner DPMAC is not of ++``TYPE_FIXED``. This means it is either of ``TYPE_PHY``, or of ++``TYPE_BACKPLANE`` (the difference being the two that in the ``TYPE_BACKPLANE`` ++mode, the MC firmware does not access the PCS registers). One can check for ++this condition using the following helper:: + +- - bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,struct fsl_mc_io *mc_io); ++ - static inline bool dpaa2_mac_is_type_phy(struct dpaa2_mac *mac); + + Before connection to a MAC, the caller must allocate and populate the + dpaa2_mac structure with the associated net_device, a pointer to the MC portal +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h +@@ -733,12 +733,7 @@ static inline unsigned int dpaa2_eth_rx_ + + static inline bool dpaa2_eth_is_type_phy(struct dpaa2_eth_priv *priv) + { +- if (priv->mac && +- (priv->mac->attr.link_type == DPMAC_LINK_TYPE_PHY || +- priv->mac->attr.link_type == DPMAC_LINK_TYPE_BACKPLANE)) +- return true; +- +- return false; ++ return dpaa2_mac_is_type_phy(priv->mac); + } + + static inline bool dpaa2_eth_has_mac(struct dpaa2_eth_priv *priv) +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h +@@ -30,8 +30,14 @@ struct dpaa2_mac { + struct phy *serdes_phy; + }; + +-bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev, +- struct fsl_mc_io *mc_io); ++static inline bool dpaa2_mac_is_type_phy(struct dpaa2_mac *mac) ++{ ++ if (!mac) ++ return false; ++ ++ return mac->attr.link_type == DPMAC_LINK_TYPE_PHY || ++ mac->attr.link_type == DPMAC_LINK_TYPE_BACKPLANE; ++} + + int dpaa2_mac_open(struct dpaa2_mac *mac); + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.h ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.h +@@ -230,12 +230,7 @@ static inline bool dpaa2_switch_supports + static inline bool + dpaa2_switch_port_is_type_phy(struct ethsw_port_priv *port_priv) + { +- if (port_priv->mac && +- (port_priv->mac->attr.link_type == DPMAC_LINK_TYPE_PHY || +- port_priv->mac->attr.link_type == DPMAC_LINK_TYPE_BACKPLANE)) +- return true; +- +- return false; ++ return dpaa2_mac_is_type_phy(port_priv->mac); + } + + static inline bool dpaa2_switch_port_has_mac(struct ethsw_port_priv *port_priv) diff --git a/target/linux/armvirt/patches-6.1/701-v6.2-0003-net-dpaa2-mac-absorb-phylink_start-call-into-dpaa2_m.patch b/target/linux/armvirt/patches-6.1/701-v6.2-0003-net-dpaa2-mac-absorb-phylink_start-call-into-dpaa2_m.patch new file mode 100644 index 00000000000..e84195f3cd7 --- /dev/null +++ b/target/linux/armvirt/patches-6.1/701-v6.2-0003-net-dpaa2-mac-absorb-phylink_start-call-into-dpaa2_m.patch @@ -0,0 +1,88 @@ +From 97c07369ab8bf9895e05d4b468f18e6567263154 Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean +Date: Tue, 29 Nov 2022 16:12:12 +0200 +Subject: [PATCH 05/14] net: dpaa2-mac: absorb phylink_start() call into + dpaa2_mac_start() + +The phylink handling is intended to be hidden inside the dpaa2_mac +object. Move the phylink_start() call into dpaa2_mac_start(), and +phylink_stop() into dpaa2_mac_stop(). + +Signed-off-by: Vladimir Oltean +Reviewed-by: Andrew Lunn +Reviewed-by: Ioana Ciornei +Tested-by: Ioana Ciornei +Signed-off-by: Paolo Abeni +--- + drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 5 +---- + drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c | 8 ++++++++ + drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c | 5 +---- + 3 files changed, 10 insertions(+), 8 deletions(-) + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +@@ -2082,10 +2082,8 @@ static int dpaa2_eth_open(struct net_dev + goto enable_err; + } + +- if (dpaa2_eth_is_type_phy(priv)) { ++ if (dpaa2_eth_is_type_phy(priv)) + dpaa2_mac_start(priv->mac); +- phylink_start(priv->mac->phylink); +- } + + return 0; + +@@ -2159,7 +2157,6 @@ static int dpaa2_eth_stop(struct net_dev + int retries = 10; + + if (dpaa2_eth_is_type_phy(priv)) { +- phylink_stop(priv->mac->phylink); + dpaa2_mac_stop(priv->mac); + } else { + netif_tx_stop_all_queues(net_dev); +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c +@@ -336,12 +336,20 @@ static void dpaa2_mac_set_supported_inte + + void dpaa2_mac_start(struct dpaa2_mac *mac) + { ++ ASSERT_RTNL(); ++ + if (mac->serdes_phy) + phy_power_on(mac->serdes_phy); ++ ++ phylink_start(mac->phylink); + } + + void dpaa2_mac_stop(struct dpaa2_mac *mac) + { ++ ASSERT_RTNL(); ++ ++ phylink_stop(mac->phylink); ++ + if (mac->serdes_phy) + phy_power_off(mac->serdes_phy); + } +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c +@@ -703,10 +703,8 @@ static int dpaa2_switch_port_open(struct + + dpaa2_switch_enable_ctrl_if_napi(ethsw); + +- if (dpaa2_switch_port_is_type_phy(port_priv)) { ++ if (dpaa2_switch_port_is_type_phy(port_priv)) + dpaa2_mac_start(port_priv->mac); +- phylink_start(port_priv->mac->phylink); +- } + + return 0; + } +@@ -718,7 +716,6 @@ static int dpaa2_switch_port_stop(struct + int err; + + if (dpaa2_switch_port_is_type_phy(port_priv)) { +- phylink_stop(port_priv->mac->phylink); + dpaa2_mac_stop(port_priv->mac); + } else { + netif_tx_stop_all_queues(netdev); diff --git a/target/linux/armvirt/patches-6.1/701-v6.2-0004-net-dpaa2-mac-remove-defensive-check-in-dpaa2_mac_di.patch b/target/linux/armvirt/patches-6.1/701-v6.2-0004-net-dpaa2-mac-remove-defensive-check-in-dpaa2_mac_di.patch new file mode 100644 index 00000000000..c3028357fe5 --- /dev/null +++ b/target/linux/armvirt/patches-6.1/701-v6.2-0004-net-dpaa2-mac-remove-defensive-check-in-dpaa2_mac_di.patch @@ -0,0 +1,50 @@ +From 095ef388f714d622aa503fcccf20dc4095b72762 Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean +Date: Tue, 29 Nov 2022 16:12:13 +0200 +Subject: [PATCH 06/14] net: dpaa2-mac: remove defensive check in + dpaa2_mac_disconnect() + +dpaa2_mac_disconnect() will only be called with a NULL mac->phylink if +dpaa2_mac_connect() failed, or was never called. + +The callers are these: + +dpaa2_eth_disconnect_mac(): + + if (dpaa2_eth_is_type_phy(priv)) + dpaa2_mac_disconnect(priv->mac); + +dpaa2_switch_port_disconnect_mac(): + + if (dpaa2_switch_port_is_type_phy(port_priv)) + dpaa2_mac_disconnect(port_priv->mac); + +priv->mac can be NULL, but in that case, dpaa2_eth_is_type_phy() returns +false, and dpaa2_mac_disconnect() is never called. Similar for +dpaa2-switch. + +When priv->mac is non-NULL, it means that dpaa2_mac_connect() returned +zero (success), and therefore, priv->mac->phylink is also a valid +pointer. + +Signed-off-by: Vladimir Oltean +Reviewed-by: Andrew Lunn +Reviewed-by: Ioana Ciornei +Tested-by: Ioana Ciornei +Signed-off-by: Paolo Abeni +--- + drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c | 3 --- + 1 file changed, 3 deletions(-) + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c +@@ -446,9 +446,6 @@ err_pcs_destroy: + + void dpaa2_mac_disconnect(struct dpaa2_mac *mac) + { +- if (!mac->phylink) +- return; +- + phylink_disconnect_phy(mac->phylink); + phylink_destroy(mac->phylink); + dpaa2_pcs_destroy(mac); diff --git a/target/linux/armvirt/patches-6.1/701-v6.2-0005-net-dpaa2-eth-assign-priv-mac-after-dpaa2_mac_connec.patch b/target/linux/armvirt/patches-6.1/701-v6.2-0005-net-dpaa2-eth-assign-priv-mac-after-dpaa2_mac_connec.patch new file mode 100644 index 00000000000..8c7ed6c793f --- /dev/null +++ b/target/linux/armvirt/patches-6.1/701-v6.2-0005-net-dpaa2-eth-assign-priv-mac-after-dpaa2_mac_connec.patch @@ -0,0 +1,101 @@ +From 06efc9b8a1360cad83cae6e71558e5458cc1fbf3 Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean +Date: Tue, 29 Nov 2022 16:12:14 +0200 +Subject: [PATCH 07/14] net: dpaa2-eth: assign priv->mac after + dpaa2_mac_connect() call + +There are 2 requirements for correct code: + +- Any time the driver accesses the priv->mac pointer at runtime, it + either holds NULL to indicate a DPNI-DPNI connection (or unconnected + DPNI), or a struct dpaa2_mac whose phylink instance was fully + initialized (created and connected to the PHY). No changes are made to + priv->mac while it is being used. Currently, rtnl_lock() watches over + the call to dpaa2_eth_connect_mac(), so it serves the purpose of + serializing this with all readers of priv->mac. + +- dpaa2_mac_connect() should run unlocked, because inside it are 2 + phylink calls with incompatible locking requirements: phylink_create() + requires that the rtnl_mutex isn't held, and phylink_fwnode_phy_connect() + requires that the rtnl_mutex is held. The only way to solve those + contradictory requirements is to let dpaa2_mac_connect() take + rtnl_lock() when it needs to. + +To solve both requirements, we need to identify the writer side of the +priv->mac pointer, which can be wrapped in a mutex private to the driver +in a future patch. The dpaa2_mac_connect() cannot be part of the writer +side critical section, because of an AB/BA deadlock with rtnl_lock(). + +So the strategy needs to be that where we prepare the DPMAC by calling +dpaa2_mac_connect(), and only make priv->mac point to it once it's fully +prepared. This ensures that the writer side critical section has the +absolute minimum surface it can. + +The reverse strategy is adopted in the dpaa2_eth_disconnect_mac() code +path. This makes sure that priv->mac is NULL when we start tearing down +the DPMAC that we disconnected from, and concurrent code will simply not +see it. + +No locking changes in this patch (concurrent code is still blocked by +the rtnl_mutex). + +Signed-off-by: Vladimir Oltean +Reviewed-by: Ioana Ciornei +Tested-by: Ioana Ciornei +Signed-off-by: Paolo Abeni +--- + .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 21 +++++++++++-------- + 1 file changed, 12 insertions(+), 9 deletions(-) + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +@@ -4443,9 +4443,8 @@ static int dpaa2_eth_connect_mac(struct + err = dpaa2_mac_open(mac); + if (err) + goto err_free_mac; +- priv->mac = mac; + +- if (dpaa2_eth_is_type_phy(priv)) { ++ if (dpaa2_mac_is_type_phy(mac)) { + err = dpaa2_mac_connect(mac); + if (err && err != -EPROBE_DEFER) + netdev_err(priv->net_dev, "Error connecting to the MAC endpoint: %pe", +@@ -4454,11 +4453,12 @@ static int dpaa2_eth_connect_mac(struct + goto err_close_mac; + } + ++ priv->mac = mac; ++ + return 0; + + err_close_mac: + dpaa2_mac_close(mac); +- priv->mac = NULL; + err_free_mac: + kfree(mac); + return err; +@@ -4466,15 +4466,18 @@ err_free_mac: + + static void dpaa2_eth_disconnect_mac(struct dpaa2_eth_priv *priv) + { +- if (dpaa2_eth_is_type_phy(priv)) +- dpaa2_mac_disconnect(priv->mac); ++ struct dpaa2_mac *mac = priv->mac; ++ ++ priv->mac = NULL; + +- if (!dpaa2_eth_has_mac(priv)) ++ if (!mac) + return; + +- dpaa2_mac_close(priv->mac); +- kfree(priv->mac); +- priv->mac = NULL; ++ if (dpaa2_mac_is_type_phy(mac)) ++ dpaa2_mac_disconnect(mac); ++ ++ dpaa2_mac_close(mac); ++ kfree(mac); + } + + static irqreturn_t dpni_irq0_handler_thread(int irq_num, void *arg) diff --git a/target/linux/armvirt/patches-6.1/701-v6.2-0006-net-dpaa2-switch-assign-port_priv-mac-after-dpaa2_ma.patch b/target/linux/armvirt/patches-6.1/701-v6.2-0006-net-dpaa2-switch-assign-port_priv-mac-after-dpaa2_ma.patch new file mode 100644 index 00000000000..e63654984a5 --- /dev/null +++ b/target/linux/armvirt/patches-6.1/701-v6.2-0006-net-dpaa2-switch-assign-port_priv-mac-after-dpaa2_ma.patch @@ -0,0 +1,73 @@ +From a5e7f7e277bd4403c45c1c7922d56d0eb08dbc7c Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean +Date: Tue, 29 Nov 2022 16:12:15 +0200 +Subject: [PATCH 08/14] net: dpaa2-switch: assign port_priv->mac after + dpaa2_mac_connect() call + +The dpaa2-switch has the exact same locking requirements when connected +to a DPMAC, so it needs port_priv->mac to always point either to NULL, +or to a DPMAC with a fully initialized phylink instance. + +Make the same preparatory change in the dpaa2-switch driver as in the +dpaa2-eth one. + +Signed-off-by: Vladimir Oltean +Reviewed-by: Ioana Ciornei +Tested-by: Ioana Ciornei +Signed-off-by: Paolo Abeni +--- + .../ethernet/freescale/dpaa2/dpaa2-switch.c | 21 +++++++++++-------- + 1 file changed, 12 insertions(+), 9 deletions(-) + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c +@@ -1450,9 +1450,8 @@ static int dpaa2_switch_port_connect_mac + err = dpaa2_mac_open(mac); + if (err) + goto err_free_mac; +- port_priv->mac = mac; + +- if (dpaa2_switch_port_is_type_phy(port_priv)) { ++ if (dpaa2_mac_is_type_phy(mac)) { + err = dpaa2_mac_connect(mac); + if (err) { + netdev_err(port_priv->netdev, +@@ -1462,11 +1461,12 @@ static int dpaa2_switch_port_connect_mac + } + } + ++ port_priv->mac = mac; ++ + return 0; + + err_close_mac: + dpaa2_mac_close(mac); +- port_priv->mac = NULL; + err_free_mac: + kfree(mac); + return err; +@@ -1474,15 +1474,18 @@ err_free_mac: + + static void dpaa2_switch_port_disconnect_mac(struct ethsw_port_priv *port_priv) + { +- if (dpaa2_switch_port_is_type_phy(port_priv)) +- dpaa2_mac_disconnect(port_priv->mac); ++ struct dpaa2_mac *mac = port_priv->mac; ++ ++ port_priv->mac = NULL; + +- if (!dpaa2_switch_port_has_mac(port_priv)) ++ if (!mac) + return; + +- dpaa2_mac_close(port_priv->mac); +- kfree(port_priv->mac); +- port_priv->mac = NULL; ++ if (dpaa2_mac_is_type_phy(mac)) ++ dpaa2_mac_disconnect(mac); ++ ++ dpaa2_mac_close(mac); ++ kfree(mac); + } + + static irqreturn_t dpaa2_switch_irq0_handler_thread(int irq_num, void *arg) diff --git a/target/linux/armvirt/patches-6.1/701-v6.2-0007-net-dpaa2-publish-MAC-stringset-to-ethtool-S-even-if.patch b/target/linux/armvirt/patches-6.1/701-v6.2-0007-net-dpaa2-publish-MAC-stringset-to-ethtool-S-even-if.patch new file mode 100644 index 00000000000..c790ba1bd50 --- /dev/null +++ b/target/linux/armvirt/patches-6.1/701-v6.2-0007-net-dpaa2-publish-MAC-stringset-to-ethtool-S-even-if.patch @@ -0,0 +1,111 @@ +From ce44b6ed9ee65efa9b3025552c513842eabcab88 Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean +Date: Tue, 29 Nov 2022 16:12:16 +0200 +Subject: [PATCH 09/14] net: dpaa2: publish MAC stringset to ethtool -S even if + MAC is missing + +DPNIs and DPSW objects can connect and disconnect at runtime from DPMAC +objects on the same fsl-mc bus. The DPMAC object also holds "ethtool -S" +unstructured counters. Those counters are only shown for the entity +owning the netdev (DPNI, DPSW) if it's connected to a DPMAC. + +The ethtool stringset code path is split into multiple callbacks, but +currently, connecting and disconnecting the DPMAC takes the rtnl_lock(). +This blocks the entire ethtool code path from running, see +ethnl_default_doit() -> rtnl_lock() -> ops->prepare_data() -> +strset_prepare_data(). + +This is going to be a problem if we are going to no longer require +rtnl_lock() when connecting/disconnecting the DPMAC, because the DPMAC +could appear between ops->get_sset_count() and ops->get_strings(). +If it appears out of the blue, we will provide a stringset into an array +that was dimensioned thinking the DPMAC wouldn't be there => array +accessed out of bounds. + +There isn't really a good way to work around that, and I don't want to +put too much pressure on the ethtool framework by playing locking games. +Just make the DPMAC counters be always available. They'll be zeroes if +the DPNI or DPSW isn't connected to a DPMAC. + +Signed-off-by: Vladimir Oltean +Reviewed-by: Andrew Lunn +Reviewed-by: Ioana Ciornei +Tested-by: Ioana Ciornei +Signed-off-by: Paolo Abeni +--- + drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c | 12 +++--------- + .../ethernet/freescale/dpaa2/dpaa2-switch-ethtool.c | 11 ++--------- + 2 files changed, 5 insertions(+), 18 deletions(-) + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c +@@ -186,7 +186,6 @@ static int dpaa2_eth_set_pauseparam(stru + static void dpaa2_eth_get_strings(struct net_device *netdev, u32 stringset, + u8 *data) + { +- struct dpaa2_eth_priv *priv = netdev_priv(netdev); + u8 *p = data; + int i; + +@@ -200,22 +199,17 @@ static void dpaa2_eth_get_strings(struct + strscpy(p, dpaa2_ethtool_extras[i], ETH_GSTRING_LEN); + p += ETH_GSTRING_LEN; + } +- if (dpaa2_eth_has_mac(priv)) +- dpaa2_mac_get_strings(p); ++ dpaa2_mac_get_strings(p); + break; + } + } + + static int dpaa2_eth_get_sset_count(struct net_device *net_dev, int sset) + { +- int num_ss_stats = DPAA2_ETH_NUM_STATS + DPAA2_ETH_NUM_EXTRA_STATS; +- struct dpaa2_eth_priv *priv = netdev_priv(net_dev); +- + switch (sset) { + case ETH_SS_STATS: /* ethtool_get_stats(), ethtool_get_drvinfo() */ +- if (dpaa2_eth_has_mac(priv)) +- num_ss_stats += dpaa2_mac_get_sset_count(); +- return num_ss_stats; ++ return DPAA2_ETH_NUM_STATS + DPAA2_ETH_NUM_EXTRA_STATS + ++ dpaa2_mac_get_sset_count(); + default: + return -EOPNOTSUPP; + } +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-ethtool.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-ethtool.c +@@ -145,14 +145,9 @@ dpaa2_switch_set_link_ksettings(struct n + static int + dpaa2_switch_ethtool_get_sset_count(struct net_device *netdev, int sset) + { +- struct ethsw_port_priv *port_priv = netdev_priv(netdev); +- int num_ss_stats = DPAA2_SWITCH_NUM_COUNTERS; +- + switch (sset) { + case ETH_SS_STATS: +- if (port_priv->mac) +- num_ss_stats += dpaa2_mac_get_sset_count(); +- return num_ss_stats; ++ return DPAA2_SWITCH_NUM_COUNTERS + dpaa2_mac_get_sset_count(); + default: + return -EOPNOTSUPP; + } +@@ -161,7 +156,6 @@ dpaa2_switch_ethtool_get_sset_count(stru + static void dpaa2_switch_ethtool_get_strings(struct net_device *netdev, + u32 stringset, u8 *data) + { +- struct ethsw_port_priv *port_priv = netdev_priv(netdev); + u8 *p = data; + int i; + +@@ -172,8 +166,7 @@ static void dpaa2_switch_ethtool_get_str + ETH_GSTRING_LEN); + p += ETH_GSTRING_LEN; + } +- if (port_priv->mac) +- dpaa2_mac_get_strings(p); ++ dpaa2_mac_get_strings(p); + break; + } + } diff --git a/target/linux/armvirt/patches-6.1/701-v6.2-0008-net-dpaa2-switch-replace-direct-MAC-access-with-dpaa.patch b/target/linux/armvirt/patches-6.1/701-v6.2-0008-net-dpaa2-switch-replace-direct-MAC-access-with-dpaa.patch new file mode 100644 index 00000000000..0663bf6fb10 --- /dev/null +++ b/target/linux/armvirt/patches-6.1/701-v6.2-0008-net-dpaa2-switch-replace-direct-MAC-access-with-dpaa.patch @@ -0,0 +1,28 @@ +From c838d9fd7e6ba9ddd6006bf0a296396266e9f121 Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean +Date: Tue, 29 Nov 2022 16:12:17 +0200 +Subject: [PATCH 10/14] net: dpaa2-switch replace direct MAC access with + dpaa2_switch_port_has_mac() + +The helper function will gain a lockdep annotation in a future patch. +Make sure to benefit from it. + +Signed-off-by: Vladimir Oltean +Reviewed-by: Ioana Ciornei +Tested-by: Ioana Ciornei +Signed-off-by: Paolo Abeni +--- + drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-ethtool.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-ethtool.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-ethtool.c +@@ -189,7 +189,7 @@ static void dpaa2_switch_ethtool_get_sta + dpaa2_switch_ethtool_counters[i].name, err); + } + +- if (port_priv->mac) ++ if (dpaa2_switch_port_has_mac(port_priv)) + dpaa2_mac_get_ethtool_stats(port_priv->mac, data + i); + } + diff --git a/target/linux/armvirt/patches-6.1/701-v6.2-0009-net-dpaa2-eth-connect-to-MAC-before-requesting-the-e.patch b/target/linux/armvirt/patches-6.1/701-v6.2-0009-net-dpaa2-eth-connect-to-MAC-before-requesting-the-e.patch new file mode 100644 index 00000000000..37831f264b8 --- /dev/null +++ b/target/linux/armvirt/patches-6.1/701-v6.2-0009-net-dpaa2-eth-connect-to-MAC-before-requesting-the-e.patch @@ -0,0 +1,93 @@ +From e0ea63162cb5f1ca7f844d6ef5fc4079448ee2d5 Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean +Date: Tue, 29 Nov 2022 16:12:18 +0200 +Subject: [PATCH 11/14] net: dpaa2-eth: connect to MAC before requesting the + "endpoint changed" IRQ + +dpaa2_eth_connect_mac() is called both from dpaa2_eth_probe() and from +dpni_irq0_handler_thread(). + +It could happen that the DPNI gets connected to a DPMAC on the fsl-mc +bus exactly during probe, as soon as the "endpoint change" interrupt is +requested in dpaa2_eth_setup_irqs(). This will cause the +dpni_irq0_handler_thread() to register a phylink instance for that DPMAC. + +Then, the probing function will also try to register a phylink instance +for the same DPMAC, operation which should fail (and this will fail the +probing of the driver). + +Reorder dpaa2_eth_setup_irqs() and dpaa2_eth_connect_mac(), such that +dpni_irq0_handler_thread() never races with the DPMAC-related portion of +the probing path. + +Also reorder dpaa2_eth_disconnect_mac() to be in the mirror position of +dpaa2_eth_connect_mac() in the teardown path. + +Signed-off-by: Vladimir Oltean +Reviewed-by: Ioana Ciornei +Tested-by: Ioana Ciornei +Signed-off-by: Paolo Abeni +--- + .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 18 +++++++++--------- + 1 file changed, 9 insertions(+), 9 deletions(-) + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +@@ -4710,6 +4710,10 @@ static int dpaa2_eth_probe(struct fsl_mc + } + #endif + ++ err = dpaa2_eth_connect_mac(priv); ++ if (err) ++ goto err_connect_mac; ++ + err = dpaa2_eth_setup_irqs(dpni_dev); + if (err) { + netdev_warn(net_dev, "Failed to set link interrupt, fall back to polling\n"); +@@ -4722,10 +4726,6 @@ static int dpaa2_eth_probe(struct fsl_mc + priv->do_link_poll = true; + } + +- err = dpaa2_eth_connect_mac(priv); +- if (err) +- goto err_connect_mac; +- + err = dpaa2_eth_dl_alloc(priv); + if (err) + goto err_dl_register; +@@ -4759,13 +4759,13 @@ err_dl_port_add: + err_dl_trap_register: + dpaa2_eth_dl_free(priv); + err_dl_register: +- dpaa2_eth_disconnect_mac(priv); +-err_connect_mac: + if (priv->do_link_poll) + kthread_stop(priv->poll_thread); + else + fsl_mc_free_irqs(dpni_dev); + err_poll_thread: ++ dpaa2_eth_disconnect_mac(priv); ++err_connect_mac: + dpaa2_eth_free_rings(priv); + err_alloc_rings: + err_csum: +@@ -4813,9 +4813,6 @@ static int dpaa2_eth_remove(struct fsl_m + #endif + + unregister_netdev(net_dev); +- rtnl_lock(); +- dpaa2_eth_disconnect_mac(priv); +- rtnl_unlock(); + + dpaa2_eth_dl_port_del(priv); + dpaa2_eth_dl_traps_unregister(priv); +@@ -4826,6 +4823,9 @@ static int dpaa2_eth_remove(struct fsl_m + else + fsl_mc_free_irqs(ls_dev); + ++ rtnl_lock(); ++ dpaa2_eth_disconnect_mac(priv); ++ rtnl_unlock(); + dpaa2_eth_free_rings(priv); + free_percpu(priv->fd); + free_percpu(priv->sgt_cache); diff --git a/target/linux/armvirt/patches-6.1/701-v6.2-0010-net-dpaa2-eth-serialize-changes-to-priv-mac-with-a-m.patch b/target/linux/armvirt/patches-6.1/701-v6.2-0010-net-dpaa2-eth-serialize-changes-to-priv-mac-with-a-m.patch new file mode 100644 index 00000000000..89f58412d40 --- /dev/null +++ b/target/linux/armvirt/patches-6.1/701-v6.2-0010-net-dpaa2-eth-serialize-changes-to-priv-mac-with-a-m.patch @@ -0,0 +1,320 @@ +From 5e448a17dfa2e95166534df7f677a3694ef6187d Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean +Date: Tue, 29 Nov 2022 16:12:19 +0200 +Subject: [PATCH 12/14] net: dpaa2-eth: serialize changes to priv->mac with a + mutex + +The dpaa2 architecture permits dynamic connections between objects on +the fsl-mc bus, specifically between a DPNI object (represented by a +struct net_device) and a DPMAC object (represented by a struct phylink). + +The DPNI driver is notified when those connections are created/broken +through the dpni_irq0_handler_thread() method. To ensure that ethtool +operations, as well as netdev up/down operations serialize with the +connection/disconnection of the DPNI with a DPMAC, +dpni_irq0_handler_thread() takes the rtnl_lock() to block those other +operations from taking place. + +There is code called by dpaa2_mac_connect() which wants to acquire the +rtnl_mutex once again, see phylink_create() -> phylink_register_sfp() -> +sfp_bus_add_upstream() -> rtnl_lock(). So the strategy doesn't quite +work out, even though it's fairly simple. + +Create a different strategy, where all code paths in the dpaa2-eth +driver access priv->mac only while they are holding priv->mac_lock. +The phylink instance is not created or connected to the PHY under the +priv->mac_lock, but only assigned to priv->mac then. This will eliminate +the reliance on the rtnl_mutex. + +Add lockdep annotations and put comments where holding the lock is not +necessary, and priv->mac can be dereferenced freely. + +Signed-off-by: Vladimir Oltean +Reviewed-by: Ioana Ciornei +Tested-by: Ioana Ciornei +Signed-off-by: Paolo Abeni +--- + .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 43 ++++++++++++-- + .../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 6 ++ + .../ethernet/freescale/dpaa2/dpaa2-ethtool.c | 58 +++++++++++++++---- + 3 files changed, 91 insertions(+), 16 deletions(-) + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +@@ -2020,8 +2020,11 @@ static int dpaa2_eth_link_state_update(s + + /* When we manage the MAC/PHY using phylink there is no need + * to manually update the netif_carrier. ++ * We can avoid locking because we are called from the "link changed" ++ * IRQ handler, which is the same as the "endpoint changed" IRQ handler ++ * (the writer to priv->mac), so we cannot race with it. + */ +- if (dpaa2_eth_is_type_phy(priv)) ++ if (dpaa2_mac_is_type_phy(priv->mac)) + goto out; + + /* Chech link state; speed / duplex changes are not treated yet */ +@@ -2060,6 +2063,8 @@ static int dpaa2_eth_open(struct net_dev + priv->dpbp_dev->obj_desc.id, priv->bpid); + } + ++ mutex_lock(&priv->mac_lock); ++ + if (!dpaa2_eth_is_type_phy(priv)) { + /* We'll only start the txqs when the link is actually ready; + * make sure we don't race against the link up notification, +@@ -2078,6 +2083,7 @@ static int dpaa2_eth_open(struct net_dev + + err = dpni_enable(priv->mc_io, 0, priv->mc_token); + if (err < 0) { ++ mutex_unlock(&priv->mac_lock); + netdev_err(net_dev, "dpni_enable() failed\n"); + goto enable_err; + } +@@ -2085,6 +2091,8 @@ static int dpaa2_eth_open(struct net_dev + if (dpaa2_eth_is_type_phy(priv)) + dpaa2_mac_start(priv->mac); + ++ mutex_unlock(&priv->mac_lock); ++ + return 0; + + enable_err: +@@ -2156,6 +2164,8 @@ static int dpaa2_eth_stop(struct net_dev + int dpni_enabled = 0; + int retries = 10; + ++ mutex_lock(&priv->mac_lock); ++ + if (dpaa2_eth_is_type_phy(priv)) { + dpaa2_mac_stop(priv->mac); + } else { +@@ -2163,6 +2173,8 @@ static int dpaa2_eth_stop(struct net_dev + netif_carrier_off(net_dev); + } + ++ mutex_unlock(&priv->mac_lock); ++ + /* On dpni_disable(), the MC firmware will: + * - stop MAC Rx and wait for all Rx frames to be enqueued to software + * - cut off WRIOP dequeues from egress FQs and wait until transmission +@@ -2488,12 +2500,20 @@ static int dpaa2_eth_ts_ioctl(struct net + static int dpaa2_eth_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) + { + struct dpaa2_eth_priv *priv = netdev_priv(dev); ++ int err; + + if (cmd == SIOCSHWTSTAMP) + return dpaa2_eth_ts_ioctl(dev, rq, cmd); + +- if (dpaa2_eth_is_type_phy(priv)) +- return phylink_mii_ioctl(priv->mac->phylink, rq, cmd); ++ mutex_lock(&priv->mac_lock); ++ ++ if (dpaa2_eth_is_type_phy(priv)) { ++ err = phylink_mii_ioctl(priv->mac->phylink, rq, cmd); ++ mutex_unlock(&priv->mac_lock); ++ return err; ++ } ++ ++ mutex_unlock(&priv->mac_lock); + + return -EOPNOTSUPP; + } +@@ -4453,7 +4473,9 @@ static int dpaa2_eth_connect_mac(struct + goto err_close_mac; + } + ++ mutex_lock(&priv->mac_lock); + priv->mac = mac; ++ mutex_unlock(&priv->mac_lock); + + return 0; + +@@ -4466,9 +4488,12 @@ err_free_mac: + + static void dpaa2_eth_disconnect_mac(struct dpaa2_eth_priv *priv) + { +- struct dpaa2_mac *mac = priv->mac; ++ struct dpaa2_mac *mac; + ++ mutex_lock(&priv->mac_lock); ++ mac = priv->mac; + priv->mac = NULL; ++ mutex_unlock(&priv->mac_lock); + + if (!mac) + return; +@@ -4487,6 +4512,7 @@ static irqreturn_t dpni_irq0_handler_thr + struct fsl_mc_device *dpni_dev = to_fsl_mc_device(dev); + struct net_device *net_dev = dev_get_drvdata(dev); + struct dpaa2_eth_priv *priv = netdev_priv(net_dev); ++ bool had_mac; + int err; + + err = dpni_get_irq_status(dpni_dev->mc_io, 0, dpni_dev->mc_handle, +@@ -4504,7 +4530,12 @@ static irqreturn_t dpni_irq0_handler_thr + dpaa2_eth_update_tx_fqids(priv); + + rtnl_lock(); +- if (dpaa2_eth_has_mac(priv)) ++ /* We can avoid locking because the "endpoint changed" IRQ ++ * handler is the only one who changes priv->mac at runtime, ++ * so we are not racing with anyone. ++ */ ++ had_mac = !!priv->mac; ++ if (had_mac) + dpaa2_eth_disconnect_mac(priv); + else + dpaa2_eth_connect_mac(priv); +@@ -4605,6 +4636,8 @@ static int dpaa2_eth_probe(struct fsl_mc + priv = netdev_priv(net_dev); + priv->net_dev = net_dev; + ++ mutex_init(&priv->mac_lock); ++ + priv->iommu_domain = iommu_get_domain_for_dev(dev); + + priv->tx_tstamp_type = HWTSTAMP_TX_OFF; +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h +@@ -580,6 +580,8 @@ struct dpaa2_eth_priv { + #endif + + struct dpaa2_mac *mac; ++ /* Serializes changes to priv->mac */ ++ struct mutex mac_lock; + struct workqueue_struct *dpaa2_ptp_wq; + struct work_struct tx_onestep_tstamp; + struct sk_buff_head tx_skbs; +@@ -733,11 +735,15 @@ static inline unsigned int dpaa2_eth_rx_ + + static inline bool dpaa2_eth_is_type_phy(struct dpaa2_eth_priv *priv) + { ++ lockdep_assert_held(&priv->mac_lock); ++ + return dpaa2_mac_is_type_phy(priv->mac); + } + + static inline bool dpaa2_eth_has_mac(struct dpaa2_eth_priv *priv) + { ++ lockdep_assert_held(&priv->mac_lock); ++ + return priv->mac ? true : false; + } + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c +@@ -86,11 +86,16 @@ static void dpaa2_eth_get_drvinfo(struct + static int dpaa2_eth_nway_reset(struct net_device *net_dev) + { + struct dpaa2_eth_priv *priv = netdev_priv(net_dev); ++ int err = -EOPNOTSUPP; ++ ++ mutex_lock(&priv->mac_lock); + + if (dpaa2_eth_is_type_phy(priv)) +- return phylink_ethtool_nway_reset(priv->mac->phylink); ++ err = phylink_ethtool_nway_reset(priv->mac->phylink); ++ ++ mutex_unlock(&priv->mac_lock); + +- return -EOPNOTSUPP; ++ return err; + } + + static int +@@ -98,10 +103,18 @@ dpaa2_eth_get_link_ksettings(struct net_ + struct ethtool_link_ksettings *link_settings) + { + struct dpaa2_eth_priv *priv = netdev_priv(net_dev); ++ int err; + +- if (dpaa2_eth_is_type_phy(priv)) +- return phylink_ethtool_ksettings_get(priv->mac->phylink, +- link_settings); ++ mutex_lock(&priv->mac_lock); ++ ++ if (dpaa2_eth_is_type_phy(priv)) { ++ err = phylink_ethtool_ksettings_get(priv->mac->phylink, ++ link_settings); ++ mutex_unlock(&priv->mac_lock); ++ return err; ++ } ++ ++ mutex_unlock(&priv->mac_lock); + + link_settings->base.autoneg = AUTONEG_DISABLE; + if (!(priv->link_state.options & DPNI_LINK_OPT_HALF_DUPLEX)) +@@ -116,11 +129,17 @@ dpaa2_eth_set_link_ksettings(struct net_ + const struct ethtool_link_ksettings *link_settings) + { + struct dpaa2_eth_priv *priv = netdev_priv(net_dev); ++ int err = -EOPNOTSUPP; + +- if (!dpaa2_eth_is_type_phy(priv)) +- return -EOPNOTSUPP; ++ mutex_lock(&priv->mac_lock); ++ ++ if (dpaa2_eth_is_type_phy(priv)) ++ err = phylink_ethtool_ksettings_set(priv->mac->phylink, ++ link_settings); + +- return phylink_ethtool_ksettings_set(priv->mac->phylink, link_settings); ++ mutex_unlock(&priv->mac_lock); ++ ++ return err; + } + + static void dpaa2_eth_get_pauseparam(struct net_device *net_dev, +@@ -129,11 +148,16 @@ static void dpaa2_eth_get_pauseparam(str + struct dpaa2_eth_priv *priv = netdev_priv(net_dev); + u64 link_options = priv->link_state.options; + ++ mutex_lock(&priv->mac_lock); ++ + if (dpaa2_eth_is_type_phy(priv)) { + phylink_ethtool_get_pauseparam(priv->mac->phylink, pause); ++ mutex_unlock(&priv->mac_lock); + return; + } + ++ mutex_unlock(&priv->mac_lock); ++ + pause->rx_pause = dpaa2_eth_rx_pause_enabled(link_options); + pause->tx_pause = dpaa2_eth_tx_pause_enabled(link_options); + pause->autoneg = AUTONEG_DISABLE; +@@ -152,9 +176,17 @@ static int dpaa2_eth_set_pauseparam(stru + return -EOPNOTSUPP; + } + +- if (dpaa2_eth_is_type_phy(priv)) +- return phylink_ethtool_set_pauseparam(priv->mac->phylink, +- pause); ++ mutex_lock(&priv->mac_lock); ++ ++ if (dpaa2_eth_is_type_phy(priv)) { ++ err = phylink_ethtool_set_pauseparam(priv->mac->phylink, ++ pause); ++ mutex_unlock(&priv->mac_lock); ++ return err; ++ } ++ ++ mutex_unlock(&priv->mac_lock); ++ + if (pause->autoneg) + return -EOPNOTSUPP; + +@@ -309,8 +341,12 @@ static void dpaa2_eth_get_ethtool_stats( + } + *(data + i++) = buf_cnt; + ++ mutex_lock(&priv->mac_lock); ++ + if (dpaa2_eth_has_mac(priv)) + dpaa2_mac_get_ethtool_stats(priv->mac, data + i); ++ ++ mutex_unlock(&priv->mac_lock); + } + + static int dpaa2_eth_prep_eth_rule(struct ethhdr *eth_value, struct ethhdr *eth_mask, diff --git a/target/linux/armvirt/patches-6.1/701-v6.2-0011-net-dpaa2-switch-serialize-changes-to-priv-mac-with.patch b/target/linux/armvirt/patches-6.1/701-v6.2-0011-net-dpaa2-switch-serialize-changes-to-priv-mac-with.patch new file mode 100644 index 00000000000..7ea446516bf --- /dev/null +++ b/target/linux/armvirt/patches-6.1/701-v6.2-0011-net-dpaa2-switch-serialize-changes-to-priv-mac-with.patch @@ -0,0 +1,203 @@ +From 80d12452a5f160c39d63efc1be07df36f9d07133 Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean +Date: Tue, 29 Nov 2022 16:12:20 +0200 +Subject: [PATCH 13/14] net: dpaa2-switch: serialize changes to priv->mac with + a mutex + +The dpaa2-switch driver uses a DPMAC in the same way as the dpaa2-eth +driver, so we need to duplicate the locking solution established by the +previous change to the switch driver as well. + +Signed-off-by: Vladimir Oltean +Reviewed-by: Ioana Ciornei +Tested-by: Ioana Ciornei +Signed-off-by: Paolo Abeni +--- + .../freescale/dpaa2/dpaa2-switch-ethtool.c | 32 +++++++++++++++---- + .../ethernet/freescale/dpaa2/dpaa2-switch.c | 31 ++++++++++++++++-- + .../ethernet/freescale/dpaa2/dpaa2-switch.h | 2 ++ + 3 files changed, 55 insertions(+), 10 deletions(-) + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-ethtool.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-ethtool.c +@@ -60,11 +60,18 @@ dpaa2_switch_get_link_ksettings(struct n + { + struct ethsw_port_priv *port_priv = netdev_priv(netdev); + struct dpsw_link_state state = {0}; +- int err = 0; ++ int err; + +- if (dpaa2_switch_port_is_type_phy(port_priv)) +- return phylink_ethtool_ksettings_get(port_priv->mac->phylink, +- link_ksettings); ++ mutex_lock(&port_priv->mac_lock); ++ ++ if (dpaa2_switch_port_is_type_phy(port_priv)) { ++ err = phylink_ethtool_ksettings_get(port_priv->mac->phylink, ++ link_ksettings); ++ mutex_unlock(&port_priv->mac_lock); ++ return err; ++ } ++ ++ mutex_unlock(&port_priv->mac_lock); + + err = dpsw_if_get_link_state(port_priv->ethsw_data->mc_io, 0, + port_priv->ethsw_data->dpsw_handle, +@@ -99,9 +106,16 @@ dpaa2_switch_set_link_ksettings(struct n + bool if_running; + int err = 0, ret; + +- if (dpaa2_switch_port_is_type_phy(port_priv)) +- return phylink_ethtool_ksettings_set(port_priv->mac->phylink, +- link_ksettings); ++ mutex_lock(&port_priv->mac_lock); ++ ++ if (dpaa2_switch_port_is_type_phy(port_priv)) { ++ err = phylink_ethtool_ksettings_set(port_priv->mac->phylink, ++ link_ksettings); ++ mutex_unlock(&port_priv->mac_lock); ++ return err; ++ } ++ ++ mutex_unlock(&port_priv->mac_lock); + + /* Interface needs to be down to change link settings */ + if_running = netif_running(netdev); +@@ -189,8 +203,12 @@ static void dpaa2_switch_ethtool_get_sta + dpaa2_switch_ethtool_counters[i].name, err); + } + ++ mutex_lock(&port_priv->mac_lock); ++ + if (dpaa2_switch_port_has_mac(port_priv)) + dpaa2_mac_get_ethtool_stats(port_priv->mac, data + i); ++ ++ mutex_unlock(&port_priv->mac_lock); + } + + const struct ethtool_ops dpaa2_switch_port_ethtool_ops = { +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c +@@ -603,8 +603,11 @@ static int dpaa2_switch_port_link_state_ + + /* When we manage the MAC/PHY using phylink there is no need + * to manually update the netif_carrier. ++ * We can avoid locking because we are called from the "link changed" ++ * IRQ handler, which is the same as the "endpoint changed" IRQ handler ++ * (the writer to port_priv->mac), so we cannot race with it. + */ +- if (dpaa2_switch_port_is_type_phy(port_priv)) ++ if (dpaa2_mac_is_type_phy(port_priv->mac)) + return 0; + + /* Interrupts are received even though no one issued an 'ifconfig up' +@@ -684,6 +687,8 @@ static int dpaa2_switch_port_open(struct + struct ethsw_core *ethsw = port_priv->ethsw_data; + int err; + ++ mutex_lock(&port_priv->mac_lock); ++ + if (!dpaa2_switch_port_is_type_phy(port_priv)) { + /* Explicitly set carrier off, otherwise + * netif_carrier_ok() will return true and cause 'ip link show' +@@ -697,6 +702,7 @@ static int dpaa2_switch_port_open(struct + port_priv->ethsw_data->dpsw_handle, + port_priv->idx); + if (err) { ++ mutex_unlock(&port_priv->mac_lock); + netdev_err(netdev, "dpsw_if_enable err %d\n", err); + return err; + } +@@ -706,6 +712,8 @@ static int dpaa2_switch_port_open(struct + if (dpaa2_switch_port_is_type_phy(port_priv)) + dpaa2_mac_start(port_priv->mac); + ++ mutex_unlock(&port_priv->mac_lock); ++ + return 0; + } + +@@ -715,6 +723,8 @@ static int dpaa2_switch_port_stop(struct + struct ethsw_core *ethsw = port_priv->ethsw_data; + int err; + ++ mutex_lock(&port_priv->mac_lock); ++ + if (dpaa2_switch_port_is_type_phy(port_priv)) { + dpaa2_mac_stop(port_priv->mac); + } else { +@@ -722,6 +732,8 @@ static int dpaa2_switch_port_stop(struct + netif_carrier_off(netdev); + } + ++ mutex_unlock(&port_priv->mac_lock); ++ + err = dpsw_if_disable(port_priv->ethsw_data->mc_io, 0, + port_priv->ethsw_data->dpsw_handle, + port_priv->idx); +@@ -1461,7 +1473,9 @@ static int dpaa2_switch_port_connect_mac + } + } + ++ mutex_lock(&port_priv->mac_lock); + port_priv->mac = mac; ++ mutex_unlock(&port_priv->mac_lock); + + return 0; + +@@ -1474,9 +1488,12 @@ err_free_mac: + + static void dpaa2_switch_port_disconnect_mac(struct ethsw_port_priv *port_priv) + { +- struct dpaa2_mac *mac = port_priv->mac; ++ struct dpaa2_mac *mac; + ++ mutex_lock(&port_priv->mac_lock); ++ mac = port_priv->mac; + port_priv->mac = NULL; ++ mutex_unlock(&port_priv->mac_lock); + + if (!mac) + return; +@@ -1495,6 +1512,7 @@ static irqreturn_t dpaa2_switch_irq0_han + struct ethsw_port_priv *port_priv; + u32 status = ~0; + int err, if_id; ++ bool had_mac; + + err = dpsw_get_irq_status(ethsw->mc_io, 0, ethsw->dpsw_handle, + DPSW_IRQ_INDEX_IF, &status); +@@ -1513,7 +1531,12 @@ static irqreturn_t dpaa2_switch_irq0_han + + if (status & DPSW_IRQ_EVENT_ENDPOINT_CHANGED) { + rtnl_lock(); +- if (dpaa2_switch_port_has_mac(port_priv)) ++ /* We can avoid locking because the "endpoint changed" IRQ ++ * handler is the only one who changes priv->mac at runtime, ++ * so we are not racing with anyone. ++ */ ++ had_mac = !!port_priv->mac; ++ if (had_mac) + dpaa2_switch_port_disconnect_mac(port_priv); + else + dpaa2_switch_port_connect_mac(port_priv); +@@ -3256,6 +3279,8 @@ static int dpaa2_switch_probe_port(struc + port_priv->netdev = port_netdev; + port_priv->ethsw_data = ethsw; + ++ mutex_init(&port_priv->mac_lock); ++ + port_priv->idx = port_idx; + port_priv->stp_state = BR_STATE_FORWARDING; + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.h ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.h +@@ -161,6 +161,8 @@ struct ethsw_port_priv { + + struct dpaa2_switch_filter_block *filter_block; + struct dpaa2_mac *mac; ++ /* Protects against changes to port_priv->mac */ ++ struct mutex mac_lock; + }; + + /* Switch data */ diff --git a/target/linux/armvirt/patches-6.1/701-v6.2-0012-net-dpaa2-mac-move-rtnl_lock-only-around-phylink.patch b/target/linux/armvirt/patches-6.1/701-v6.2-0012-net-dpaa2-mac-move-rtnl_lock-only-around-phylink.patch new file mode 100644 index 00000000000..976c2a03354 --- /dev/null +++ b/target/linux/armvirt/patches-6.1/701-v6.2-0012-net-dpaa2-mac-move-rtnl_lock-only-around-phylink.patch @@ -0,0 +1,113 @@ +From 4ea2faf5bb13d9ba9f07e996d495c4cbe34a4236 Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean +Date: Tue, 29 Nov 2022 16:12:21 +0200 +Subject: [PATCH 14/14] net: dpaa2-mac: move rtnl_lock() only around + phylink_{,dis}connect_phy() + +After the introduction of a private mac_lock that serializes access to +priv->mac (and port_priv->mac in the switch), the only remaining purpose +of rtnl_lock() is to satisfy the locking requirements of +phylink_fwnode_phy_connect() and phylink_disconnect_phy(). + +But the functions these live in, dpaa2_mac_connect() and +dpaa2_mac_disconnect(), have contradictory locking requirements. +While phylink_fwnode_phy_connect() wants rtnl_lock() to be held, +phylink_create() wants it to not be held. + +Move the rtnl_lock() from top-level (in the dpaa2-eth and dpaa2-switch +drivers) to only surround the phylink calls that require it, in the +dpaa2-mac library code. + +This is possible because dpaa2_mac_connect() and dpaa2_mac_disconnect() +run unlocked, and there isn't any danger of an AB/BA deadlock between +the rtnl_mutex and other private locks. + +Signed-off-by: Vladimir Oltean +Reviewed-by: Ioana Ciornei +Tested-by: Ioana Ciornei +Signed-off-by: Paolo Abeni +--- + drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 4 ---- + drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c | 5 +++++ + drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c | 4 ---- + 3 files changed, 5 insertions(+), 8 deletions(-) + +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +@@ -4529,7 +4529,6 @@ static irqreturn_t dpni_irq0_handler_thr + dpaa2_eth_set_mac_addr(netdev_priv(net_dev)); + dpaa2_eth_update_tx_fqids(priv); + +- rtnl_lock(); + /* We can avoid locking because the "endpoint changed" IRQ + * handler is the only one who changes priv->mac at runtime, + * so we are not racing with anyone. +@@ -4539,7 +4538,6 @@ static irqreturn_t dpni_irq0_handler_thr + dpaa2_eth_disconnect_mac(priv); + else + dpaa2_eth_connect_mac(priv); +- rtnl_unlock(); + } + + return IRQ_HANDLED; +@@ -4856,9 +4854,7 @@ static int dpaa2_eth_remove(struct fsl_m + else + fsl_mc_free_irqs(ls_dev); + +- rtnl_lock(); + dpaa2_eth_disconnect_mac(priv); +- rtnl_unlock(); + dpaa2_eth_free_rings(priv); + free_percpu(priv->fd); + free_percpu(priv->sgt_cache); +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c +@@ -428,7 +428,9 @@ int dpaa2_mac_connect(struct dpaa2_mac * + } + mac->phylink = phylink; + ++ rtnl_lock(); + err = phylink_fwnode_phy_connect(mac->phylink, dpmac_node, 0); ++ rtnl_unlock(); + if (err) { + netdev_err(net_dev, "phylink_fwnode_phy_connect() = %d\n", err); + goto err_phylink_destroy; +@@ -446,7 +448,10 @@ err_pcs_destroy: + + void dpaa2_mac_disconnect(struct dpaa2_mac *mac) + { ++ rtnl_lock(); + phylink_disconnect_phy(mac->phylink); ++ rtnl_unlock(); ++ + phylink_destroy(mac->phylink); + dpaa2_pcs_destroy(mac); + of_phy_put(mac->serdes_phy); +--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c ++++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c +@@ -1530,7 +1530,6 @@ static irqreturn_t dpaa2_switch_irq0_han + } + + if (status & DPSW_IRQ_EVENT_ENDPOINT_CHANGED) { +- rtnl_lock(); + /* We can avoid locking because the "endpoint changed" IRQ + * handler is the only one who changes priv->mac at runtime, + * so we are not racing with anyone. +@@ -1540,7 +1539,6 @@ static irqreturn_t dpaa2_switch_irq0_han + dpaa2_switch_port_disconnect_mac(port_priv); + else + dpaa2_switch_port_connect_mac(port_priv); +- rtnl_unlock(); + } + + out: +@@ -2958,9 +2956,7 @@ static void dpaa2_switch_remove_port(str + { + struct ethsw_port_priv *port_priv = ethsw->ports[port_idx]; + +- rtnl_lock(); + dpaa2_switch_port_disconnect_mac(port_priv); +- rtnl_unlock(); + free_netdev(port_priv->netdev); + ethsw->ports[port_idx] = NULL; + } -- 2.30.2