RE: [PATCH v3 2/3] hw/i386: Add a new check to configure smp dies for EPYC

[Babu Moger]

> -----Original Message-----
> From: Igor Mammedov <imammedo@redhat.com>
> Sent: Friday, August 7, 2020 2:27 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com; qemu-
> devel@nongnu.org; mst@redhat.com
> Subject: Re: [PATCH v3 2/3] hw/i386: Add a new check to configure smp dies for
> EPYC
> 
> On Fri, 07 Aug 2020 11:32:51 -0500
> Babu Moger <babu.moger@amd.com> wrote:
> 
> > Adding a new check to warn the users to configure 'dies' when
> s/warn .../error out .../
> 
> > topology is numa configured. It makes it easy to build the topology
> > for EPYC models.
> 
> probably it should mention that will break configs that do not have correct
> topology configured.

Sure. Will do that.

> 
> 
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> >  hw/i386/x86.c |    7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/hw/i386/x86.c b/hw/i386/x86.c index
> > 67bee1bcb8..2a6ce56ef1 100644
> > --- a/hw/i386/x86.c
> > +++ b/hw/i386/x86.c
> > @@ -138,6 +138,13 @@ 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)) {
> > +            error_setg(&error_fatal, "Numa configuration requires smp 
> > 'dies' "
> > +                       "parameter. Configure the cpu topology properly 
> > with "
> > +                       "max_cpus = sockets * dies * cores * threads");
> > +            return;
> > +        }
> >          x86_set_epyc_topo_handlers(ms);
> >      }
> 
> we also should error out in case
>     ms->numa_state->num_nodes != (ms->smp.sockets * x86ms->smp_dies and
> ask user to configure numa to match total number of used dies.
> 

I have already added the same check above. Do you want anything more?