|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [v2][PATCH 2/5] hw:pci-host:piix: split i440fx_init
On Thu, Jul 31, 2014 at 06:12:32PM +0800, 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.
And this works with all legacy drivers and with windows
drivers?
> 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);
This is a hack anyway, how about reverse-decoding
required device id from the supplied card instead of
poking at the host?
You can do it when card is initialized.
>
> 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?
>
> Tiejun
Stick it in the constructor for your xen pt device.
> >
> >----
> >i440fx: make types configurable at run-time
> >
> >Xen wants to supply a different pci and host devices,
> >inheriting i440fx devices. Make types configurable.
> >
> >Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> >
> >-->
> >
> >diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >index be8fdfe..86f295a 100644
> >--- a/include/hw/i386/pc.h
> >+++ b/include/hw/i386/pc.h
> >@@ -230,7 +230,11 @@ extern int no_hpet;
> > struct PCII440FXState;
> > typedef struct PCII440FXState PCII440FXState;
> >
> >-PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
> >+#define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
> >+#define TYPE_I440FX_PCI_DEVICE "i440FX"
> >+
> >+PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >+ PCII440FXState **pi440fx_state, int *piix_devfn,
> > ISABus **isa_bus, qemu_irq *pic,
> > MemoryRegion *address_space_mem,
> > MemoryRegion *address_space_io,
> >diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >index 31125b7..e0979cd 100644
> >--- a/hw/i386/pc_piix.c
> >+++ b/hw/i386/pc_piix.c
> >@@ -194,7 +194,9 @@ static void pc_init1(MachineState *machine,
> > }
> >
> > if (pci_enabled) {
> >- pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
> >+ pci_bus = i440fx_init(TYPE_I440FX_PCI_HOST_BRIDGE,
> >+ TYPE_I440FX_PCI_DEVICE,
> >+ &i440fx_state, &piix3_devfn, &isa_bus, gsi,
> > system_memory, system_io, machine->ram_size,
> > below_4g_mem_size,
> > above_4g_mem_size,
> >diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >index e0e0946..0cd82b8 100644
> >--- a/hw/pci-host/piix.c
> >+++ b/hw/pci-host/piix.c
> >@@ -40,7 +40,6 @@
> > * http://download.intel.com/design/chipsets/datashts/29054901.pdf
> > */
> >
> >-#define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
> > #define I440FX_PCI_HOST_BRIDGE(obj) \
> > OBJECT_CHECK(I440FXState, (obj), TYPE_I440FX_PCI_HOST_BRIDGE)
> >
> >@@ -91,7 +90,6 @@ typedef struct PIIX3State {
> > MemoryRegion rcr_mem;
> > } PIIX3State;
> >
> >-#define TYPE_I440FX_PCI_DEVICE "i440FX"
> > #define I440FX_PCI_DEVICE(obj) \
> > OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> >
> >@@ -305,7 +303,8 @@ static int i440fx_initfn(PCIDevice *dev)
> > return 0;
> > }
> >
> >-PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> >+PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >+ PCII440FXState **pi440fx_state,
> > int *piix3_devfn,
> > ISABus **isa_bus, qemu_irq *pic,
> > MemoryRegion *address_space_mem,
> >@@ -325,7 +324,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> > unsigned i;
> > I440FXState *i440fx;
> >
> >- dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
> >+ dev = qdev_create(NULL, host_type);
> > s = PCI_HOST_BRIDGE(dev);
> > b = pci_bus_new(dev, NULL, pci_address_space,
> > address_space_io, 0, TYPE_PCI_BUS);
> >@@ -333,7 +332,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> > object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev),
> > NULL);
> > qdev_init_nofail(dev);
> >
> >- d = pci_create_simple(b, 0, TYPE_I440FX_PCI_DEVICE);
> >+ d = pci_create_simple(b, 0, pci_type);
> > *pi440fx_state = I440FX_PCI_DEVICE(d);
> > f = *pi440fx_state;
> > f->system_memory = address_space_mem;
> >
> >
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |