From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56143) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VgFL2-0005LG-68 for qemu-devel@nongnu.org; Tue, 12 Nov 2013 09:53:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VgFKu-0004QS-SR for qemu-devel@nongnu.org; Tue, 12 Nov 2013 09:53:08 -0500 Received: from cantor2.suse.de ([195.135.220.15]:34881 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VgFKu-0004Q8-IH for qemu-devel@nongnu.org; Tue, 12 Nov 2013 09:53:00 -0500 Message-ID: <528240C7.6050006@suse.de> Date: Tue, 12 Nov 2013 15:52:55 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= 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> In-Reply-To: <528055EC.6080600@163.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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?6LW15bCP5by6?= Cc: Chen Fan , pbonzini@redhat.com, qemu-devel@nongnu.org, aliguori@amazon.com Resending yesterday's message since it hasn't arrived on qemu-devel... Am 11.11.2013 04:58, schrieb =E8=B5=B5=E5=B0=8F=E5=BC=BA: > =E4=BA=8E 11/05/2013 04:51 PM, =E8=B5=B5=E5=B0=8F=E5=BC=BA =E5=86=99=E9= =81=93: >> =E4=BA=8E 2013=E5=B9=B411=E6=9C=8805=E6=97=A5 16:25, Chen Fan =E5=86=99= =E9=81=93: >>> 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: >=20 > 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). >=20 > 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 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg