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

[Borislav Petkov <bp@alien8.de>]

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