[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v4 2+/9] spapr: allocate the ICPState object from
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-ppc] [PATCH v4 2+/9] spapr: allocate the ICPState object from under sPAPRCPUCore |
Date: |
Sun, 2 Apr 2017 19:16:16 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 04/02/2017 10:25 AM, David Gibson wrote:
> On Thu, Mar 30, 2017 at 05:08:04PM +0200, Cédric Le Goater wrote:
>> Today, all the ICPs are created before the CPUs, stored in an array
>> under the sPAPR machine and linked to the CPU when the core threads
>> are realized. This modeling brings some complexity when a lookup in
>> the array is required and it can be simplified by allocating the ICPs
>> when the CPUs are.
>>
>> This is the purpose of this proposal which introduces a new 'icp_type'
>> field under the machine and creates the ICP objects of the right type
>> (KVM or not) before the PowerPCCPU object are.
>>
>> This change allows more cleanups : the removal of the icps array under
>> the sPAPR machine and the removal xics_get_cpu_index_by_dt_id() helper.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> ---
>>
>> Tested with KVM and TCG. migration is fine.
>
> Reviewed-by: David Gibson <address@hidden>
>
> But please resend with the whole series, I've lost track of which
> versions are which.
sure. I was sending material to clarify what I was saying. I will
resend tomorrow.
Thanks,
C.
>>
>> hw/intc/xics.c | 11 -----------
>> hw/ppc/spapr.c | 47 ++++++++++++++---------------------------------
>> hw/ppc/spapr_cpu_core.c | 18 ++++++++++++++----
>> include/hw/ppc/spapr.h | 2 +-
>> include/hw/ppc/xics.h | 2 --
>> 5 files changed, 29 insertions(+), 51 deletions(-)
>>
>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> index 56fe70cd10e9..d4428b41b03a 100644
>> --- a/hw/intc/xics.c
>> +++ b/hw/intc/xics.c
>> @@ -38,17 +38,6 @@
>> #include "monitor/monitor.h"
>> #include "hw/intc/intc.h"
>>
>> -int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
>> -{
>> - PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
>> -
>> - if (cpu) {
>> - return cpu->parent_obj.cpu_index;
>> - }
>> -
>> - return -1;
>> -}
>> -
>> void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu)
>> {
>> CPUState *cs = CPU(cpu);
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index b9f7f8607869..18d8fe7458c1 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -103,7 +103,6 @@ static int try_create_xics(sPAPRMachineState *spapr,
>> const char *type_ics,
>> XICSFabric *xi = XICS_FABRIC(spapr);
>> Error *err = NULL, *local_err = NULL;
>> ICSState *ics = NULL;
>> - int i;
>>
>> ics = ICS_SIMPLE(object_new(type_ics));
>> object_property_add_child(OBJECT(spapr), "ics", OBJECT(ics), NULL);
>> @@ -112,34 +111,14 @@ static int try_create_xics(sPAPRMachineState *spapr,
>> const char *type_ics,
>> object_property_set_bool(OBJECT(ics), true, "realized", &local_err);
>> error_propagate(&err, local_err);
>> if (err) {
>> - goto error;
>> + error_propagate(errp, err);
>> + return -1;
>> }
>>
>> - spapr->icps = g_malloc0(nr_servers * sizeof(ICPState));
>> spapr->nr_servers = nr_servers;
>> -
>> - for (i = 0; i < nr_servers; i++) {
>> - ICPState *icp = &spapr->icps[i];
>> -
>> - object_initialize(icp, sizeof(*icp), type_icp);
>> - object_property_add_child(OBJECT(spapr), "icp[*]", OBJECT(icp),
>> NULL);
>> - object_property_add_const_link(OBJECT(icp), "xics", OBJECT(xi),
>> NULL);
>> - object_property_set_bool(OBJECT(icp), true, "realized", &err);
>> - if (err) {
>> - goto error;
>> - }
>> - object_unref(OBJECT(icp));
>> - }
>> -
>> spapr->ics = ics;
>> + spapr->icp_type = type_icp;
>> return 0;
>> -
>> -error:
>> - error_propagate(errp, err);
>> - if (ics) {
>> - object_unparent(OBJECT(ics));
>> - }
>> - return -1;
>> }
>>
>> static int xics_system_init(MachineState *machine,
>> @@ -1366,9 +1345,10 @@ static int spapr_post_load(void *opaque, int
>> version_id)
>> int err = 0;
>>
>> if (!object_dynamic_cast(OBJECT(spapr->ics), TYPE_ICS_KVM)) {
>> - int i;
>> - for (i = 0; i < spapr->nr_servers; i++) {
>> - icp_resend(&spapr->icps[i]);
>> + CPUState *cs;
>> + CPU_FOREACH(cs) {
>> + PowerPCCPU *cpu = POWERPC_CPU(cs);
>> + icp_resend(ICP(cpu->intc));
>> }
>> }
>>
>> @@ -3026,20 +3006,21 @@ static void spapr_ics_resend(XICSFabric *dev)
>>
>> static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
>> {
>> - sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
>> - int server = xics_get_cpu_index_by_dt_id(cpu_dt_id);
>> + PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
>>
>> - return (server < spapr->nr_servers) ? &spapr->icps[server] : NULL;
>> + return cpu ? ICP(cpu->intc) : NULL;
>> }
>>
>> static void spapr_pic_print_info(InterruptStatsProvider *obj,
>> Monitor *mon)
>> {
>> sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
>> - int i;
>> + CPUState *cs;
>> +
>> + CPU_FOREACH(cs) {
>> + PowerPCCPU *cpu = POWERPC_CPU(cs);
>>
>> - for (i = 0; i < spapr->nr_servers; i++) {
>> - icp_pic_print_info(&spapr->icps[i], mon);
>> + icp_pic_print_info(ICP(cpu->intc), mon);
>> }
>>
>> ics_pic_print_info(spapr->ics, mon);
>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>> index 4e1a99591d19..c80eb7c34f9f 100644
>> --- a/hw/ppc/spapr_cpu_core.c
>> +++ b/hw/ppc/spapr_cpu_core.c
>> @@ -63,8 +63,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr,
>> PowerPCCPU *cpu,
>> Error **errp)
>> {
>> CPUPPCState *env = &cpu->env;
>> - XICSFabric *xi = XICS_FABRIC(spapr);
>> - ICPState *icp = xics_icp_get(xi, cpu->cpu_dt_id);
>>
>> /* Set time-base frequency to 512 MHz */
>> cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
>> @@ -82,8 +80,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr,
>> PowerPCCPU *cpu,
>> }
>> }
>>
>> - xics_cpu_setup(xi, cpu, icp);
>> -
>> qemu_register_reset(spapr_cpu_reset, cpu);
>> spapr_cpu_reset(cpu);
>> }
>> @@ -143,18 +139,32 @@ static void spapr_cpu_core_realize_child(Object
>> *child, Error **errp)
>> sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>> CPUState *cs = CPU(child);
>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>> + Object *obj;
>> +
>> + obj = object_new(spapr->icp_type);
>> + object_property_add_child(OBJECT(spapr), "icp[*]", obj, NULL);
>> + object_property_add_const_link(obj, "xics", OBJECT(spapr), NULL);
>> + object_property_set_bool(obj, true, "realized", &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + return;
>> + }
>>
>> object_property_set_bool(child, true, "realized", &local_err);
>> if (local_err) {
>> + object_unparent(obj);
>> error_propagate(errp, local_err);
>> return;
>> }
>>
>> spapr_cpu_init(spapr, cpu, &local_err);
>> if (local_err) {
>> + object_unparent(obj);
>> error_propagate(errp, local_err);
>> return;
>> }
>> +
>> + xics_cpu_setup(XICS_FABRIC(spapr), cpu, ICP(obj));
>> }
>>
>> static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index 808aac870359..db3d4acb18a6 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -96,7 +96,7 @@ struct sPAPRMachineState {
>> MemoryHotplugState hotplug_memory;
>>
>> uint32_t nr_servers;
>> - ICPState *icps;
>> + const char *icp_type;
>> };
>>
>> #define H_SUCCESS 0
>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>> index 5e0244447fcd..88e0f021b168 100644
>> --- a/include/hw/ppc/xics.h
>> +++ b/include/hw/ppc/xics.h
>> @@ -172,8 +172,6 @@ void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu,
>> ICPState *icp);
>> void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu);
>>
>> /* Internal XICS interfaces */
>> -int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
>> -
>> void icp_set_cppr(ICPState *icp, uint8_t cppr);
>> void icp_set_mfrr(ICPState *icp, uint8_t mfrr);
>> uint32_t icp_accept(ICPState *ss);
>