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

[Borislav Petkov]

On Tue, Apr 08, 2025 at 11:52:42AM -0400, Yazen Ghannam wrote:
> 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.

Now you're talking! :-P

> Please see patch below.

... which I simplified even more.

I'm thinking whoever is going to test fmpm on something else besides MI300A,
they will have to extend this address masking thing and then we can cross that
bridge when we get to it.

So this is keeping it simple for now.

Ack?

---

commit 58029c39cdc54ac4f4dc40b4a9c05eed9f9b808a (HEAD -> refs/heads/edac-urgent)
Author: Yazen Ghannam <yazen.ghannam@xxxxxxx>
Date:   Thu Feb 27 19:31:32 2025 +0000

    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.
    
    Mask the address for physical rows on MI300.
    
      [ bp: Simplify. ]
    
    Fixes: 6f15e617cc99 ("RAS: Introduce a FRU memory poison manager")
    Signed-off-by: Yazen Ghannam <yazen.ghannam@xxxxxxx>
    Signed-off-by: Borislav Petkov (AMD) <bp@xxxxxxxxx>
    Cc: stable@xxxxxxxxxxxxxxx

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..8877c6ff64c4 100644
--- a/drivers/ras/amd/fmpm.c
+++ b/drivers/ras/amd/fmpm.c
@@ -250,6 +250,13 @@ static bool rec_has_valid_entries(struct fru_rec *rec)
 	return true;
 }
 
+/*
+ * 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.
+ */
+#define MASK_ADDR(addr)	((addr) & ~(MI300_UMC_MCA_ROW13 | MI300_UMC_MCA_COL))
+
 static bool fpds_equal(struct cper_fru_poison_desc *old, struct cper_fru_poison_desc *new)
 {
 	/*
@@ -258,7 +265,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 (MASK_ADDR(old->addr) != MASK_ADDR(new->addr))
 		return false;
 
 	if (old->hw_id != new->hw_id)

-- 
Regards/Gruss,
    Boris.

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