| 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
| ||
|
Message-ID: <94cd8a1f-1f2a-4cd6-8ab8-49b9d1e9fa2d@kernel.org>
Date: Tue, 30 Sep 2025 13:07:47 -0500
From: "Mario Limonciello (AMD) (kernel.org)" <superm1@...nel.org>
To: Yazen Ghannam <yazen.ghannam@....com>, x86@...nel.org
Cc: linux-kernel@...r.kernel.org, Filip Barczyk <filip.barczyk@...o.net>
Subject: Re: [PATCH 1/2] x86/amd_node: Fix AMD root device caching
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@....com>
> Cc: stable@...r.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, ®))
> - 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;
> }
>
Powered by blists - more mailing lists