Some registers may be modified by parallel execution contexts and
require protections to prevent corruption.
A review of the driver revealed the need for these additional
protections.
Doug Berger (3):
net: bcmgenet: synchronize EXT_RGMII_OOB_CTRL access
net: bcmgenet: synchronize use of bcmgenet_set_rx_mode()
net: bcmgenet: synchronize UMAC_CMD access
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 14 +++++++++++++-
drivers/net/ethernet/broadcom/genet/bcmgenet.h | 2 ++
drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c | 6 ++++++
drivers/net/ethernet/broadcom/genet/bcmmii.c | 4 ++++
4 files changed, 25 insertions(+), 1 deletion(-)
--
These commits are dependent on the previously submitted:
[PATCH stable 5.4 0/2] net: bcmgenet: revisit MAC reset
2.34.1
[ Upstream commit d85cf67a339685beae1d0aee27b7f61da95455be ]
The EXT_RGMII_OOB_CTRL register can be written from different
contexts. It is predominantly written from the adjust_link
handler which is synchronized by the phydev->lock, but can
also be written from a different context when configuring the
mii in bcmgenet_mii_config().
The chances of contention are quite low, but it is conceivable
that adjust_link could occur during resume when WoL is enabled
so use the phydev->lock synchronizer in bcmgenet_mii_config()
to be sure.
Fixes: afe3f907d20f ("net: bcmgenet: power on MII block for all MII modes")
Cc: [email protected]
Signed-off-by: Doug Berger <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
---
drivers/net/ethernet/broadcom/genet/bcmmii.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 56fad34461f7..d0b59d1f6c73 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -269,6 +269,7 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
* block for the interface to work
*/
if (priv->ext_phy) {
+ mutex_lock(&phydev->lock);
reg = bcmgenet_ext_readl(priv, EXT_RGMII_OOB_CTRL);
reg |= id_mode_dis;
if (GENET_IS_V1(priv) || GENET_IS_V2(priv) || GENET_IS_V3(priv))
@@ -276,6 +277,7 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
else
reg |= RGMII_MODE_EN;
bcmgenet_ext_writel(priv, reg, EXT_RGMII_OOB_CTRL);
+ mutex_unlock(&phydev->lock);
}
if (init)
--
2.34.1
[ Upstream commit 2dbe5f19368caae63b1f59f5bc2af78c7d522b3a ]
The ndo_set_rx_mode function is synchronized with the
netif_addr_lock spinlock and BHs disabled. Since this
function is also invoked directly from the driver the
same synchronization should be applied.
Fixes: 72f96347628e ("net: bcmgenet: set Rx mode before starting netif")
Cc: [email protected]
Signed-off-by: Doug Berger <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index bf52bd643846..e34df8da65e7 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2874,7 +2874,9 @@ static void bcmgenet_netif_start(struct net_device *dev)
struct bcmgenet_priv *priv = netdev_priv(dev);
/* Start the network engine */
+ netif_addr_lock_bh(dev);
bcmgenet_set_rx_mode(dev);
+ netif_addr_unlock_bh(dev);
bcmgenet_enable_rx_napi(priv);
umac_enable_set(priv, CMD_TX_EN | CMD_RX_EN, true);
--
2.34.1
[ Upstream commit 0d5e2a82232605b337972fb2c7d0cbc46898aca1 ]
The UMAC_CMD register is written from different execution
contexts and has insufficient synchronization protections to
prevent possible corruption. Of particular concern are the
acceses from the phy_device delayed work context used by the
adjust_link call and the BH context that may be used by the
ndo_set_rx_mode call.
A spinlock is added to the driver to protect contended register
accesses (i.e. reg_lock) and it is used to synchronize accesses
to UMAC_CMD.
Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
Cc: [email protected]
Signed-off-by: Doug Berger <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 12 +++++++++++-
drivers/net/ethernet/broadcom/genet/bcmgenet.h | 2 ++
drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c | 6 ++++++
drivers/net/ethernet/broadcom/genet/bcmmii.c | 2 ++
4 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index e34df8da65e7..da9df1d3662b 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1990,14 +1990,18 @@ static void umac_enable_set(struct bcmgenet_priv *priv, u32 mask, bool enable)
{
u32 reg;
+ spin_lock_bh(&priv->reg_lock);
reg = bcmgenet_umac_readl(priv, UMAC_CMD);
- if (reg & CMD_SW_RESET)
+ if (reg & CMD_SW_RESET) {
+ spin_unlock_bh(&priv->reg_lock);
return;
+ }
if (enable)
reg |= mask;
else
reg &= ~mask;
bcmgenet_umac_writel(priv, reg, UMAC_CMD);
+ spin_unlock_bh(&priv->reg_lock);
/* UniMAC stops on a packet boundary, wait for a full-size packet
* to be processed
@@ -2013,8 +2017,10 @@ static void reset_umac(struct bcmgenet_priv *priv)
udelay(10);
/* issue soft reset and disable MAC while updating its registers */
+ spin_lock_bh(&priv->reg_lock);
bcmgenet_umac_writel(priv, CMD_SW_RESET, UMAC_CMD);
udelay(2);
+ spin_unlock_bh(&priv->reg_lock);
}
static void bcmgenet_intr_disable(struct bcmgenet_priv *priv)
@@ -3140,16 +3146,19 @@ static void bcmgenet_set_rx_mode(struct net_device *dev)
* 3. The number of filters needed exceeds the number filters
* supported by the hardware.
*/
+ spin_lock(&priv->reg_lock);
reg = bcmgenet_umac_readl(priv, UMAC_CMD);
if ((dev->flags & (IFF_PROMISC | IFF_ALLMULTI)) ||
(nfilter > MAX_MDF_FILTER)) {
reg |= CMD_PROMISC;
bcmgenet_umac_writel(priv, reg, UMAC_CMD);
+ spin_unlock(&priv->reg_lock);
bcmgenet_umac_writel(priv, 0, UMAC_MDF_CTRL);
return;
} else {
reg &= ~CMD_PROMISC;
bcmgenet_umac_writel(priv, reg, UMAC_CMD);
+ spin_unlock(&priv->reg_lock);
}
/* update MDF filter */
@@ -3507,6 +3516,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
goto err;
}
+ spin_lock_init(&priv->reg_lock);
spin_lock_init(&priv->lock);
SET_NETDEV_DEV(dev, &pdev->dev);
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 29bf256d13f6..9efc503a9c8b 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -608,6 +608,8 @@ struct bcmgenet_rx_ring {
/* device context */
struct bcmgenet_priv {
void __iomem *base;
+ /* reg_lock: lock to serialize access to shared registers */
+ spinlock_t reg_lock;
enum bcmgenet_version version;
struct net_device *dev;
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c b/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c
index 8ebca6bf300e..973275d116b6 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c
@@ -133,6 +133,7 @@ int bcmgenet_wol_power_down_cfg(struct bcmgenet_priv *priv,
}
/* Can't suspend with WoL if MAC is still in reset */
+ spin_lock_bh(&priv->reg_lock);
reg = bcmgenet_umac_readl(priv, UMAC_CMD);
if (reg & CMD_SW_RESET)
reg &= ~CMD_SW_RESET;
@@ -140,6 +141,7 @@ int bcmgenet_wol_power_down_cfg(struct bcmgenet_priv *priv,
/* disable RX */
reg &= ~CMD_RX_EN;
bcmgenet_umac_writel(priv, reg, UMAC_CMD);
+ spin_unlock_bh(&priv->reg_lock);
mdelay(10);
reg = bcmgenet_umac_readl(priv, UMAC_MPD_CTRL);
@@ -163,6 +165,7 @@ int bcmgenet_wol_power_down_cfg(struct bcmgenet_priv *priv,
retries);
/* Enable CRC forward */
+ spin_lock_bh(&priv->reg_lock);
reg = bcmgenet_umac_readl(priv, UMAC_CMD);
priv->crc_fwd_en = 1;
reg |= CMD_CRC_FWD;
@@ -170,6 +173,7 @@ int bcmgenet_wol_power_down_cfg(struct bcmgenet_priv *priv,
/* Receiver must be enabled for WOL MP detection */
reg |= CMD_RX_EN;
bcmgenet_umac_writel(priv, reg, UMAC_CMD);
+ spin_unlock_bh(&priv->reg_lock);
return 0;
}
@@ -191,8 +195,10 @@ void bcmgenet_wol_power_up_cfg(struct bcmgenet_priv *priv,
bcmgenet_umac_writel(priv, reg, UMAC_MPD_CTRL);
/* Disable CRC Forward */
+ spin_lock_bh(&priv->reg_lock);
reg = bcmgenet_umac_readl(priv, UMAC_CMD);
reg &= ~CMD_CRC_FWD;
bcmgenet_umac_writel(priv, reg, UMAC_CMD);
+ spin_unlock_bh(&priv->reg_lock);
priv->crc_fwd_en = 0;
}
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index d0b59d1f6c73..bd532e5b9f73 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -91,6 +91,7 @@ void bcmgenet_mii_setup(struct net_device *dev)
reg |= RGMII_LINK;
bcmgenet_ext_writel(priv, reg, EXT_RGMII_OOB_CTRL);
+ spin_lock_bh(&priv->reg_lock);
reg = bcmgenet_umac_readl(priv, UMAC_CMD);
reg &= ~((CMD_SPEED_MASK << CMD_SPEED_SHIFT) |
CMD_HD_EN |
@@ -103,6 +104,7 @@ void bcmgenet_mii_setup(struct net_device *dev)
reg |= CMD_TX_EN | CMD_RX_EN;
}
bcmgenet_umac_writel(priv, reg, UMAC_CMD);
+ spin_unlock_bh(&priv->reg_lock);
priv->eee.eee_active = phy_init_eee(phydev, 0) >= 0;
bcmgenet_eee_enable_set(dev,
--
2.34.1
On Thu, May 16, 2024 at 04:01:48PM -0700, Doug Berger wrote:
> Some registers may be modified by parallel execution contexts and
> require protections to prevent corruption.
>
> A review of the driver revealed the need for these additional
> protections.
>
> Doug Berger (3):
> net: bcmgenet: synchronize EXT_RGMII_OOB_CTRL access
> net: bcmgenet: synchronize use of bcmgenet_set_rx_mode()
> net: bcmgenet: synchronize UMAC_CMD access
>
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 14 +++++++++++++-
> drivers/net/ethernet/broadcom/genet/bcmgenet.h | 2 ++
> drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c | 6 ++++++
> drivers/net/ethernet/broadcom/genet/bcmmii.c | 4 ++++
> 4 files changed, 25 insertions(+), 1 deletion(-)
>
> --
> These commits are dependent on the previously submitted:
> [PATCH stable 5.4 0/2] net: bcmgenet: revisit MAC reset
>
> 2.34.1
>
>
All now queued up, thanks.
greg k-h