Re: [PATCH v4 2/3] hw/i386: Update the EPYC topology to use socket/dies/core/thread model

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 14 Aug 2020 16:39:33 -0500
Babu Moger <babu.moger@amd.com> wrote:

> Update the EPYC topology to use socket/dies/core/thread model. The EPYC
> model does not use the smp dies to build the topology. Instead, it uses
> numa nodes to build the topology. Internally both are similar concept
> which divides the cores on L3 boundary. Combining both into one terminology
> makes it simple to program.
> 
> Add a new check to error out when smp dies are not provided when EPYC
> model is numa configured. Next task is to remove node_id, nr_nodes and
> nodes_per_pkg from EPYC topology which will be done in next patch.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  hw/i386/x86.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 67bee1bcb8..e90c42d2fc 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -138,6 +138,14 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version)
>  
>      /* Check for apicid encoding */
>      if (cpu_x86_use_epyc_apic_id_encoding(ms->cpu_type)) {
> +        if ((ms->numa_state->num_nodes > 0) &&

> +            ms->numa_state->num_nodes != (ms->smp.sockets * x86ms->smp_dies)) {
this case is gated by (ms->numa_state->num_nodes > 0) so it won't work in case
 -smp dies=>1 but there is no -numa options at all

we need to error out and ask to provide numa nodes corresponding to
   (ms->numa_state->num_nodes == 0) && (ms->smp.sockets * x86ms->smp_dies)

or better alternative would be to enable autonuma when EPYC cpu is enabled,
that will insure that this patch will work even if user hasn't specified -numa option,
since it will create a single numa node automatically.

The later will take care of (-smp 1,dies=1) case, which is broken due to
lack of explicit -numa we end up with CPU_UNSET_NUMA_NODE_ID in CPUID_Fn8000001E_ECX.

> +            error_setg(&error_fatal, "Numa configuration here requires smp "
> +                       "'dies' parameter. Configure the cpu topology properly "
> +                       "with max_cpus = sockets * dies * cores * threads. Dies"
> +                       " is equivalent to number of numa nodes in a socket.");
> +            return;
> +        }
>          x86_set_epyc_topo_handlers(ms);
>      }
>  
>