[PATCH 00/10] EDAC/{ie31200,igen6}: Fix address decoding and detect IMCs at runtime

[PATCH 00/10] EDAC/{ie31200,igen6}: Fix address decoding and detect IMCs at runtime

[Qiuxu Zhuo]

Intel client EDAC drivers duplicate address translation logic and rely
on platform-specific IMC counts even though the IMCs can be detected
at runtime. This series fixes several decoding bugs, replaces static IMC
counts with runtime detection, and consolidates the common translation
logic to simplify future maintenance.

Patches 1-5: Fix several correctness issues in the ie31200 and igen6
drivers, including interleave boundary handling, channel hash selection,
non-hash address decoding, and logged error address translation.

Patches 6-7: Do small cleanups.

Patches 8-9: Detect the number of IMCs at runtime, allowing new SoCs
to be supported without maintaining per-platform IMC counts while
eliminating redundant resource configuration tables.

Patch 10: Refactor the address translation logic by introducing
shared helpers for the common interleave and hash translation operations.
This removes duplicated implementations across multiple decoding paths,
making future changes easier while preserving existing behavior.

Qiuxu Zhuo (10):
  EDAC/ie31200: Decouple DIMM width decoding from enum order
  EDAC/igen6: Fix interleave boundary condition
  EDAC/igen6: Fix channel selection hash
  EDAC/igen6: Fix channel address decode for non-hash mode
  EDAC/igen6: Fix Raptor Lake-P logged error address
  EDAC/igen6: Remove unnecessary XOR on the zero-valued interleave bit
  EDAC/igen6: Simplify compute die ID comments
  EDAC/igen6: Detect present memory controllers at runtime
  EDAC/igen6: Remove redundant resource configuration tables
  EDAC/igen6: Refactor address translation logic

 drivers/edac/ie31200_edac.c |  18 +-
 drivers/edac/igen6_edac.c   | 466 ++++++++++++++++++++----------------
 2 files changed, 278 insertions(+), 206 deletions(-)


base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
-- 
2.43.0

[PATCH 01/10] EDAC/ie31200: Decouple DIMM width decoding from enum order

[Qiuxu Zhuo]

The current method to get DIMM width relied on DEV_* enum ordering via a
linear offset (+ DEV_X8), tightly coupling hardware encoding to enum layout.

Replace it with explicit decoding to remove this dependency, as the
enum is expected to grow with additional device widths.

Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
---
 drivers/edac/ie31200_edac.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/edac/ie31200_edac.c b/drivers/edac/ie31200_edac.c
index e3bd6436669b..bfd54012ae47 100644
--- a/drivers/edac/ie31200_edac.c
+++ b/drivers/edac/ie31200_edac.c
@@ -416,7 +416,23 @@ static void populate_dimm_info(struct dimm_data *dd, u32 addr_decode, int dimm,
 {
 	dd->size = field_get(cfg->reg_mad_dimm_size_mask[dimm], addr_decode) * cfg->reg_mad_dimm_size_granularity;
 	dd->ranks = field_get(cfg->reg_mad_dimm_rank_mask[dimm], addr_decode) + 1;
-	dd->dtype = field_get(cfg->reg_mad_dimm_width_mask[dimm], addr_decode) + DEV_X8;
+
+	switch (field_get(cfg->reg_mad_dimm_width_mask[dimm], addr_decode)) {
+	case 0:
+		dd->dtype = DEV_X8;
+		break;
+	case 1:
+		dd->dtype = DEV_X16;
+		break;
+	case 2:
+		dd->dtype = DEV_X32;
+		break;
+	case 3:
+		dd->dtype = DEV_X64;
+		break;
+	default:
+		dd->dtype = DEV_UNKNOWN;
+	}
 }
 
 static void ie31200_get_dimm_config(struct mem_ctl_info *mci, void __iomem *window,
-- 
2.43.0

[PATCH 02/10] EDAC/igen6: Fix interleave boundary condition

[Qiuxu Zhuo]

The address translation logic splits the memory space into interleaved
and non-interleaved regions using a boundary at 2 * s_size.

The current check uses '>' and incorrectly classifies the boundary
address (2 * s_size) as part of the interleaved region. This leads to
incorrect channel/sub-channel selection at the region boundary.

Fix the classification by using '>=' so that the boundary address is
handled in the non-interleaved region, matching the hardware layout.

Fixes: 10590a9d4f23 ("EDAC/igen6: Add EDAC driver for Intel client SoCs using IBECC")
Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
---
 drivers/edac/igen6_edac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/edac/igen6_edac.c b/drivers/edac/igen6_edac.c
index f1fc20d4ebf6..43b56a2eb547 100644
--- a/drivers/edac/igen6_edac.c
+++ b/drivers/edac/igen6_edac.c
@@ -1035,7 +1035,7 @@ static void decode_addr(u64 addr, u32 hash, u64 s_size, int l_map,
 {
 	int intlv_bit = CHANNEL_HASH_LSB_MASK_BIT(hash) + 6;
 
-	if (addr > 2 * s_size) {
+	if (addr >= 2 * s_size) {
 		*sub_addr = addr - s_size;
 		*idx = l_map;
 		return;
-- 
2.43.0

[PATCH 03/10] EDAC/igen6: Fix channel selection hash

[Qiuxu Zhuo]

In channel selection hash mode, the hardware decoding logic always
includes the channel interleave bit in XOR operations. However, the
hash mask may or may not include this channel interleave bit. When
the mask does include this bit, the current igen6_edac code performs
XOR on the interleave bit twice, effectively ignoring it - which is
incorrect.

Fix this issue by ensuring the hash mask always includes the interleave
bit, so XOR is performed on the interleave bit exactly once.

Fixes: 10590a9d4f23 ("EDAC/igen6: Add EDAC driver for Intel client SoCs using IBECC")
Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
---
 drivers/edac/igen6_edac.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/igen6_edac.c b/drivers/edac/igen6_edac.c
index 43b56a2eb547..f1fb644154ae 100644
--- a/drivers/edac/igen6_edac.c
+++ b/drivers/edac/igen6_edac.c
@@ -1009,14 +1009,22 @@ static void set_dimm_params(struct igen6_imc *imc, int chan)
 
 static int decode_chan_idx(u64 addr, u64 mask, int intlv_bit)
 {
-	u64 hash_addr = addr & mask, hash = 0;
-	u64 intlv = (addr >> intlv_bit) & 1;
+	u64 hash_addr, hash = 0;
 	int i;
 
+	/*
+	 * In hash mode, the @intlv_bit is the lowest selected bit of @addr
+	 * to be XORed. While @mask may or may not include this @intlv_bit,
+	 * we enforce that @mask includes @intlv_bit to ensure @intlv_bit is
+	 * XORed exactly once.
+	 */
+	mask |= 1 << intlv_bit;
+	hash_addr = addr & mask;
+
 	for (i = 6; i < 20; i++)
 		hash ^= (hash_addr >> i) & 1;
 
-	return (int)hash ^ intlv;
+	return (int)hash;
 }
 
 static u64 decode_channel_addr(u64 addr, int intlv_bit)
-- 
2.43.0

[PATCH 04/10] EDAC/igen6: Fix channel address decode for non-hash mode

[Qiuxu Zhuo]

In non-hash mode, decode_channel_addr() and channel index extraction
used a hardcoded interleave bit position 6 instead of the actual
intlv_bit parameter, causing incorrect channel address decoding.

Fix this by using intlv_bit consistently in both hash and non-hash modes.

Fixes: 10590a9d4f23 ("EDAC/igen6: Add EDAC driver for Intel client SoCs using IBECC")
Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
---
 drivers/edac/igen6_edac.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/edac/igen6_edac.c b/drivers/edac/igen6_edac.c
index f1fb644154ae..ea5628d780eb 100644
--- a/drivers/edac/igen6_edac.c
+++ b/drivers/edac/igen6_edac.c
@@ -1049,13 +1049,12 @@ static void decode_addr(u64 addr, u32 hash, u64 s_size, int l_map,
 		return;
 	}
 
-	if (CHANNEL_HASH_MODE(hash)) {
-		*sub_addr = decode_channel_addr(addr, intlv_bit);
+	*sub_addr = decode_channel_addr(addr, intlv_bit);
+
+	if (CHANNEL_HASH_MODE(hash))
 		*idx = decode_chan_idx(addr, CHANNEL_HASH_MASK(hash), intlv_bit);
-	} else {
-		*sub_addr = decode_channel_addr(addr, 6);
-		*idx = GET_BITFIELD(addr, 6, 6);
-	}
+	else
+		*idx = GET_BITFIELD(addr, intlv_bit, intlv_bit);
 }
 
 static int igen6_decode(struct decoded_addr *res)
-- 
2.43.0

[PATCH 05/10] EDAC/igen6: Fix Raptor Lake-P logged error address

[Qiuxu Zhuo]

Raptor Lake-P was treated as using a different IBECC (In-Band ECC) error
address format and therefore had a dedicated extraction path that shifted
the logged address.

However, Raptor Lake-P uses the same cache-line-granularity error address
format as other IBECC platforms. The special handling causes the logged
address to be decoded incorrectly.

Fix the issue by removing Raptor Lake-P specific extraction logic and using
the common path instead. This also allows reusing Alder Lake resource
configuration data.

Fixes: d23627a7688f ("EDAC/igen6: Add Intel Raptor Lake-P SoCs support")
Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
---
 drivers/edac/igen6_edac.c | 39 ++++++---------------------------------
 1 file changed, 6 insertions(+), 33 deletions(-)

diff --git a/drivers/edac/igen6_edac.c b/drivers/edac/igen6_edac.c
index ea5628d780eb..12d718a50e1c 100644
--- a/drivers/edac/igen6_edac.c
+++ b/drivers/edac/igen6_edac.c
@@ -175,8 +175,6 @@ static struct res_config {
 	/* Set imc->dimm_{l_size,s_size,l_map}[chan]. */
 	void (*set_dimm_params)(struct igen6_imc *imc, int chan);
 	bool (*ibecc_available)(struct pci_dev *pdev);
-	/* Extract error address logged in IBECC */
-	u64 (*err_addr)(u64 ecclog);
 	/* Convert error address logged in IBECC to system physical address */
 	u64 (*err_addr_to_sys_addr)(u64 eaddr, int mc);
 	/* Convert error address logged in IBECC to integrated memory controller address */
@@ -522,11 +520,6 @@ static u64 adl_err_addr_to_imc_addr(u64 eaddr, int mc)
 	return imc_addr;
 }
 
-static u64 rpl_p_err_addr(u64 ecclog)
-{
-	return field_get(res_cfg->reg_eccerrlog_addr_mask, ecclog);
-}
-
 static enum mem_type ptl_h_get_mem_type(struct igen6_imc *imc)
 {
 	u32 mtype, val;
@@ -716,22 +709,6 @@ static struct res_config adl_n_cfg = {
 	.err_addr_to_imc_addr	= adl_err_addr_to_imc_addr,
 };
 
-static struct res_config rpl_p_cfg = {
-	.machine_check		= true,
-	.num_imc		= 2,
-	.reg_mchbar_mask	= GENMASK_ULL(41, 17),
-	.reg_tom_mask		= GENMASK_ULL(41, 20),
-	.reg_touud_mask		= GENMASK_ULL(41, 20),
-	.reg_eccerrlog_addr_mask = GENMASK_ULL(45, 5),
-	.imc_base		= 0xd800,
-	.ibecc_base		= 0xd400,
-	.ibecc_error_log_offset	= 0x68,
-	.ibecc_available	= tgl_ibecc_available,
-	.err_addr		= rpl_p_err_addr,
-	.err_addr_to_sys_addr	= adl_err_addr_to_sys_addr,
-	.err_addr_to_imc_addr	= adl_err_addr_to_imc_addr,
-};
-
 static struct res_config mtl_ps_cfg = {
 	.machine_check				= true,
 	.num_imc				= 2,
@@ -877,11 +854,11 @@ static struct pci_device_id igen6_pci_tbl[] = {
 	{ PCI_VDEVICE(INTEL, DID_ASL_SKU1), .driver_data = (kernel_ulong_t)&adl_n_cfg },
 	{ PCI_VDEVICE(INTEL, DID_ASL_SKU2), .driver_data = (kernel_ulong_t)&adl_n_cfg },
 	{ PCI_VDEVICE(INTEL, DID_ASL_SKU3), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_RPL_P_SKU1), .driver_data = (kernel_ulong_t)&rpl_p_cfg },
-	{ PCI_VDEVICE(INTEL, DID_RPL_P_SKU2), .driver_data = (kernel_ulong_t)&rpl_p_cfg },
-	{ PCI_VDEVICE(INTEL, DID_RPL_P_SKU3), .driver_data = (kernel_ulong_t)&rpl_p_cfg },
-	{ PCI_VDEVICE(INTEL, DID_RPL_P_SKU4), .driver_data = (kernel_ulong_t)&rpl_p_cfg },
-	{ PCI_VDEVICE(INTEL, DID_RPL_P_SKU5), .driver_data = (kernel_ulong_t)&rpl_p_cfg },
+	{ PCI_VDEVICE(INTEL, DID_RPL_P_SKU1), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_RPL_P_SKU2), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_RPL_P_SKU3), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_RPL_P_SKU4), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_RPL_P_SKU5), .driver_data = (kernel_ulong_t)&adl_cfg },
 	{ PCI_VDEVICE(INTEL, DID_MTL_PS_SKU1), .driver_data = (kernel_ulong_t)&mtl_ps_cfg },
 	{ PCI_VDEVICE(INTEL, DID_MTL_PS_SKU2), .driver_data = (kernel_ulong_t)&mtl_ps_cfg },
 	{ PCI_VDEVICE(INTEL, DID_MTL_PS_SKU3), .driver_data = (kernel_ulong_t)&mtl_ps_cfg },
@@ -1237,11 +1214,7 @@ static void ecclog_work_cb(struct work_struct *work)
 
 	llist_for_each_entry_safe(node, tmp, head, llnode) {
 		memset(&res, 0, sizeof(res));
-		if (res_cfg->err_addr)
-			eaddr = res_cfg->err_addr(node->ecclog);
-		else
-			eaddr = node->ecclog & res_cfg->reg_eccerrlog_addr_mask;
-
+		eaddr	     = node->ecclog & res_cfg->reg_eccerrlog_addr_mask;
 		res.mc	     = node->mc;
 		res.sys_addr = res_cfg->err_addr_to_sys_addr(eaddr, res.mc);
 		res.imc_addr = res_cfg->err_addr_to_imc_addr(eaddr, res.mc);
-- 
2.43.0

[PATCH 06/10] EDAC/igen6: Remove unnecessary XOR on the zero-valued interleave bit

[Qiuxu Zhuo]

When reconstructing the removed interleave bit from an inflated memory
slice address, where a zero was inserted at the interleave bit position,
it's unnecessary to XOR this zero-valued interleave bit.

Remove this unnecessary XOR operation. No functional changes intended.

Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
---
 drivers/edac/igen6_edac.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/edac/igen6_edac.c b/drivers/edac/igen6_edac.c
index 12d718a50e1c..71ed50daeb19 100644
--- a/drivers/edac/igen6_edac.c
+++ b/drivers/edac/igen6_edac.c
@@ -448,16 +448,16 @@ static u64 mem_addr_to_sys_addr(u64 maddr)
 	return maddr;
 }
 
-static u64 mem_slice_hash(u64 addr, u64 mask, u64 hash_init, int intlv_bit)
+static u64 mem_slice_hash(u64 addr, u64 mask, u64 hash_init)
 {
+	/* The interleave bit in @addr is a zero. */
 	u64 hash_addr = addr & mask, hash = hash_init;
-	u64 intlv = (addr >> intlv_bit) & 1;
 	int i;
 
 	for (i = 6; i < 20; i++)
 		hash ^= (hash_addr >> i) & 1;
 
-	return hash ^ intlv;
+	return hash;
 }
 
 static u64 tgl_err_addr_to_mem_addr(u64 eaddr, int mc)
@@ -478,7 +478,7 @@ static u64 tgl_err_addr_to_mem_addr(u64 eaddr, int mc)
 	maddr = GET_BITFIELD(eaddr, intlv_bit, 63) << (intlv_bit + 1) |
 		GET_BITFIELD(eaddr, 0, intlv_bit - 1);
 
-	hash = mem_slice_hash(maddr, mask, mc, intlv_bit);
+	hash = mem_slice_hash(maddr, mask, mc);
 
 	return maddr | (hash << intlv_bit);
 }
-- 
2.43.0

[PATCH 07/10] EDAC/igen6: Simplify compute die ID comments

[Qiuxu Zhuo]

The existing comments repeat information already implied by the code
structure. Shorten them to SoC names only to reduce clutter and
improve readability.

No functional changes intended.

Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
---
 drivers/edac/igen6_edac.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/edac/igen6_edac.c b/drivers/edac/igen6_edac.c
index 71ed50daeb19..2960af172c57 100644
--- a/drivers/edac/igen6_edac.c
+++ b/drivers/edac/igen6_edac.c
@@ -223,7 +223,8 @@ static char ecclog_buf[ECCLOG_POOL_SIZE];
 static struct irq_work ecclog_irq_work;
 static struct work_struct ecclog_work;
 
-/* Compute die IDs for Elkhart Lake with IBECC */
+/* SoC compute die IDs with IBECC capability. */
+/* Elkhart Lake */
 #define DID_EHL_SKU5	0x4514
 #define DID_EHL_SKU6	0x4528
 #define DID_EHL_SKU7	0x452a
@@ -236,22 +237,22 @@ static struct work_struct ecclog_work;
 #define DID_EHL_SKU14	0x4534
 #define DID_EHL_SKU15	0x4536
 
-/* Compute die IDs for ICL-NNPI with IBECC */
+/* ICL-NNPI */
 #define DID_ICL_SKU8	0x4581
 #define DID_ICL_SKU10	0x4585
 #define DID_ICL_SKU11	0x4589
 #define DID_ICL_SKU12	0x458d
 
-/* Compute die IDs for Tiger Lake with IBECC */
+/* Tiger Lake */
 #define DID_TGL_SKU	0x9a14
 
-/* Compute die IDs for Alder Lake with IBECC */
+/* Alder Lake */
 #define DID_ADL_SKU1	0x4601
 #define DID_ADL_SKU2	0x4602
 #define DID_ADL_SKU3	0x4621
 #define DID_ADL_SKU4	0x4641
 
-/* Compute die IDs for Alder Lake-N with IBECC */
+/* Alder Lake-N */
 #define DID_ADL_N_SKU1	0x4614
 #define DID_ADL_N_SKU2	0x4617
 #define DID_ADL_N_SKU3	0x461b
@@ -265,38 +266,38 @@ static struct work_struct ecclog_work;
 #define DID_ADL_N_SKU11	0x467c
 #define DID_ADL_N_SKU12	0x4632
 
-/* Compute die IDs for Arizona Beach with IBECC */
+/* Arizona Beach */
 #define DID_AZB_SKU1	0x4676
 
-/* Compute did IDs for Amston Lake with IBECC */
+/* Amston Lake */
 #define DID_ASL_SKU1	0x464a
 #define DID_ASL_SKU2	0x4646
 #define DID_ASL_SKU3	0x4652
 
-/* Compute die IDs for Raptor Lake-P with IBECC */
+/* Raptor Lake-P */
 #define DID_RPL_P_SKU1	0xa706
 #define DID_RPL_P_SKU2	0xa707
 #define DID_RPL_P_SKU3	0xa708
 #define DID_RPL_P_SKU4	0xa716
 #define DID_RPL_P_SKU5	0xa718
 
-/* Compute die IDs for Meteor Lake-PS with IBECC */
+/* Meteor Lake-PS */
 #define DID_MTL_PS_SKU1	0x7d21
 #define DID_MTL_PS_SKU2	0x7d22
 #define DID_MTL_PS_SKU3	0x7d23
 #define DID_MTL_PS_SKU4	0x7d24
 
-/* Compute die IDs for Meteor Lake-P with IBECC */
+/* Meteor Lake-P */
 #define DID_MTL_P_SKU1	0x7d01
 #define DID_MTL_P_SKU2	0x7d02
 #define DID_MTL_P_SKU3	0x7d14
 
-/* Compute die IDs for Arrow Lake-UH with IBECC */
+/* Arrow Lake-UH */
 #define DID_ARL_UH_SKU1	0x7d06
 #define DID_ARL_UH_SKU2	0x7d20
 #define DID_ARL_UH_SKU3	0x7d30
 
-/* Compute die IDs for Panther Lake-H with IBECC */
+/* Panther Lake-H */
 #define DID_PTL_H_SKU1	0xb000
 #define DID_PTL_H_SKU2	0xb001
 #define DID_PTL_H_SKU3	0xb002
@@ -312,10 +313,10 @@ static struct work_struct ecclog_work;
 #define DID_PTL_H_SKU13	0xb02a
 #define DID_PTL_H_SKU14	0xb00a
 
-/* Compute die IDs for Wildcat Lake with IBECC */
+/* Wildcat Lake */
 #define DID_WCL_SKU1	0xfd00
 
-/* Compute die IDs for Nova Lake-H/HX with IBECC */
+/* Nova Lake-H/HX */
 #define DID_NVL_H_SKU1	0xd701
 #define DID_NVL_H_SKU2	0xd702
 #define DID_NVL_H_SKU3	0xd704
-- 
2.43.0

[PATCH 08/10] EDAC/igen6: Detect present memory controllers at runtime

[Qiuxu Zhuo]

The igen6_edac currently relies on res_config::num_imc to describe the
number of memory controllers supported by each SoC. As a result, adding
support for a new platform requires updating this configuration even
though the hardware can be discovered at runtime.

Instead, detect the number of present memory controllers at runtime and
size the driver state accordingly. This eliminates the need to update
res_config whenever a new SoC variant is added.

Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
---
 drivers/edac/igen6_edac.c | 97 ++++++++++++++++++++++++++-------------
 1 file changed, 64 insertions(+), 33 deletions(-)

diff --git a/drivers/edac/igen6_edac.c b/drivers/edac/igen6_edac.c
index 2960af172c57..d468c655348c 100644
--- a/drivers/edac/igen6_edac.c
+++ b/drivers/edac/igen6_edac.c
@@ -42,7 +42,8 @@
 
 #define GET_BITFIELD(v, lo, hi) (((v) & GENMASK_ULL(hi, lo)) >> (lo))
 
-#define NUM_IMC				2 /* Max memory controllers */
+/* Probing upper bound, not a hardware capability limit. */
+#define MAX_IMC_TO_PROBE		8
 #define NUM_CHANNELS			2 /* Max channels */
 #define NUM_DIMMS			2 /* Max DIMMs per channel */
 
@@ -182,11 +183,11 @@ static struct res_config {
 } *res_cfg;
 
 static struct igen6_pvt {
-	struct igen6_imc imc[NUM_IMC];
 	void __iomem *memss_pma_cr;
 	u64 ms_hash;
 	u64 ms_s_size;
 	int ms_l_map;
+	struct igen6_imc imc[];
 } *igen6_pvt;
 
 /* The top of low usable DRAM */
@@ -353,6 +354,46 @@ static int get_mchbar(struct pci_dev *pdev, u64 *mchbar)
 	return 0;
 }
 
+/* Check whether the memory controller is absent. */
+static bool imc_absent(void __iomem *window)
+{
+	return readl(window + MAD_INTER_CHANNEL_OFFSET) == ~0;
+}
+
+/* Return MMIO base address of the memory controller if it's present, otherwise return NULL. */
+static void __iomem *map_imc_window(u64 mchbar, int pmc)
+{
+	void __iomem *window;
+
+	window = ioremap(mchbar + pmc * MCHBAR_SIZE, MCHBAR_SIZE);
+	if (!window)
+		return NULL;
+
+	if (imc_absent(window)) {
+		iounmap(window);
+		return NULL;
+	}
+
+	return window;
+}
+
+/* Return the number of present memory controllers. */
+static int get_imc_num(u64 mchbar)
+{
+	void __iomem *window;
+	int lmc, pmc;
+
+	for (lmc = 0, pmc = 0; pmc < MAX_IMC_TO_PROBE; pmc++) {
+		window = map_imc_window(mchbar, pmc);
+		if (window) {
+			iounmap(window);
+			lmc++;
+		}
+	}
+
+	return lmc;
+}
+
 static bool ehl_ibecc_available(struct pci_dev *pdev)
 {
 	u32 v;
@@ -1457,18 +1498,27 @@ static struct igen6_pvt *igen6_pvt_setup(struct pci_dev *pdev)
 {
 	void __iomem *memss_pma_cr;
 	struct igen6_pvt *pvt;
+	int imc_num, rc;
 	u64 mchbar;
-	int rc;
-
-	pvt = kzalloc_obj(*igen6_pvt);
-	if (!pvt)
-		return NULL;
 
 	rc = get_mchbar(pdev, &mchbar);
-	if (rc) {
-		kfree(pvt);
+	if (rc)
+		return NULL;
+
+	imc_num = get_imc_num(mchbar);
+	if (!imc_num) {
+		igen6_printk(KERN_ERR, "No mc found.\n");
 		return NULL;
 	}
+	edac_dbg(2, "%d mcs found.\n", imc_num);
+
+	/* Use the runtime detected IMC count. */
+	if (res_cfg->num_imc != imc_num)
+		res_cfg->num_imc = imc_num;
+
+	pvt = kzalloc_flex(*pvt, imc, imc_num);
+	if (!pvt)
+		return NULL;
 
 	memss_pma_cr = ioremap(mchbar, MCHBAR_SIZE * 2);
 	if (!memss_pma_cr) {
@@ -1553,12 +1603,6 @@ static void igen6_check(struct mem_ctl_info *mci)
 		irq_work_queue(&ecclog_irq_work);
 }
 
-/* Check whether the memory controller is absent. */
-static bool igen6_imc_absent(void __iomem *window)
-{
-	return readl(window + MAD_INTER_CHANNEL_OFFSET) == ~0;
-}
-
 static void imc_release(struct device *dev)
 {
 	/* Nothing to do, the 'imc' owns the 'dev' and will also release it. */
@@ -1670,26 +1714,15 @@ static int igen6_register_mcis(struct pci_dev *pdev, u64 mchbar)
 {
 	void __iomem *window;
 	int lmc, pmc, rc;
-	u64 base;
 
-	for (lmc = 0, pmc = 0; pmc < NUM_IMC; pmc++) {
-		base   = mchbar + pmc * MCHBAR_SIZE;
-		window = ioremap(base, MCHBAR_SIZE);
-		if (!window) {
-			igen6_printk(KERN_ERR, "Failed to ioremap 0x%llx for mc%d\n", base, pmc);
-			rc = -ENOMEM;
-			goto out_unregister_mcis;
-		}
-
-		if (igen6_imc_absent(window)) {
-			iounmap(window);
-			edac_dbg(2, "Skip absent mc%d\n", pmc);
+	for (lmc = 0, pmc = 0; pmc < MAX_IMC_TO_PROBE; pmc++) {
+		window = map_imc_window(mchbar, pmc);
+		if (!window)
 			continue;
-		}
 
 		rc = igen6_register_mci(lmc, window, pdev);
 		if (rc)
-			goto out_iounmap;
+			goto err_unregister;
 
 		/* Done, if all present MCs are detected and registered. */
 		if (++lmc >= res_cfg->num_imc)
@@ -1709,10 +1742,8 @@ static int igen6_register_mcis(struct pci_dev *pdev, u64 mchbar)
 
 	return 0;
 
-out_iounmap:
+err_unregister:
 	iounmap(window);
-
-out_unregister_mcis:
 	igen6_unregister_mcis();
 
 	return rc;
-- 
2.43.0

[PATCH 09/10] EDAC/igen6: Remove redundant resource configuration tables

[Qiuxu Zhuo]

Several resource configuration tables differ only in their num_imc
value, while all other fields are identical. Their only purpose is to
describe the number of memory controllers supported by a platform.

Since IMC count is now detected at runtime, these duplicate tables no
longer carry any unique platform information. Reuse the shared
configurations and remove the redundant tables.

Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
---
 drivers/edac/igen6_edac.c | 64 +++++++++++----------------------------
 1 file changed, 17 insertions(+), 47 deletions(-)

diff --git a/drivers/edac/igen6_edac.c b/drivers/edac/igen6_edac.c
index d468c655348c..ea34adac822b 100644
--- a/drivers/edac/igen6_edac.c
+++ b/drivers/edac/igen6_edac.c
@@ -736,21 +736,6 @@ static struct res_config adl_cfg = {
 	.err_addr_to_imc_addr	= adl_err_addr_to_imc_addr,
 };
 
-static struct res_config adl_n_cfg = {
-	.machine_check		= true,
-	.num_imc		= 1,
-	.reg_mchbar_mask	= GENMASK_ULL(41, 17),
-	.reg_tom_mask		= GENMASK_ULL(41, 20),
-	.reg_touud_mask		= GENMASK_ULL(41, 20),
-	.reg_eccerrlog_addr_mask = GENMASK_ULL(45, 5),
-	.imc_base		= 0xd800,
-	.ibecc_base		= 0xd400,
-	.ibecc_error_log_offset	= 0x68,
-	.ibecc_available	= tgl_ibecc_available,
-	.err_addr_to_sys_addr	= adl_err_addr_to_sys_addr,
-	.err_addr_to_imc_addr	= adl_err_addr_to_imc_addr,
-};
-
 static struct res_config mtl_ps_cfg = {
 	.machine_check				= true,
 	.num_imc				= 2,
@@ -813,21 +798,6 @@ static struct res_config ptl_h_cfg = {
 	.err_addr_to_imc_addr		= adl_err_addr_to_imc_addr,
 };
 
-static struct res_config wcl_cfg = {
-	.machine_check		= true,
-	.num_imc		= 1,
-	.reg_mchbar_mask	= GENMASK_ULL(41, 17),
-	.reg_tom_mask		= GENMASK_ULL(41, 20),
-	.reg_touud_mask		= GENMASK_ULL(41, 20),
-	.reg_eccerrlog_addr_mask = GENMASK_ULL(38, 5),
-	.imc_base		= 0xd800,
-	.ibecc_base		= 0xd400,
-	.ibecc_error_log_offset	= 0x170,
-	.ibecc_available	= mtl_p_ibecc_available,
-	.err_addr_to_sys_addr	= adl_err_addr_to_sys_addr,
-	.err_addr_to_imc_addr	= adl_err_addr_to_imc_addr,
-};
-
 static struct res_config nvl_h_cfg = {
 	.machine_check			= true,
 	.num_imc			= 2,
@@ -880,22 +850,22 @@ static struct pci_device_id igen6_pci_tbl[] = {
 	{ PCI_VDEVICE(INTEL, DID_ADL_SKU2), .driver_data = (kernel_ulong_t)&adl_cfg },
 	{ PCI_VDEVICE(INTEL, DID_ADL_SKU3), .driver_data = (kernel_ulong_t)&adl_cfg },
 	{ PCI_VDEVICE(INTEL, DID_ADL_SKU4), .driver_data = (kernel_ulong_t)&adl_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU1), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU2), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU3), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU4), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU5), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU6), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU7), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU8), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU9), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU10), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU11), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU12), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_AZB_SKU1), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ASL_SKU1), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ASL_SKU2), .driver_data = (kernel_ulong_t)&adl_n_cfg },
-	{ PCI_VDEVICE(INTEL, DID_ASL_SKU3), .driver_data = (kernel_ulong_t)&adl_n_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU1), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU2), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU3), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU4), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU5), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU6), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU7), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU8), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU9), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU10), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU11), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ADL_N_SKU12), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_AZB_SKU1), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ASL_SKU1), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ASL_SKU2), .driver_data = (kernel_ulong_t)&adl_cfg },
+	{ PCI_VDEVICE(INTEL, DID_ASL_SKU3), .driver_data = (kernel_ulong_t)&adl_cfg },
 	{ PCI_VDEVICE(INTEL, DID_RPL_P_SKU1), .driver_data = (kernel_ulong_t)&adl_cfg },
 	{ PCI_VDEVICE(INTEL, DID_RPL_P_SKU2), .driver_data = (kernel_ulong_t)&adl_cfg },
 	{ PCI_VDEVICE(INTEL, DID_RPL_P_SKU3), .driver_data = (kernel_ulong_t)&adl_cfg },
@@ -911,6 +881,7 @@ static struct pci_device_id igen6_pci_tbl[] = {
 	{ PCI_VDEVICE(INTEL, DID_ARL_UH_SKU1), .driver_data = (kernel_ulong_t)&mtl_p_cfg },
 	{ PCI_VDEVICE(INTEL, DID_ARL_UH_SKU2), .driver_data = (kernel_ulong_t)&mtl_p_cfg },
 	{ PCI_VDEVICE(INTEL, DID_ARL_UH_SKU3), .driver_data = (kernel_ulong_t)&mtl_p_cfg },
+	{ PCI_VDEVICE(INTEL, DID_WCL_SKU1), .driver_data = (kernel_ulong_t)&mtl_p_cfg },
 	{ PCI_VDEVICE(INTEL, DID_PTL_H_SKU1), .driver_data = (kernel_ulong_t)&ptl_h_cfg },
 	{ PCI_VDEVICE(INTEL, DID_PTL_H_SKU2), .driver_data = (kernel_ulong_t)&ptl_h_cfg },
 	{ PCI_VDEVICE(INTEL, DID_PTL_H_SKU3), .driver_data = (kernel_ulong_t)&ptl_h_cfg },
@@ -925,7 +896,6 @@ static struct pci_device_id igen6_pci_tbl[] = {
 	{ PCI_VDEVICE(INTEL, DID_PTL_H_SKU12), .driver_data = (kernel_ulong_t)&ptl_h_cfg },
 	{ PCI_VDEVICE(INTEL, DID_PTL_H_SKU13), .driver_data = (kernel_ulong_t)&ptl_h_cfg },
 	{ PCI_VDEVICE(INTEL, DID_PTL_H_SKU14), .driver_data = (kernel_ulong_t)&ptl_h_cfg },
-	{ PCI_VDEVICE(INTEL, DID_WCL_SKU1), .driver_data = (kernel_ulong_t)&wcl_cfg },
 	{ PCI_VDEVICE(INTEL, DID_NVL_H_SKU1), .driver_data = (kernel_ulong_t)&nvl_h_cfg },
 	{ PCI_VDEVICE(INTEL, DID_NVL_H_SKU2), .driver_data = (kernel_ulong_t)&nvl_h_cfg },
 	{ PCI_VDEVICE(INTEL, DID_NVL_H_SKU3), .driver_data = (kernel_ulong_t)&nvl_h_cfg },
-- 
2.43.0

[PATCH 10/10] EDAC/igen6: Refactor address translation logic

[Qiuxu Zhuo]

The igen6 EDAC driver implements similar interleave and hash translation
logic at multiple levels of the memory hierarchy.

The separate implementations duplicate decoding logic, making future
changes harder and increasing the risk of behavior diverging.

Consolidate the common address translation operations into shared
helpers so all decoding paths use a single implementation.

No functional changes intended.

Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
---
 drivers/edac/igen6_edac.c | 244 +++++++++++++++++++++++++-------------
 1 file changed, 159 insertions(+), 85 deletions(-)

diff --git a/drivers/edac/igen6_edac.c b/drivers/edac/igen6_edac.c
index ea34adac822b..96da6f70539b 100644
--- a/drivers/edac/igen6_edac.c
+++ b/drivers/edac/igen6_edac.c
@@ -123,6 +123,43 @@
 #define MEM_SLICE_HASH_MASK(v)		(GET_BITFIELD(v, 6, 19) << 6)
 #define MEM_SLICE_HASH_LSB_MASK_BIT(v)	GET_BITFIELD(v, 24, 26)
 
+/*
+ * A slice represents a portion of memory space participating in an
+ * interleave relationship within the memory hierarchy.
+ *
+ * It can represent in different levels such as:
+ *
+ *   - a pair of memory controllers
+ *   - a memory controller
+ *   - a memory channel
+ *   - a memory sub-channel / DIMM
+ *
+ * +--------+
+ * |        |
+ * | Zone 1 |
+ * |        |
+ * +--------+  +--------+
+ * |        |  |        |
+ * |        |  |        |
+ * | Zone 0 |  | Zone 0 |
+ * |        |  |        |
+ * |        |  |        |
+ * +--------+  +--------+
+ *
+ *  Slice L     Slice S
+ *
+ * Memory space is divided into:
+ *
+ *   - Zone 0 : Interleaved region
+ *   - Zone 1 : Non-interleaved region (upper part of the large slice).
+ */
+struct slice {
+	/* Slice address. */
+	u64 addr;
+	/* Slice that @addr belongs to. */
+	int id;
+};
+
 struct igen6_imc {
 	int mc;
 	struct mem_ctl_info *mci;
@@ -323,6 +360,102 @@ static struct work_struct ecclog_work;
 #define DID_NVL_H_SKU3	0xd704
 #define DID_NVL_H_SKU4	0xd705
 
+/* Remove the interleave bit and shift upper part down to fill gap. */
+static u64 squeeze_addr(u64 addr, int intlv_bit)
+{
+	u64 slice_addr;
+
+	slice_addr  = GET_BITFIELD(addr, intlv_bit + 1, 63) << intlv_bit;
+	slice_addr |= GET_BITFIELD(addr, 0, intlv_bit - 1);
+
+	return slice_addr;
+}
+
+/* Shift the upper bits up and insert a zero at the @intlv_bit bit position. */
+static u64 inflate_addr(u64 addr, int intlv_bit)
+{
+	u64 inflated_addr;
+
+	/* Insert a zero at @intlv_bit position. */
+	inflated_addr  = GET_BITFIELD(addr, intlv_bit, 63) << (intlv_bit + 1);
+	inflated_addr |= GET_BITFIELD(addr, 0, intlv_bit - 1);
+
+	return inflated_addr;
+}
+
+static u64 compute_hash(u64 addr, u64 hash_mask, u64 hash_base, int intlv_bit)
+{
+	u64 hash_addr;
+	int i;
+
+	/*
+	 * In hash mode, @intlv_bit is the lowest selected bit of @addr
+	 * to be XORed. While @mask may or may not include this @intlv_bit,
+	 * we enforce that @mask includes @intlv_bit to ensure @intlv_bit is
+	 * XORed exactly once.
+	 */
+	hash_mask |= BIT_ULL(intlv_bit);
+	hash_addr  = addr & hash_mask;
+
+	for (i = 6; i < 20; i++)
+		hash_base ^= (hash_addr >> i) & 1;
+
+	return hash_base;
+}
+
+/*
+ * Converts a higher-level address (system / IMC / channel) into a lower-level
+ * slice address and identifier.
+ */
+static void translate_to_lower_level(u64 addr, u64 hash_mask, u64 hash_base,
+				     int intlv_bit, u64 s_size, int l_map,
+				     struct slice *slice)
+{
+	/* In non-interleave zone. */
+	if (addr >= 2 * s_size) {
+		slice->addr = addr - s_size;
+		slice->id  = l_map;
+		return;
+	}
+
+	/* In interleave zone. */
+	slice->addr = squeeze_addr(addr, intlv_bit);
+
+	/* Non-hash mode. */
+	if (!hash_mask) {
+		slice->id = GET_BITFIELD(addr, intlv_bit, intlv_bit);
+		return;
+	}
+
+	/* Hash mode. */
+	slice->id = compute_hash(addr, hash_mask, hash_base, intlv_bit);
+}
+
+/* Reconstruct address for upper memory hierarchy level. */
+static u64 translate_to_upper_level(u64 addr, u64 hash_mask, u64 hash_base,
+				    int intlv_bit, u64 s_size)
+{
+	u64 inflated_addr, hash_val;
+
+	/* In non-interleave zone. */
+	if (addr >= s_size)
+		return addr + s_size;
+
+	/*
+	 * In interleave zone.
+	 *
+	 * Insert a zero at @intlv_bit position.
+	 */
+	inflated_addr = inflate_addr(addr, intlv_bit);
+
+	/*
+	 * Reconstruct the removed interleave bit and use it to replace
+	 * the zero at @intlv_bit position.
+	 */
+	hash_val = compute_hash(inflated_addr, hash_mask, hash_base, intlv_bit);
+	return inflated_addr | (hash_val << intlv_bit);
+}
+
 static int get_mchbar(struct pci_dev *pdev, u64 *mchbar)
 {
 	union  {
@@ -490,21 +623,9 @@ static u64 mem_addr_to_sys_addr(u64 maddr)
 	return maddr;
 }
 
-static u64 mem_slice_hash(u64 addr, u64 mask, u64 hash_init)
-{
-	/* The interleave bit in @addr is a zero. */
-	u64 hash_addr = addr & mask, hash = hash_init;
-	int i;
-
-	for (i = 6; i < 20; i++)
-		hash ^= (hash_addr >> i) & 1;
-
-	return hash;
-}
-
 static u64 tgl_err_addr_to_mem_addr(u64 eaddr, int mc)
 {
-	u64 maddr, hash, mask, ms_s_size;
+	u64 mask, ms_s_size;
 	int intlv_bit;
 	u32 ms_hash;
 
@@ -517,12 +638,7 @@ static u64 tgl_err_addr_to_mem_addr(u64 eaddr, int mc)
 	mask = MEM_SLICE_HASH_MASK(ms_hash);
 	intlv_bit = MEM_SLICE_HASH_LSB_MASK_BIT(ms_hash) + 6;
 
-	maddr = GET_BITFIELD(eaddr, intlv_bit, 63) << (intlv_bit + 1) |
-		GET_BITFIELD(eaddr, 0, intlv_bit - 1);
-
-	hash = mem_slice_hash(maddr, mask, mc);
-
-	return maddr | (hash << intlv_bit);
+	return translate_to_upper_level(eaddr, mask, mc, intlv_bit, ms_s_size);
 }
 
 static u64 tgl_err_addr_to_sys_addr(u64 eaddr, int mc)
@@ -544,8 +660,9 @@ static u64 adl_err_addr_to_sys_addr(u64 eaddr, int mc)
 
 static u64 adl_err_addr_to_imc_addr(u64 eaddr, int mc)
 {
-	u64 imc_addr, ms_s_size = igen6_pvt->ms_s_size;
+	u64 ms_s_size = igen6_pvt->ms_s_size;
 	struct igen6_imc *imc = &igen6_pvt->imc[mc];
+	struct slice slice;
 	int intlv_bit;
 	u32 mc_hash;
 
@@ -556,10 +673,8 @@ static u64 adl_err_addr_to_imc_addr(u64 eaddr, int mc)
 
 	intlv_bit = MAC_MC_HASH_LSB(mc_hash) + 6;
 
-	imc_addr = GET_BITFIELD(eaddr, intlv_bit + 1, 63) << intlv_bit |
-		   GET_BITFIELD(eaddr, 0, intlv_bit - 1);
-
-	return imc_addr;
+	translate_to_lower_level(eaddr, 0, 0, intlv_bit, ms_s_size, 0, &slice);
+	return slice.addr;
 }
 
 static enum mem_type ptl_h_get_mem_type(struct igen6_imc *imc)
@@ -996,62 +1111,13 @@ static void set_dimm_params(struct igen6_imc *imc, int chan)
 	imc->dimm_s_size[chan] = MAD_DIMM_CH_DIMM_S_SIZE(val);
 }
 
-static int decode_chan_idx(u64 addr, u64 mask, int intlv_bit)
-{
-	u64 hash_addr, hash = 0;
-	int i;
-
-	/*
-	 * In hash mode, the @intlv_bit is the lowest selected bit of @addr
-	 * to be XORed. While @mask may or may not include this @intlv_bit,
-	 * we enforce that @mask includes @intlv_bit to ensure @intlv_bit is
-	 * XORed exactly once.
-	 */
-	mask |= 1 << intlv_bit;
-	hash_addr = addr & mask;
-
-	for (i = 6; i < 20; i++)
-		hash ^= (hash_addr >> i) & 1;
-
-	return (int)hash;
-}
-
-static u64 decode_channel_addr(u64 addr, int intlv_bit)
-{
-	u64 channel_addr;
-
-	/* Remove the interleave bit and shift upper part down to fill gap */
-	channel_addr  = GET_BITFIELD(addr, intlv_bit + 1, 63) << intlv_bit;
-	channel_addr |= GET_BITFIELD(addr, 0, intlv_bit - 1);
-
-	return channel_addr;
-}
-
-static void decode_addr(u64 addr, u32 hash, u64 s_size, int l_map,
-			int *idx, u64 *sub_addr)
-{
-	int intlv_bit = CHANNEL_HASH_LSB_MASK_BIT(hash) + 6;
-
-	if (addr >= 2 * s_size) {
-		*sub_addr = addr - s_size;
-		*idx = l_map;
-		return;
-	}
-
-	*sub_addr = decode_channel_addr(addr, intlv_bit);
-
-	if (CHANNEL_HASH_MODE(hash))
-		*idx = decode_chan_idx(addr, CHANNEL_HASH_MASK(hash), intlv_bit);
-	else
-		*idx = GET_BITFIELD(addr, intlv_bit, intlv_bit);
-}
-
 static int igen6_decode(struct decoded_addr *res)
 {
 	struct igen6_imc *imc = &igen6_pvt->imc[res->mc];
-	u64 addr = res->imc_addr, sub_addr, s_size;
-	int idx, l_map;
-	u32 hash;
+	u64 addr = res->imc_addr, s_size;
+	int intlv_bit, l_map;
+	u32 hash, hash_mask;
+	struct slice slice;
 
 	if (addr >= igen6_tom) {
 		edac_dbg(0, "Address 0x%llx out of range\n", addr);
@@ -1062,17 +1128,25 @@ static int igen6_decode(struct decoded_addr *res)
 	hash   = readl(imc->window + CHANNEL_HASH_OFFSET);
 	s_size = imc->ch_s_size;
 	l_map  = imc->ch_l_map;
-	decode_addr(addr, hash, s_size, l_map, &idx, &sub_addr);
-	res->channel_idx  = idx;
-	res->channel_addr = sub_addr;
+	hash_mask = CHANNEL_HASH_MODE(hash) ? CHANNEL_HASH_MASK(hash) : 0;
+	intlv_bit = CHANNEL_HASH_LSB_MASK_BIT(hash) + 6;
+
+	translate_to_lower_level(addr, hash_mask, 0, intlv_bit, s_size, l_map, &slice);
+
+	res->channel_idx  = slice.id;
+	res->channel_addr = slice.addr;
 
 	/* Decode sub-channel/DIMM */
 	hash   = readl(imc->window + CHANNEL_EHASH_OFFSET);
-	s_size = imc->dimm_s_size[idx];
-	l_map  = imc->dimm_l_map[idx];
-	decode_addr(res->channel_addr, hash, s_size, l_map, &idx, &sub_addr);
-	res->sub_channel_idx  = idx;
-	res->sub_channel_addr = sub_addr;
+	s_size = imc->dimm_s_size[res->channel_idx];
+	l_map  = imc->dimm_l_map[res->channel_idx];
+	hash_mask = CHANNEL_HASH_MODE(hash) ? CHANNEL_HASH_MASK(hash) : 0;
+	intlv_bit = CHANNEL_HASH_LSB_MASK_BIT(hash) + 6;
+
+	translate_to_lower_level(res->channel_addr, hash_mask, 0, intlv_bit, s_size, l_map, &slice);
+
+	res->sub_channel_idx  = slice.id;
+	res->sub_channel_addr = slice.addr;
 
 	return 0;
 }
-- 
2.43.0

Re: [PATCH 09/10] EDAC/igen6: Remove redundant resource configuration tables

[Luck, Tony]

On Mon, Jul 06, 2026 at 11:22:32AM +0800, Qiuxu Zhuo wrote:
> Several resource configuration tables differ only in their num_imc
> value, while all other fields are identical. Their only purpose is to
> describe the number of memory controllers supported by a platform.
> 
> Since IMC count is now detected at runtime, these duplicate tables no
> longer carry any unique platform information. Reuse the shared
> configurations and remove the redundant tables.
...
> +	{ PCI_VDEVICE(INTEL, DID_WCL_SKU1), .driver_data = (kernel_ulong_t)&mtl_p_cfg },

Sharing these is good. But also might be confusing since the structure
names include the three character short form of the CPU name.

Please add some comments before each structure definition to list
which CPU models are sharing. E.g.

/* Shared by Meteor Lake-P, Arrow Lake, Wildcat Lake. */
static struct res_config mtl_p_cfg = {
 ...
}

-Tony

RE: [PATCH 09/10] EDAC/igen6: Remove redundant resource configuration tables

[Zhuo, Qiuxu]

Hi Tony,

> From: Luck, Tony <tony.luck@intel.com>
> Sent: Tuesday, July 7, 2026 7:29 AM
> To: Zhuo, Qiuxu <qiuxu.zhuo@intel.com>
> Cc: Borislav Petkov <bp@alien8.de>; Lai, Yi1 <yi1.lai@intel.com>; linux-
> edac@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 09/10] EDAC/igen6: Remove redundant resource
> configuration tables
> 
> On Mon, Jul 06, 2026 at 11:22:32AM +0800, Qiuxu Zhuo wrote:
> > Several resource configuration tables differ only in their num_imc
> > value, while all other fields are identical. Their only purpose is to
> > describe the number of memory controllers supported by a platform.
> >
> > Since IMC count is now detected at runtime, these duplicate tables no
> > longer carry any unique platform information. Reuse the shared
> > configurations and remove the redundant tables.
> ...
> > +	{ PCI_VDEVICE(INTEL, DID_WCL_SKU1), .driver_data =
> > +(kernel_ulong_t)&mtl_p_cfg },
> 
> Sharing these is good. But also might be confusing since the structure names
> include the three character short form of the CPU name.
> 
> Please add some comments before each structure definition to list which CPU
> models are sharing. E.g.
> 
> /* Shared by Meteor Lake-P, Arrow Lake, Wildcat Lake. */ static struct
> res_config mtl_p_cfg = {  ...
> }

Thanks for the review.
OK, I'll add comments as you suggested in v2.

Thanks!
-Qiuxu