Re: [Qemu-devel] [Qemu-ppc] [PATCH 03/25] spapr: introduce a spapr_icp_create() helper

[Cédric Le Goater]

On 11/24/2017 10:09 AM, Greg Kurz wrote:
> On Thu, 23 Nov 2017 14:29:33 +0100
> Cédric Le Goater <address@hidden> wrote:
> 
>> On sPAPR, the creation of the interrupt presenter depends on some of
>> the machine attributes. When the XIVE interrupt mode is available,
>> this will get more complex. So provide a machine-level helper to
>> isolate the process and hide the details to the sPAPR core realize
>> function.
>>
> 
> Not sure it makes sense to introduce this helper that early in the series...
> what about folding it in patch 23 where it is really needed ?

It does 'icp_type' and the 'xics_fabric' which are machine concepts 
around the sPAPR interrupt controller model.
 
But yes, it could come before patch 23. May be not folded, though.

C.


>> Signed-off-by: Cédric Le Goater <address@hidden>
>> ---
>>  hw/ppc/spapr.c          | 14 ++++++++++++++
>>  hw/ppc/spapr_cpu_core.c |  2 +-
>>  include/hw/ppc/spapr.h  |  2 ++
>>  3 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 174e7ff0678d..925cbd3c1bf4 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -3556,6 +3556,20 @@ static ICPState *spapr_icp_get(XICSFabric *xi, int 
>> vcpu_id)
>>      return cpu ? ICP(cpu->intc) : NULL;
>>  }
>>  
>> +Object *spapr_icp_create(sPAPRMachineState *spapr, CPUState *cs, Error 
>> **errp)
>> +{
>> +    Error *local_err = NULL;
>> +    Object *obj;
>> +
>> +    obj = icp_create(cs, spapr->icp_type, XICS_FABRIC(spapr), &local_err);
>> +    if (local_err) {
>> +        error_propagate(errp, local_err);
>> +        return NULL;
>> +    }
>> +
>> +    return obj;
>> +}
>> +
>>  static void spapr_pic_print_info(InterruptStatsProvider *obj,
>>                                   Monitor *mon)
>>  {
>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>> index f7cc74512481..61a9850e688b 100644
>> --- a/hw/ppc/spapr_cpu_core.c
>> +++ b/hw/ppc/spapr_cpu_core.c
>> @@ -122,7 +122,7 @@ static void spapr_cpu_core_realize_child(Object *child,
>>          goto error;
>>      }
>>  
>> -    cpu->intc = icp_create(cs, spapr->icp_type, XICS_FABRIC(spapr), 
>> &local_err);
>> +    cpu->intc = spapr_icp_create(spapr, cs, &local_err);
>>      if (local_err) {
>>          goto error;
>>      }
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index 9d21ca9bde3a..9da38de34277 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -707,4 +707,6 @@ void spapr_do_system_reset_on_cpu(CPUState *cs, 
>> run_on_cpu_data arg);
>>  int spapr_vcpu_id(PowerPCCPU *cpu);
>>  PowerPCCPU *spapr_find_cpu(int vcpu_id);
>>  
>> +Object *spapr_icp_create(sPAPRMachineState *spapr, CPUState *cs, Error 
>> **errp);
>> +
>>  #endif /* HW_SPAPR_H */
>