Received-SPF: pass (google.com: domain of linux-kernel+bounces-177407-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Hariprasad Kelam <hkelam@marvell.com>
To: Wei Fang <wei.fang@nxp.com>, "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org"
	<kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "shenwei.wang@nxp.com" <shenwei.wang@nxp.com>,
        "xiaoning.wang@nxp.com"
	<xiaoning.wang@nxp.com>,
        "richardcochran@gmail.com"
	<richardcochran@gmail.com>,
        "andrew@lunn.ch" <andrew@lunn.ch>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "imx@lists.linux.dev" <imx@lists.linux.dev>
Subject: [PATCH net] net: fec: avoid lock evasion when reading pps_enable
Thread-Topic: [PATCH net] net: fec: avoid lock evasion when reading pps_enable
Thread-Index: AQHapRfDaiJFn1FWf0uT/Cuv3lOavw==
Date: Mon, 13 May 2024 09:27:23 +0000
Message-ID: 
 <PH0PR18MB4474D5050F6CBA0B2D4A6041DEE22@PH0PR18MB4474.namprd18.prod.outlook.com>
References: <20240513015127.961360-1-wei.fang@nxp.com>
In-Reply-To: <20240513015127.961360-1-wei.fang@nxp.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?6O7PsR0Lr3KhiHAhOj1FqDV3pd9Jhcyz4OB5y0Yfv3PNQ6FV3EfYDyEGkkzQ?=
 =?us-ascii?Q?/Ru8JjcksKlqUNR0qmFH1Lg0wAY8hIrT/fnGufmw0Bns1HC0ksb2zv04wq8U?=
 =?us-ascii?Q?24JLQfgocuzzLS32Uyb4kFWFSgI+WdGsnwYUK2isvbHtalXhz63GG4s7kD8g?=
 =?us-ascii?Q?qdpzmH4yjuU2ZNIBWUdPrc972z/VRHHy+9RvQsWI+Q2eYh3c1fVQX8EMaWfU?=
 =?us-ascii?Q?azSTjmRPWVO6C10UjgbVlrBOBx7MFwhH/AQAZWPer7r5aVN65HCqIuVUV/tf?=
 =?us-ascii?Q?3lMxA5/Y6vAd3LC7MCA6AszPF1q5KgNhEooyoj6B2eaqEvVNDLZ3hT+6/cs1?=
 =?us-ascii?Q?dTO3KFJRxQIA2qjzG6mDQEWvP50/AlDfE6vTg/pXZuiTZ12OYPFbBsozEEVW?=
 =?us-ascii?Q?HVrCQO0pugz+7nlv2HN3IfCvagNttO8l7wqgXDwdaI3QOLr/aiNDw5tAJeP9?=
 =?us-ascii?Q?iq5skixLlCACu1uEc/KxPeMsjZsVHUMxnQxlfZy+2O3POKrFMjAHDi+6wMzd?=
 =?us-ascii?Q?PhfY+N0FzQZXwnT19xAJVgZRqEjxva0fzLO1naEtlTl5RFFbXx88bv4dqENq?=
 =?us-ascii?Q?WUQLHumfl0pQZm7zc24fPHCzabpGg6Jjo7w00RUsy7q63c3VjqPxH8rY6Xsk?=
 =?us-ascii?Q?rRmizeIg4HVzaVFg2ku8R1vxqbn3Uu/JLcV3LzYkfG9NQHUNmTHJTL7P0Qah?=
 =?us-ascii?Q?vcjMTUdxMxkYzq1VOlKit8CnQBWQRI3ru7pI9IBBkgytxYMM11djJ7FD+OgY?=
 =?us-ascii?Q?GUep74b93T/t4kKHwF2VcC6hkCqA0ykV3nZP2O0jSCv5jZsoTFQXyZ3gMp5a?=
 =?us-ascii?Q?DBMtTYoQl1c4VvcJUf5ccQ/CcP0LeXlPXrMe8tB7rJKLXIE91Ibu3NlutdAA?=
 =?us-ascii?Q?2a9ns3rsKssyrnGEzVoZToFrq4AIDglNvYShnXYYyRrbH279bVcPsONMzTYR?=
 =?us-ascii?Q?qQCXPp5IF7cvEazAtSm2mldow31tZPh9AUuwLP4NueSHOJNYshcSp/yDaEYp?=
 =?us-ascii?Q?qcwMrNcXeSE/s0sKnyi43h93vzkcz+8OHTL8HFW/6e7Jhf7UglFY8MBaTJ95?=
 =?us-ascii?Q?RZYIEBzr4rOXCJLEVl9w0QlugO23kxsgRcFg2B5uG4GIqP0+C/yE2ztPSqKd?=
 =?us-ascii?Q?Rinl+H6vEicCpcu9m7UeJcWXmAgfMsdoTgwJ/NoGka7csrEm5+Awpsps/KIE?=
 =?us-ascii?Q?p/kXVhuhnQD5gGRg69BQqcfiTsuabNEDqnvkeoJJxUi+di3LK3XbW7sLtlYE?=
 =?us-ascii?Q?FhFbWoaKM8ga80ej6oJgHQRE8JY/KiccUk/Ft+Zfz/Z2u26tV2gmDR8GcvE8?=
 =?us-ascii?Q?8q96WGzjXzKFi7/ByGF/GLLbh9Hitg4OWBlL5C5G02Nj7pQ4YSTK6HaXTRIl?=
 =?us-ascii?Q?v+5SDVLtv6RMW5y8ERGYkAGe6T0N2VhTX/U13nRZgPdWuysDuMPMhg7VOCfk?=
 =?us-ascii?Q?a4HrKhlYTxoZ50LCPxK4oiSaO3aWIJKIjoOk53rbKiBQbTf1CVD/d7cqI+m9?=
 =?us-ascii?Q?dd+ugqkRFOWqFzfNr6LKT6m7gwciNp9TVYcbHR2jz757ysmOb9H2+AS2PDWK?=
 =?us-ascii?Q?sTs8UZ51m5QYbopZEpQ=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR18MB4474.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5aa0273c-b137-4435-6f39-08dc732ee5a7
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 May 2024 09:27:23.8333
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: McNWMuo77eaPrfQ9mattNy11SRgv9YowH+c2/sd5A2UzmhgfpYkeC4+3uuKEIrLzi2mSUjrg2UkzZHnXQQm/Rg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR18MB3498
X-Proofpoint-GUID: qpw4X80iu3_doLr74ASfpbcj8y0IfHFh
X-Proofpoint-ORIG-GUID: qpw4X80iu3_doLr74ASfpbcj8y0IfHFh
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-05-13_06,2024-05-10_02,2023-05-22_02

See inline,

> The assignment of pps_enable is protected by tmreg_lock, but the read
> operation of pps_enable is not. So the Coverity tool reports a lock evasi=
on
> warning which may cause data race to occur when running in a multithread
> environment. Although this issue is almost impossible to occur, we'd bett=
er fix
> it, at least it seems more logically reasonable, and it also prevents Cov=
erity
> from continuing to issue warnings.
>=20
> Fixes: 278d24047891 ("net: fec: ptp: Enable PPS output based on ptp clock=
")
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>  drivers/net/ethernet/freescale/fec_ptp.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>=20
> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
> b/drivers/net/ethernet/freescale/fec_ptp.c
> index 181d9bfbee22..8d37274a3fb0 100644
> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> @@ -104,14 +104,16 @@ static int fec_ptp_enable_pps(struct
> fec_enet_private *fep, uint enable)
>  	struct timespec64 ts;
>  	u64 ns;
>=20
> -	if (fep->pps_enable =3D=3D enable)
> -		return 0;
> -
>  	fep->pps_channel =3D DEFAULT_PPS_CHANNEL;
>  	fep->reload_period =3D PPS_OUPUT_RELOAD_PERIOD;
>=20
>  	spin_lock_irqsave(&fep->tmreg_lock, flags);
>=20
> +	if (fep->pps_enable =3D=3D enable) {

Can we atomic_set/get instead of spin_lock here.

Thanks,
Hariprasad k
> +		spin_unlock_irqrestore(&fep->tmreg_lock, flags);
> +		return 0;
> +	}
> +
>  	if (enable) {
>  		/* clear capture or output compare interrupt status if have.
>  		 */
> --
> 2.34.1
>=20