From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [Qemu-devel] [PATCH v5 06/16] apic: Introduce backend/frontend infrastructure for KVM reuse Date: Mon, 19 Dec 2011 18:38:34 -0600 Message-ID: <4EEFD90A.1000204@codemonkey.ws> References: <4EEFB72E.7030508@codemonkey.ws> <4EEFC970.9030205@web.de> <4EEFD69F.6080700@codemonkey.ws> <4EEFD786.8030609@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Anthony Liguori , kvm@vger.kernel.org, "Michael S. Tsirkin" , Marcelo Tosatti , qemu-devel , Blue Swirl , Avi Kivity To: Jan Kiszka Return-path: Received: from mail-gx0-f174.google.com ([209.85.161.174]:35243 "EHLO mail-gx0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752335Ab1LTAii (ORCPT ); Mon, 19 Dec 2011 19:38:38 -0500 Received: by ggdk6 with SMTP id k6so4420103ggd.19 for ; Mon, 19 Dec 2011 16:38:37 -0800 (PST) In-Reply-To: <4EEFD786.8030609@web.de> Sender: kvm-owner@vger.kernel.org List-ID: 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 >