Re: [Qemu-devel] [PATCH 4/4] xics: Support for in-kernel XICS interrupt controller

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 08/03/2013 12:57 AM, Alexey Kardashevskiy wrote:
> On 08/01/2013 11:29 AM, Andreas Färber wrote:
>> Am 01.08.2013 02:14, schrieb Alexey Kardashevskiy:
>>> On 08/01/2013 05:52 AM, Andreas Färber wrote:
>>>> Am 17.07.2013 08:37, schrieb Alexey Kardashevskiy:
> 
>>>>> +
>>>>> +    icp->ss = g_malloc0(icp->nr_servers*sizeof(ICPState));
>>>>> +    for (i = 0; i < icp->nr_servers; i++) {
>>>>> +        char buffer[32];
>>>>> +        object_initialize(&icp->ss[i], TYPE_ICP_KVM);
>>>>> +        snprintf(buffer, sizeof(buffer), "icp[%d]", i);
>>>>> +        object_property_add_child(OBJECT(icp), buffer, OBJECT(&icp->ss[i]), NULL);
>>>>> +        qdev_init_nofail(DEVICE(&icp->ss[i]));
>>>>
>>>> object_property_set_bool()
>>>
>>>
>>> ? Anthony did XICS refactoring recently and that has qdev_init_nofail().
>>
>> Nobody is perfect. ;)
>>
>> The point is, this is an object, and in realize you shouldn't abort but
>> set errp and leave error printing and handling to your caller. The QOM
>> API as opposed to qdev works with an Error object that you can
>> error_propagate() to your caller.
>>
>> (Also using qdev_* for something that is new-style QOM is ugly IMO.)
>>
>>>> Where does icp->nr_servers come from?
>>>
>>> Via properties in try_create_xics() (hw/ppc/spapr.c).
>>
>> Sounds tricky... Peter introduced static array properties for a similar
>> purpose, I believe. Don't know if that would help here.
> 
> Peter who? For what purpose? Could you please point to it? Cannot find it
> anywhere.
> 
> 
>>>
>>>> Is there no way to split this into
>>>> instance_init and realize?
>>>
>>>
>>> Why would we want to split?
>>
>> Because realize is too late to create new devices: With our targetted
>> late, recursive realization model it will not be possible to see and
>> modify such objects from management interface - only before realize.
> 
> 
> Oh. I lost you here. I need to create XICS with some child ICP devices (now
> they are devices). The number of child devices comes from spapr.c via
> properties. Now you are saying it is too late to create devices (even so
> lame ones) in realize(). So I have to put the creation in instance_init().
> Could you please point now I can create device-and-pass-propetries in one
> shot? Thanks!


No, I am still missing some bits.
In spapr we try to initialize xics-kvm and if we cannot have it, we switch
to emulated xics.

I.e.:
dev = qdev_create(NULL, type);
qdev_prop_set_uint32(dev, "nr_servers", nr_servers);
qdev_prop_set_uint32(dev, "nr_irqs", nr_irqs);
if (qdev_init(dev) < 0) {
    return NULL;
}

qdev_create() calls instance_init() (which cannot fail) and qdev_init()
calls realize() (which can fail and this is how I know that xics-kvm cannot
work).

instance_init() and realize() are called at once, right here. But as I
understand, realize() is not supposed to be called this way, it should be
called by QOM, no?


And about properties - before I create child devices, I need both
properties set. So it looks like I need a third property such as
"realized", call it "real_instance_init" and assign a callback on it as it
is done for realize(). Correct?


-- 
Alexey