Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 16/22] ppc/xics: register the reset handler of ICP objects

["Cédric Le Goater" <clg@kaod.org>]

On 02/23/2017 03:42 AM, David Gibson wrote:
> On Thu, Feb 16, 2017 at 02:47:39PM +0100, Cédric Le Goater wrote:
>> The reset of the ICP objects is currently handled by XICS but this can
>> be done for each individual ICP.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> Hrm.  I think whether device_reset() gets called automatically depends
> on how the device is wired into the composition tree, and I'm not sure
> the icps are in the right place for it to work.

reset gets called only if it under sysbus or if you have registered 
a reset_handler. previously, XICS was a sysbus object so 
xics_common_reset() was getting called automatically.

> This doesn't replace the code in xics_common_reset() so if it does
> work it means we must have previously been resetting the ICPs twice.
> Is that right?

no. but there has been some confusion with the recent changes
on XICS.

What replace the code in xics_common_reset() is :

	qdev_set_parent_bus(DEVICE(icp), sysbus_get_default());

That's how the reset handlers get called from QOM objects. 

C.

>> ---
>>  hw/intc/xics.c | 18 ------------------
>>  hw/ppc/spapr.c |  1 +
>>  2 files changed, 1 insertion(+), 18 deletions(-)
>>
>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> index dd41340d41a5..3ad7e8cf8ec4 100644
>> --- a/hw/intc/xics.c
>> +++ b/hw/intc/xics.c
>> @@ -137,29 +137,11 @@ static void ics_simple_pic_print_info(InterruptStatsProvider *obj,
>>  /*
>>   * XICS Common class - parent for emulated XICS and KVM-XICS
>>   */
>> -static void xics_common_reset(DeviceState *d)
>> -{
>> -    XICSState *xics = XICS_COMMON(d);
>> -    int i;
>> -
>> -    for (i = 0; i < xics->nr_servers; i++) {
>> -        device_reset(DEVICE(&xics->ss[i]));
>> -    }
>> -}
>> -
>> -static void xics_common_class_init(ObjectClass *oc, void *data)
>> -{
>> -    DeviceClass *dc = DEVICE_CLASS(oc);
>> -
>> -    dc->reset = xics_common_reset;
>> -}
>> -
>>  static const TypeInfo xics_common_info = {
>>      .name          = TYPE_XICS_COMMON,
>>      .parent        = TYPE_DEVICE,
>>      .instance_size = sizeof(XICSState),
>>      .class_size    = sizeof(XICSStateClass),
>> -    .class_init    = xics_common_class_init,
>>  };
>>  
>>  /*
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 9c1772f93155..445d9a6ddad4 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -130,6 +130,7 @@ static XICSState *try_create_xics(sPAPRMachineState *spapr,
>>          ICPState *icp = &xics->ss[i];
>>  
>>          object_initialize(icp, sizeof(*icp), type_icp);
>> +        qdev_set_parent_bus(DEVICE(icp), sysbus_get_default());
>>          object_property_add_child(OBJECT(xics), "icp[*]", OBJECT(icp), NULL);
>>          object_property_add_const_link(OBJECT(icp), "xics", OBJECT(xics), NULL);
>>          object_property_set_bool(OBJECT(icp), true, "realized", &err);
>