[PATCH] x86: Avoid divide by 0 in amd_smn_init()

[PATCH] x86: Avoid divide by 0 in amd_smn_init()

[Jason Andryuk]

Xen synthesizes the CPU topology, so the num_nodes and num_roots values
may be surprising for amd_smn_init().  Specifically:

    roots_per_node = num_roots / num_nodes;

may results in roots_per_node == 0 which leads to divide by zero in

    count % roots_per_node

As an example, I have a system with a Xen PVH dom0 that reports:
Found 1 AMD root devices
Found 2 AMD nodes

Ensure roots_per_node is at least 1 to avoid the divide by zero errors.
num_nodes are allocated for amd_roots, so roots_per_node = 1 will
populate all the entries.

Also add a pr_debug() for the number of nodes.

Cc: stable@vger.kernel.org
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
This is an alternative to
https://lore.kernel.org/xen-devel/20260506055528.476493-2-penny.zheng@amd.com/
but it leaves smn available for dom0.
---
 arch/x86/kernel/amd_node.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
index 0be01725a2a4..f335c5f1ae1d 100644
--- a/arch/x86/kernel/amd_node.c
+++ b/arch/x86/kernel/amd_node.c
@@ -282,11 +282,14 @@ static int __init amd_smn_init(void)
 		return -ENODEV;
 
 	num_nodes = amd_num_nodes();
+	pr_debug("Found %d AMD nodes\n", num_nodes);
 	amd_roots = kzalloc_objs(*amd_roots, num_nodes);
 	if (!amd_roots)
 		return -ENOMEM;
 
 	roots_per_node = num_roots / num_nodes;
+	if (roots_per_node == 0)
+		roots_per_node = 1;
 
 	count = 0;
 	node = 0;
-- 
2.34.1

Re: [PATCH] x86: Avoid divide by 0 in amd_smn_init()

[Borislav Petkov]

On Tue, Jun 23, 2026 at 05:19:03PM -0400, Jason Andryuk wrote:
> Xen synthesizes the CPU topology, so the num_nodes and num_roots values
> may be surprising for amd_smn_init().  Specifically:
> 
>     roots_per_node = num_roots / num_nodes;
> 
> may results in roots_per_node == 0 which leads to divide by zero in
> 
>     count % roots_per_node
> 
> As an example, I have a system with a Xen PVH dom0 that reports:
> Found 1 AMD root devices
> Found 2 AMD nodes
> 
> Ensure roots_per_node is at least 1 to avoid the divide by zero errors.
> num_nodes are allocated for amd_roots, so roots_per_node = 1 will
> populate all the entries.
> 
> Also add a pr_debug() for the number of nodes.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> This is an alternative to
> https://lore.kernel.org/xen-devel/20260506055528.476493-2-penny.zheng@amd.com/
> but it leaves smn available for dom0.

Does this alternative work too?

https://lore.kernel.org/r/20260605230949.GBaiNXPZ2ztjVL7DBg@fat_crate.local

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86: Avoid divide by 0 in amd_smn_init()

[Jason Andryuk]

On 2026-06-23 17:35, Borislav Petkov wrote:
> On Tue, Jun 23, 2026 at 05:19:03PM -0400, Jason Andryuk wrote:
>> Xen synthesizes the CPU topology, so the num_nodes and num_roots values
>> may be surprising for amd_smn_init().  Specifically:
>>
>>      roots_per_node = num_roots / num_nodes;
>>
>> may results in roots_per_node == 0 which leads to divide by zero in
>>
>>      count % roots_per_node
>>
>> As an example, I have a system with a Xen PVH dom0 that reports:
>> Found 1 AMD root devices
>> Found 2 AMD nodes
>>
>> Ensure roots_per_node is at least 1 to avoid the divide by zero errors.
>> num_nodes are allocated for amd_roots, so roots_per_node = 1 will
>> populate all the entries.
>>
>> Also add a pr_debug() for the number of nodes.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>> ---
>> This is an alternative to
>> https://lore.kernel.org/xen-devel/20260506055528.476493-2-penny.zheng@amd.com/
>> but it leaves smn available for dom0.
> 
> Does this alternative work too?
> 
> https://lore.kernel.org/r/20260605230949.GBaiNXPZ2ztjVL7DBg@fat_crate.local

It boots as dom0, but smn is disabled.

Yours:
$ journalctl -b  | grep -e amd-pmf -e ypervisor
Hypervisor detected: Xen HVM
amd-pmf AMDI0107:00: error -EINVAL: error in reading from 0x13b102e8
amd-pmf AMDI0107:00: probe with driver amd-pmf failed with error -22

Mine:
$ journalctl -b -2 | grep -e amd-pmf -e ypervisor
Hypervisor detected: Xen HVM
amd-pmf AMDI0107:00: No Smart PC policy present
amd-pmf AMDI0107:00: registered PMF device successfully

amd-pmc also fails with yours.

dom0 is the privileged hardware domain and sees the physical PCI 
devices.  get_next_root() is looking for AMD or Hygon vendor ids, and 
dom0 will find those.  A regular domain (domU) would see QEMU's emulated 
pci host bridges, which will be Intel.

I have wip s0ix support with Xen where dom0 issues the amd-pmc calls to 
enter s0ix.  I'm not sure of all the uses of SMN, but with Xen 
dom0/hardware domain running most drivers, I think it should be available.

Regards,
Jason

Re: [PATCH] x86: Avoid divide by 0 in amd_smn_init()

[Borislav Petkov]

+ Andy

On Wed, Jun 24, 2026 at 11:41:16AM -0400, Jason Andryuk wrote:
> I have wip s0ix support with Xen where dom0 issues the amd-pmc calls to
> enter s0ix.  I'm not sure of all the uses of SMN, but with Xen dom0/hardware
> domain running most drivers, I think it should be available.

Well, how should we make it available if dom0 doesn't really allow us to
enumerate PCI roots and thus count AMD nodes?

Andy, see upthread.

What would you suggest we do here on dom0? We're trying to enumerate AMD
nodes but dom0 is doing something special wrt topology and PCI roots - see
get_next_root() in amd_node.c.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86: Avoid divide by 0 in amd_smn_init()

[Jason Andryuk]

On 2026-06-24 11:59, Borislav Petkov wrote:
> + Andy
> 
> On Wed, Jun 24, 2026 at 11:41:16AM -0400, Jason Andryuk wrote:
>> I have wip s0ix support with Xen where dom0 issues the amd-pmc calls to
>> enter s0ix.  I'm not sure of all the uses of SMN, but with Xen dom0/hardware
>> domain running most drivers, I think it should be available.
> 
> Well, how should we make it available if dom0 doesn't really allow us to
> enumerate PCI roots and thus count AMD nodes?

dom0 enumerates the physical PCI roots.  It's just the dom0 vCPUs and 
topology are synthetic.

> Andy, see upthread.
> 
> What would you suggest we do here on dom0? We're trying to enumerate AMD
> nodes but dom0 is doing something special wrt topology and PCI roots - see
> get_next_root() in amd_node.c.
> 
> Thx.
> 

I think this is the issue:

     The "root" device search was introduced to support SMN access for Zen
     systems. This device represents a PCIe root complex. It is not the
     same as the "CPU/node" devices found at slots 0x18-0x1F.

We don't want dom0 to access the "CPU/node" devices.  It's the "root" 
device SMN access I am trying to retain.  Many amd_smn_read/write calls 
have hardcoded node 0, like for amd-pmc.

Regards,
Jason

Re: [PATCH] x86: Avoid divide by 0 in amd_smn_init()

[Borislav Petkov]

On Wed, Jun 24, 2026 at 12:41:48PM -0400, Jason Andryuk wrote:
> I think this is the issue:
> 
>     The "root" device search was introduced to support SMN access for Zen
>     systems. This device represents a PCIe root complex. It is not the
>     same as the "CPU/node" devices found at slots 0x18-0x1F.

What is that? AI output?

> We don't want dom0 to access the "CPU/node" devices.  It's the "root" device
> SMN access I am trying to retain.

I know what you're trying to do - you want to use SMN accesses on dom0. And
I'm trying to figure out a stable detection method on Xen which is future
proof.

> Many amd_smn_read/write calls have hardcoded node 0, like for amd-pmc.

Maybe.

Whatever it is, it needs to be a long-term solution and properly vetted by Xen
folks so that we don't do crazy hacks for Xen's sake everytime.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86: Avoid divide by 0 in amd_smn_init()

[Jason Andryuk]

On 2026-06-24 13:15, Borislav Petkov wrote:
> On Wed, Jun 24, 2026 at 12:41:48PM -0400, Jason Andryuk wrote:
>> I think this is the issue:
>>
>>      The "root" device search was introduced to support SMN access for Zen
>>      systems. This device represents a PCIe root complex. It is not the
>>      same as the "CPU/node" devices found at slots 0x18-0x1F.
> 
> What is that? AI output?

It's from the commit message of 40a5f6ffdfc8f8ed0d8c535dfa3733b31c66a88c

>> We don't want dom0 to access the "CPU/node" devices.  It's the "root" device
>> SMN access I am trying to retain.
> 
> I know what you're trying to do - you want to use SMN accesses on dom0. And
> I'm trying to figure out a stable detection method on Xen which is future
> proof.
> 
>> Many amd_smn_read/write calls have hardcoded node 0, like for amd-pmc.
> 
> Maybe.
> 
> Whatever it is, it needs to be a long-term solution and properly vetted by Xen
> folks so that we don't do crazy hacks for Xen's sake everytime.

Sure.

I think Ingo's suggestion to re-add the check will at least get systems 
booting again.  Then when Xen SMN accesses is sorted out, that can be 
changed as necessary.

Regards,
Jason

Re: [PATCH] x86: Avoid divide by 0 in amd_smn_init()

[Ingo Molnar]

* Jason Andryuk <jason.andryuk@amd.com> wrote:

> Xen synthesizes the CPU topology, so the num_nodes and num_roots values
> may be surprising for amd_smn_init().  Specifically:
> 
>     roots_per_node = num_roots / num_nodes;
> 
> may results in roots_per_node == 0 which leads to divide by zero in
> 
>     count % roots_per_node
> 
> As an example, I have a system with a Xen PVH dom0 that reports:
> Found 1 AMD root devices
> Found 2 AMD nodes
> 
> Ensure roots_per_node is at least 1 to avoid the divide by zero errors.
> num_nodes are allocated for amd_roots, so roots_per_node = 1 will
> populate all the entries.
> 
> Also add a pr_debug() for the number of nodes.

So arguably this Xen PHV dom0 PCI configuration is bogus,
because it violates the roots % nodes rule, right?

Why should we not go back to something similar to the pre-40a5f6ffdfc8
state of things, which warned about such bogus configs in the syslog,
so that it could be seen and fixed:

-               /*
-                * There should be _exactly_ N roots for each DF/SMN
-                * interface.
-                */
-               if (!roots_per_misc || (root_count % roots_per_misc)) {
-                       pr_info("Unsupported AMD DF/PCI configuration found\n");
-                       return -ENODEV;
-               }

Instead of your patch which just silently works around the
borkage and issues a pr_debug() that nobody reads?

AFAICS the following fix:

  0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching")

Never restored that sanity check & warning about such firmware
bogosity.

Thanks,

	Ingo

Re: [PATCH] x86: Avoid divide by 0 in amd_smn_init()

[Borislav Petkov]

On Wed, Jun 24, 2026 at 11:21:51AM +0200, Ingo Molnar wrote:
> Why should we not go back to something similar to the pre-40a5f6ffdfc8

Because this code obviously cannot run in a guest. See my other reply.

-- 
Regards/Gruss,
    Boris.

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