[Qemu-devel] [PATCH v6 01/11] machine: Don't allow CPU toplogies with partially filled cores

[Bharata B Rao <bharata@linux.vnet.ibm.com>]

Prevent guests from booting with CPU topologies that have partially
filled CPU cores or can result in partially filled CPU cores after
CPU hotplug like

-smp 15,sockets=1,cores=4,threads=4,maxcpus=16 or
-smp 15,sockets=1,cores=4,threads=4,maxcpus=17.

This is enforced by introducing MachineClass::validate_smp_config()
that gets called from generic SMP parsing code. Machine type versions
that don't want to enforce this can override this method.

TODO: Only sPAPR and PC changes are done in this patch, other archs
will be touched after there is agreement on this approach.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 hw/core/machine.c   | 20 ++++++++++++++++++++
 hw/i386/pc_piix.c   |  7 +++++++
 hw/i386/pc_q35.c    |  7 +++++++
 hw/ppc/spapr.c      |  7 +++++++
 include/hw/boards.h |  1 +
 vl.c                |  4 ++++
 6 files changed, 46 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index c46ddc7..b66c101 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -336,6 +336,25 @@ static void machine_init_notify(Notifier *notifier, void *data)
     foreach_dynamic_sysbus_device(error_on_sysbus_device, NULL);
 }
 
+static int validate_smp_config_generic(int smp_cpus, int max_cpus,
+                                       int smp_threads)
+{
+        if (smp_cpus % smp_threads) {
+            error_report("cpu topology: "
+                         "smp_cpus (%u) should be multiple of threads (%u) ",
+                         smp_cpus, smp_threads);
+            return 1;
+        }
+
+        if (max_cpus % smp_threads) {
+            error_report("cpu topology: "
+                         "max_cpus (%u) should be multiple of threads (%u) ",
+                         max_cpus, smp_threads);
+            return 1;
+        }
+        return 0;
+}
+
 static void machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -343,6 +362,7 @@ static void machine_class_init(ObjectClass *oc, void *data)
     /* Default 128 MB as guest ram size */
     mc->default_ram_size = 128 * M_BYTE;
     mc->rom_file_has_mr = true;
+    mc->validate_smp_config = validate_smp_config_generic;
 }
 
 static void machine_class_base_init(ObjectClass *oc, void *data)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 438cdae..dd4bba1 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -397,6 +397,12 @@ static void pc_xen_hvm_init(MachineState *machine)
 }
 #endif
 
+static int pc_validate_smp_config_default(int smp_cpus, int max_cpus,
+                                          int smp_threads)
+{
+    return 0;
+}
+
 #define DEFINE_I440FX_MACHINE(suffix, name, compatfn, optionfn) \
     static void pc_init_##suffix(MachineState *machine) \
     { \
@@ -434,6 +440,7 @@ static void pc_i440fx_2_5_machine_options(MachineClass *m)
     pc_i440fx_2_6_machine_options(m);
     m->alias = NULL;
     m->is_default = 0;
+    m->validate_smp_config = pc_validate_smp_config_default;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
 }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 412b3cd..80ce9e8 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -325,6 +325,12 @@ static void pc_compat_1_4(MachineState *machine)
     pc_compat_1_5(machine);
 }
 
+static int pc_q35_validate_smp_config_default(int smp_cpus, int max_cpus,
+                                              int smp_threads)
+{
+    return 0;
+}
+
 #define DEFINE_Q35_MACHINE(suffix, name, compatfn, optionfn) \
     static void pc_init_##suffix(MachineState *machine) \
     { \
@@ -362,6 +368,7 @@ static void pc_q35_2_5_machine_options(MachineClass *m)
 {
     pc_q35_2_6_machine_options(m);
     m->alias = NULL;
+    m->validate_smp_config = pc_q35_validate_smp_config_default;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
 }
 
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 414e0f9b..a330169 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2320,6 +2320,12 @@ static const TypeInfo spapr_machine_info = {
     },
 };
 
+static int spapr_validate_smp_config_default(int smp_cpus, int max_cpus,
+                                          int smp_threads)
+{
+    return 0;
+}
+
 #define DEFINE_SPAPR_MACHINE(suffix, verstr, latest)                 \
     static void spapr_machine_##suffix##_class_init(ObjectClass *oc, \
                                                     void *data)      \
@@ -2379,6 +2385,7 @@ static void spapr_machine_2_5_class_options(MachineClass *mc)
     spapr_machine_2_6_class_options(mc);
     smc->use_ohci_by_default = true;
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_5);
+    mc->validate_smp_config = spapr_validate_smp_config_default;
 }
 
 DEFINE_SPAPR_MACHINE(2_5, "2.5", false);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 0f30959..39091bf 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -99,6 +99,7 @@ struct MachineClass {
     HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
                                            DeviceState *dev);
     unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
+    int (*validate_smp_config)(int smp_cpus, int max_cpus, int smp_threads);
 };
 
 /**
diff --git a/vl.c b/vl.c
index 5aaea77..4b36a49 100644
--- a/vl.c
+++ b/vl.c
@@ -4132,6 +4132,10 @@ int main(int argc, char **argv, char **envp)
 
     smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
 
+    if (machine_class->validate_smp_config(smp_cpus, max_cpus, smp_threads)) {
+        exit(1);
+    }
+
     machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */
     if (max_cpus > machine_class->max_cpus) {
         error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
-- 
2.1.0