linux-kernel - Re: [PATCH 2/2] RAS/AMD/{ATL,FMPM}: Get masked address

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250408155242.GA2523543@yaz-khff2.amd.com>
Date: Tue, 8 Apr 2025 11:52:42 -0400
From: Yazen Ghannam <yazen.ghannam@....com>
To: Borislav Petkov <bp@...en8.de>
Cc: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@....com>,
	Muralidhara M K <muralidhara.mk@....com>,
	linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] RAS/AMD/{ATL,FMPM}: Get masked address

On Tue, Apr 08, 2025 at 12:14:15PM +0200, Borislav Petkov wrote:
> On Mon, Apr 07, 2025 at 11:16:57AM -0400, Yazen Ghannam wrote:
> > Okay, will do.
> 
> Yah, except that df_cfg crap needs an export now:
> 
> ERROR: modpost: "df_cfg" [drivers/ras/amd/fmpm.ko] undefined!
> make[2]: *** [scripts/Makefile.modpost:147: Module.symvers] Error 1
> make[1]: *** [/mnt/kernel/kernel/linux/Makefile:1956: modpost] Error 2
> make: *** [Makefile:248: __sub-make] Error 2
> 

*sigh*, sorry for the noise. I did a 'make -f' build rather than a full
build.

> Looking at the call chain how we land in that fpds_equal() in fmpm, can we
> read out from the error records themselves that those are from MI300 and not
> need df_cfg at all?
> 
> There's a struct mce here:
> 
> static void update_fru_record(struct fru_rec *rec, struct mce *m)
> 
> which has CPUID for example.
> 
> Otherwise we'll have to go back to your original thing if we're going to have
> to export *something*...
> 

At the moment, FMPM only loads on MI300A. We can just have a local
function to mask the addresses. I was thinking we can have function
pointers to make things generic. But maybe we keep it simple until
really necessary by just using the MI300 version by default.

Please see patch below.

Thanks,
Yazen

----------------

>From 7f6e2196cb54bb2134e5c1854e5a54530eec77a1 Mon Sep 17 00:00:00 2001
From: Yazen Ghannam <yazen.ghannam@....com>
Date: Thu, 27 Feb 2025 19:31:32 +0000
Subject: [PATCH] RAS/AMD/FMPM: Get masked address

Some operations require checking, or ignoring, specific bits in an
address value. For example, this can be comparing address values to
identify unique structures.

Currently, the full address value is compared when filtering for
duplicates. This results in over counting and creation of extra records.
This gives the impression that more unique events occurred than did in
reality.

Introduce a helper to return a masked address. Start with the case for
physical rows on MI300.

Fixes: 6f15e617cc99 ("RAS: Introduce a FRU memory poison manager")
Signed-off-by: Yazen Ghannam <yazen.ghannam@....com>
Cc: stable@...r.kernel.org
---
 drivers/ras/amd/atl/internal.h |  3 +++
 drivers/ras/amd/atl/umc.c      |  2 --
 drivers/ras/amd/fmpm.c         | 14 +++++++++++++-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/ras/amd/atl/internal.h b/drivers/ras/amd/atl/internal.h
index f9be26d25348..d096b58cd0ae 100644
--- a/drivers/ras/amd/atl/internal.h
+++ b/drivers/ras/amd/atl/internal.h
@@ -362,4 +362,7 @@ static inline void atl_debug_on_bad_intlv_mode(struct addr_ctx *ctx)
 	atl_debug(ctx, "Unrecognized interleave mode: %u", ctx->map.intlv_mode);
 }
 
+#define MI300_UMC_MCA_COL	GENMASK(5, 1)
+#define MI300_UMC_MCA_ROW13	BIT(23)
+
 #endif /* __AMD_ATL_INTERNAL_H__ */
diff --git a/drivers/ras/amd/atl/umc.c b/drivers/ras/amd/atl/umc.c
index cb8ace3d4e42..6e072b7667e9 100644
--- a/drivers/ras/amd/atl/umc.c
+++ b/drivers/ras/amd/atl/umc.c
@@ -229,7 +229,6 @@ int get_umc_info_mi300(void)
  * Additionally, the PC and Bank bits may be hashed. This must be accounted for before
  * reconstructing the normalized address.
  */
-#define MI300_UMC_MCA_COL	GENMASK(5, 1)
 #define MI300_UMC_MCA_BANK	GENMASK(9, 6)
 #define MI300_UMC_MCA_ROW	GENMASK(24, 10)
 #define MI300_UMC_MCA_PC	BIT(25)
@@ -360,7 +359,6 @@ static void _retire_row_mi300(struct atl_err *a_err)
  *
  * See MI300_UMC_MCA_ROW for the row bits in MCA_ADDR_UMC value.
  */
-#define MI300_UMC_MCA_ROW13	BIT(23)
 static void retire_row_mi300(struct atl_err *a_err)
 {
 	_retire_row_mi300(a_err);
diff --git a/drivers/ras/amd/fmpm.c b/drivers/ras/amd/fmpm.c
index 90de737fbc90..6119bd0d6ae1 100644
--- a/drivers/ras/amd/fmpm.c
+++ b/drivers/ras/amd/fmpm.c
@@ -167,6 +167,18 @@ static unsigned int spa_nr_entries;
  */
 static DEFINE_MUTEX(fmpm_update_mutex);
 
+/*
+ * Row retirement is done on MI300 systems, and some bits are 'don't care'
+ * for comparing addresses with unique physical rows.
+ * This includes all column bits and the row[13] bit.
+ */
+static u64 mi300_mask_address(u64 addr)
+{
+	return addr & ~(MI300_UMC_MCA_ROW13 | MI300_UMC_MCA_COL);
+}
+
+static u64 (*fmpm_mask_address)(u64 addr) = mi300_mask_address;
+
 #define for_each_fru(i, rec) \
 	for (i = 0; rec = fru_records[i], i < max_nr_fru; i++)
 
@@ -258,7 +270,7 @@ static bool fpds_equal(struct cper_fru_poison_desc *old, struct cper_fru_poison_
 	 *
 	 * Also, order the checks from most->least likely to fail to shortcut the code.
 	 */
-	if (old->addr != new->addr)
+	if (fmpm_mask_address(old->addr) != fmpm_mask_address(new->addr))
 		return false;
 
 	if (old->hw_id != new->hw_id)
-- 
2.49.0