[PATCH 0/2] AMD root search fix

[PATCH 0/2] AMD root search fix

[Yazen Ghannam]

Hi all,

Recently, there have been a couple of reports of the AMD64 EDAC module
failing to load on v6.14+.

This has been root caused to the recent AMD node rework. Specifically,
the root device caching method breaks on non-reference boards.

Patch 1 is a proposed fix.

Patch 2 is a minor cleanup.

Thanks,
Yazen

---
Yazen Ghannam (2):
      x86/amd_node: Fix AMD root device caching
      x86/amd_node: Use new root search helper

 arch/x86/include/asm/amd/node.h |   1 -
 arch/x86/kernel/amd_node.c      | 107 ++++++++++++++--------------------------
 2 files changed, 37 insertions(+), 71 deletions(-)
---
base-commit: 30d4efb2f5a515a60fe6b0ca85362cbebea21e2f
change-id: 20250930-fix-amd-root-2a6529e8162a

[PATCH 1/2] x86/amd_node: Fix AMD root device caching

[Yazen Ghannam]

Recent AMD node rework removed the "search and count" method of caching
AMD root devices. This depended on the value from a Data Fabric register
that was expected to hold the PCI bus of one of the root devices
attached to that fabric.

However, this expectation is incorrect. The register, when read from PCI
config space, returns the bitwise-OR of the buses of all attached root
devices.

This behavior is benign on AMD reference design boards, since the bus
numbers are aligned. This results in a bitwise-OR value matching one of
the buses. For example, 0x00 | 0x40 | 0xA0 | 0xE0 = 0xE0.

This behavior breaks on boards where the bus numbers are not exactly
aligned. For example, 0x00 | 0x07 | 0xE0 | 0x15 = 0x1F.

The bus numbering style in the reference boards is not a requirement.
The numbering found in other boards is not incorrect. Therefore, the
root device caching method needs to be adjusted.

Go back to the "search and count" method used before the recent rework.
Search for root devices using PCI class code rather than fixed PCI IDs.

This keeps the goal of the rework (remove dependency on PCI IDs) while
being able to support various board designs.

Fixes: 40a5f6ffdfc8 ("x86/amd_nb: Simplify root device search")
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Cc: stable@vger.kernel.org
---
 arch/x86/include/asm/amd/node.h |  1 -
 arch/x86/kernel/amd_node.c      | 95 ++++++++++++++++-------------------------
 2 files changed, 36 insertions(+), 60 deletions(-)

diff --git a/arch/x86/include/asm/amd/node.h b/arch/x86/include/asm/amd/node.h
index 23fe617898a8..a672b8765fa8 100644
--- a/arch/x86/include/asm/amd/node.h
+++ b/arch/x86/include/asm/amd/node.h
@@ -23,7 +23,6 @@
 #define AMD_NODE0_PCI_SLOT	0x18
 
 struct pci_dev *amd_node_get_func(u16 node, u8 func);
-struct pci_dev *amd_node_get_root(u16 node);
 
 static inline u16 amd_num_nodes(void)
 {
diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
index a40176b62eb5..4a3d9ca6e225 100644
--- a/arch/x86/kernel/amd_node.c
+++ b/arch/x86/kernel/amd_node.c
@@ -34,62 +34,6 @@ struct pci_dev *amd_node_get_func(u16 node, u8 func)
 	return pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(AMD_NODE0_PCI_SLOT + node, func));
 }
 
-#define DF_BLK_INST_CNT		0x040
-#define	DF_CFG_ADDR_CNTL_LEGACY	0x084
-#define	DF_CFG_ADDR_CNTL_DF4	0xC04
-
-#define DF_MAJOR_REVISION	GENMASK(27, 24)
-
-static u16 get_cfg_addr_cntl_offset(struct pci_dev *df_f0)
-{
-	u32 reg;
-
-	/*
-	 * Revision fields added for DF4 and later.
-	 *
-	 * Major revision of '0' is found pre-DF4. Field is Read-as-Zero.
-	 */
-	if (pci_read_config_dword(df_f0, DF_BLK_INST_CNT, &reg))
-		return 0;
-
-	if (reg & DF_MAJOR_REVISION)
-		return DF_CFG_ADDR_CNTL_DF4;
-
-	return DF_CFG_ADDR_CNTL_LEGACY;
-}
-
-struct pci_dev *amd_node_get_root(u16 node)
-{
-	struct pci_dev *root;
-	u16 cntl_off;
-	u8 bus;
-
-	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
-		return NULL;
-
-	/*
-	 * D18F0xXXX [Config Address Control] (DF::CfgAddressCntl)
-	 * Bits [7:0] (SecBusNum) holds the bus number of the root device for
-	 * this Data Fabric instance. The segment, device, and function will be 0.
-	 */
-	struct pci_dev *df_f0 __free(pci_dev_put) = amd_node_get_func(node, 0);
-	if (!df_f0)
-		return NULL;
-
-	cntl_off = get_cfg_addr_cntl_offset(df_f0);
-	if (!cntl_off)
-		return NULL;
-
-	if (pci_read_config_byte(df_f0, cntl_off, &bus))
-		return NULL;
-
-	/* Grab the pointer for the actual root device instance. */
-	root = pci_get_domain_bus_and_slot(0, bus, 0);
-
-	pci_dbg(root, "is root for AMD node %u\n", node);
-	return root;
-}
-
 static struct pci_dev **amd_roots;
 
 /* Protect the PCI config register pairs used for SMN. */
@@ -274,16 +218,49 @@ DEFINE_SHOW_STORE_ATTRIBUTE(smn_node);
 DEFINE_SHOW_STORE_ATTRIBUTE(smn_address);
 DEFINE_SHOW_STORE_ATTRIBUTE(smn_value);
 
+static struct pci_dev *get_next_root(struct pci_dev *root)
+{
+	while ((root = pci_get_class(PCI_CLASS_BRIDGE_HOST << 8, root))) {
+		/* Root device is Device 0 Function 0. */
+		if (root->devfn)
+			continue;
+
+		if (root->vendor != PCI_VENDOR_ID_AMD &&
+		    root->vendor != PCI_VENDOR_ID_HYGON)
+			continue;
+
+		break;
+	}
+
+	return root;
+}
+
 static int amd_cache_roots(void)
 {
-	u16 node, num_nodes = amd_num_nodes();
+	u16 count = 0, num_roots = 0, roots_per_node, node = 0, num_nodes = amd_num_nodes();
+	struct pci_dev *root = NULL;
 
 	amd_roots = kcalloc(num_nodes, sizeof(*amd_roots), GFP_KERNEL);
 	if (!amd_roots)
 		return -ENOMEM;
 
-	for (node = 0; node < num_nodes; node++)
-		amd_roots[node] = amd_node_get_root(node);
+	while ((root = get_next_root(root))) {
+		pci_dbg(root, "is an AMD root device\n");
+		num_roots++;
+	}
+
+	pr_debug("Found %d AMD root devices\n", num_roots);
+
+	roots_per_node = num_roots / num_nodes;
+
+	while ((root = get_next_root(root)) && node < num_nodes) {
+		/* Use one root for each node and skip the rest. */
+		if (count++ % roots_per_node)
+			continue;
+
+		pci_dbg(root, "is root for AMD node %u\n", node);
+		amd_roots[node++] = root;
+	}
 
 	return 0;
 }

-- 
2.51.0

Re: [PATCH 1/2] x86/amd_node: Fix AMD root device caching

[Mario Limonciello (AMD) (kernel.org)]

On 9/30/2025 11:45 AM, Yazen Ghannam wrote:
> Recent AMD node rework removed the "search and count" method of caching
> AMD root devices. This depended on the value from a Data Fabric register
> that was expected to hold the PCI bus of one of the root devices
> attached to that fabric.
> 
> However, this expectation is incorrect. The register, when read from PCI
> config space, returns the bitwise-OR of the buses of all attached root
> devices.
> 
> This behavior is benign on AMD reference design boards, since the bus
> numbers are aligned. This results in a bitwise-OR value matching one of
> the buses. For example, 0x00 | 0x40 | 0xA0 | 0xE0 = 0xE0.
> 
> This behavior breaks on boards where the bus numbers are not exactly
> aligned. For example, 0x00 | 0x07 | 0xE0 | 0x15 = 0x1F.
> 
> The bus numbering style in the reference boards is not a requirement.
> The numbering found in other boards is not incorrect. Therefore, the
> root device caching method needs to be adjusted.
> 
> Go back to the "search and count" method used before the recent rework.
> Search for root devices using PCI class code rather than fixed PCI IDs.
> 
> This keeps the goal of the rework (remove dependency on PCI IDs) while
> being able to support various board designs.
> 
> Fixes: 40a5f6ffdfc8 ("x86/amd_nb: Simplify root device search")

Was this a publicly reported failure?

If so is there a link to LKML or a Bugzilla with the details of the 
failure you can include here?

> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Cc: stable@vger.kernel.org
> ---
>   arch/x86/include/asm/amd/node.h |  1 -
>   arch/x86/kernel/amd_node.c      | 95 ++++++++++++++++-------------------------
>   2 files changed, 36 insertions(+), 60 deletions(-)
> 
> diff --git a/arch/x86/include/asm/amd/node.h b/arch/x86/include/asm/amd/node.h
> index 23fe617898a8..a672b8765fa8 100644
> --- a/arch/x86/include/asm/amd/node.h
> +++ b/arch/x86/include/asm/amd/node.h
> @@ -23,7 +23,6 @@
>   #define AMD_NODE0_PCI_SLOT	0x18
>   
>   struct pci_dev *amd_node_get_func(u16 node, u8 func);
> -struct pci_dev *amd_node_get_root(u16 node);
>   
>   static inline u16 amd_num_nodes(void)
>   {
> diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
> index a40176b62eb5..4a3d9ca6e225 100644
> --- a/arch/x86/kernel/amd_node.c
> +++ b/arch/x86/kernel/amd_node.c
> @@ -34,62 +34,6 @@ struct pci_dev *amd_node_get_func(u16 node, u8 func)
>   	return pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(AMD_NODE0_PCI_SLOT + node, func));
>   }
>   
> -#define DF_BLK_INST_CNT		0x040
> -#define	DF_CFG_ADDR_CNTL_LEGACY	0x084
> -#define	DF_CFG_ADDR_CNTL_DF4	0xC04
> -
> -#define DF_MAJOR_REVISION	GENMASK(27, 24)
> -
> -static u16 get_cfg_addr_cntl_offset(struct pci_dev *df_f0)
> -{
> -	u32 reg;
> -
> -	/*
> -	 * Revision fields added for DF4 and later.
> -	 *
> -	 * Major revision of '0' is found pre-DF4. Field is Read-as-Zero.
> -	 */
> -	if (pci_read_config_dword(df_f0, DF_BLK_INST_CNT, &reg))
> -		return 0;
> -
> -	if (reg & DF_MAJOR_REVISION)
> -		return DF_CFG_ADDR_CNTL_DF4;
> -
> -	return DF_CFG_ADDR_CNTL_LEGACY;
> -}
> -
> -struct pci_dev *amd_node_get_root(u16 node)
> -{
> -	struct pci_dev *root;
> -	u16 cntl_off;
> -	u8 bus;
> -
> -	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
> -		return NULL;
> -
> -	/*
> -	 * D18F0xXXX [Config Address Control] (DF::CfgAddressCntl)
> -	 * Bits [7:0] (SecBusNum) holds the bus number of the root device for
> -	 * this Data Fabric instance. The segment, device, and function will be 0.
> -	 */
> -	struct pci_dev *df_f0 __free(pci_dev_put) = amd_node_get_func(node, 0);
> -	if (!df_f0)
> -		return NULL;
> -
> -	cntl_off = get_cfg_addr_cntl_offset(df_f0);
> -	if (!cntl_off)
> -		return NULL;
> -
> -	if (pci_read_config_byte(df_f0, cntl_off, &bus))
> -		return NULL;
> -
> -	/* Grab the pointer for the actual root device instance. */
> -	root = pci_get_domain_bus_and_slot(0, bus, 0);
> -
> -	pci_dbg(root, "is root for AMD node %u\n", node);
> -	return root;
> -}
> -
>   static struct pci_dev **amd_roots;
>   
>   /* Protect the PCI config register pairs used for SMN. */
> @@ -274,16 +218,49 @@ DEFINE_SHOW_STORE_ATTRIBUTE(smn_node);
>   DEFINE_SHOW_STORE_ATTRIBUTE(smn_address);
>   DEFINE_SHOW_STORE_ATTRIBUTE(smn_value);
>   
> +static struct pci_dev *get_next_root(struct pci_dev *root)
> +{
> +	while ((root = pci_get_class(PCI_CLASS_BRIDGE_HOST << 8, root))) {
> +		/* Root device is Device 0 Function 0. */
> +		if (root->devfn)
> +			continue;
> +
> +		if (root->vendor != PCI_VENDOR_ID_AMD &&
> +		    root->vendor != PCI_VENDOR_ID_HYGON)
> +			continue;
> +
> +		break;
> +	}
> +
> +	return root;
> +}
> +
>   static int amd_cache_roots(void)
>   {
> -	u16 node, num_nodes = amd_num_nodes();
> +	u16 count = 0, num_roots = 0, roots_per_node, node = 0, num_nodes = amd_num_nodes();
> +	struct pci_dev *root = NULL;
>   
>   	amd_roots = kcalloc(num_nodes, sizeof(*amd_roots), GFP_KERNEL);
>   	if (!amd_roots)
>   		return -ENOMEM;
>   
> -	for (node = 0; node < num_nodes; node++)
> -		amd_roots[node] = amd_node_get_root(node);
> +	while ((root = get_next_root(root))) {
> +		pci_dbg(root, "is an AMD root device\n");
> +		num_roots++;
> +	}
> +
> +	pr_debug("Found %d AMD root devices\n", num_roots);
> +
> +	roots_per_node = num_roots / num_nodes;
> +
> +	while ((root = get_next_root(root)) && node < num_nodes) {
> +		/* Use one root for each node and skip the rest. */
> +		if (count++ % roots_per_node)
> +			continue;
> +
> +		pci_dbg(root, "is root for AMD node %u\n", node);
> +		amd_roots[node++] = root;
> +	}
>   
>   	return 0;
>   }
> 

Re: [PATCH 1/2] x86/amd_node: Fix AMD root device caching

[Yazen Ghannam]

On Tue, Sep 30, 2025 at 01:07:47PM -0500, Mario Limonciello (AMD) (kernel.org) wrote:
> 
> 
> On 9/30/2025 11:45 AM, Yazen Ghannam wrote:
> > Recent AMD node rework removed the "search and count" method of caching
> > AMD root devices. This depended on the value from a Data Fabric register
> > that was expected to hold the PCI bus of one of the root devices
> > attached to that fabric.
> > 
> > However, this expectation is incorrect. The register, when read from PCI
> > config space, returns the bitwise-OR of the buses of all attached root
> > devices.
> > 
> > This behavior is benign on AMD reference design boards, since the bus
> > numbers are aligned. This results in a bitwise-OR value matching one of
> > the buses. For example, 0x00 | 0x40 | 0xA0 | 0xE0 = 0xE0.
> > 
> > This behavior breaks on boards where the bus numbers are not exactly
> > aligned. For example, 0x00 | 0x07 | 0xE0 | 0x15 = 0x1F.
> > 
> > The bus numbering style in the reference boards is not a requirement.
> > The numbering found in other boards is not incorrect. Therefore, the
> > root device caching method needs to be adjusted.
> > 
> > Go back to the "search and count" method used before the recent rework.
> > Search for root devices using PCI class code rather than fixed PCI IDs.
> > 
> > This keeps the goal of the rework (remove dependency on PCI IDs) while
> > being able to support various board designs.
> > 
> > Fixes: 40a5f6ffdfc8 ("x86/amd_nb: Simplify root device search")
> 
> Was this a publicly reported failure?
> 
> If so is there a link to LKML or a Bugzilla with the details of the failure
> you can include here?
> 

No, it was reported off-list.

Thanks,
Yazen

Re: [PATCH 1/2] x86/amd_node: Fix AMD root device caching

[Filip Barczyk]

<span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">On Tue, Oct 01, 2025 at 03:46 PM +0200, Yazen Ghannam wrote:</span><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; On Tue, Sep 30, 2025 at 01:07:47PM -0500, Mario Limonciello (AMD) (kernel.org) wrote:</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt;</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt;</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; On 9/30/2025 11:45 AM, Yazen Ghannam wrote:</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; Recent AMD node rework removed the "search and count" method of caching</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; AMD root devices. This depended on the value from a Data Fabric register</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; that was expected to hold the PCI bus of one of the root devices</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; attached to that fabric.</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt;</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; However, this expectation is incorrect. The register, when read from PCI</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; config space, returns the bitwise-OR of the buses of all attached root</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; devices.</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt;</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; This behavior is benign on AMD reference design boards, since the bus</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; numbers are aligned. This results in a bitwise-OR value matching one of</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; the buses. For example, 0x00 | 0x40 | 0xA0 | 0xE0 = 0xE0.</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt;</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; This behavior breaks on boards where the bus numbers are not exactly</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; aligned. For example, 0x00 | 0x07 | 0xE0 | 0x15 = F.</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt;</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; The bus numbering style in the reference boards is not a requirement.</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; The numbering found in other boards is not incorrect. Therefore, the</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; root device caching method needs to be adjusted.</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt;</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; Go back to the "search and count" method used before the recent rework.</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; Search for root devices using PCI class code rather than fixed PCI IDs.</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt;</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; This keeps the goal of the rework (remove dependency on PCI IDs) while</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; being able to support various board designs.</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt;</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; &gt; Fixes: 40a5f6ffdfc8 ("x86/amd_nb: Simplify root device search")</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt;</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; Was this a publicly reported failure?</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt;</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; If so is there a link to LKML or a Bugzilla with the details of the failure</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt; you can include here?</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; &gt;</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; No, it was reported off-list.</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; Thanks,</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">&gt; Yazen</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"><br></div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">I confirm that an issue where EDAC fails to discover DIMMs on Dell PowerEdge R7725, 2x EPYC 9475F @ kernel 6.14.3 is fixed with this patch.</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">Thanks,</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">Filip</div><div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"><br></div>
________________________________
 Pico Quantitative Trading LLC ("PQT"). This e-mail (including any attachments) is intended only for use by the addressee(s) named above, and may contain confidential, proprietary or legally privileged information. If you are not the intended recipient of this e-mail, any review, use, disclosure, dissemination, distribution, printing or copying of this e-mail or any attachment is strictly prohibited. If you have received this e-mail in error, please notify Pico immediately by return e-mail and permanently delete the original from your system and any hard copy printout thereof. E-mails are not encrypted and cannot be guaranteed to be secure or error-free and, as with all Internet communications, information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. Accordingly, Pico accepts no liability for any errors or omissions in the content contained herein. In compliance with applicable laws, rules and regulations and/or at its discretion, Pico may review and archive incoming and outgoing e-mail communications, copies of which may be produced at the request of regulators.

Re: [PATCH 1/2] x86/amd_node: Fix AMD root device caching

[Filip Barczyk]

On Tue, Oct 01, 2025 at 03:46 PM +0200, Yazen Ghannam wrote:
> On Tue, Sep 30, 2025 at 01:07:47PM -0500, Mario Limonciello (AMD) (kernel.org) wrote:
> >
> >
> > On 9/30/2025 11:45 AM, Yazen Ghannam wrote:
> > > Recent AMD node rework removed the "search and count" method of caching
> > > AMD root devices. This depended on the value from a Data Fabric register
> > > that was expected to hold the PCI bus of one of the root devices
> > > attached to that fabric.
> > >
> > > However, this expectation is incorrect. The register, when read from PCI
> > > config space, returns the bitwise-OR of the buses of all attached root
> > > devices.
> > >
> > > This behavior is benign on AMD reference design boards, since the bus
> > > numbers are aligned. This results in a bitwise-OR value matching one of
> > > the buses. For example, 0x00 | 0x40 | 0xA0 | 0xE0 = 0xE0.
> > >
> > > This behavior breaks on boards where the bus numbers are not exactly
> > > aligned. For example, 0x00 | 0x07 | 0xE0 | 0x15 = F.
> > >
> > > The bus numbering style in the reference boards is not a requirement.
> > > The numbering found in other boards is not incorrect. Therefore, the
> > > root device caching method needs to be adjusted.
> > >
> > > Go back to the "search and count" method used before the recent rework.
> > > Search for root devices using PCI class code rather than fixed PCI IDs.
> > >
> > > This keeps the goal of the rework (remove dependency on PCI IDs) while
> > > being able to support various board designs.
> > >
> > > Fixes: 40a5f6ffdfc8 ("x86/amd_nb: Simplify root device search")
> >
> > Was this a publicly reported failure?
> >
> > If so is there a link to LKML or a Bugzilla with the details of the failure
> > you can include here?
> >
> No, it was reported off-list.
> Thanks,
> Yazen

I confirm that an issue when EDAC fails to enumerate DIMMs on Dell PowerEdge R7725, 2x EPYC 9475F @ kernel 6.14.3 is fixed with this patch.
Thanks,
Filip
________________________________
 Pico Quantitative Trading LLC ("PQT"). This e-mail (including any attachments) is intended only for use by the addressee(s) named above, and may contain confidential, proprietary or legally privileged information. If you are not the intended recipient of this e-mail, any review, use, disclosure, dissemination, distribution, printing or copying of this e-mail or any attachment is strictly prohibited. If you have received this e-mail in error, please notify Pico immediately by return e-mail and permanently delete the original from your system and any hard copy printout thereof. E-mails are not encrypted and cannot be guaranteed to be secure or error-free and, as with all Internet communications, information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. Accordingly, Pico accepts no liability for any errors or omissions in the content contained herein. In compliance with applicable laws, rules and regulations and/or at its discretion, Pico may review and archive incoming and outgoing e-mail communications, copies of which may be produced at the request of regulators.

Re: [PATCH 1/2] x86/amd_node: Fix AMD root device caching

[Borislav Petkov]

On Tue, Sep 30, 2025 at 04:45:45PM +0000, Yazen Ghannam wrote:
> Recent AMD node rework removed the "search and count" method of caching
> AMD root devices. This depended on the value from a Data Fabric register
> that was expected to hold the PCI bus of one of the root devices
> attached to that fabric.
> 
> However, this expectation is incorrect. The register, when read from PCI
> config space, returns the bitwise-OR of the buses of all attached root
> devices.
> 
> This behavior is benign on AMD reference design boards, since the bus
> numbers are aligned. This results in a bitwise-OR value matching one of
> the buses. For example, 0x00 | 0x40 | 0xA0 | 0xE0 = 0xE0.
> 
> This behavior breaks on boards where the bus numbers are not exactly
> aligned. For example, 0x00 | 0x07 | 0xE0 | 0x15 = 0x1F.

Do I see it correctly that one of the root device's PCI bus is always 0x0 so
you can simply read that one and you can keep the current code?

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 1/2] x86/amd_node: Fix AMD root device caching

[Yazen Ghannam]

On Tue, Oct 21, 2025 at 02:29:09PM +0200, Borislav Petkov wrote:
> On Tue, Sep 30, 2025 at 04:45:45PM +0000, Yazen Ghannam wrote:
> > Recent AMD node rework removed the "search and count" method of caching
> > AMD root devices. This depended on the value from a Data Fabric register
> > that was expected to hold the PCI bus of one of the root devices
> > attached to that fabric.
> > 
> > However, this expectation is incorrect. The register, when read from PCI
> > config space, returns the bitwise-OR of the buses of all attached root
> > devices.
> > 
> > This behavior is benign on AMD reference design boards, since the bus
> > numbers are aligned. This results in a bitwise-OR value matching one of
> > the buses. For example, 0x00 | 0x40 | 0xA0 | 0xE0 = 0xE0.
> > 
> > This behavior breaks on boards where the bus numbers are not exactly
> > aligned. For example, 0x00 | 0x07 | 0xE0 | 0x15 = 0x1F.
> 
> Do I see it correctly that one of the root device's PCI bus is always 0x0 so
> you can simply read that one and you can keep the current code?
> 

It's correct that one of the root devices will be on bus 0x0. But that
device will be part of AMD node 0. So we still need to pick a root
device for the other, if any, AMD nodes in the system.

For example, a system with 2 AMD nodes could have this:
Node 0 : 0x00 0x07 0x0e 0x15
Node 1 : 0x1c 0x23 0x2a 0x31

Thanks,
Yazen

Re: [PATCH 1/2] x86/amd_node: Fix AMD root device caching

[Borislav Petkov]

On Tue, Oct 21, 2025 at 09:37:45AM -0400, Yazen Ghannam wrote:
> It's correct that one of the root devices will be on bus 0x0. But that
> device will be part of AMD node 0. So we still need to pick a root
> device for the other, if any, AMD nodes in the system.
> 
> For example, a system with 2 AMD nodes could have this:
> Node 0 : 0x00 0x07 0x0e 0x15
> Node 1 : 0x1c 0x23 0x2a 0x31

Aha, so next question: isn't there a way to figure out which is the *first*
PCI bus of a AMD node? My gut feeling tells me there should be some info
somewhere which we can use...

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 1/2] x86/amd_node: Fix AMD root device caching

[Yazen Ghannam]

On Tue, Oct 21, 2025 at 04:15:00PM +0200, Borislav Petkov wrote:
> On Tue, Oct 21, 2025 at 09:37:45AM -0400, Yazen Ghannam wrote:
> > It's correct that one of the root devices will be on bus 0x0. But that
> > device will be part of AMD node 0. So we still need to pick a root
> > device for the other, if any, AMD nodes in the system.
> > 
> > For example, a system with 2 AMD nodes could have this:
> > Node 0 : 0x00 0x07 0x0e 0x15
> > Node 1 : 0x1c 0x23 0x2a 0x31
> 
> Aha, so next question: isn't there a way to figure out which is the *first*
> PCI bus of a AMD node? My gut feeling tells me there should be some info
> somewhere which we can use...
> 

Yep, that was one of the goals of the recent rework. I thought the DF
register held that info, but I was mistaken.

I haven't been able to find anything else. We can keep looking, but I
wanted to send out this patch as a fix in the meantime.

Thanks,
Yazen

Re: [PATCH 1/2] x86/amd_node: Fix AMD root device caching

[Borislav Petkov]

On Tue, Sep 30, 2025 at 04:45:45PM +0000, Yazen Ghannam wrote:
> This behavior is benign on AMD reference design boards, since the bus
> numbers are aligned. This results in a bitwise-OR value matching one of
> the buses. For example, 0x00 | 0x40 | 0xA0 | 0xE0 = 0xE0.
> 
> This behavior breaks on boards where the bus numbers are not exactly
> aligned. For example, 0x00 | 0x07 | 0xE0 | 0x15 = 0x1F.

<---

Please add here something along the lines of:

"And even if one could say, they both have bus 0x0 containing the root
devices, this is not true on the other AMD nodes besides 0."

>  static int amd_cache_roots(void)
>  {
> -	u16 node, num_nodes = amd_num_nodes();
> +	u16 count = 0, num_roots = 0, roots_per_node, node = 0, num_nodes = amd_num_nodes();
> +	struct pci_dev *root = NULL;
>  
>  	amd_roots = kcalloc(num_nodes, sizeof(*amd_roots), GFP_KERNEL);
>  	if (!amd_roots)
>  		return -ENOMEM;
>  
> -	for (node = 0; node < num_nodes; node++)
> -		amd_roots[node] = amd_node_get_root(node);
> +	while ((root = get_next_root(root))) {
> +		pci_dbg(root, "is an AMD root device\n");
> +		num_roots++;
> +	}
> +
> +	pr_debug("Found %d AMD root devices\n", num_roots);
> +
> +	roots_per_node = num_roots / num_nodes;

What happens if num_roots = 0? IOW, you need to handle that here.

> +
> +	while ((root = get_next_root(root)) && node < num_nodes) {
> +		/* Use one root for each node and skip the rest. */
> +		if (count++ % roots_per_node)
> +			continue;
> +
> +		pci_dbg(root, "is root for AMD node %u\n", node);
> +		amd_roots[node++] = root;
> +	}

If I squint my eyes hard enough, I can see you getting rid of the *three*
while loops here. So please try again.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 1/2] x86/amd_node: Fix AMD root device caching

[Yazen Ghannam]

On Wed, Oct 22, 2025 at 01:13:42PM +0200, Borislav Petkov wrote:
> On Tue, Sep 30, 2025 at 04:45:45PM +0000, Yazen Ghannam wrote:
> > This behavior is benign on AMD reference design boards, since the bus
> > numbers are aligned. This results in a bitwise-OR value matching one of
> > the buses. For example, 0x00 | 0x40 | 0xA0 | 0xE0 = 0xE0.
> > 
> > This behavior breaks on boards where the bus numbers are not exactly
> > aligned. For example, 0x00 | 0x07 | 0xE0 | 0x15 = 0x1F.
> 
> <---
> 
> Please add here something along the lines of:
> 
> "And even if one could say, they both have bus 0x0 containing the root
> devices, this is not true on the other AMD nodes besides 0."
> 

Okay, will do.

> >  static int amd_cache_roots(void)
> >  {
> > -	u16 node, num_nodes = amd_num_nodes();
> > +	u16 count = 0, num_roots = 0, roots_per_node, node = 0, num_nodes = amd_num_nodes();
> > +	struct pci_dev *root = NULL;
> >  
> >  	amd_roots = kcalloc(num_nodes, sizeof(*amd_roots), GFP_KERNEL);
> >  	if (!amd_roots)
> >  		return -ENOMEM;
> >  
> > -	for (node = 0; node < num_nodes; node++)
> > -		amd_roots[node] = amd_node_get_root(node);
> > +	while ((root = get_next_root(root))) {
> > +		pci_dbg(root, "is an AMD root device\n");
> > +		num_roots++;
> > +	}
> > +
> > +	pr_debug("Found %d AMD root devices\n", num_roots);
> > +
> > +	roots_per_node = num_roots / num_nodes;
> 
> What happens if num_roots = 0? IOW, you need to handle that here.

Yes, will do.

> 
> > +
> > +	while ((root = get_next_root(root)) && node < num_nodes) {
> > +		/* Use one root for each node and skip the rest. */
> > +		if (count++ % roots_per_node)
> > +			continue;
> > +
> > +		pci_dbg(root, "is root for AMD node %u\n", node);
> > +		amd_roots[node++] = root;
> > +	}
> 
> If I squint my eyes hard enough, I can see you getting rid of the *three*
> while loops here. So please try again.
> 

I don't follow. Do you mean to combine the other loops into this one? Or
that this loop should be expanded into three loops explicitly doing one
thing each?

Thanks,
Yazen

Re: [PATCH 1/2] x86/amd_node: Fix AMD root device caching

[Borislav Petkov]

On Wed, Oct 22, 2025 at 09:19:04AM -0400, Yazen Ghannam wrote:
> I don't follow. Do you mean to combine the other loops into this one? Or
> that this loop should be expanded into three loops explicitly doing one
> thing each?

See if you can do everything in one loop. I.e., limit the amount of looping to
the least possible.

:-)

-- 
Regards/Gruss,
    Boris.

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

[PATCH 2/2] x86/amd_node: Use new root search helper

[Yazen Ghannam]

...to reduce code duplication.

No functional change intended.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
 arch/x86/kernel/amd_node.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
index 4a3d9ca6e225..02254e2034bd 100644
--- a/arch/x86/kernel/amd_node.c
+++ b/arch/x86/kernel/amd_node.c
@@ -268,18 +268,8 @@ static int amd_cache_roots(void)
 static int reserve_root_config_spaces(void)
 {
 	struct pci_dev *root = NULL;
-	struct pci_bus *bus = NULL;
-
-	while ((bus = pci_find_next_bus(bus))) {
-		/* Root device is Device 0 Function 0 on each Primary Bus. */
-		root = pci_get_slot(bus, 0);
-		if (!root)
-			continue;
-
-		if (root->vendor != PCI_VENDOR_ID_AMD &&
-		    root->vendor != PCI_VENDOR_ID_HYGON)
-			continue;
 
+	while ((root = get_next_root(root))) {
 		pci_dbg(root, "Reserving PCI config space\n");
 
 		/*

-- 
2.51.0

Re: [PATCH 0/2] AMD root search fix

[Mario Limonciello (AMD) (kernel.org)]

On 9/30/2025 11:45 AM, Yazen Ghannam wrote:
> Hi all,
> 
> Recently, there have been a couple of reports of the AMD64 EDAC module
> failing to load on v6.14+.
> 
> This has been root caused to the recent AMD node rework. Specifically,
> the root device caching method breaks on non-reference boards.
> 
> Patch 1 is a proposed fix.
> 
> Patch 2 is a minor cleanup.
> 
> Thanks,
> Yazen
> 
> ---
> Yazen Ghannam (2):
>        x86/amd_node: Fix AMD root device caching
>        x86/amd_node: Use new root search helper
> 
>   arch/x86/include/asm/amd/node.h |   1 -
>   arch/x86/kernel/amd_node.c      | 107 ++++++++++++++--------------------------
>   2 files changed, 37 insertions(+), 71 deletions(-)
> ---
> base-commit: 30d4efb2f5a515a60fe6b0ca85362cbebea21e2f
> change-id: 20250930-fix-amd-root-2a6529e8162a
> 

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>