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

[Ingo Molnar]

* Jason Andryuk <jason.andryuk@xxxxxxx> 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