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".

A new pc-specified pc_smp_parse() is introduced and to keep the interface
consistent, refactoring legacy smp_parse() to __smp_parse() is necessary.

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 <like.xu@linux.intel.com>
---
 qemu-options.hx | 17 +++++-----
 vl.c            | 89 ++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 97 insertions(+), 9 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 5daa5a8fb0..7fad5b50ff 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 8d92e2d209..66b577f447 100644
--- a/vl.c
+++ b/vl.c
@@ -63,6 +63,7 @@ int main(int argc, char **argv)
 #include "sysemu/watchdog.h"
 #include "hw/firmware/smbios.h"
 #include "hw/acpi/acpi.h"
+#include "hw/i386/pc.h"
 #include "hw/xen/xen.h"
 #include "hw/qdev.h"
 #include "hw/loader.h"
@@ -1248,6 +1249,9 @@ static QemuOptsList qemu_smp_opts = {
         }, {
             .name = "sockets",
             .type = QEMU_OPT_NUMBER,
+        }, {
+            .name = "dies",
+            .type = QEMU_OPT_NUMBER,
         }, {
             .name = "cores",
             .type = QEMU_OPT_NUMBER,
@@ -1262,7 +1266,7 @@ static QemuOptsList qemu_smp_opts = {
     },
 };
 
-static void smp_parse(QemuOpts *opts)
+static void __smp_parse(QemuOpts *opts)
 {
     if (opts) {
         unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
@@ -1334,6 +1338,89 @@ static void smp_parse(QemuOpts *opts)
     }
 }
 
+static void pc_smp_parse(QemuOpts *opts)
+{
+    PCMachineState *pcms = (PCMachineState *)
+        object_dynamic_cast(OBJECT(current_machine), TYPE_PC_MACHINE);
+
+    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);
+
+    /* compute missing values, prefer sockets over cores over threads */
+    if (cpus == 0 || sockets == 0) {
+        cores = cores > 0 ? cores : 1;
+        threads = threads > 0 ? threads : 1;
+        if (cpus == 0) {
+            sockets = sockets > 0 ? sockets : 1;
+            cpus = cores * threads * dies * sockets;
+        } else {
+            current_machine->smp.max_cpus =
+                    qemu_opt_get_number(opts, "maxcpus", cpus);
+            sockets = current_machine->smp.max_cpus / (cores * threads * dies);
+        }
+    } else if (cores == 0) {
+        threads = threads > 0 ? threads : 1;
+        cores = cpus / (sockets * dies * threads);
+        cores = cores > 0 ? cores : 1;
+    } else if (threads == 0) {
+        threads = cpus / (cores * dies * sockets);
+        threads = threads > 0 ? threads : 1;
+    } else if (sockets * dies * cores * threads < cpus) {
+        error_report("cpu topology: "
+                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
+                        "smp_cpus (%u)",
+                        sockets, dies, cores, threads, cpus);
+        exit(1);
+    }
+
+    current_machine->smp.max_cpus =
+            qemu_opt_get_number(opts, "maxcpus", cpus);
+
+    if (current_machine->smp.max_cpus < cpus) {
+        error_report("maxcpus must be equal to or greater than smp");
+        exit(1);
+    }
+
+    if (sockets * dies * cores * threads > current_machine->smp.max_cpus) {
+        error_report("cpu topology: "
+                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) > "
+                        "maxcpus (%u)",
+                        sockets, dies, cores, threads,
+                        current_machine->smp.max_cpus);
+        exit(1);
+    }
+
+    if (sockets * dies * cores * threads != current_machine->smp.max_cpus) {
+        warn_report("Invalid CPU topology deprecated: "
+                    "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
+                    "!= maxcpus (%u)",
+                    sockets, dies, cores, threads,
+                    current_machine->smp.max_cpus);
+    }
+
+    current_machine->smp.cpus = cpus;
+    current_machine->smp.cores = cores;
+    current_machine->smp.threads = threads;
+    pcms->smp_dies = dies;
+
+    if (current_machine->smp.cpus > 1) {
+        Error *blocker = NULL;
+        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
+        replay_add_blocker(blocker);
+    }
+}
+
+static void smp_parse(QemuOpts *opts)
+{
+    if (object_dynamic_cast(OBJECT(current_machine), TYPE_PC_MACHINE))
+        pc_smp_parse(opts);
+    else
+        __smp_parse(opts);
+}
+
 static void realtime_init(void)
 {
     if (enable_mlock) {
-- 
2.21.0

On Tue, May 21, 2019 at 12:50:54AM +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".
> 
> A new pc-specified pc_smp_parse() is introduced and to keep the interface
> consistent, refactoring legacy smp_parse() to __smp_parse() is necessary.
> 
> 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 <like.xu@linux.intel.com>
> ---
>  qemu-options.hx | 17 +++++-----
>  vl.c            | 89 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 97 insertions(+), 9 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 5daa5a8fb0..7fad5b50ff 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 8d92e2d209..66b577f447 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -63,6 +63,7 @@ int main(int argc, char **argv)
>  #include "sysemu/watchdog.h"
>  #include "hw/firmware/smbios.h"
>  #include "hw/acpi/acpi.h"
> +#include "hw/i386/pc.h"
>  #include "hw/xen/xen.h"
>  #include "hw/qdev.h"
>  #include "hw/loader.h"
> @@ -1248,6 +1249,9 @@ static QemuOptsList qemu_smp_opts = {
>          }, {
>              .name = "sockets",
>              .type = QEMU_OPT_NUMBER,
> +        }, {
> +            .name = "dies",
> +            .type = QEMU_OPT_NUMBER,
>          }, {
>              .name = "cores",
>              .type = QEMU_OPT_NUMBER,
> @@ -1262,7 +1266,7 @@ static QemuOptsList qemu_smp_opts = {
>      },
>  };
>  
> -static void smp_parse(QemuOpts *opts)
> +static void __smp_parse(QemuOpts *opts)
>  {
>      if (opts) {
>          unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
> @@ -1334,6 +1338,89 @@ static void smp_parse(QemuOpts *opts)
>      }
>  }
>  
> +static void pc_smp_parse(QemuOpts *opts)
> +{
> +    PCMachineState *pcms = (PCMachineState *)
> +        object_dynamic_cast(OBJECT(current_machine), TYPE_PC_MACHINE);
> +
> +    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);
> +
> +    /* compute missing values, prefer sockets over cores over threads */
> +    if (cpus == 0 || sockets == 0) {
> +        cores = cores > 0 ? cores : 1;
> +        threads = threads > 0 ? threads : 1;
> +        if (cpus == 0) {
> +            sockets = sockets > 0 ? sockets : 1;
> +            cpus = cores * threads * dies * sockets;
> +        } else {
> +            current_machine->smp.max_cpus =
> +                    qemu_opt_get_number(opts, "maxcpus", cpus);
> +            sockets = current_machine->smp.max_cpus / (cores * threads * dies);
> +        }
> +    } else if (cores == 0) {
> +        threads = threads > 0 ? threads : 1;
> +        cores = cpus / (sockets * dies * threads);
> +        cores = cores > 0 ? cores : 1;
> +    } else if (threads == 0) {
> +        threads = cpus / (cores * dies * sockets);
> +        threads = threads > 0 ? threads : 1;
> +    } else if (sockets * dies * cores * threads < cpus) {
> +        error_report("cpu topology: "
> +                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
> +                        "smp_cpus (%u)",
> +                        sockets, dies, cores, threads, cpus);
> +        exit(1);
> +    }
> +
> +    current_machine->smp.max_cpus =
> +            qemu_opt_get_number(opts, "maxcpus", cpus);
> +
> +    if (current_machine->smp.max_cpus < cpus) {
> +        error_report("maxcpus must be equal to or greater than smp");
> +        exit(1);
> +    }
> +
> +    if (sockets * dies * cores * threads > current_machine->smp.max_cpus) {
> +        error_report("cpu topology: "
> +                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) > "
> +                        "maxcpus (%u)",
> +                        sockets, dies, cores, threads,
> +                        current_machine->smp.max_cpus);
> +        exit(1);
> +    }
> +
> +    if (sockets * dies * cores * threads != current_machine->smp.max_cpus) {
> +        warn_report("Invalid CPU topology deprecated: "
> +                    "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
> +                    "!= maxcpus (%u)",
> +                    sockets, dies, cores, threads,
> +                    current_machine->smp.max_cpus);
> +    }
> +
> +    current_machine->smp.cpus = cpus;
> +    current_machine->smp.cores = cores;
> +    current_machine->smp.threads = threads;
> +    pcms->smp_dies = dies;
> +
> +    if (current_machine->smp.cpus > 1) {
> +        Error *blocker = NULL;
> +        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
> +        replay_add_blocker(blocker);
> +    }
> +}
> +
> +static void smp_parse(QemuOpts *opts)
> +{
> +    if (object_dynamic_cast(OBJECT(current_machine), TYPE_PC_MACHINE))
> +        pc_smp_parse(opts);

This is not exactly what I meant when I suggested creating a
pc_smp_parse() function.

The idea was to place the PC-specific function inside
hw/i386/pc.c, and have a MachineClass::smp_parse function pointer
that machine types could override.

The existing generic smp_parse() code, on the other hand, could
be moved to hw/core/machine.c, and be the default implementation
of MachineClass::smp_parse.

Suggestion: If you first create a copy of smp_parse() in one
patch, and then add CPU die support in another patch, it will be
much easier to review the changes you added to the pc_smp_parse()
code.


> +    else
> +        __smp_parse(opts);
> +}
> +
>  static void realtime_init(void)
>  {
>      if (enable_mlock) {
> -- 
> 2.21.0
> 
> 

-- 
Eduardo