1 From 5e448a17dfa2e95166534df7f677a3694ef6187d Mon Sep 17 00:00:00 2001
2 From: Vladimir Oltean <vladimir.oltean@nxp.com>
3 Date: Tue, 29 Nov 2022 16:12:19 +0200
4 Subject: [PATCH 12/14] net: dpaa2-eth: serialize changes to priv->mac with a
7 The dpaa2 architecture permits dynamic connections between objects on
8 the fsl-mc bus, specifically between a DPNI object (represented by a
9 struct net_device) and a DPMAC object (represented by a struct phylink).
11 The DPNI driver is notified when those connections are created/broken
12 through the dpni_irq0_handler_thread() method. To ensure that ethtool
13 operations, as well as netdev up/down operations serialize with the
14 connection/disconnection of the DPNI with a DPMAC,
15 dpni_irq0_handler_thread() takes the rtnl_lock() to block those other
16 operations from taking place.
18 There is code called by dpaa2_mac_connect() which wants to acquire the
19 rtnl_mutex once again, see phylink_create() -> phylink_register_sfp() ->
20 sfp_bus_add_upstream() -> rtnl_lock(). So the strategy doesn't quite
21 work out, even though it's fairly simple.
23 Create a different strategy, where all code paths in the dpaa2-eth
24 driver access priv->mac only while they are holding priv->mac_lock.
25 The phylink instance is not created or connected to the PHY under the
26 priv->mac_lock, but only assigned to priv->mac then. This will eliminate
27 the reliance on the rtnl_mutex.
29 Add lockdep annotations and put comments where holding the lock is not
30 necessary, and priv->mac can be dereferenced freely.
32 Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
33 Reviewed-by: Ioana Ciornei <ioana.ciornei@nxp.com>
34 Tested-by: Ioana Ciornei <ioana.ciornei@nxp.com>
35 Signed-off-by: Paolo Abeni <pabeni@redhat.com>
37 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 43 ++++++++++++--
38 .../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 6 ++
39 .../ethernet/freescale/dpaa2/dpaa2-ethtool.c | 58 +++++++++++++++----
40 3 files changed, 91 insertions(+), 16 deletions(-)
42 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
43 +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
44 @@ -2021,8 +2021,11 @@ static int dpaa2_eth_link_state_update(s
46 /* When we manage the MAC/PHY using phylink there is no need
47 * to manually update the netif_carrier.
48 + * We can avoid locking because we are called from the "link changed"
49 + * IRQ handler, which is the same as the "endpoint changed" IRQ handler
50 + * (the writer to priv->mac), so we cannot race with it.
52 - if (dpaa2_eth_is_type_phy(priv))
53 + if (dpaa2_mac_is_type_phy(priv->mac))
56 /* Chech link state; speed / duplex changes are not treated yet */
57 @@ -2061,6 +2064,8 @@ static int dpaa2_eth_open(struct net_dev
58 priv->dpbp_dev->obj_desc.id, priv->bpid);
61 + mutex_lock(&priv->mac_lock);
63 if (!dpaa2_eth_is_type_phy(priv)) {
64 /* We'll only start the txqs when the link is actually ready;
65 * make sure we don't race against the link up notification,
66 @@ -2079,6 +2084,7 @@ static int dpaa2_eth_open(struct net_dev
68 err = dpni_enable(priv->mc_io, 0, priv->mc_token);
70 + mutex_unlock(&priv->mac_lock);
71 netdev_err(net_dev, "dpni_enable() failed\n");
74 @@ -2086,6 +2092,8 @@ static int dpaa2_eth_open(struct net_dev
75 if (dpaa2_eth_is_type_phy(priv))
76 dpaa2_mac_start(priv->mac);
78 + mutex_unlock(&priv->mac_lock);
83 @@ -2157,6 +2165,8 @@ static int dpaa2_eth_stop(struct net_dev
87 + mutex_lock(&priv->mac_lock);
89 if (dpaa2_eth_is_type_phy(priv)) {
90 dpaa2_mac_stop(priv->mac);
92 @@ -2164,6 +2174,8 @@ static int dpaa2_eth_stop(struct net_dev
93 netif_carrier_off(net_dev);
96 + mutex_unlock(&priv->mac_lock);
98 /* On dpni_disable(), the MC firmware will:
99 * - stop MAC Rx and wait for all Rx frames to be enqueued to software
100 * - cut off WRIOP dequeues from egress FQs and wait until transmission
101 @@ -2489,12 +2501,20 @@ static int dpaa2_eth_ts_ioctl(struct net
102 static int dpaa2_eth_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
104 struct dpaa2_eth_priv *priv = netdev_priv(dev);
107 if (cmd == SIOCSHWTSTAMP)
108 return dpaa2_eth_ts_ioctl(dev, rq, cmd);
110 - if (dpaa2_eth_is_type_phy(priv))
111 - return phylink_mii_ioctl(priv->mac->phylink, rq, cmd);
112 + mutex_lock(&priv->mac_lock);
114 + if (dpaa2_eth_is_type_phy(priv)) {
115 + err = phylink_mii_ioctl(priv->mac->phylink, rq, cmd);
116 + mutex_unlock(&priv->mac_lock);
120 + mutex_unlock(&priv->mac_lock);
124 @@ -4458,7 +4478,9 @@ static int dpaa2_eth_connect_mac(struct
128 + mutex_lock(&priv->mac_lock);
130 + mutex_unlock(&priv->mac_lock);
134 @@ -4471,9 +4493,12 @@ err_free_mac:
136 static void dpaa2_eth_disconnect_mac(struct dpaa2_eth_priv *priv)
138 - struct dpaa2_mac *mac = priv->mac;
139 + struct dpaa2_mac *mac;
141 + mutex_lock(&priv->mac_lock);
144 + mutex_unlock(&priv->mac_lock);
148 @@ -4492,6 +4517,7 @@ static irqreturn_t dpni_irq0_handler_thr
149 struct fsl_mc_device *dpni_dev = to_fsl_mc_device(dev);
150 struct net_device *net_dev = dev_get_drvdata(dev);
151 struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
155 err = dpni_get_irq_status(dpni_dev->mc_io, 0, dpni_dev->mc_handle,
156 @@ -4509,7 +4535,12 @@ static irqreturn_t dpni_irq0_handler_thr
157 dpaa2_eth_update_tx_fqids(priv);
160 - if (dpaa2_eth_has_mac(priv))
161 + /* We can avoid locking because the "endpoint changed" IRQ
162 + * handler is the only one who changes priv->mac at runtime,
163 + * so we are not racing with anyone.
165 + had_mac = !!priv->mac;
167 dpaa2_eth_disconnect_mac(priv);
169 dpaa2_eth_connect_mac(priv);
170 @@ -4610,6 +4641,8 @@ static int dpaa2_eth_probe(struct fsl_mc
171 priv = netdev_priv(net_dev);
172 priv->net_dev = net_dev;
174 + mutex_init(&priv->mac_lock);
176 priv->iommu_domain = iommu_get_domain_for_dev(dev);
178 priv->tx_tstamp_type = HWTSTAMP_TX_OFF;
179 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
180 +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
181 @@ -580,6 +580,8 @@ struct dpaa2_eth_priv {
184 struct dpaa2_mac *mac;
185 + /* Serializes changes to priv->mac */
186 + struct mutex mac_lock;
187 struct workqueue_struct *dpaa2_ptp_wq;
188 struct work_struct tx_onestep_tstamp;
189 struct sk_buff_head tx_skbs;
190 @@ -733,11 +735,15 @@ static inline unsigned int dpaa2_eth_rx_
192 static inline bool dpaa2_eth_is_type_phy(struct dpaa2_eth_priv *priv)
194 + lockdep_assert_held(&priv->mac_lock);
196 return dpaa2_mac_is_type_phy(priv->mac);
199 static inline bool dpaa2_eth_has_mac(struct dpaa2_eth_priv *priv)
201 + lockdep_assert_held(&priv->mac_lock);
203 return priv->mac ? true : false;
206 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
207 +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
208 @@ -86,11 +86,16 @@ static void dpaa2_eth_get_drvinfo(struct
209 static int dpaa2_eth_nway_reset(struct net_device *net_dev)
211 struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
212 + int err = -EOPNOTSUPP;
214 + mutex_lock(&priv->mac_lock);
216 if (dpaa2_eth_is_type_phy(priv))
217 - return phylink_ethtool_nway_reset(priv->mac->phylink);
218 + err = phylink_ethtool_nway_reset(priv->mac->phylink);
220 + mutex_unlock(&priv->mac_lock);
222 - return -EOPNOTSUPP;
227 @@ -98,10 +103,18 @@ dpaa2_eth_get_link_ksettings(struct net_
228 struct ethtool_link_ksettings *link_settings)
230 struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
233 - if (dpaa2_eth_is_type_phy(priv))
234 - return phylink_ethtool_ksettings_get(priv->mac->phylink,
236 + mutex_lock(&priv->mac_lock);
238 + if (dpaa2_eth_is_type_phy(priv)) {
239 + err = phylink_ethtool_ksettings_get(priv->mac->phylink,
241 + mutex_unlock(&priv->mac_lock);
245 + mutex_unlock(&priv->mac_lock);
247 link_settings->base.autoneg = AUTONEG_DISABLE;
248 if (!(priv->link_state.options & DPNI_LINK_OPT_HALF_DUPLEX))
249 @@ -116,11 +129,17 @@ dpaa2_eth_set_link_ksettings(struct net_
250 const struct ethtool_link_ksettings *link_settings)
252 struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
253 + int err = -EOPNOTSUPP;
255 - if (!dpaa2_eth_is_type_phy(priv))
256 - return -EOPNOTSUPP;
257 + mutex_lock(&priv->mac_lock);
259 + if (dpaa2_eth_is_type_phy(priv))
260 + err = phylink_ethtool_ksettings_set(priv->mac->phylink,
263 - return phylink_ethtool_ksettings_set(priv->mac->phylink, link_settings);
264 + mutex_unlock(&priv->mac_lock);
269 static void dpaa2_eth_get_pauseparam(struct net_device *net_dev,
270 @@ -129,11 +148,16 @@ static void dpaa2_eth_get_pauseparam(str
271 struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
272 u64 link_options = priv->link_state.options;
274 + mutex_lock(&priv->mac_lock);
276 if (dpaa2_eth_is_type_phy(priv)) {
277 phylink_ethtool_get_pauseparam(priv->mac->phylink, pause);
278 + mutex_unlock(&priv->mac_lock);
282 + mutex_unlock(&priv->mac_lock);
284 pause->rx_pause = dpaa2_eth_rx_pause_enabled(link_options);
285 pause->tx_pause = dpaa2_eth_tx_pause_enabled(link_options);
286 pause->autoneg = AUTONEG_DISABLE;
287 @@ -152,9 +176,17 @@ static int dpaa2_eth_set_pauseparam(stru
291 - if (dpaa2_eth_is_type_phy(priv))
292 - return phylink_ethtool_set_pauseparam(priv->mac->phylink,
294 + mutex_lock(&priv->mac_lock);
296 + if (dpaa2_eth_is_type_phy(priv)) {
297 + err = phylink_ethtool_set_pauseparam(priv->mac->phylink,
299 + mutex_unlock(&priv->mac_lock);
303 + mutex_unlock(&priv->mac_lock);
308 @@ -307,8 +339,12 @@ static void dpaa2_eth_get_ethtool_stats(
310 *(data + i++) = buf_cnt;
312 + mutex_lock(&priv->mac_lock);
314 if (dpaa2_eth_has_mac(priv))
315 dpaa2_mac_get_ethtool_stats(priv->mac, data + i);
317 + mutex_unlock(&priv->mac_lock);
320 static int dpaa2_eth_prep_eth_rule(struct ethhdr *eth_value, struct ethhdr *eth_mask,