[PATCH] EDAC/bluefield: dont use bluefield_edac_readl result on error

[PATCH] EDAC/bluefield: dont use bluefield_edac_readl result on error

[David Thompson]

The "bluefield_edac_readl()" routine returns an uninitialized result
during error paths. In those cases the calling routine should not use
the uninitialized result. The driver should simply log the error, and
then return early.

Fixes: e41967575474 ("EDAC/bluefield: Use Arm SMC for EMI access on BlueField-2")
Signed-off-by: David Thompson <davthompson@nvidia.com>
Reviewed-by: Shravan Kumar Ramani <shravankr@nvidia.com>
---
 drivers/edac/bluefield_edac.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/edac/bluefield_edac.c b/drivers/edac/bluefield_edac.c
index 4942a240c30f..ae3bb7afa103 100644
--- a/drivers/edac/bluefield_edac.c
+++ b/drivers/edac/bluefield_edac.c
@@ -199,8 +199,10 @@ static void bluefield_gather_report_ecc(struct mem_ctl_info *mci,
 	 * error without the detailed information.
 	 */
 	err = bluefield_edac_readl(priv, MLXBF_SYNDROM, &dram_syndrom);
-	if (err)
+	if (err) {
 		dev_err(priv->dev, "DRAM syndrom read failed.\n");
+		return;
+	}
 
 	serr = FIELD_GET(MLXBF_SYNDROM__SERR, dram_syndrom);
 	derr = FIELD_GET(MLXBF_SYNDROM__DERR, dram_syndrom);
@@ -213,20 +215,26 @@ static void bluefield_gather_report_ecc(struct mem_ctl_info *mci,
 	}
 
 	err = bluefield_edac_readl(priv, MLXBF_ADD_INFO, &dram_additional_info);
-	if (err)
+	if (err) {
 		dev_err(priv->dev, "DRAM additional info read failed.\n");
+		return;
+	}
 
 	err_prank = FIELD_GET(MLXBF_ADD_INFO__ERR_PRANK, dram_additional_info);
 
 	ecc_dimm = (err_prank >= 2 && priv->dimm_ranks[0] <= 2) ? 1 : 0;
 
 	err = bluefield_edac_readl(priv, MLXBF_ERR_ADDR_0, &edea0);
-	if (err)
+	if (err) {
 		dev_err(priv->dev, "Error addr 0 read failed.\n");
+		return;
+	}
 
 	err = bluefield_edac_readl(priv, MLXBF_ERR_ADDR_1, &edea1);
-	if (err)
+	if (err) {
 		dev_err(priv->dev, "Error addr 1 read failed.\n");
+		return;
+	}
 
 	ecc_dimm_addr = ((u64)edea1 << 32) | edea0;
 
@@ -250,8 +258,10 @@ static void bluefield_edac_check(struct mem_ctl_info *mci)
 		return;
 
 	err = bluefield_edac_readl(priv, MLXBF_ECC_CNT, &ecc_count);
-	if (err)
+	if (err) {
 		dev_err(priv->dev, "ECC count read failed.\n");
+		return;
+	}
 
 	single_error_count = FIELD_GET(MLXBF_ECC_CNT__SERR_CNT, ecc_count);
 	double_error_count = FIELD_GET(MLXBF_ECC_CNT__DERR_CNT, ecc_count);
-- 
2.43.2

RE: [PATCH] EDAC/bluefield: dont use bluefield_edac_readl result on error

[David Thompson]

> -----Original Message-----
> From: David Thompson <davthompson@nvidia.com>
> Sent: Tuesday, March 18, 2025 5:48 PM
> To: Shravan Ramani <shravankr@nvidia.com>; bp@alien8.de;
> tony.luck@intel.com; james.morse@arm.com; mchehab@kernel.org;
> rric@kernel.org
> Cc: linux-edac@vger.kernel.org; linux-kernel@vger.kernel.org; David Thompson
> <davthompson@nvidia.com>
> Subject: [PATCH] EDAC/bluefield: dont use bluefield_edac_readl result on error
> 
> The "bluefield_edac_readl()" routine returns an uninitialized result during error
> paths. In those cases the calling routine should not use the uninitialized result.
> The driver should simply log the error, and then return early.
> 
> Fixes: e41967575474 ("EDAC/bluefield: Use Arm SMC for EMI access on
> BlueField-2")
> Signed-off-by: David Thompson <davthompson@nvidia.com>
> Reviewed-by: Shravan Kumar Ramani <shravankr@nvidia.com>
> ---
>  drivers/edac/bluefield_edac.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 

Refreshing this review...

Does anyone have feedback on this EDAC driver patch?

Thanks, Dave

Re: [PATCH] EDAC/bluefield: dont use bluefield_edac_readl result on error

[Borislav Petkov]

On Thu, May 22, 2025 at 03:41:23PM +0000, David Thompson wrote:
> > -----Original Message-----
> > From: David Thompson <davthompson@nvidia.com>
> > Sent: Tuesday, March 18, 2025 5:48 PM
> > To: Shravan Ramani <shravankr@nvidia.com>; bp@alien8.de;
> > tony.luck@intel.com; james.morse@arm.com; mchehab@kernel.org;
> > rric@kernel.org
> > Cc: linux-edac@vger.kernel.org; linux-kernel@vger.kernel.org; David Thompson
> > <davthompson@nvidia.com>
> > Subject: [PATCH] EDAC/bluefield: dont use bluefield_edac_readl result on error
> > 
> > The "bluefield_edac_readl()" routine returns an uninitialized result during error
> > paths. In those cases the calling routine should not use the uninitialized result.
> > The driver should simply log the error, and then return early.
> > 
> > Fixes: e41967575474 ("EDAC/bluefield: Use Arm SMC for EMI access on
> > BlueField-2")
> > Signed-off-by: David Thompson <davthompson@nvidia.com>
> > Reviewed-by: Shravan Kumar Ramani <shravankr@nvidia.com>
> > ---
> >  drivers/edac/bluefield_edac.c | 20 +++++++++++++++-----
> >  1 file changed, 15 insertions(+), 5 deletions(-)
> > 
> 
> Refreshing this review...
> 
> Does anyone have feedback on this EDAC driver patch?

Looks like it fell through the cracks... :-\

Queued now.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette