Resending yesterday's message since it hasn't arrived on qemu-devel...
Am 11.11.2013 04:58, schrieb 赵小强:
> 于 11/05/2013 04:51 PM, 赵小强 写道:
>> 于 2013年11月05日 16:25, Chen Fan 写道:
>>> On Tue, 2013-11-05 at 15:55 +0800, xiaoqiang zhao wrote:
[...]
>>>> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
>>>> index b550070..b32a549 100644
>>>> --- a/include/hw/cpu/icc_bus.h
>>>> +++ b/include/hw/cpu/icc_bus.h
>>>> @@ -66,7 +66,8 @@ typedef struct ICCDeviceClass {
>>>> DeviceClass parent_class;
>>>> /*< public >*/
>>>> - int (*init)(ICCDevice *dev); /* TODO replace with QOM realize */
>>>> + /* QOM realize */
>>>> + DeviceRealize realize;
>>> Maybe adding 'realize' in ICCDeviceClass is redundant, because parent
>>> class has included 'realize' field.
>>>
>>>> } ICCDeviceClass;
>>>> #define TYPE_ICC_DEVICE "icc-device"
>>>> diff --git a/include/hw/i386/apic_internal.h
>>>> b/include/hw/i386/apic_internal.h
>>>> index 1b0a7fb..bd3a5fc 100644
>>>> --- a/include/hw/i386/apic_internal.h
>>>> +++ b/include/hw/i386/apic_internal.h
>>>> @@ -79,8 +79,9 @@ typedef struct APICCommonState APICCommonState;
>>>> typedef struct APICCommonClass
>>>> {
>>>> ICCDeviceClass parent_class;
>>>> -
>>>> - void (*init)(APICCommonState *s);
>>>> +
>>>> + /* QOM realize */
>>>> + DeviceRealize realize;
>>> as above.
>>>
>>> Thanks,
>>> Chen
>>>> void (*set_base)(APICCommonState *s, uint64_t val);
>>>> void (*set_tpr)(APICCommonState *s, uint8_t val);
>>>> uint8_t (*get_tpr)(APICCommonState *s);
>>>
>> Thanks for your review!!
> Hi, Chen Fan:
>
> In my understanding, If we use only one 'realize'(which in
> 'DeviceClass'), I think all the initialization work must be done in the
> leaf child. if we add 'redundant' realize to each parent, then we can
> call the initialization chain from DeviceClass down to leaf child's
> parent, with each parent complete a bit further(of cause, a parent can
> do nothing and pass to it's child directly).
>
> What do you think?
Your analysis is correct. v2 is like I requested you to do it and v3
still does iirc, so let's stick with that for "consistency". Bad word in
this context, I know. ;)
If you have some time, it would be nice if you could check whether these
devices (the non-KVM versions at least) are covered by make check. For
ICC bus I am certain that it is. In particular I'm wondering if we need
certain -cpu arguments to enable the *APIC devices and have the realize
functions actually exercised? (My assumption is no, but a confirmation
would save time.)
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
