Re: [Qemu-devel] [PATCH v2 2/4] apic: QOM'ify apic & icc_bus

[赵小强 <zxq_yx_007@163.com>]

于 11/13/2013 02:06 PM, 赵小强 写道:
> 于 11/12/2013 10:52 PM, Andreas Färber 写道:
>> 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
>>
> I see. thanks for your reply!
   I have been busy with my work this days. I wish I could have some 
time soon. A few questions about your last reply.
> 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. 
   1. Does "make check" mean the build target in the Makefile ?  if it 
is, I can not find anything about "icc_bus" under "tests" directory.  Or 
you refer to something else?
   2. What does "enable the *APIC devices and have the realize functions 
actually exercised?" mean?