Re: [Qemu-devel] [PATCH v3 9/9] vl.c: Add -smp, dies=* command line support and update doc

[Eduardo Habkost]

On Wed, Jun 12, 2019 at 04:41:04PM +0800, Like Xu wrote:
> For PC target, users could configure the number of dies per one package
> via command line with this patch, such as "-smp dies=2,cores=4".
> 
> The parsing rules of new cpu-topology model obey the same restrictions/logic
> as the legacy socket/core/thread model especially on missing values computing.
> 
> Signed-off-by: Like Xu <address@hidden>
> ---
>  hw/i386/pc.c    | 32 ++++++++++++++++++--------------
>  qemu-options.hx | 17 +++++++++--------
>  vl.c            |  3 +++
>  3 files changed, 30 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 63b44bd2bd..8a5da4f0c1 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1543,10 +1543,13 @@ static void pc_new_cpu(PCMachineState *pcms, int64_t 
> apic_id, Error **errp)
>  
>  void pc_smp_parse(MachineState *ms, QemuOpts *opts)
>  {
> -    /* copy it from legacy smp_parse() in vl.c */
> +    PCMachineState *pcms = (PCMachineState *)
> +        object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE);
> +

Any specific reason for doing this instead of PC_MACHINE(ms)?

The rest of the patch looks good.


>      if (opts) {
>          unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
>          unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
> +        unsigned dies = qemu_opt_get_number(opts, "dies", 1);
>          unsigned cores   = qemu_opt_get_number(opts, "cores", 0);
>          unsigned threads = qemu_opt_get_number(opts, "threads", 0);
>  
> @@ -1556,24 +1559,24 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
>              threads = threads > 0 ? threads : 1;
>              if (cpus == 0) {
>                  sockets = sockets > 0 ? sockets : 1;
> -                cpus = cores * threads * sockets;
> +                cpus = cores * threads * dies * sockets;
>              } else {
>                  ms->smp.max_cpus =
>                          qemu_opt_get_number(opts, "maxcpus", cpus);
> -                sockets = ms->smp.max_cpus / (cores * threads);
> +                sockets = ms->smp.max_cpus / (cores * threads * dies);
>              }
>          } else if (cores == 0) {
>              threads = threads > 0 ? threads : 1;
> -            cores = cpus / (sockets * threads);
> +            cores = cpus / (sockets * dies * threads);
>              cores = cores > 0 ? cores : 1;
>          } else if (threads == 0) {
> -            threads = cpus / (cores * sockets);
> +            threads = cpus / (cores * dies * sockets);
>              threads = threads > 0 ? threads : 1;
> -        } else if (sockets * cores * threads < cpus) {
> +        } else if (sockets * dies * cores * threads < cpus) {
>              error_report("cpu topology: "
> -                         "sockets (%u) * cores (%u) * threads (%u) < "
> +                         "sockets (%u) * dies (%u) * cores (%u) * threads 
> (%u) < "
>                           "smp_cpus (%u)",
> -                         sockets, cores, threads, cpus);
> +                         sockets, dies, cores, threads, cpus);
>              exit(1);
>          }
>  
> @@ -1585,26 +1588,27 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
>              exit(1);
>          }
>  
> -        if (sockets * cores * threads > ms->smp.max_cpus) {
> +        if (sockets * dies * cores * threads > ms->smp.max_cpus) {
>              error_report("cpu topology: "
> -                         "sockets (%u) * cores (%u) * threads (%u) > "
> +                         "sockets (%u) * dies (%u) * cores (%u) * threads 
> (%u) > "
>                           "maxcpus (%u)",
> -                         sockets, cores, threads,
> +                         sockets, dies, cores, threads,
>                           ms->smp.max_cpus);
>              exit(1);
>          }
>  
> -        if (sockets * cores * threads != ms->smp.max_cpus) {
> +        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
>              warn_report("Invalid CPU topology deprecated: "
> -                        "sockets (%u) * cores (%u) * threads (%u) "
> +                        "sockets (%u) * dies (%u) * cores (%u) * threads 
> (%u) "
>                          "!= maxcpus (%u)",
> -                        sockets, cores, threads,
> +                        sockets, dies, cores, threads,
>                          ms->smp.max_cpus);
>          }
>  
>          ms->smp.cpus = cpus;
>          ms->smp.cores = cores;
>          ms->smp.threads = threads;
> +        pcms->smp_dies = dies;
>      }
>  
>      if (ms->smp.cpus > 1) {
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 0d8beb4afd..a5b314a448 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -138,25 +138,26 @@ no incompatible TCG features have been enabled (e.g. 
> icount/replay).
>  ETEXI
>  
>  DEF("smp", HAS_ARG, QEMU_OPTION_smp,
> -    "-smp 
> [cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads][,sockets=sockets]\n"
> +    "-smp 
> [cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads][,dies=dies][,sockets=sockets]\n"
>      "                set the number of CPUs to 'n' [default=1]\n"
>      "                maxcpus= maximum number of total cpus, including\n"
>      "                offline CPUs for hotplug, etc\n"
> -    "                cores= number of CPU cores on one socket\n"
> +    "                cores= number of CPU cores on one socket (for PC, it's 
> on one die)\n"
>      "                threads= number of threads on one CPU core\n"
> +    "                dies= number of CPU dies on one socket (for PC only)\n"
>      "                sockets= number of discrete sockets in the system\n",
>          QEMU_ARCH_ALL)
>  STEXI
> -@item -smp 
> [cpus=]@var{n}[,cores=@var{cores}][,threads=@var{threads}][,sockets=@var{sockets}][,maxcpus=@var{maxcpus}]
> +@item -smp 
> [cpus=]@var{n}[,cores=@var{cores}][,threads=@var{threads}][,dies=dies][,sockets=@var{sockets}][,maxcpus=@var{maxcpus}]
>  @findex -smp
>  Simulate an SMP system with @var{n} CPUs. On the PC target, up to 255
>  CPUs are supported. On Sparc32 target, Linux limits the number of usable CPUs
>  to 4.
> -For the PC target, the number of @var{cores} per socket, the number
> -of @var{threads} per cores and the total number of @var{sockets} can be
> -specified. Missing values will be computed. If any on the three values is
> -given, the total number of CPUs @var{n} can be omitted. @var{maxcpus}
> -specifies the maximum number of hotpluggable CPUs.
> +For the PC target, the number of @var{cores} per die, the number of 
> @var{threads}
> +per cores, the number of @var{dies} per packages and the total number of
> +@var{sockets} can be specified. Missing values will be computed.
> +If any on the three values is given, the total number of CPUs @var{n} can be 
> omitted.
> +@var{maxcpus} specifies the maximum number of hotpluggable CPUs.
>  ETEXI
>  
>  DEF("numa", HAS_ARG, QEMU_OPTION_numa,
> diff --git a/vl.c b/vl.c
> index 53ea9b6d6f..c6d1339442 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1231,6 +1231,9 @@ static QemuOptsList qemu_smp_opts = {
>          }, {
>              .name = "sockets",
>              .type = QEMU_OPT_NUMBER,
> +        }, {
> +            .name = "dies",
> +            .type = QEMU_OPT_NUMBER,
>          }, {
>              .name = "cores",
>              .type = QEMU_OPT_NUMBER,
> -- 
> 2.21.0
> 

-- 
Eduardo