On Fri, Jan 08, 2016 at 12:25:09PM +0530, Bharata B Rao wrote: > 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 <bhar...@linux.vnet.ibm.com>
Personally, I think this restriciton should be enforced always, and
for non-full sockets as well. But I'm happy enough with this if it
lets us move forwards.
Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
> 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 "
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
