Received-SPF: pass (google.com: domain of linux-kernel+bounces-32372-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: "Jenishkumar Patel [C]" <jpatel2@marvell.com>
To: 'Maxime Chevallier' <maxime.chevallier@bootlin.com>
CC: "marcin.s.wojtas@gmail.com" <marcin.s.wojtas@gmail.com>,
        "linux@armlinux.org.uk" <linux@armlinux.org.uk>,
        "davem@davemloft.net"
	<davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Antoine Tenart
	<atenart@kernel.org>
Subject: RE: [EXT] Re: [net v4 PATCH 1/1] net: mvpp2: clear BM pool before
 initialization
Thread-Topic: [EXT] Re: [net v4 PATCH 1/1] net: mvpp2: clear BM pool before
 initialization
Thread-Index: AQHaSoviVtEU90vMIk6A5vQHbYZ71LDhK9iAgAQdgyA=
Date: Mon, 22 Jan 2024 06:02:07 +0000
Message-ID: 
 <PH0PR18MB4543B5E92792ECD5D3683861EC752@PH0PR18MB4543.namprd18.prod.outlook.com>
References: <20240119035914.2595665-1-jpatel2@marvell.com>
 <20240119150451.476d6ba2@device-28.home>
In-Reply-To: <20240119150451.476d6ba2@device-28.home>
Accept-Language: en-GB, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?jxW/kHsnB4x65JvZtE6HdSxQSLdfcQOotUdQ2nwt6FlGi3uS2OSZCb+UDO8w?=
 =?us-ascii?Q?jsjauFY9a2xSk51yKrlQ+E/sGHOKJYfgGqMaJG9s6gF2x7mMJVeHLJL1rGRm?=
 =?us-ascii?Q?e/9WA6Hlsies6NDBQhpupCdFCYV9j3PS+K/MegqNypVnGVfoHL+0mDdW7wQO?=
 =?us-ascii?Q?z4BcyJl1tAEaWxuXGeq4FHCUUVCag+V55nISBYfGplP8a1Pi69FRq2isFpQt?=
 =?us-ascii?Q?EaKzQ3sHRaCKsD6Q7yfJCklpYb2IJU7N6ht7zAMSquuUTG9wwhy12T51fGlz?=
 =?us-ascii?Q?aRLEKOqIF3JAWry9uR81syv1fkCMgaMd1+Z8a9RlL9HgmQMEMsIW3igUwJPg?=
 =?us-ascii?Q?ScGa9U7MBl5Xaz0qyU/m95ce0Gp2iqmmg1vXq8/k18Eu1zOURZkJ7iPIIP81?=
 =?us-ascii?Q?3kG/ysL5dv8FNfYefkejOXl/GGCER9jeXBxyIge3ZZrd6/1NgN06gT9w7q0H?=
 =?us-ascii?Q?vvxDRsT4mohll81+Nx9p/366ZgoVAPrcL5qNgW/3XxaUG2gw1jojJR84Pa0n?=
 =?us-ascii?Q?5OEtzzFluXQu3Ao7iN+2zH2/6q3Hi77V2ZP5E+EmgJw72MITvcB4i7faQQAd?=
 =?us-ascii?Q?HkHq9usN6cqfEM37w5/19G2uPiwaBrvoHO5C+N60FQFdBGgcZtoHlZj4m/An?=
 =?us-ascii?Q?PZYjLukWQIzeOSHOWv7xW4brdRQC8LvWH4qCcajkn9iZm0YFbX+DgMWo08+m?=
 =?us-ascii?Q?mdTtu+b3s8tVeGBoOtJE1z0jez9O4oDKT2qJdkcOIzSAxyWBSG/JqEuvReRW?=
 =?us-ascii?Q?ETDFd2nc4UWlOy3fkyNLyUizEm8ES3lqDw/tbU4pA+RBl+b7/MQwOfm7J2cF?=
 =?us-ascii?Q?lgy5y20ICfzNgYbDhnAQ2mPeTbFO8Vza2X8uC2ul4CJN6Ebp5+rQZYUicl75?=
 =?us-ascii?Q?klGhdYU9lqJcj71hmaRdCDR/zuSKefpdRWZ20rl8krjlqRexD9GiZ3sK1Dez?=
 =?us-ascii?Q?xKudyBCkuIPkJOZROF7a75hBeaXmFObxiFUG94fXItr6VZL24yp07zRuQDwp?=
 =?us-ascii?Q?EwvfgDrQUiz5I49KaGQmLo+qezfY+vBUQmmNuBW0Yog7a7J4z406ZN20Aa50?=
 =?us-ascii?Q?VAEiXlD/ceqJpVqlBAcqObW/XK7oPPHfwN/WbIOsgMCKz75N8b9XHjSdYyJ9?=
 =?us-ascii?Q?4v80x9Rp0me1T9mt/l/ZK5V+MwzmqYYbQpVRQMuW6IY5visjaFuK9q9qEOQD?=
 =?us-ascii?Q?4vOtb/UPeWaodsYm7/48cVyBQmjmdBal5tlPwb5eXzdcVbJJ+7ct7AnLk7Lp?=
 =?us-ascii?Q?OxB441WjlrzQW/9NPdNSEG+qXRW/BzztSmoqWpD5M9awGrawZrtyLpemUJlo?=
 =?us-ascii?Q?3uvF9mHL0K553vvnOHINzf+Iq63TZsfHnWZ26baPC5eXjj2DZpdNlJ0M8L9Q?=
 =?us-ascii?Q?yVXx8funUu0fu76ypYhYl2BoLSu7w2uOOOGSYd4OzXdTcQjIpz2dQEcmDvct?=
 =?us-ascii?Q?NM4cFYSpstKWjNEOjn6vLzh2dUEci0u9IMbRzPJNPOwa7g3wxHoOCK98cfr9?=
 =?us-ascii?Q?gxUMNfVuHqYyYNiUiAwK/dpT04fTURs/5aok8ykegzhChyjFjdkuKEo6P+hT?=
 =?us-ascii?Q?HfqCGrLKq0d7At8iRMw=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: PH0PR18MB4543.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5081f3fa-aa8d-4d01-2cee-08dc1b0faa38
X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Jan 2024 06:02:07.3697
 (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: FlamZJvjRVRAXO8w4tXxuusLXVQnuyT9YS3S+nS7vN1Q5GOWm605X9ToAQ9c4x5GC48E6/IrijNS//t3wPh+OA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV3PR18MB5706
X-Proofpoint-ORIG-GUID: DaEWjW5DGqgNpM16L_6bHMNGiMSYDDUa
X-Proofpoint-GUID: DaEWjW5DGqgNpM16L_6bHMNGiMSYDDUa
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-01-21_04,2024-01-19_02,2023-05-22_02


-----Original Message-----
From: Maxime Chevallier <maxime.chevallier@bootlin.com>=20
Sent: Friday, January 19, 2024 7:35 PM
To: Jenishkumar Patel [C] <jpatel2@marvell.com>
Cc: marcin.s.wojtas@gmail.com; linux@armlinux.org.uk; davem@davemloft.net; =
edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel=
org; linux-kernel@vger.kernel.org; Antoine Tenart <atenart@kernel.org>
Subject: [EXT] Re: [net v4 PATCH 1/1] net: mvpp2: clear BM pool before init=
ialization

External Email

----------------------------------------------------------------------
Hello,

On Thu, 18 Jan 2024 19:59:14 -0800
Jenishkumar Maheshbhai Patel <jpatel2@marvell.com> wrote:

> Register value persist after booting the kernel using kexec which=20
> results in kernel panic. Thus clear the BM pool registers before=20
> initialisation to fix the issue.
>=20
> Fixes: 3f518509dedc ("ethernet: Add new driver for Marvell Armada 375=20
> network unit")
> Signed-off-by: Jenishkumar Maheshbhai Patel <jpatel2@marvell.com>
> ---
> v1-v2:
> -Move comments outside the loop
> -remove unrequired brances.
> v2-v3:
> -improve readability
> -correct register read API
> v3-v4:
> -optimize the code
> -improve readability

Thanks for taking the reviews into account, however ...

>  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 27 ++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
>=20
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c=20
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 820b1fabe297..23adf53c2aa1 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -614,12 +614,38 @@ static void mvpp23_bm_set_8pool_mode(struct mvpp2 *=
priv)
>  	mvpp2_write(priv, MVPP22_BM_POOL_BASE_ADDR_HIGH_REG, val);  }
> =20
> +/* Cleanup pool before actual initialization in the OS */ static void=20
> +mvpp2_bm_pool_cleanup(struct mvpp2 *priv, int pool_id) {
> +	unsigned int thread =3D mvpp2_cpu_to_thread(priv, get_cpu());
> +	u32 val;
> +	int i;
> +
> +	/* Drain the BM from all possible residues left by firmware */
> +	for (i =3D 0; i < MVPP2_BM_POOL_SIZE_MAX; i++)
> +		mvpp2_thread_read(priv, thread, MVPP2_BM_PHY_ALLOC_REG(pool_id));

.. I think you didn't answer Antoine's comment on that loop from the V2, r=
egarding what this does exactly. From the other sites this is used, it seem=
s to perform an allocation from the pool, can you clarify how safe it is to=
 do so here, if for example the BM was never configured by the firmware bef=
orehand and is therefore already in a Stopped state ?

Reading the register provides a pointer to buffer that is already allocated=
 during BM initialization. When multiple reading is done on the register, i=
t will drain all the pointers that are stored by previous firmware. Also re=
ading this register does not perform any allocation as it is only performin=
g register read operation, thus when the BM is not configured earlier then =
it will not lead to any stop state.

And are we not risking any leak if there was something in the pool that we =
don't release ?

The data on the pointer given by register read is written after the read op=
eration is preformed, which means the pointer does not contain any data, th=
us there is no leak.

Thanks,

Maxime