Re: [Qemu-devel] [RFC PATCH v2 6/9] spapr: CPU core device

[Igor Mammedov <imammedo@redhat.com>]

On Mon, 14 Mar 2016 11:56:52 +0100
Thomas Huth <thuth@redhat.com> wrote:

> On 14.03.2016 11:25, Igor Mammedov wrote:
> > On Fri, 11 Mar 2016 10:24:35 +0530
> > Bharata B Rao <bharata@linux.vnet.ibm.com> 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 <bharata@linux.vnet.ibm.com>
> >> ---
> >>  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
>