From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42523) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XD2ln-0006yz-D8 for qemu-devel@nongnu.org; Thu, 31 Jul 2014 22:40:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XD2lf-0001Ek-UZ for qemu-devel@nongnu.org; Thu, 31 Jul 2014 22:40:35 -0400 Received: from mga11.intel.com ([192.55.52.93]:49897) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XD2lf-0001EJ-OO for qemu-devel@nongnu.org; Thu, 31 Jul 2014 22:40:27 -0400 Message-ID: <53DAFE0C.7020505@intel.com> Date: Fri, 01 Aug 2014 10:40:12 +0800 From: "Chen, Tiejun" MIME-Version: 1.0 References: <1406788299-8394-1-git-send-email-tiejun.chen@intel.com> <1406788299-8394-3-git-send-email-tiejun.chen@intel.com> <20140731091050.GA31192@redhat.com> <53DA0BD1.9080509@intel.com> <20140731095328.GB31192@redhat.com> <53DA1690.6020905@intel.com> <53DA324D.7060000@intel.com> <20140731154724.GC3898@redhat.com> In-Reply-To: <20140731154724.GC3898@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [v2][PATCH 2/5] hw:pci-host:piix: split i440fx_init List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: pbonzini@redhat.com, xen-devel@lists.xensource.com, qemu-devel@nongnu.org, stefano.stabellini@eu.citrix.com On 2014/7/31 23:47, Michael S. Tsirkin wrote: > On Thu, Jul 31, 2014 at 08:10:53PM +0800, Chen, Tiejun wrote: >> On 2014/7/31 18:12, Chen, Tiejun wrote: >>> On 2014/7/31 17:53, Michael S. Tsirkin wrote: >>>> On Thu, Jul 31, 2014 at 05:26:41PM +0800, Chen, Tiejun wrote: >>>>> On 2014/7/31 17:10, Michael S. Tsirkin wrote: >>>>>> On Thu, Jul 31, 2014 at 02:31:36PM +0800, Tiejun Chen wrote: >>>>>>> We'd like to split i440fx_init and then we can share something >>>>>>> with other stuff. >>>>>>> >>>>>>> Signed-off-by: Tiejun Chen >>>>>> >>>>>> I think this is too much work for very little benefit. >>>>>> Just pass const char *type to i440fx_init. >>>>> >>>>> You know we will introduce that faked PCIe device represented that PCH >>>>> later, >>>> >>>> Later when? On top of this patch series? Would like to see it all >>>> before applying this ... >>> >>> I will send this with other IGD stuff after this series is fine to you >>> since its just creating a simple PCIe device. >>> >>> I think you should know this whole story since as you guys discussed we >>> don't fix that PCH at 1f.0. So it may be like this, >>> >>> static int create_pseudo_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice >>> *hdev) >>> { >>> >>> struct PCIDevice *dev; >>> >>> char rid; >>> >>> /* We havt to use a simple PCI device to fake this ISA bridge >>> * to avoid making some confusion to BIOS and ACPI. >>> */ >>> dev = pci_create(bus, -1, "pseudo-intel-pch-isa-bridge"); >>> >>> qdev_init_nofail(&dev->qdev); >>> pci_config_set_vendor_id(dev->config, XEN_SUBSYSTEM_ID)); >>> ^ >>> I don't remember this exactly. >>> pci_config_set_device_id(dev->config, hdev->device_id); >>> >>> return 0; >>> } >>> >>>> >>>>> so how to distinguish them? Are you saying I should check the type >>>>> like this? >>>>> >>>>> if(Xen-Type) >>>>> {} >>>>> else >>>>> {} >>>>> >>>> >>>> No! Put the code in init function for the respective class, >>>> pass type as an argument: >>> >>> If you mean we don't introduce any "if/else", I still don't understand >>> how to insert such that function above, could you show this exactly? >>> >> >> I think I can do this inside that into >> xen_igd_passthrough_pc_machine_pci_bus_init(). There, we can just follow >> pci_bus = i440fx_init() since create_pseudo_pch_isa_bridge() just needs >> pci_bus as a parameter. >> >> Thanks >> Tiejun > > Do it in xen_igd_i440fx_class_init: constructor for your class. > This device is not hot-pluggable right? > If so I would prefer not to make this xen specific. > Instead, find the device and tweak the id when you > initialize the card. Michael, I mean in v3 patches series to do something like, xen_igd_passthrough_pc_machine_pci_bus_init() { if (pci_enabled) { pci_bus = i440fx_init(); create_pseudo_pch_isa_bridge() } Thanks Tiejun