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

[Thomas Huth]

On 14.03.2016 11:25, Igor Mammedov wrote:
> On Fri, 11 Mar 2016 10:24:35 +0530
> Bharata B Rao <address@hidden> 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 <address@hidden>
>> ---
>>  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??

 Thomas