net: phy: marvell: fix paged access races
authorRussell King <rmk+kernel@armlinux.org.uk>
Tue, 2 Jan 2018 10:58:48 +0000 (10:58 +0000)
committerDavid S. Miller <davem@davemloft.net>
Wed, 3 Jan 2018 16:00:23 +0000 (11:00 -0500)
commit424ca4c5512173e42b4086322dafd33ee882baf3
tree4573f372c623eeec4faa5b21b7dd32ebf2230e32
parent78ffc4acceff48522b92d8fbf8f4a0ffe78838b2
net: phy: marvell: fix paged access races

For paged accesses to be truely safe, we need to hold the bus lock to
prevent anyone else gaining access to the registers while we modify
them.

The phydev->lock mutex does not do this: userspace via the MII ioctl
can still sneak in and read or write any register while we are on a
different page, and the suspend/resume methods can be called by a
thread different to the thread polling the phy status.

Races have been observed with mvneta on SolidRun Clearfog with phylink,
particularly between the phylib worker reading the PHYs status, and
the thread resuming mvneta, calling phy_start() which then calls
through to m88e1121_config_aneg_rgmii_delays(), which tries to
read-modify-write the MSCR register:

CPU0 CPU1
marvell_read_status_page()
marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE)
...
m88e1121_config_aneg_rgmii_delays()
set_page(MII_MARVELL_MSCR_PAGE)
phy_read(phydev, MII_88E1121_PHY_MSCR_REG)
marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
...
phy_write(phydev, MII_88E1121_PHY_MSCR_REG)

The result of this is we end up writing the copper page register 21,
which causes the copper PHY to be disabled, and the link partner sees
the link immediately go down.

Solve this by taking the bus lock instead of the PHY lock, thereby
preventing other accesses to the PHY while we are accessing other PHY
pages.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/phy/marvell.c