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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251022111342.GNaPi8ZqATfwpja2GR@fat_crate.local>
Date: Wed, 22 Oct 2025 13:13:42 +0200
From: Borislav Petkov <bp@...en8.de>
To: Yazen Ghannam <yazen.ghannam@....com>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org,
	Mario Limonciello <mario.limonciello@....com>,
	Filip Barczyk <filip.barczyk@...o.net>
Subject: Re: [PATCH 1/2] x86/amd_node: Fix AMD root device caching

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