| Message ID | 51f88e8c944034037eba3835ced2763880ac4c10.1415091929.git.hutao@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 2014-11-04 at 17:12 +0800, Hu Tao wrote: > So that standard pci host device can share them. > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > hw/pci-host/piix.c | 20 -------------------- > hw/pci-host/q35.c | 7 ------- > hw/pci/pci_host.c | 32 ++++++++++++++++++++++++++++++++ > 3 files changed, 32 insertions(+), 27 deletions(-) > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > index eb92bde..683465c 100644 > --- a/hw/pci-host/piix.c > +++ b/hw/pci-host/piix.c > @@ -256,14 +256,8 @@ static void i440fx_pcihost_get_pci_hole64_end(Object *obj, Visitor *v, > > static void i440fx_pcihost_initfn(Object *obj) > { > - PCIHostState *s = PCI_HOST_BRIDGE(obj); > I440FXState *d = I440FX_PCI_HOST_BRIDGE(obj); > > - memory_region_init_io(&s->conf_mem, obj, &pci_host_conf_le_ops, s, > - "pci-conf-idx", 4); > - memory_region_init_io(&s->data_mem, obj, &pci_host_data_le_ops, s, > - "pci-conf-data", 4); > - > object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "int", > i440fx_pcihost_get_pci_hole_start, > NULL, NULL, NULL, NULL); > @@ -283,18 +277,6 @@ static void i440fx_pcihost_initfn(Object *obj) > d->pci_info.w32.end = IO_APIC_DEFAULT_ADDRESS; > } > > -static void i440fx_pcihost_realize(DeviceState *dev, Error **errp) > -{ > - PCIHostState *s = PCI_HOST_BRIDGE(dev); > - SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > - > - sysbus_add_io(sbd, PC_PCI_CONFIG_ADDR, &s->conf_mem); > - sysbus_init_ioports(sbd, PC_PCI_CONFIG_ADDR, 4); > - > - sysbus_add_io(sbd, PC_PCI_CONFIG_DATA, &s->data_mem); > - sysbus_init_ioports(sbd, PC_PCI_CONFIG_DATA, 4); > -} > - > static int i440fx_initfn(PCIDevice *dev) > { > PCII440FXState *d = I440FX_PCI_DEVICE(dev); > @@ -755,8 +737,6 @@ static void i440fx_pcihost_class_init(ObjectClass *klass, void *data) > PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass); > > hc->root_bus_path = i440fx_pcihost_root_bus_path; > - dc->realize = i440fx_pcihost_realize; > - dc->fw_name = "pci"; > dc->props = i440fx_props; > } > > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > index 9e66835..81eddd7 100644 > --- a/hw/pci-host/q35.c > +++ b/hw/pci-host/q35.c > @@ -138,18 +138,11 @@ static void q35_host_class_init(ObjectClass *klass, void *data) > dc->realize = q35_host_realize; > dc->props = mch_props; > set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > - dc->fw_name = "pci"; > } > > static void q35_host_initfn(Object *obj) > { > Q35PCIHost *s = Q35_HOST_DEVICE(obj); > - PCIHostState *phb = PCI_HOST_BRIDGE(obj); > - > - memory_region_init_io(&phb->conf_mem, obj, &pci_host_conf_le_ops, phb, > - "pci-conf-idx", 4); > - memory_region_init_io(&phb->data_mem, obj, &pci_host_data_le_ops, phb, > - "pci-conf-data", 4); > > object_initialize(&s->mch, sizeof(s->mch), TYPE_MCH_PCI_DEVICE); > object_property_add_child(OBJECT(s), "mch", OBJECT(&s->mch), NULL); > diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c > index f2a69ea..406c747 100644 > --- a/hw/pci/pci_host.c > +++ b/hw/pci/pci_host.c > @@ -176,12 +176,44 @@ const MemoryRegionOps pci_host_data_be_ops = { > .endianness = DEVICE_BIG_ENDIAN, > }; > > +static void pci_host_initfn(Object *obj) > +{ > + PCIHostState *phb = PCI_HOST_BRIDGE(obj); > + > + memory_region_init_io(&phb->conf_mem, obj, &pci_host_conf_le_ops, phb, > + "pci-conf-idx", 4); > + memory_region_init_io(&phb->data_mem, obj, &pci_host_data_le_ops, phb, > + "pci-conf-data", 4); > +} > + > +static void pci_host_realize(DeviceState *dev, Error **errp) > +{ > + PCIHostState *s = PCI_HOST_BRIDGE(dev); > + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > + > + sysbus_add_io(sbd, PC_PCI_CONFIG_ADDR, &s->conf_mem); > + sysbus_init_ioports(sbd, PC_PCI_CONFIG_ADDR, 4); > + > + sysbus_add_io(sbd, PC_PCI_CONFIG_DATA, &s->data_mem); > + sysbus_init_ioports(sbd, PC_PCI_CONFIG_DATA, 4); Hi, If I got this right, now we have this pci_host_realize run for each object of classes deriving from TYPE_PCI_HOST_BRIDGE. Please correct me if I am wrong. The machines PC and Q35 share the same code, however we have other host bridges deriving that I think they behave different: hw/mips/gt64xxx_pci.c:1207: .parent = TYPE_PCI_HOST_BRIDGE, hw/pci-bridge/dec.c:150: .parent = TYPE_PCI_HOST_BRIDGE, hw/pci-host/apb.c:839: .parent = TYPE_PCI_HOST_BRIDGE, hw/pci-host/bonito.c:832: .parent = TYPE_PCI_HOST_BRIDGE, hw/pci-host/grackle.c:156: .parent = TYPE_PCI_HOST_BRIDGE, hw/pci-host/ppce500.c:440: .parent = TYPE_PCI_HOST_BRIDGE, hw/pci-host/prep.c:392: .parent = TYPE_PCI_HOST_BRIDGE, hw/pci-host/uninorth.c:456: .parent = TYPE_PCI_HOST_BRIDGE, hw/pci-host/uninorth.c:470: .parent = TYPE_PCI_HOST_BRIDGE, hw/pci-host/uninorth.c:484: .parent = TYPE_PCI_HOST_BRIDGE, hw/pci-host/uninorth.c:498: .parent = TYPE_PCI_HOST_BRIDGE, hw/pci-host/versatile.c:508: .parent = TYPE_PCI_HOST_BRIDGE, hw/pci/pci_host.c:179: .name = TYPE_PCI_HOST_BRIDGE, hw/ppc/ppc4xx_pci.c:405: .parent = TYPE_PCI_HOST_BRIDGE, hw/ppc/spapr_pci.c:801: .parent = TYPE_PCI_HOST_BRIDGE, hw/sh4/sh_pci.c:193: .parent = TYPE_PCI_HOST_BRIDGE, If I am right, putting this so "high" in the hierarchy may not be the right solution. Thanks, Marcel > +} > + > +static void pci_host_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->realize = pci_host_realize; > + dc->fw_name = "pci"; > +} > + > static const TypeInfo pci_host_type_info = { > .name = TYPE_PCI_HOST_BRIDGE, > .parent = TYPE_SYS_BUS_DEVICE, > .abstract = true, > .class_size = sizeof(PCIHostBridgeClass), > + .class_init = pci_host_class_init, > .instance_size = sizeof(PCIHostState), > + .instance_init = pci_host_initfn, > }; > > static void pci_host_register_types(void)
On Tue, Nov 04, 2014 at 04:21:41PM +0200, Marcel Apfelbaum wrote: > On Tue, 2014-11-04 at 17:12 +0800, Hu Tao wrote: > > So that standard pci host device can share them. > > > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > > --- > > hw/pci-host/piix.c | 20 -------------------- > > hw/pci-host/q35.c | 7 ------- > > hw/pci/pci_host.c | 32 ++++++++++++++++++++++++++++++++ > > 3 files changed, 32 insertions(+), 27 deletions(-) > > > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > > index eb92bde..683465c 100644 > > --- a/hw/pci-host/piix.c > > +++ b/hw/pci-host/piix.c > > @@ -256,14 +256,8 @@ static void i440fx_pcihost_get_pci_hole64_end(Object *obj, Visitor *v, > > > > static void i440fx_pcihost_initfn(Object *obj) > > { > > - PCIHostState *s = PCI_HOST_BRIDGE(obj); > > I440FXState *d = I440FX_PCI_HOST_BRIDGE(obj); > > > > - memory_region_init_io(&s->conf_mem, obj, &pci_host_conf_le_ops, s, > > - "pci-conf-idx", 4); > > - memory_region_init_io(&s->data_mem, obj, &pci_host_data_le_ops, s, > > - "pci-conf-data", 4); > > - > > object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "int", > > i440fx_pcihost_get_pci_hole_start, > > NULL, NULL, NULL, NULL); > > @@ -283,18 +277,6 @@ static void i440fx_pcihost_initfn(Object *obj) > > d->pci_info.w32.end = IO_APIC_DEFAULT_ADDRESS; > > } > > > > -static void i440fx_pcihost_realize(DeviceState *dev, Error **errp) > > -{ > > - PCIHostState *s = PCI_HOST_BRIDGE(dev); > > - SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > > - > > - sysbus_add_io(sbd, PC_PCI_CONFIG_ADDR, &s->conf_mem); > > - sysbus_init_ioports(sbd, PC_PCI_CONFIG_ADDR, 4); > > - > > - sysbus_add_io(sbd, PC_PCI_CONFIG_DATA, &s->data_mem); > > - sysbus_init_ioports(sbd, PC_PCI_CONFIG_DATA, 4); > > -} > > - > > static int i440fx_initfn(PCIDevice *dev) > > { > > PCII440FXState *d = I440FX_PCI_DEVICE(dev); > > @@ -755,8 +737,6 @@ static void i440fx_pcihost_class_init(ObjectClass *klass, void *data) > > PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass); > > > > hc->root_bus_path = i440fx_pcihost_root_bus_path; > > - dc->realize = i440fx_pcihost_realize; > > - dc->fw_name = "pci"; > > dc->props = i440fx_props; > > } > > > > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > > index 9e66835..81eddd7 100644 > > --- a/hw/pci-host/q35.c > > +++ b/hw/pci-host/q35.c > > @@ -138,18 +138,11 @@ static void q35_host_class_init(ObjectClass *klass, void *data) > > dc->realize = q35_host_realize; > > dc->props = mch_props; > > set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > > - dc->fw_name = "pci"; > > } > > > > static void q35_host_initfn(Object *obj) > > { > > Q35PCIHost *s = Q35_HOST_DEVICE(obj); > > - PCIHostState *phb = PCI_HOST_BRIDGE(obj); > > - > > - memory_region_init_io(&phb->conf_mem, obj, &pci_host_conf_le_ops, phb, > > - "pci-conf-idx", 4); > > - memory_region_init_io(&phb->data_mem, obj, &pci_host_data_le_ops, phb, > > - "pci-conf-data", 4); > > > > object_initialize(&s->mch, sizeof(s->mch), TYPE_MCH_PCI_DEVICE); > > object_property_add_child(OBJECT(s), "mch", OBJECT(&s->mch), NULL); > > diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c > > index f2a69ea..406c747 100644 > > --- a/hw/pci/pci_host.c > > +++ b/hw/pci/pci_host.c > > @@ -176,12 +176,44 @@ const MemoryRegionOps pci_host_data_be_ops = { > > .endianness = DEVICE_BIG_ENDIAN, > > }; > > > > +static void pci_host_initfn(Object *obj) > > +{ > > + PCIHostState *phb = PCI_HOST_BRIDGE(obj); > > + > > + memory_region_init_io(&phb->conf_mem, obj, &pci_host_conf_le_ops, phb, > > + "pci-conf-idx", 4); > > + memory_region_init_io(&phb->data_mem, obj, &pci_host_data_le_ops, phb, > > + "pci-conf-data", 4); > > +} > > + > > +static void pci_host_realize(DeviceState *dev, Error **errp) > > +{ > > + PCIHostState *s = PCI_HOST_BRIDGE(dev); > > + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > > + > > + sysbus_add_io(sbd, PC_PCI_CONFIG_ADDR, &s->conf_mem); > > + sysbus_init_ioports(sbd, PC_PCI_CONFIG_ADDR, 4); > > + > > + sysbus_add_io(sbd, PC_PCI_CONFIG_DATA, &s->data_mem); > > + sysbus_init_ioports(sbd, PC_PCI_CONFIG_DATA, 4); > Hi, > > If I got this right, now we have this pci_host_realize run > for each object of classes deriving from TYPE_PCI_HOST_BRIDGE. > Please correct me if I am wrong. You're absolutely right unless the subclass overides the realize function. > The machines PC and Q35 share the same code, however > we have other host bridges deriving that I think they behave different: > > hw/mips/gt64xxx_pci.c:1207: .parent = TYPE_PCI_HOST_BRIDGE, > hw/pci-bridge/dec.c:150: .parent = TYPE_PCI_HOST_BRIDGE, > hw/pci-host/apb.c:839: .parent = TYPE_PCI_HOST_BRIDGE, > hw/pci-host/bonito.c:832: .parent = TYPE_PCI_HOST_BRIDGE, > hw/pci-host/grackle.c:156: .parent = TYPE_PCI_HOST_BRIDGE, > hw/pci-host/ppce500.c:440: .parent = TYPE_PCI_HOST_BRIDGE, > hw/pci-host/prep.c:392: .parent = TYPE_PCI_HOST_BRIDGE, > hw/pci-host/uninorth.c:456: .parent = TYPE_PCI_HOST_BRIDGE, > hw/pci-host/uninorth.c:470: .parent = TYPE_PCI_HOST_BRIDGE, > hw/pci-host/uninorth.c:484: .parent = TYPE_PCI_HOST_BRIDGE, > hw/pci-host/uninorth.c:498: .parent = TYPE_PCI_HOST_BRIDGE, > hw/pci-host/versatile.c:508: .parent = TYPE_PCI_HOST_BRIDGE, > hw/pci/pci_host.c:179: .name = TYPE_PCI_HOST_BRIDGE, > hw/ppc/ppc4xx_pci.c:405: .parent = TYPE_PCI_HOST_BRIDGE, > hw/ppc/spapr_pci.c:801: .parent = TYPE_PCI_HOST_BRIDGE, > hw/sh4/sh_pci.c:193: .parent = TYPE_PCI_HOST_BRIDGE, > > If I am right, putting this so "high" in the hierarchy > may not be the right solution. It is wrong indeed for those devices having different settings. So please just ignore this patch. Thanks for review! > > Thanks, > Marcel > > > +} > > + > > +static void pci_host_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *dc = DEVICE_CLASS(klass); > > + > > + dc->realize = pci_host_realize; > > + dc->fw_name = "pci"; > > +} > > + > > static const TypeInfo pci_host_type_info = { > > .name = TYPE_PCI_HOST_BRIDGE, > > .parent = TYPE_SYS_BUS_DEVICE, > > .abstract = true, > > .class_size = sizeof(PCIHostBridgeClass), > > + .class_init = pci_host_class_init, > > .instance_size = sizeof(PCIHostState), > > + .instance_init = pci_host_initfn, > > }; > > > > static void pci_host_register_types(void) > > >
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index eb92bde..683465c 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -256,14 +256,8 @@ static void i440fx_pcihost_get_pci_hole64_end(Object *obj, Visitor *v, static void i440fx_pcihost_initfn(Object *obj) { - PCIHostState *s = PCI_HOST_BRIDGE(obj); I440FXState *d = I440FX_PCI_HOST_BRIDGE(obj); - memory_region_init_io(&s->conf_mem, obj, &pci_host_conf_le_ops, s, - "pci-conf-idx", 4); - memory_region_init_io(&s->data_mem, obj, &pci_host_data_le_ops, s, - "pci-conf-data", 4); - object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "int", i440fx_pcihost_get_pci_hole_start, NULL, NULL, NULL, NULL); @@ -283,18 +277,6 @@ static void i440fx_pcihost_initfn(Object *obj) d->pci_info.w32.end = IO_APIC_DEFAULT_ADDRESS; } -static void i440fx_pcihost_realize(DeviceState *dev, Error **errp) -{ - PCIHostState *s = PCI_HOST_BRIDGE(dev); - SysBusDevice *sbd = SYS_BUS_DEVICE(dev); - - sysbus_add_io(sbd, PC_PCI_CONFIG_ADDR, &s->conf_mem); - sysbus_init_ioports(sbd, PC_PCI_CONFIG_ADDR, 4); - - sysbus_add_io(sbd, PC_PCI_CONFIG_DATA, &s->data_mem); - sysbus_init_ioports(sbd, PC_PCI_CONFIG_DATA, 4); -} - static int i440fx_initfn(PCIDevice *dev) { PCII440FXState *d = I440FX_PCI_DEVICE(dev); @@ -755,8 +737,6 @@ static void i440fx_pcihost_class_init(ObjectClass *klass, void *data) PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass); hc->root_bus_path = i440fx_pcihost_root_bus_path; - dc->realize = i440fx_pcihost_realize; - dc->fw_name = "pci"; dc->props = i440fx_props; } diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 9e66835..81eddd7 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -138,18 +138,11 @@ static void q35_host_class_init(ObjectClass *klass, void *data) dc->realize = q35_host_realize; dc->props = mch_props; set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); - dc->fw_name = "pci"; } static void q35_host_initfn(Object *obj) { Q35PCIHost *s = Q35_HOST_DEVICE(obj); - PCIHostState *phb = PCI_HOST_BRIDGE(obj); - - memory_region_init_io(&phb->conf_mem, obj, &pci_host_conf_le_ops, phb, - "pci-conf-idx", 4); - memory_region_init_io(&phb->data_mem, obj, &pci_host_data_le_ops, phb, - "pci-conf-data", 4); object_initialize(&s->mch, sizeof(s->mch), TYPE_MCH_PCI_DEVICE); object_property_add_child(OBJECT(s), "mch", OBJECT(&s->mch), NULL); diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c index f2a69ea..406c747 100644 --- a/hw/pci/pci_host.c +++ b/hw/pci/pci_host.c @@ -176,12 +176,44 @@ const MemoryRegionOps pci_host_data_be_ops = { .endianness = DEVICE_BIG_ENDIAN, }; +static void pci_host_initfn(Object *obj) +{ + PCIHostState *phb = PCI_HOST_BRIDGE(obj); + + memory_region_init_io(&phb->conf_mem, obj, &pci_host_conf_le_ops, phb, + "pci-conf-idx", 4); + memory_region_init_io(&phb->data_mem, obj, &pci_host_data_le_ops, phb, + "pci-conf-data", 4); +} + +static void pci_host_realize(DeviceState *dev, Error **errp) +{ + PCIHostState *s = PCI_HOST_BRIDGE(dev); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); + + sysbus_add_io(sbd, PC_PCI_CONFIG_ADDR, &s->conf_mem); + sysbus_init_ioports(sbd, PC_PCI_CONFIG_ADDR, 4); + + sysbus_add_io(sbd, PC_PCI_CONFIG_DATA, &s->data_mem); + sysbus_init_ioports(sbd, PC_PCI_CONFIG_DATA, 4); +} + +static void pci_host_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->realize = pci_host_realize; + dc->fw_name = "pci"; +} + static const TypeInfo pci_host_type_info = { .name = TYPE_PCI_HOST_BRIDGE, .parent = TYPE_SYS_BUS_DEVICE, .abstract = true, .class_size = sizeof(PCIHostBridgeClass), + .class_init = pci_host_class_init, .instance_size = sizeof(PCIHostState), + .instance_init = pci_host_initfn, }; static void pci_host_register_types(void)
So that standard pci host device can share them. Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- hw/pci-host/piix.c | 20 -------------------- hw/pci-host/q35.c | 7 ------- hw/pci/pci_host.c | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 27 deletions(-)