| Message ID | 20220513175445.89616-4-shentey@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | QOM'ify PIIX southbridge creation | expand |
On 13/05/2022 18:54, Bernhard Beschow wrote: > PCI interrupt wiring and device creation (piix4 only) were performed > in create() functions which are obsolete. Move these tasks into QOM > functions to modernize the code. > > In order to avoid duplicate checking for xen_enabled() the piix3 realize > methods are now split. > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > hw/isa/piix3.c | 67 +++++++++++++++++++++++++++++++++----------------- > hw/isa/piix4.c | 20 +++++++++------ > 2 files changed, 57 insertions(+), 30 deletions(-) > > diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c > index 7d69420967..d15117a7c7 100644 > --- a/hw/isa/piix3.c > +++ b/hw/isa/piix3.c > @@ -24,6 +24,7 @@ > > #include "qemu/osdep.h" > #include "qemu/range.h" > +#include "qapi/error.h" > #include "hw/southbridge/piix.h" > #include "hw/irq.h" > #include "hw/isa/isa.h" > @@ -280,7 +281,7 @@ static const MemoryRegionOps rcr_ops = { > .endianness = DEVICE_LITTLE_ENDIAN > }; > > -static void piix3_realize(PCIDevice *dev, Error **errp) > +static void pci_piix3_realize(PCIDevice *dev, Error **errp) > { > PIIX3State *d = PIIX3_PCI_DEVICE(dev); > > @@ -305,7 +306,6 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data) > dc->desc = "ISA bridge"; > dc->vmsd = &vmstate_piix3; > dc->hotpluggable = false; > - k->realize = piix3_realize; > k->vendor_id = PCI_VENDOR_ID_INTEL; > /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */ > k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0; > @@ -329,11 +329,28 @@ static const TypeInfo piix3_pci_type_info = { > }, > }; > > +static void piix3_realize(PCIDevice *dev, Error **errp) > +{ > + ERRP_GUARD(); > + PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev); > + PCIBus *pci_bus = pci_get_bus(dev); > + > + pci_piix3_realize(dev, errp); > + if (*errp) { > + return; > + } > + > + pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq, > + piix3, PIIX_NUM_PIRQS); > + pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq); > +}; > + Oooof. So the reason this looks a bit odd is because we don't have an equivalent of device_class_set_parent_realize() for PCIDevice realize(). Having had a look in pci.c this looks like a similar approach for handling errors, execpt that here errp is accessed directly. I think this is probably the best we can do for now though. Have you tried forcing pci_piix3_realize() to throw an error during testing to make sure this works as expected? > static void piix3_class_init(ObjectClass *klass, void *data) > { > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > k->config_write = piix3_write_config; > + k->realize = piix3_realize; > } > > static const TypeInfo piix3_info = { > @@ -342,11 +359,33 @@ static const TypeInfo piix3_info = { > .class_init = piix3_class_init, > }; > > +static void piix3_xen_realize(PCIDevice *dev, Error **errp) > +{ > + ERRP_GUARD(); > + PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev); > + PCIBus *pci_bus = pci_get_bus(dev); > + > + pci_piix3_realize(dev, errp); > + if (*errp) { > + return; > + } > + > + /* > + * Xen supports additional interrupt routes from the PCI devices to > + * the IOAPIC: the four pins of each PCI device on the bus are also > + * connected to the IOAPIC directly. > + * These additional routes can be discovered through ACPI. > + */ > + pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq, > + piix3, XEN_PIIX_NUM_PIRQS); > +}; > + > static void piix3_xen_class_init(ObjectClass *klass, void *data) > { > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > k->config_write = piix3_write_config_xen; > + k->realize = piix3_xen_realize; > }; > > static const TypeInfo piix3_xen_info = { > @@ -368,27 +407,11 @@ PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus) > { > PIIX3State *piix3; > PCIDevice *pci_dev; > + const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE > + : TYPE_PIIX3_DEVICE; > > - /* > - * Xen supports additional interrupt routes from the PCI devices to > - * the IOAPIC: the four pins of each PCI device on the bus are also > - * connected to the IOAPIC directly. > - * These additional routes can be discovered through ACPI. > - */ > - if (xen_enabled()) { > - pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, > - TYPE_PIIX3_XEN_DEVICE); > - piix3 = PIIX3_PCI_DEVICE(pci_dev); > - pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq, > - piix3, XEN_PIIX_NUM_PIRQS); > - } else { > - pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, > - TYPE_PIIX3_DEVICE); > - piix3 = PIIX3_PCI_DEVICE(pci_dev); > - pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq, > - piix3, PIIX_NUM_PIRQS); > - pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq); > - } > + pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type); > + piix3 = PIIX3_PCI_DEVICE(pci_dev); > *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0")); > > return piix3; > diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c > index a223b69e24..134d23aea7 100644 > --- a/hw/isa/piix4.c > +++ b/hw/isa/piix4.c > @@ -204,6 +204,8 @@ static const MemoryRegionOps piix4_rcr_ops = { > static void piix4_realize(PCIDevice *dev, Error **errp) > { > PIIX4State *s = PIIX4_PCI_DEVICE(dev); > + PCIDevice *pci; > + PCIBus *pci_bus = pci_get_bus(dev); > ISABus *isa_bus; > qemu_irq *i8259_out_irq; > > @@ -242,6 +244,15 @@ static void piix4_realize(PCIDevice *dev, Error **errp) > return; > } > s->rtc.irq = isa_get_irq(ISA_DEVICE(&s->rtc), s->rtc.isairq); > + > + /* IDE */ > + pci = pci_create_simple(pci_bus, dev->devfn + 1, "piix4-ide"); > + pci_ide_create_devs(pci); > + > + /* USB */ > + pci_create_simple(pci_bus, dev->devfn + 2, "piix4-usb-uhci"); > + > + pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS); > } > > static void piix4_init(Object *obj) > @@ -292,7 +303,6 @@ type_init(piix4_register_types) > > DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus) > { > - PIIX4State *s; > PCIDevice *pci; > DeviceState *dev; > int devfn = PCI_DEVFN(10, 0); > @@ -300,22 +310,16 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus) > pci = pci_create_simple_multifunction(pci_bus, devfn, true, > TYPE_PIIX4_PCI_DEVICE); > dev = DEVICE(pci); > - s = PIIX4_PCI_DEVICE(pci); > + > if (isa_bus) { > *isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0")); > } > > - pci = pci_create_simple(pci_bus, devfn + 1, "piix4-ide"); > - pci_ide_create_devs(pci); > - > - pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci"); > if (smbus) { > *smbus = piix4_pm_init(pci_bus, devfn + 3, 0x1100, > qdev_get_gpio_in_named(dev, "isa", 9), > NULL, 0, NULL); > } > > - pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS); > - > return dev; > } As long as the error handling works as required: Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> ATB, Mark.
On Sat, May 21, 2022 at 10:27 AM Mark Cave-Ayland < mark.cave-ayland@ilande.co.uk> wrote: > On 13/05/2022 18:54, Bernhard Beschow wrote: > > > PCI interrupt wiring and device creation (piix4 only) were performed > > in create() functions which are obsolete. Move these tasks into QOM > > functions to modernize the code. > > > > In order to avoid duplicate checking for xen_enabled() the piix3 realize > > methods are now split. > > > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > > --- > > hw/isa/piix3.c | 67 +++++++++++++++++++++++++++++++++----------------- > > hw/isa/piix4.c | 20 +++++++++------ > > 2 files changed, 57 insertions(+), 30 deletions(-) > > > > diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c > > index 7d69420967..d15117a7c7 100644 > > --- a/hw/isa/piix3.c > > +++ b/hw/isa/piix3.c > > @@ -24,6 +24,7 @@ > > > > #include "qemu/osdep.h" > > #include "qemu/range.h" > > +#include "qapi/error.h" > > #include "hw/southbridge/piix.h" > > #include "hw/irq.h" > > #include "hw/isa/isa.h" > > @@ -280,7 +281,7 @@ static const MemoryRegionOps rcr_ops = { > > .endianness = DEVICE_LITTLE_ENDIAN > > }; > > > > -static void piix3_realize(PCIDevice *dev, Error **errp) > > +static void pci_piix3_realize(PCIDevice *dev, Error **errp) > > { > > PIIX3State *d = PIIX3_PCI_DEVICE(dev); > > > > @@ -305,7 +306,6 @@ static void pci_piix3_class_init(ObjectClass *klass, > void *data) > > dc->desc = "ISA bridge"; > > dc->vmsd = &vmstate_piix3; > > dc->hotpluggable = false; > > - k->realize = piix3_realize; > > k->vendor_id = PCI_VENDOR_ID_INTEL; > > /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */ > > k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0; > > @@ -329,11 +329,28 @@ static const TypeInfo piix3_pci_type_info = { > > }, > > }; > > > > +static void piix3_realize(PCIDevice *dev, Error **errp) > > +{ > > + ERRP_GUARD(); > > + PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev); > > + PCIBus *pci_bus = pci_get_bus(dev); > > + > > + pci_piix3_realize(dev, errp); > > + if (*errp) { > > + return; > > + } > > + > > + pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq, > > + piix3, PIIX_NUM_PIRQS); > > + pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq); > > +}; > > + > > Oooof. So the reason this looks a bit odd is because we don't have an > equivalent of > device_class_set_parent_realize() for PCIDevice realize(). Having had a > look in pci.c > this looks like a similar approach for handling errors, execpt that here > errp is > accessed directly. > Yes, I was surprised by this as well. So I took inspiration from sdhci_common_realize(). > > I think this is probably the best we can do for now though. Have you tried > forcing > pci_piix3_realize() to throw an error during testing to make sure this > works as expected? > I'll post the results in the cover letter of v2. > > > static void piix3_class_init(ObjectClass *klass, void *data) > > { > > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > > > k->config_write = piix3_write_config; > > + k->realize = piix3_realize; > > } > > > > static const TypeInfo piix3_info = { > > @@ -342,11 +359,33 @@ static const TypeInfo piix3_info = { > > .class_init = piix3_class_init, > > }; > > > > +static void piix3_xen_realize(PCIDevice *dev, Error **errp) > > +{ > > + ERRP_GUARD(); > > + PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev); > > + PCIBus *pci_bus = pci_get_bus(dev); > > + > > + pci_piix3_realize(dev, errp); > > + if (*errp) { > > + return; > > + } > > + > > + /* > > + * Xen supports additional interrupt routes from the PCI devices to > > + * the IOAPIC: the four pins of each PCI device on the bus are also > > + * connected to the IOAPIC directly. > > + * These additional routes can be discovered through ACPI. > > + */ > > + pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq, > > + piix3, XEN_PIIX_NUM_PIRQS); > > +}; > > + > > static void piix3_xen_class_init(ObjectClass *klass, void *data) > > { > > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > > > k->config_write = piix3_write_config_xen; > > + k->realize = piix3_xen_realize; > > }; > > > > static const TypeInfo piix3_xen_info = { > > @@ -368,27 +407,11 @@ PIIX3State *piix3_create(PCIBus *pci_bus, ISABus > **isa_bus) > > { > > PIIX3State *piix3; > > PCIDevice *pci_dev; > > + const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE > > + : TYPE_PIIX3_DEVICE; > > > > - /* > > - * Xen supports additional interrupt routes from the PCI devices to > > - * the IOAPIC: the four pins of each PCI device on the bus are also > > - * connected to the IOAPIC directly. > > - * These additional routes can be discovered through ACPI. > > - */ > > - if (xen_enabled()) { > > - pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, > > - > TYPE_PIIX3_XEN_DEVICE); > > - piix3 = PIIX3_PCI_DEVICE(pci_dev); > > - pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq, > > - piix3, XEN_PIIX_NUM_PIRQS); > > - } else { > > - pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, > > - TYPE_PIIX3_DEVICE); > > - piix3 = PIIX3_PCI_DEVICE(pci_dev); > > - pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq, > > - piix3, PIIX_NUM_PIRQS); > > - pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq); > > - } > > + pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type); > > + piix3 = PIIX3_PCI_DEVICE(pci_dev); > > *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0")); > > > > return piix3; > > diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c > > index a223b69e24..134d23aea7 100644 > > --- a/hw/isa/piix4.c > > +++ b/hw/isa/piix4.c > > @@ -204,6 +204,8 @@ static const MemoryRegionOps piix4_rcr_ops = { > > static void piix4_realize(PCIDevice *dev, Error **errp) > > { > > PIIX4State *s = PIIX4_PCI_DEVICE(dev); > > + PCIDevice *pci; > > + PCIBus *pci_bus = pci_get_bus(dev); > > ISABus *isa_bus; > > qemu_irq *i8259_out_irq; > > > > @@ -242,6 +244,15 @@ static void piix4_realize(PCIDevice *dev, Error > **errp) > > return; > > } > > s->rtc.irq = isa_get_irq(ISA_DEVICE(&s->rtc), s->rtc.isairq); > > + > > + /* IDE */ > > + pci = pci_create_simple(pci_bus, dev->devfn + 1, "piix4-ide"); > > + pci_ide_create_devs(pci); > > + > > + /* USB */ > > + pci_create_simple(pci_bus, dev->devfn + 2, "piix4-usb-uhci"); > > + > > + pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, > PIIX_NUM_PIRQS); > > } > > > > static void piix4_init(Object *obj) > > @@ -292,7 +303,6 @@ type_init(piix4_register_types) > > > > DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus > **smbus) > > { > > - PIIX4State *s; > > PCIDevice *pci; > > DeviceState *dev; > > int devfn = PCI_DEVFN(10, 0); > > @@ -300,22 +310,16 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus > **isa_bus, I2CBus **smbus) > > pci = pci_create_simple_multifunction(pci_bus, devfn, true, > > TYPE_PIIX4_PCI_DEVICE); > > dev = DEVICE(pci); > > - s = PIIX4_PCI_DEVICE(pci); > > + > > if (isa_bus) { > > *isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0")); > > } > > > > - pci = pci_create_simple(pci_bus, devfn + 1, "piix4-ide"); > > - pci_ide_create_devs(pci); > > - > > - pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci"); > > if (smbus) { > > *smbus = piix4_pm_init(pci_bus, devfn + 3, 0x1100, > > qdev_get_gpio_in_named(dev, "isa", 9), > > NULL, 0, NULL); > > } > > > > - pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, > PIIX_NUM_PIRQS); > > - > > return dev; > > } > > As long as the error handling works as required: > > Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > > > ATB, > > Mark. >
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c index 7d69420967..d15117a7c7 100644 --- a/hw/isa/piix3.c +++ b/hw/isa/piix3.c @@ -24,6 +24,7 @@ #include "qemu/osdep.h" #include "qemu/range.h" +#include "qapi/error.h" #include "hw/southbridge/piix.h" #include "hw/irq.h" #include "hw/isa/isa.h" @@ -280,7 +281,7 @@ static const MemoryRegionOps rcr_ops = { .endianness = DEVICE_LITTLE_ENDIAN }; -static void piix3_realize(PCIDevice *dev, Error **errp) +static void pci_piix3_realize(PCIDevice *dev, Error **errp) { PIIX3State *d = PIIX3_PCI_DEVICE(dev); @@ -305,7 +306,6 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data) dc->desc = "ISA bridge"; dc->vmsd = &vmstate_piix3; dc->hotpluggable = false; - k->realize = piix3_realize; k->vendor_id = PCI_VENDOR_ID_INTEL; /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */ k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0; @@ -329,11 +329,28 @@ static const TypeInfo piix3_pci_type_info = { }, }; +static void piix3_realize(PCIDevice *dev, Error **errp) +{ + ERRP_GUARD(); + PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev); + PCIBus *pci_bus = pci_get_bus(dev); + + pci_piix3_realize(dev, errp); + if (*errp) { + return; + } + + pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq, + piix3, PIIX_NUM_PIRQS); + pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq); +}; + static void piix3_class_init(ObjectClass *klass, void *data) { PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); k->config_write = piix3_write_config; + k->realize = piix3_realize; } static const TypeInfo piix3_info = { @@ -342,11 +359,33 @@ static const TypeInfo piix3_info = { .class_init = piix3_class_init, }; +static void piix3_xen_realize(PCIDevice *dev, Error **errp) +{ + ERRP_GUARD(); + PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev); + PCIBus *pci_bus = pci_get_bus(dev); + + pci_piix3_realize(dev, errp); + if (*errp) { + return; + } + + /* + * Xen supports additional interrupt routes from the PCI devices to + * the IOAPIC: the four pins of each PCI device on the bus are also + * connected to the IOAPIC directly. + * These additional routes can be discovered through ACPI. + */ + pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq, + piix3, XEN_PIIX_NUM_PIRQS); +}; + static void piix3_xen_class_init(ObjectClass *klass, void *data) { PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); k->config_write = piix3_write_config_xen; + k->realize = piix3_xen_realize; }; static const TypeInfo piix3_xen_info = { @@ -368,27 +407,11 @@ PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus) { PIIX3State *piix3; PCIDevice *pci_dev; + const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE + : TYPE_PIIX3_DEVICE; - /* - * Xen supports additional interrupt routes from the PCI devices to - * the IOAPIC: the four pins of each PCI device on the bus are also - * connected to the IOAPIC directly. - * These additional routes can be discovered through ACPI. - */ - if (xen_enabled()) { - pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, - TYPE_PIIX3_XEN_DEVICE); - piix3 = PIIX3_PCI_DEVICE(pci_dev); - pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq, - piix3, XEN_PIIX_NUM_PIRQS); - } else { - pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, - TYPE_PIIX3_DEVICE); - piix3 = PIIX3_PCI_DEVICE(pci_dev); - pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq, - piix3, PIIX_NUM_PIRQS); - pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq); - } + pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type); + piix3 = PIIX3_PCI_DEVICE(pci_dev); *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0")); return piix3; diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c index a223b69e24..134d23aea7 100644 --- a/hw/isa/piix4.c +++ b/hw/isa/piix4.c @@ -204,6 +204,8 @@ static const MemoryRegionOps piix4_rcr_ops = { static void piix4_realize(PCIDevice *dev, Error **errp) { PIIX4State *s = PIIX4_PCI_DEVICE(dev); + PCIDevice *pci; + PCIBus *pci_bus = pci_get_bus(dev); ISABus *isa_bus; qemu_irq *i8259_out_irq; @@ -242,6 +244,15 @@ static void piix4_realize(PCIDevice *dev, Error **errp) return; } s->rtc.irq = isa_get_irq(ISA_DEVICE(&s->rtc), s->rtc.isairq); + + /* IDE */ + pci = pci_create_simple(pci_bus, dev->devfn + 1, "piix4-ide"); + pci_ide_create_devs(pci); + + /* USB */ + pci_create_simple(pci_bus, dev->devfn + 2, "piix4-usb-uhci"); + + pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS); } static void piix4_init(Object *obj) @@ -292,7 +303,6 @@ type_init(piix4_register_types) DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus) { - PIIX4State *s; PCIDevice *pci; DeviceState *dev; int devfn = PCI_DEVFN(10, 0); @@ -300,22 +310,16 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus) pci = pci_create_simple_multifunction(pci_bus, devfn, true, TYPE_PIIX4_PCI_DEVICE); dev = DEVICE(pci); - s = PIIX4_PCI_DEVICE(pci); + if (isa_bus) { *isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0")); } - pci = pci_create_simple(pci_bus, devfn + 1, "piix4-ide"); - pci_ide_create_devs(pci); - - pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci"); if (smbus) { *smbus = piix4_pm_init(pci_bus, devfn + 3, 0x1100, qdev_get_gpio_in_named(dev, "isa", 9), NULL, 0, NULL); } - pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS); - return dev; }
PCI interrupt wiring and device creation (piix4 only) were performed in create() functions which are obsolete. Move these tasks into QOM functions to modernize the code. In order to avoid duplicate checking for xen_enabled() the piix3 realize methods are now split. Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- hw/isa/piix3.c | 67 +++++++++++++++++++++++++++++++++----------------- hw/isa/piix4.c | 20 +++++++++------ 2 files changed, 57 insertions(+), 30 deletions(-)