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

[Daniel P . Berrangé]

On Fri, Aug 07, 2020 at 11:32:51AM -0500, Babu Moger wrote:
> Adding a new check to warn the users to configure 'dies' when
> topology is numa configured. It makes it easy to build the
> topology for EPYC models.

This says you're adding a warning....

> 
> 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");

...but you're actually making this a fatal error, not a warning.

I'm not sure this is really OK. Wouldn't this mean that existing VMs
deployed today, risk triggering this fatal error next time they
are booted, or live migrated.  If it is possible someone is using
such a config today, I don't think we can break it.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|