Re: [Xen-devel] [Qemu-devel] [v2][PATCH 2/5] hw:pci-host:piix: split i440fx_init

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

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 <tiejun.chen@xxxxxxxxx>
> > > > > > 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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel