From patchwork Mon Aug 11 02:50:48 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tiejun Chen X-Patchwork-Id: 378878 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id BA989140132 for ; Mon, 11 Aug 2014 12:51:22 +1000 (EST) Received: from localhost ([::1]:33412 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XGfhg-0001nK-Pe for incoming@patchwork.ozlabs.org; Sun, 10 Aug 2014 22:51:20 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48064) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XGfhM-0001Uv-2n for qemu-devel@nongnu.org; Sun, 10 Aug 2014 22:51:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XGfhH-0001WZ-Gh for qemu-devel@nongnu.org; Sun, 10 Aug 2014 22:51:00 -0400 Received: from mga11.intel.com ([192.55.52.93]:22034) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XGfhH-0001To-5a for qemu-devel@nongnu.org; Sun, 10 Aug 2014 22:50:55 -0400 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 10 Aug 2014 19:50:53 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.01,838,1400050800"; d="scan'208";a="582958989" Received: from tiejunch-mobl.ccr.corp.intel.com (HELO [10.238.130.113]) ([10.238.130.113]) by fmsmga002.fm.intel.com with ESMTP; 10 Aug 2014 19:50:48 -0700 Message-ID: <53E82F88.5020209@intel.com> Date: Mon, 11 Aug 2014 10:50:48 +0800 From: "Chen, Tiejun" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: "Michael S. Tsirkin" References: <1407307835-9692-1-git-send-email-tiejun.chen@intel.com> <1407307835-9692-4-git-send-email-tiejun.chen@intel.com> <20140806094509.GB22307@redhat.com> <53E2009E.4080107@intel.com> <20140806210714.GD22307@redhat.com> <53E2D921.8040501@intel.com> <20140810202759.GA5199@redhat.com> In-Reply-To: <20140810202759.GA5199@redhat.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 192.55.52.93 Cc: pbonzini@redhat.com, xen-devel@lists.xensource.com, qemu-devel@nongnu.org, stefano.stabellini@eu.citrix.com Subject: Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 2014/8/11 4:27, Michael S. Tsirkin wrote: > On Thu, Aug 07, 2014 at 09:40:49AM +0800, Chen, Tiejun wrote: >> >> >> On 2014/8/7 5:07, Michael S. Tsirkin wrote: >>> On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote: >>>> On 2014/8/6 17:45, Michael S. Tsirkin wrote: >>>>> On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote: >>>>>> We need to use this index to reuse this macro later >>>>>> >>>>>> Signed-off-by: Tiejun Chen >>>>> >>>>> Which index? >>>>> Most users don't need to change. >>>>> Just open-code OBJECT_CHECK where necessary, or add >>>>> a new wrapper. >>>> >>>> Okay so what about this? >>>> >>>> hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE >>>> >>>> We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get >>>> object with type, then we can reuse i440fx_init() simply. >>>> >>>> Signed-off-by: Tiejun Chen >>>> >>>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c >>>> index 0cd82b8..8c74653 100644 >>>> --- a/hw/pci-host/piix.c >>>> +++ b/hw/pci-host/piix.c >>>> @@ -93,6 +93,9 @@ typedef struct PIIX3State { >>>> #define I440FX_PCI_DEVICE(obj) \ >>>> OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE) >>>> >>>> +#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \ >>>> + OBJECT_CHECK(PCII440FXState, (obj), type) >>> >>> This is just wrong. If you are casting to PCII440FXState, >> >> Why? We will have two different QOM typenames of PCII440FXStates. >> >>> there is no reason not to use TYPE_I440FX_PCI_DEVICE. >> >> As you know we already have this original, >> >> static const TypeInfo i440fx_info = { >> .name = TYPE_I440FX_PCI_DEVICE, >> .parent = TYPE_PCI_DEVICE, >> .instance_size = sizeof(PCII440FXState), >> .class_init = i440fx_class_init, >> }; >> >> and in patch #4, we will register that new host bridge to IGD passthrough: >> >> static const TypeInfo xen_igd_passthrough_i440fx_info = { >> .name = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE, >> .parent = TYPE_PCI_DEVICE, >> .instance_size = sizeof(PCII440FXState), >> .class_init = xen_igd_passthrough_i440fx_class_init, >> }; > > My idea is to inherit TYPE_I440FX_PCI_DEVICE instead. So here, this mean xen_igd_passthrough_i440fx_info's parent should be TYPE_I440FX_PCI_DEVICE, right? > Then you can reuse regular piix code which casts to > TYPE_I440FX_PCI_DEVICE because > TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE would > be a subclass of TYPE_I440FX_PCI_DEVICE. > So since then, we can reuse i440fx_initfn. As a summary, what we should do is like the following, xen:hw:pci-host:piix: create host bridge to passthrough Implement a pci host bridge specific to passthrough. Actually this just inherits the standard one. This is based on http://patchwork.ozlabs.org/patch/363810/. Signed-off-by: Tiejun Chen --- hw/pci-host/piix.c | 28 ++++++++++++++++++++++++++++ include/hw/i386/pc.h | 2 ++ 2 files changed, 30 insertions(+) ISABus **isa_bus, qemu_irq *pic, diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index 0cd82b8..2ccd9ee 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -703,6 +703,33 @@ static const TypeInfo i440fx_info = { .class_init = i440fx_class_init, }; +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); + + k->init = i440fx_initfn; + k->vendor_id = PCI_VENDOR_ID_INTEL; + k->device_id = PCI_DEVICE_ID_INTEL_82441; + k->revision = 0x02; + k->class_id = PCI_CLASS_BRIDGE_HOST; + dc->desc = "IGD PT XEN Host bridge"; + dc->vmsd = &vmstate_i440fx; + /* + * PCI-facing part of the host bridge, not usable without the + * host-facing part, which can't be device_add'ed, yet. + */ + dc->cannot_instantiate_with_device_add_yet = true; + dc->hotpluggable = false; +} + +static const TypeInfo xen_igd_passthrough_i440fx_info = { + .name = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE, + .parent = TYPE_I440FX_PCI_DEVICE, + .instance_size = sizeof(PCII440FXState), + .class_init = xen_igd_passthrough_i440fx_class_init, +}; + static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge, PCIBus *rootbus) { @@ -744,6 +771,7 @@ static const TypeInfo i440fx_pcihost_info = { static void i440fx_register_types(void) { type_register_static(&i440fx_info); + type_register_static(&xen_igd_passthrough_i440fx_info); type_register_static(&piix3_info); type_register_static(&piix3_xen_info); type_register_static(&i440fx_pcihost_info); diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 11fb72f..de34aa6 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState; #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost" #define TYPE_I440FX_PCI_DEVICE "i440FX" +#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX" + PCIBus *i440fx_init(const char *host_type, const char *pci_type, PCII440FXState **pi440fx_state, int *piix_devfn,