From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54392) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VmCpw-0004e7-9r for qemu-devel@nongnu.org; Thu, 28 Nov 2013 20:25:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VmCpo-0006sS-NJ for qemu-devel@nongnu.org; Thu, 28 Nov 2013 20:25:40 -0500 Received: from m50-132.163.com ([123.125.50.132]:55762) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VmCpn-0006sH-RZ for qemu-devel@nongnu.org; Thu, 28 Nov 2013 20:25:32 -0500 Message-ID: <5297ED2A.700@163.com> Date: Fri, 29 Nov 2013 09:26:02 +0800 From: =?UTF-8?B?6LW15bCP5by6?= MIME-Version: 1.0 References: <1383638143-11812-1-git-send-email-zxq_yx_007@163.com> <1383638143-11812-3-git-send-email-zxq_yx_007@163.com> <1383639922.556.10.camel@G08FNSTD131468> <5278B1A3.40109@163.com> <528055EC.6080600@163.com> <528240C7.6050006@suse.de> <528316FC.4090901@163.com> In-Reply-To: <528316FC.4090901@163.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v2 2/4] apic: QOM'ify apic & icc_bus List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Cc: Chen Fan , pbonzini@redhat.com, qemu-devel@nongnu.org, aliguori@amazon.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?