From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Hauke Mehrtens <hauke@hauke-m.de>,
        "David S. Miller" <davem@davemloft.net>,
        Sasha Levin <sashal@kernel.org>
Subject: Re: [PATCH 5.4 169/355] net: dsa: lantiq_gswip: serialize access to
 the PCE table
Thread-Topic: [PATCH 5.4 169/355] net: dsa: lantiq_gswip: serialize access to
 the PCE table
Thread-Index: AQHX2kSnOkmxsoPWP0Cc60mbFnJbHqwFPqgA
Date:   Mon, 15 Nov 2021 23:36:23 +0000
Message-ID: <20211115233622.qdsnq72hlusszzxr@skbuf>
References: <20211115165313.549179499@linuxfoundation.org>
 <20211115165319.255917034@linuxfoundation.org>
In-Reply-To: <20211115165319.255917034@linuxfoundation.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?s9TZhN7X+WEC3Gf08uCgoPM9cFvaZNfIq2W7j9XUo4TLHvqC5LLQiGOeXY1a?=
 =?us-ascii?Q?tfXnsRAgQPada3eXP9hLv6HIyImpHvWALASLJUXS+4LSjJUTiD8euYR8SmO+?=
 =?us-ascii?Q?wsVe8G1KT8E51TigK4evaZwpSkdNZ1dK+4HpSH9bzQL/ofxoZPQ+yGIT+1Vr?=
 =?us-ascii?Q?Wt9MQVqqcDSYtiRILR0Sg1INra87weN03VPvQB2QmGOwR8lm8dMi4ieVNeOA?=
 =?us-ascii?Q?eYKFHMAItF5jZ2KIgOsdr+VCSOomJMS76TJWTA5LjKDeLryoOMMlHJ9htiPw?=
 =?us-ascii?Q?F2h0usxNNhpbb5ae7K5RM/2Z8hZPHr0cpr0KjxxtuYOaZwA+Oo1OQW7jDDWx?=
 =?us-ascii?Q?8jLGHlgK1I4qtZRmQtxJlEb7obsU1Nk3AWITkrXavqi33mHcZ/dhHYX1XHHA?=
 =?us-ascii?Q?VqOM+lLdJLIYYv70ein+4tPMJorCwM8QkmLcHH6yA5ta9Xz9CImxOO1FabXm?=
 =?us-ascii?Q?zPyDP2rT6QdnZc85oHhLkI51fKEQnXarKCyS18jqgoBLtn54myZQX6+evzUG?=
 =?us-ascii?Q?MRFHEh0sE1ZHiUFo5j0Vvl2sterbRWGsnX2tNuhn4xOeWiKEB140/1K1N6pn?=
 =?us-ascii?Q?QXuYZZhrfSfIZxp3Ka5yDC2E1336naEVafCpjCo2x5yOdA5QcXN2GLWLdfOg?=
 =?us-ascii?Q?bctE+lxoimFDK8Td5KwoqpIuDetRrbsMqauWKIXgMOl+s9ZyJFhzweUwZ71O?=
 =?us-ascii?Q?3XNCLWfhyt+I7oVWS39xZuaSVPysmHKM8gSV+wdqEdxZY3srGTRhHMr7X/hr?=
 =?us-ascii?Q?/RBiP+8MuN3MjbkFjB0swyGAQAMUdoOpc3+GEAUYk3lzS94DgwXzmTvaSIxo?=
 =?us-ascii?Q?OVF3QxBc8YsH5FNLjIMfZrv+osrhRLHjCMMlbdCW0OjqlH66yM9Yn74u5y4t?=
 =?us-ascii?Q?sW3lPG/TxyyBXz7z+ckwQhOtRMOIlrt7cbtsy0RtGApr0jJ27XBfS/M8fTMy?=
 =?us-ascii?Q?0R1suaU3hn7aTT/B33OhdlUqd0QDcDV7hxcgKf/lfe2CadDVcWQy5qCoGhi9?=
 =?us-ascii?Q?xK7+y/C70t8SHGrlb72RwLNX7PUAzP6Ze7Crs+nhBS60nAczEal9I/Ph6j+L?=
 =?us-ascii?Q?tSjnY+kqLd8zlF43KqkQd47K6wS1SU8C3+saBllowFrC+Ygh8x2zDtsshG9U?=
 =?us-ascii?Q?qsY6yehXx536t7B38ehxPuJztkS0aTuPKHP0CJGS/9ja4egk+ZH4ruzD0wA2?=
 =?us-ascii?Q?/QF88spWV5D5KIfrneoLprZXA6mln31dXE36QpG3wXowM0xx0XHts6M3iYPA?=
 =?us-ascii?Q?BgeUI6ULNwTFxCs8OtkMXLPDJ52ZbyxCd0iYg7j3MJ4cpmbPFPUBMCecjTN4?=
 =?us-ascii?Q?b2KveUgjWbkUW5z8YiBTnRyYI2DnwFxIm4F5yk9sGCBiCxb5EC+V6P+0rGit?=
 =?us-ascii?Q?lEzHbtCkFPbK0+0ZpCQwMijEMGZzpk6DANwDHMnTH9Zk8yxEEsAjKI9bjT+d?=
 =?us-ascii?Q?YLOpss/MB3GKM4mgwjlVic/LM+R5/jimMmq7o9ieVlYOa7mPAftyTq6OwkLm?=
 =?us-ascii?Q?UfIsn/OUhSER84rdPhDuISYgqX8Erelrnn5AnPO9pobQksrD1xYsuBNwBGAR?=
 =?us-ascii?Q?i9rb0RzAaNp1NkpEwVPq+oOcq5hz45ZiY8W7ENH4XpwBbh6KUBuiSi6QjgDz?=
 =?us-ascii?Q?IpwIcVdVzVpTHoV4ScEJr+E=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <CBAA8669196C554192E3D77D0EFDA869@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5136.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 960af84b-518e-4070-b023-08d9a890bc0a
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Nov 2021 23:36:23.2150
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: PGV/6NPjYJa4sUWQ+W7I9Ey3I1dtJuj3IlbZKLy4ivLkdU2rTS2DqhVug1SwXHtk0pL1xLTsjsY+H7Xj7n+Rnw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB5696
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Nov 15, 2021 at 06:01:33PM +0100, Greg Kroah-Hartman wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>=20
> [ Upstream commit 49753a75b9a32de4c0393bb8d1e51ea223fda8e4 ]
>=20
> Looking at the code, the GSWIP switch appears to hold bridging service
> structures (VLANs, FDBs, forwarding rules) in PCE table entries.
> Hardware access to the PCE table is non-atomic, and is comprised of
> several register reads and writes.
>=20
> These accesses are currently serialized by the rtnl_lock, but DSA is
> changing its driver API and that lock will no longer be held when
> calling ->port_fdb_add() and ->port_fdb_del().
>=20
> So this driver needs to serialize the access to the PCE table using its
> own locking scheme. This patch adds that.
>=20
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  drivers/net/dsa/lantiq_gswip.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
>=20
> diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswi=
p.c
> index 60e36f46f8abe..d612ef8648baa 100644
> --- a/drivers/net/dsa/lantiq_gswip.c
> +++ b/drivers/net/dsa/lantiq_gswip.c
> @@ -274,6 +274,7 @@ struct gswip_priv {
>  	int num_gphy_fw;
>  	struct gswip_gphy_fw *gphy_fw;
>  	u32 port_vlan_filter;
> +	struct mutex pce_table_lock;
>  };
> =20
>  struct gswip_pce_table_entry {
> @@ -521,10 +522,14 @@ static int gswip_pce_table_entry_read(struct gswip_=
priv *priv,
>  	u16 addr_mode =3D tbl->key_mode ? GSWIP_PCE_TBL_CTRL_OPMOD_KSRD :
>  					GSWIP_PCE_TBL_CTRL_OPMOD_ADRD;
> =20
> +	mutex_lock(&priv->pce_table_lock);
> +
>  	err =3D gswip_switch_r_timeout(priv, GSWIP_PCE_TBL_CTRL,
>  				     GSWIP_PCE_TBL_CTRL_BAS);
> -	if (err)
> +	if (err) {
> +		mutex_unlock(&priv->pce_table_lock);
>  		return err;
> +	}
> =20
>  	gswip_switch_w(priv, tbl->index, GSWIP_PCE_TBL_ADDR);
>  	gswip_switch_mask(priv, GSWIP_PCE_TBL_CTRL_ADDR_MASK |
> @@ -534,8 +539,10 @@ static int gswip_pce_table_entry_read(struct gswip_p=
riv *priv,
> =20
>  	err =3D gswip_switch_r_timeout(priv, GSWIP_PCE_TBL_CTRL,
>  				     GSWIP_PCE_TBL_CTRL_BAS);
> -	if (err)
> +	if (err) {
> +		mutex_unlock(&priv->pce_table_lock);
>  		return err;
> +	}
> =20
>  	for (i =3D 0; i < ARRAY_SIZE(tbl->key); i++)
>  		tbl->key[i] =3D gswip_switch_r(priv, GSWIP_PCE_TBL_KEY(i));
> @@ -551,6 +558,8 @@ static int gswip_pce_table_entry_read(struct gswip_pr=
iv *priv,
>  	tbl->valid =3D !!(crtl & GSWIP_PCE_TBL_CTRL_VLD);
>  	tbl->gmap =3D (crtl & GSWIP_PCE_TBL_CTRL_GMAP_MASK) >> 7;
> =20
> +	mutex_unlock(&priv->pce_table_lock);
> +
>  	return 0;
>  }
> =20
> @@ -563,10 +572,14 @@ static int gswip_pce_table_entry_write(struct gswip=
_priv *priv,
>  	u16 addr_mode =3D tbl->key_mode ? GSWIP_PCE_TBL_CTRL_OPMOD_KSWR :
>  					GSWIP_PCE_TBL_CTRL_OPMOD_ADWR;
> =20
> +	mutex_lock(&priv->pce_table_lock);
> +
>  	err =3D gswip_switch_r_timeout(priv, GSWIP_PCE_TBL_CTRL,
>  				     GSWIP_PCE_TBL_CTRL_BAS);
> -	if (err)
> +	if (err) {
> +		mutex_unlock(&priv->pce_table_lock);
>  		return err;
> +	}
> =20
>  	gswip_switch_w(priv, tbl->index, GSWIP_PCE_TBL_ADDR);
>  	gswip_switch_mask(priv, GSWIP_PCE_TBL_CTRL_ADDR_MASK |
> @@ -598,8 +611,12 @@ static int gswip_pce_table_entry_write(struct gswip_=
priv *priv,
>  	crtl |=3D GSWIP_PCE_TBL_CTRL_BAS;
>  	gswip_switch_w(priv, crtl, GSWIP_PCE_TBL_CTRL);
> =20
> -	return gswip_switch_r_timeout(priv, GSWIP_PCE_TBL_CTRL,
> -				      GSWIP_PCE_TBL_CTRL_BAS);
> +	err =3D gswip_switch_r_timeout(priv, GSWIP_PCE_TBL_CTRL,
> +				     GSWIP_PCE_TBL_CTRL_BAS);
> +
> +	mutex_unlock(&priv->pce_table_lock);
> +
> +	return err;
>  }
> =20
>  /* Add the LAN port into a bridge with the CPU port by
> @@ -2020,6 +2037,7 @@ static int gswip_probe(struct platform_device *pdev=
)
>  	priv->ds->priv =3D priv;
>  	priv->ds->ops =3D &gswip_switch_ops;
>  	priv->dev =3D dev;
> +	mutex_init(&priv->pce_table_lock);
>  	version =3D gswip_switch_r(priv, GSWIP_VERSION);
> =20
>  	/* bring up the mdio bus */
> --=20
> 2.33.0
>=20
>=20
>

As discussed on the v5.14 backport, this patch can be dropped. Same
thing for the 5.10 version of the backport.=