From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50090) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afRJ7-000750-7Y for qemu-devel@nongnu.org; Mon, 14 Mar 2016 08:09:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1afRJ3-00064z-Sv for qemu-devel@nongnu.org; Mon, 14 Mar 2016 08:09:09 -0400 Date: Mon, 14 Mar 2016 13:08:54 +0100 From: Igor Mammedov Message-ID: <20160314130854.43a259e7@nial.brq.redhat.com> In-Reply-To: <56E698F4.9000401@redhat.com> References: <1457672078-17307-1-git-send-email-bharata@linux.vnet.ibm.com> <1457672078-17307-7-git-send-email-bharata@linux.vnet.ibm.com> <20160314112523.1c43461f@nial.brq.redhat.com> <56E698F4.9000401@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH v2 6/9] spapr: CPU core device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: mjrosato@linux.vnet.ibm.com, agraf@suse.de, pkrempa@redhat.com, ehabkost@redhat.com, aik@ozlabs.ru, armbru@redhat.com, qemu-devel@nongnu.org, borntraeger@de.ibm.com, qemu-ppc@nongnu.org, Bharata B Rao , pbonzini@redhat.com, mdroth@linux.vnet.ibm.com, afaerber@suse.de, david@gibson.dropbear.id.au On Mon, 14 Mar 2016 11:56:52 +0100 Thomas Huth wrote: > On 14.03.2016 11:25, Igor Mammedov wrote: > > On Fri, 11 Mar 2016 10:24:35 +0530 > > Bharata B Rao wrote: > > > >> Add sPAPR specific CPU core device that is based on generic CPU core device. > >> Creating this core device will result in creation of all the CPU thread > >> devices that are part of this core. > >> > >> Introduce sPAPRMachineClass.dr_cpu_enabled to indicate support for > >> CPU core hotplug. Initialize boot time CPUs as core deivces and prevent > >> topologies that result in partially filled cores. Both of these are done > >> only if CPU core hotplug is supported. > >> > >> Note: An unrelated change in the call to xics_system_init() is done > >> in this patch as it makes sense to use the local variable smt introduced > >> in this patch instead of kvmppc_smt_threads() call here. > >> > >> Signed-off-by: Bharata B Rao > >> --- > >> hw/ppc/Makefile.objs | 1 + > >> hw/ppc/spapr.c | 68 +++++++++++--- > >> hw/ppc/spapr_cpu_core.c | 199 ++++++++++++++++++++++++++++++++++++++++ > >> include/hw/ppc/spapr.h | 4 + > >> include/hw/ppc/spapr_cpu_core.h | 28 ++++++ > >> 5 files changed, 287 insertions(+), 13 deletions(-) > >> create mode 100644 hw/ppc/spapr_cpu_core.c > >> create mode 100644 include/hw/ppc/spapr_cpu_core.h > >> > >> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > >> index c1ffc77..5cc6608 100644 > >> --- a/hw/ppc/Makefile.objs > >> +++ b/hw/ppc/Makefile.objs > >> @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o > >> obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o > >> obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o > >> obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o > >> +obj-$(CONFIG_PSERIES) += spapr_cpu_core.o > >> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) > >> obj-y += spapr_pci_vfio.o > >> endif > >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > >> index 64c4acc..cffe8c8 100644 > >> --- a/hw/ppc/spapr.c > >> +++ b/hw/ppc/spapr.c > ... > >> @@ -1800,13 +1818,34 @@ static void ppc_spapr_init(MachineState *machine) > >> if (machine->cpu_model == NULL) { > >> machine->cpu_model = kvm_enabled() ? "host" : "POWER7"; > >> } > >> - for (i = 0; i < smp_cpus; i++) { > >> - cpu = cpu_ppc_init(machine->cpu_model); > >> - if (cpu == NULL) { > >> - error_report("Unable to find PowerPC CPU definition"); > >> - exit(1); > >> + > >> + if (smc->dr_cpu_enabled) { > >> + spapr->cores = g_new0(Object *, spapr_max_cores); > > The spapr->cores _pointer_ is allocate with g_new0 here ... > > >> + for (i = 0; i < spapr_max_cores; i++) { > >> + int core_dt_id = i * smt; > >> + > >> + if (i < spapr_cores) { > >> + Object *core = object_new(TYPE_SPAPR_CPU_CORE); > >> + > >> + object_property_set_str(core, machine->cpu_model, "cpu_model", > >> + &error_fatal); > >> + object_property_set_int(core, smp_threads, "threads", > >> + &error_fatal); > >> + object_property_set_int(core, core_dt_id, CPU_CORE_PROP_CORE, > >> + &error_fatal); > >> + object_property_set_bool(core, true, "realized", &error_fatal); > >> + } > >> } > >> - spapr_cpu_init(spapr, cpu, &error_fatal); > >> + } else { > >> + for (i = 0; i < smp_cpus; i++) { > >> + PowerPCCPU *cpu = cpu_ppc_init(machine->cpu_model); > >> + if (cpu == NULL) { > >> + error_report("Unable to find PowerPC CPU definition"); > >> + exit(1); > >> + } > >> + spapr_cpu_init(spapr, cpu, &error_fatal); > >> + } > >> } > ... > >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > >> index 098d85d..c099c3c 100644 > >> --- a/include/hw/ppc/spapr.h > >> +++ b/include/hw/ppc/spapr.h > >> @@ -36,6 +36,7 @@ struct sPAPRMachineClass { > >> > >> /*< public >*/ > >> bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */ > >> + bool dr_cpu_enabled; /* enable dynamic-reconfig/hotplug of CPUs */ > >> bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */ > >> }; > >> > >> @@ -79,6 +80,7 @@ struct sPAPRMachineState { > >> /*< public >*/ > >> char *kvm_type; > >> MemoryHotplugState hotplug_memory; > >> + Object **cores; > > I'd prefer "Object *cores[0];" as it tells us that it's an array > > ... so you can not declare it as an array here, can you?? I can't > > Thomas >