From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34896) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCFhM-0005Su-Dq for qemu-devel@nongnu.org; Mon, 26 Mar 2012 15:35:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SCFhK-00022Q-CP for qemu-devel@nongnu.org; Mon, 26 Mar 2012 15:35:23 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:51696) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCFhK-00022C-70 for qemu-devel@nongnu.org; Mon, 26 Mar 2012 15:35:22 -0400 Received: by obbwd20 with SMTP id wd20so6390018obb.4 for ; Mon, 26 Mar 2012 12:35:20 -0700 (PDT) Message-ID: <4F70C4F6.8090900@codemonkey.ws> Date: Mon, 26 Mar 2012 14:35:18 -0500 From: Anthony Liguori 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> In-Reply-To: <4F70C3E0.4000708@web.de> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit 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: Jan Kiszka Cc: Gavin Shan , "qemu-devel@nongnu.org" , Isaku Yamahata , Avi Kivity , Paolo Bonzini , Wanpeng Li 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. 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 original >>> 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. It needs to be at least a forward reference. So we can avoid stuff like: int apic_accept_pic_intr(DeviceState *s); It should be: int apic_accept_pic_intr(APICState *s); So we can make use of the lovely type checking provided by the compiler to us. > We even abstracted the specifics of the > RTC away when it is embedded into a super-IO and interacts with an HPET. > If QOM requires such poking, then that requirement should be reassessed. 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. >> >> 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. I'm not really worried about it. It's so easier to refactor this later. Why rush it now? Regards, Anthony Liguori > Jan