Re: [Qemu-arm] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init

[Paolo Bonzini]

On 10/06/2016 19:40, Andrew Jones wrote:
> +    if (sockets == -1 || cores == -1 || threads == -1 ||
> +        maxcpus == -1 || cpus == -1) {
> +        error_report("cpu topology: "
> +                     "all machine properties must be specified");
> +        exit(1);
> +    }
> +

I think it's sane to accept some defaults.  It must not be the DWIM
thing that -smp does (which is targeted to Windows's dislike of
multi-socket machine on consumer hardware).  It must be something that
makes sense, and my proposal is:

- threads: 1
- cores: 1
- sockets:
  - maxcpus / (cores * threads) if maxcpus given
  - cpus / (cores * threads) if cpus given
  - else 1
- maxcpus: cores * threads * sockets
- cpus: maxcpus

This is a simple, linear logic that assigns defaults to each argument in
sequence.  At the same time -machine should fail for all invalid
combinations, namely:

- any argument < 1
- any argument > some compile-time value (1024?) to avoid overflows
- cpus % (cores * threads) != 0
- cpus > sockets * cores * threads
- maxcpus != cores * threads * sockets

Alone the last relation shows that requiring all four of maxcpus, cores,
threads and sockets is unnecessary. :)

This will catch situations where assigning a default for sockets doesn't
work well.  For example cores=2,cpus=4,maxcpus=5 will assign
threads=1,sockets=2; then the last relation will fail and QEMU will give
an error.

These invalid combination also make sure that other useful default cases
work.  For example "maxcpus given, sockets and cpus not" works thanks to
the following deduction:

- sockets = cpus/(cores*threads)
- maxcpus = cores*threads*sockets
- because cpus%(cores*threads) must be 0, this gives maxcpus=cpus

-smp should do its legacy magic and assign all five values based on it.
If the results do not match the obvious s/smp/-machine/ command line it
should warn, and perhaps suggest the equivalent -machine command line.
It doesn't have to be a minimal command line, just equivalent.

Apart from this point, these are lovely patches. :)

Paolo