From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:53881) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rcnj5-0002C5-O4 for qemu-devel@nongnu.org; Mon, 19 Dec 2011 19:38:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rcnj4-0002Pi-F2 for qemu-devel@nongnu.org; Mon, 19 Dec 2011 19:38:39 -0500 Received: from mail-gx0-f173.google.com ([209.85.161.173]:45406) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rcnj4-0002Pe-8z for qemu-devel@nongnu.org; Mon, 19 Dec 2011 19:38:38 -0500 Received: by ggnk1 with SMTP id k1so5072524ggn.4 for ; Mon, 19 Dec 2011 16:38:37 -0800 (PST) Message-ID: <4EEFD90A.1000204@codemonkey.ws> Date: Mon, 19 Dec 2011 18:38:34 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <4EEFB72E.7030508@codemonkey.ws> <4EEFC970.9030205@web.de> <4EEFD69F.6080700@codemonkey.ws> <4EEFD786.8030609@web.de> In-Reply-To: <4EEFD786.8030609@web.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 06/16] apic: Introduce backend/frontend infrastructure for KVM reuse List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Anthony Liguori , kvm@vger.kernel.org, "Michael S. Tsirkin" , Marcelo Tosatti , qemu-devel , Blue Swirl , Avi Kivity On 12/19/2011 06:32 PM, Jan Kiszka wrote: > On 2011-12-20 01:28, Anthony Liguori wrote: >> On 12/19/2011 05:32 PM, Jan Kiszka wrote: >>>> struct APICCommonInfo { >>>> DeviceInfo qdev; >>>> void (*init)(APICState *s); >>>> void (*set_base)(APICState *s, uint64_t val); >>>> void (*set_tpr)(APICState *s, uint8_t val); >>>> void (*external_nmi)(APICState *s); >>>> }; >>>> >>>> Take a look at SCSIDevice for an example of this in practice. This is >>>> nicer because as we move save/load into devices methods, it becomes >>>> natural to define the state and save/load function in the base class. >>>> Provided it only uses base class state, it lets save/load be compatible >>>> between both in-kernel and in-qemu device model. >>> >>> The difference is (unless I completely miss your point) that a common >>> SCSI base class is used by different derived classes. >> >> The 'frontend' is the common code and the 'backend' are the bits that >> are different, no? >> >> We ultimately want there to be two devices that share all of the >> 'frontend' code by providing different 'backend' implementations. >> >> So make the 'frontend' a base class that provides a set of abstract >> virtual methods (the set you have as the 'backend' interface). Each >> device instance then inherits from the base class and provides its own >> implementation of the virtual methods. >> >>> Here we have a >>> common frontend class but different base classes, so to say. And we have >>> a mechanism to chose where to inherit from on instantiation. Precisely >>> this allows to keep the compatibility between in-kernel and user space >>> model in this series. >> >> Okay, so I really think this is the problem. The in-kernel APIC is a >> separate device, no a property of the userspace APIC device. >> >> It should be modeled as two separate devices. > > That was v1 of my patches. Avi didn't like it, I tried it like this, and > in the end I had to agree. So, no, I don't think we want such a model. Yes, we do :-) The in-kernel APIC is a different implementation of the APIC device. It's not an "accelerator" for the userspace APIC. All that you're doing here is reinventing qdev. You're defining your own type system (APICBackend), creating a new regression system for it, and then defining your own factory function for creating it (through a qdev property). I'm struggling to understand the reason to avoid using the infrastructure we already have to do all of this. Regards, Anthony Liguori > > Jan >