From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35374) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCFj1-0006xa-8T for qemu-devel@nongnu.org; Mon, 26 Mar 2012 15:37:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SCFiz-0002TW-40 for qemu-devel@nongnu.org; Mon, 26 Mar 2012 15:37:06 -0400 Received: from fmmailgate07.web.de ([217.72.192.248]:58755) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCFiy-0002Rl-Qt for qemu-devel@nongnu.org; Mon, 26 Mar 2012 15:37:05 -0400 Received: from moweb001.kundenserver.de (moweb001.kundenserver.de [172.19.20.114]) by fmmailgate07.web.de (Postfix) with ESMTP id 7AE66FAE0C2 for ; Mon, 26 Mar 2012 21:37:03 +0200 (CEST) Message-ID: <4F70C55D.4030203@web.de> Date: Mon, 26 Mar 2012 21:37:01 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1332727608-26523-1-git-send-email-liwp@linux.vnet.ibm.com> <4F705F08.4010002@siemens.com> <4F70A877.3060809@codemonkey.ws> <4F70C3E0.4000708@web.de> <4F70C4F6.8090900@codemonkey.ws> In-Reply-To: <4F70C4F6.8090900@codemonkey.ws> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigF3F1B96DD22B9F70F129D8C7" Subject: Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Gavin Shan , "qemu-devel@nongnu.org" , Isaku Yamahata , Avi Kivity , Paolo Bonzini , Wanpeng Li This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigF3F1B96DD22B9F70F129D8C7 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable On 2012-03-26 21:35, Anthony Liguori wrote: > On 03/26/2012 02:30 PM, Jan Kiszka wrote: >> On 2012-03-26 19:33, Anthony Liguori wrote: >>> On 03/26/2012 07:20 AM, Jan Kiszka wrote: >>>> On 2012-03-26 04:06, Wanpeng Li wrote: >>>>> From: Anthony Liguori >>>>> >>>>> >>>>> This series aggressively refactors the PC machine initialization to= >>>>> be more >>>>> modelled and less ad-hoc. The highlights of this series are: >>>>> >>>>> 1) Things like -m and -bios-name are now device model properties= >>>>> >>>>> 2) The i440fx and piix3 are now modelled in a thorough fashion >>>>> >>>>> 3) Most of the chipset features of the piix3 are modelled >>>>> through composition >>>>> >>>>> 4) i440fx_init is trivialized to creating devices and setting >>>>> properties >>>>> >>>>> 5) convert MemoryRegion to QOM >>>>> >>>>> 6) convert PCI host bridge to QOM >>>>> >>>>> The point (4) is the most important one. As we refactor in this >>>>> fashion, >>>>> we should quickly get to the point where machine->init disappears >>>>> completely in >>>>> favor of just creating a handful of devices. >>>>> >>>>> The two stage initialization of QOM is important here.=20 >>>>> instance_init() is when >>>>> composed devices are created which means that after you've created >>>>> a device, all >>>>> of its children are visible in the device model. This lets you set= >>>>> properties >>>>> of the parent and its children. >>>>> >>>>> realize() (which is still called DeviceState::init today) will be >>>>> called right >>>>> before the guest starts up for the first time. >>>> >>>> While I see the value of the overall direction, I still disagree on >>>> making internal data structures of HPET, RTC and 8254 publicly >>>> available. That's a wrong step back. I'm sure there are smarter >>>> solutions, alse as there were some proposals back then in the origin= al >>>> thread. >>> >>> I'm not fully decided myself. A couple things are clear to me though= : >>> >>> 1) We must expose type proper types in header files. We need there >>> to be a >>> globally accessible RTCState type and functions that operate on it. >> >> I'm not sure what "proper type" means in this context, but I'm quite >> sure that there should be no need for poking into the internal of the >> class outside of mc146818rtc.c. >=20 > It needs to be at least a forward reference. So we can avoid stuff lik= e: >=20 > int apic_accept_pic_intr(DeviceState *s); >=20 > It should be: >=20 > int apic_accept_pic_intr(APICState *s); >=20 > So we can make use of the lovely type checking provided by the compiler= > to us. I do not disagree. A pointer is harmless. >=20 >> We even abstracted the specifics of the >> RTC away when it is embedded into a super-IO and interacts with an HPE= T. >> If QOM requires such poking, then that requirement should be reassesse= d. >=20 > There are a couple of ways to make types private while still having > forward declarations. None of them are straight forward. That's why I= > suggest we save this for another day. >=20 >>> >>> 2) We can simplify memory management by knowing the size of the type >>> in the >>> header files too. >> >> Is this more than a malloc-free pair? >> >>> >>> Since this is an easily refactorable thing to look at later, I think >>> we should >>> start with extracting the types. >> >> My worry is that those three refactorings set bad examples for others.= >> So I'd like to avoid such back and forth if possible. >=20 > I'm not really worried about it. It's so easier to refactor this > later. Why rush it now? You rush changing the current layout, not me. :) Jan --------------enigF3F1B96DD22B9F70F129D8C7 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk9wxV0ACgkQitSsb3rl5xSrWgCfWzDm+/r5+DuJJiWUCxUNw6B+ KTQAnRLe0kMA1w7gXXn2TJcda2EWDiG3 =gUy4 -----END PGP SIGNATURE----- --------------enigF3F1B96DD22B9F70F129D8C7--