Linux EDAC development
 help / color / mirror / Atom feed
* [PATCH] EDAC/amd64: Fix incorrect Node ID mapping on CPU-only Zen4+ systems
@ 2026-06-08  7:26 Phineas Su
  2026-06-09 16:07 ` Yazen Ghannam
  0 siblings, 1 reply; 7+ messages in thread
From: Phineas Su @ 2026-06-08  7:26 UTC (permalink / raw)
  To: yazen.ghannam, bp, tony.luck
  Cc: muralidhara.mk, linux-edac, linux-kernel, Phineas Su

On CPU-only systems using AMD Zen4 (Genoa) and newer processors, memory
ECC errors can be reported with an incorrect Node ID (incremented by 1).
This happens because these CPUs use SMCA_UMC_V2 banks, which triggers
the GPU node ID fixup logic in fixup_node_id().

Since it is a CPU-only system, gpu_node_map is not initialized and
gpu_node_map.base_node_id is 0. The check (nid < gpu_node_map.base_node_id)
evaluates to (nid < 0), which is always false for the unsigned u8 nid.
The function then incorrectly applies the fixup (nid - 0 + 1), resulting
in the Node ID being incremented.

Fix this by ensuring that gpu_node_map has been initialized (i.e.,
node_count is non-zero) before applying any GPU-specific fixups.

Fixes: 4251566ebc1c ("EDAC/amd64: Cache and use GPU node map")
Signed-off-by: Phineas Su <pohaosu@google.com>
---
 drivers/edac/amd64_edac.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index c6aa69dbd9fb..1e688123a50c 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1047,6 +1047,10 @@ static int fixup_node_id(int node_id, struct mce *m)
 	if (smca_get_bank_type(m->extcpu, m->bank) != SMCA_UMC_V2)
 		return node_id;
 
+	/* If no GPU nodes are present, no fixup is needed. */
+	if (!gpu_node_map.node_count)
+		return node_id;
+
 	/* Nodes below the GPU base node are CPU nodes and don't need a fixup. */
 	if (nid < gpu_node_map.base_node_id)
 		return node_id;
-- 
2.54.0.1032.g2f8565e1d1-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] EDAC/amd64: Fix incorrect Node ID mapping on CPU-only Zen4+ systems
  2026-06-08  7:26 [PATCH] EDAC/amd64: Fix incorrect Node ID mapping on CPU-only Zen4+ systems Phineas Su
@ 2026-06-09 16:07 ` Yazen Ghannam
  2026-06-14 14:15   ` Phineas Su
  0 siblings, 1 reply; 7+ messages in thread
From: Yazen Ghannam @ 2026-06-09 16:07 UTC (permalink / raw)
  To: Phineas Su; +Cc: bp, tony.luck, muralidhara.mk, linux-edac, linux-kernel

On Mon, Jun 08, 2026 at 07:26:07AM +0000, Phineas Su wrote:
> On CPU-only systems using AMD Zen4 (Genoa) and newer processors, memory
> ECC errors can be reported with an incorrect Node ID (incremented by 1).
> This happens because these CPUs use SMCA_UMC_V2 banks, which triggers
> the GPU node ID fixup logic in fixup_node_id().
> 

Can you please share the MCA_IPID register value for these banks?

The statement 'Zen4 and newer processors...use SMCA_UMC_V2 banks' isn't
generally true.

This system may be a special case. Or there may be a hardware issue. Or
there may be a bug in the kernel enumeration code.

Thanks,
Yazen

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] EDAC/amd64: Fix incorrect Node ID mapping on CPU-only Zen4+ systems
  2026-06-09 16:07 ` Yazen Ghannam
@ 2026-06-14 14:15   ` Phineas Su
  2026-06-14 15:06     ` [PATCH v2] EDAC/amd64: Only translate Node ID if GPU nodes are present Phineas Su
  0 siblings, 1 reply; 7+ messages in thread
From: Phineas Su @ 2026-06-14 14:15 UTC (permalink / raw)
  To: Yazen Ghannam; +Cc: bp, tony.luck, muralidhara.mk, linux-edac, linux-kernel

On Wed, Jun 10, 2026 at 12:07 AM Yazen Ghannam <yazen.ghannam@amd.com> wrote:
>
> On Mon, Jun 08, 2026 at 07:26:07AM +0000, Phineas Su wrote:
> > On CPU-only systems using AMD Zen4 (Genoa) and newer processors, memory
> > ECC errors can be reported with an incorrect Node ID (incremented by 1).
> > This happens because these CPUs use SMCA_UMC_V2 banks, which triggers
> > the GPU node ID fixup logic in fixup_node_id().
> >
>
> Can you please share the MCA_IPID register value for these banks?
>
> The statement 'Zen4 and newer processors...use SMCA_UMC_V2 banks' isn't
> generally true.
>
> This system may be a special case. Or there may be a hardware issue. Or
> there may be a bug in the kernel enumeration code.
>
> Thanks,
> Yazen

Hi Yazen,

Thanks, you are correct. I think we misunderstood the code regarding
Zen4 CPUs generally using UMC_V2.
I was troubleshooting an internal memory error reporting issue on a
CPU-only system and initially thought this might be the problem.
However, I think making the code more robust and clearer is
worthwhile. If a CPU-only system were to report UMC_V2 (due to future
hardware iterations, firmware bugs, or enumeration anomalies), the
current logic would incorrectly apply the GPU fixup.
I will send a v2 of the patch with an updated commit message framing
this as a robustness improvement.

Thanks,
Phineas Su

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] EDAC/amd64: Only translate Node ID if GPU nodes are present
  2026-06-14 14:15   ` Phineas Su
@ 2026-06-14 15:06     ` Phineas Su
  2026-06-22 17:07       ` Yazen Ghannam
  0 siblings, 1 reply; 7+ messages in thread
From: Phineas Su @ 2026-06-14 15:06 UTC (permalink / raw)
  To: yazen.ghannam
  Cc: bp, tony.luck, muralidhara.mk, linux-edac, linux-kernel,
	Phineas Su

The fixup_node_id() function adjusts the AMD Node ID for GPU memory
controllers (which report as SMCA_UMC_V2) on heterogeneous systems.
CPU memory controllers typically report as SMCA_UMC (v1) and do not
require translation.

If a CPU memory controller on a CPU-only system were to report as
SMCA_UMC_V2 (e.g. due to future hardware iterations, firmware reporting,
or kernel enumeration differences), the code would attempt to apply the
GPU translation. Since gpu_node_map is uninitialized on CPU-only systems,
this would lead to an unintended translation being applied.

Harden the translation logic by checking that GPU nodes are actually
present in the system (gpu_node_map.node_count > 0) before applying the
translation. This ensures CPU-only systems are always safely bypassed.

Signed-off-by: Phineas Su <pohaosu@google.com>
---
v2:
  - Reframed commit message to focus on robustness rather than a confirmed
    bug, as Zen4 CPUs do not generally report UMC_V2. (Yazen)

 drivers/edac/amd64_edac.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index c6aa69dbd9fb..1e688123a50c 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1047,6 +1047,10 @@ static int fixup_node_id(int node_id, struct mce *m)
 	if (smca_get_bank_type(m->extcpu, m->bank) != SMCA_UMC_V2)
 		return node_id;
 
+	/* If no GPU nodes are present, no fixup is needed. */
+	if (!gpu_node_map.node_count)
+		return node_id;
+
 	/* Nodes below the GPU base node are CPU nodes and don't need a fixup. */
 	if (nid < gpu_node_map.base_node_id)
 		return node_id;
-- 
2.54.0.1136.gdb2ca164c4-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] EDAC/amd64: Only translate Node ID if GPU nodes are present
  2026-06-14 15:06     ` [PATCH v2] EDAC/amd64: Only translate Node ID if GPU nodes are present Phineas Su
@ 2026-06-22 17:07       ` Yazen Ghannam
  2026-06-23  0:49         ` Borislav Petkov
  0 siblings, 1 reply; 7+ messages in thread
From: Yazen Ghannam @ 2026-06-22 17:07 UTC (permalink / raw)
  To: Phineas Su; +Cc: bp, tony.luck, muralidhara.mk, linux-edac, linux-kernel

On Sun, Jun 14, 2026 at 03:06:18PM +0000, Phineas Su wrote:
> The fixup_node_id() function adjusts the AMD Node ID for GPU memory
> controllers (which report as SMCA_UMC_V2) on heterogeneous systems.
> CPU memory controllers typically report as SMCA_UMC (v1) and do not
> require translation.
> 
> If a CPU memory controller on a CPU-only system were to report as
> SMCA_UMC_V2 (e.g. due to future hardware iterations, firmware reporting,
> or kernel enumeration differences), the code would attempt to apply the
> GPU translation. Since gpu_node_map is uninitialized on CPU-only systems,
> this would lead to an unintended translation being applied.
> 
> Harden the translation logic by checking that GPU nodes are actually
> present in the system (gpu_node_map.node_count > 0) before applying the
> translation. This ensures CPU-only systems are always safely bypassed.
> 
> Signed-off-by: Phineas Su <pohaosu@google.com>
> ---
> v2:
>   - Reframed commit message to focus on robustness rather than a confirmed
>     bug, as Zen4 CPUs do not generally report UMC_V2. (Yazen)
> 
>  drivers/edac/amd64_edac.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index c6aa69dbd9fb..1e688123a50c 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -1047,6 +1047,10 @@ static int fixup_node_id(int node_id, struct mce *m)
>  	if (smca_get_bank_type(m->extcpu, m->bank) != SMCA_UMC_V2)
>  		return node_id;
>  
> +	/* If no GPU nodes are present, no fixup is needed. */
> +	if (!gpu_node_map.node_count)
> +		return node_id;
> +

Thanks Phineas for the patch.

I do agree, in principle, about hardening against possible new
configurations.

I think your patch could go a bit further.

I don't see any users of gpu_node_map.node_count. So rather than add a
user, we could remove it.

In fact, we don't even need the entire struct. We just need the
'gpu_base_node_id'.

>  	/* Nodes below the GPU base node are CPU nodes and don't need a fixup. */
>  	if (nid < gpu_node_map.base_node_id)
>  		return node_id;

You could add the "unintialized" check here. ^^^

	if (!gpu_base_node_id || nid < gpu_base_node_id)
	    	return node_id;

What do you think?

Thanks,
Yazen

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] EDAC/amd64: Only translate Node ID if GPU nodes are present
  2026-06-22 17:07       ` Yazen Ghannam
@ 2026-06-23  0:49         ` Borislav Petkov
  2026-06-23 15:49           ` Yazen Ghannam
  0 siblings, 1 reply; 7+ messages in thread
From: Borislav Petkov @ 2026-06-23  0:49 UTC (permalink / raw)
  To: Yazen Ghannam
  Cc: Phineas Su, tony.luck, muralidhara.mk, linux-edac, linux-kernel

On Mon, Jun 22, 2026 at 01:07:17PM -0400, Yazen Ghannam wrote:
> I do agree, in principle, about hardening against possible new
> configurations.

Are those configurations going to happen?

If so, we can harden when that happens. We don't do hypothetical coding yet.

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] EDAC/amd64: Only translate Node ID if GPU nodes are present
  2026-06-23  0:49         ` Borislav Petkov
@ 2026-06-23 15:49           ` Yazen Ghannam
  0 siblings, 0 replies; 7+ messages in thread
From: Yazen Ghannam @ 2026-06-23 15:49 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Phineas Su, tony.luck, muralidhara.mk, linux-edac, linux-kernel

On Mon, Jun 22, 2026 at 05:49:59PM -0700, Borislav Petkov wrote:
> On Mon, Jun 22, 2026 at 01:07:17PM -0400, Yazen Ghannam wrote:
> > I do agree, in principle, about hardening against possible new
> > configurations.
> 
> Are those configurations going to happen?

Not that I'm aware.

> 
> If so, we can harden when that happens. We don't do hypothetical coding yet.
> 

Okay, no problem.

Thanks,
Yazen

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-06-23 15:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-08  7:26 [PATCH] EDAC/amd64: Fix incorrect Node ID mapping on CPU-only Zen4+ systems Phineas Su
2026-06-09 16:07 ` Yazen Ghannam
2026-06-14 14:15   ` Phineas Su
2026-06-14 15:06     ` [PATCH v2] EDAC/amd64: Only translate Node ID if GPU nodes are present Phineas Su
2026-06-22 17:07       ` Yazen Ghannam
2026-06-23  0:49         ` Borislav Petkov
2026-06-23 15:49           ` Yazen Ghannam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox