Philippe Mathieu-Daudé <phi...@redhat.com> 于2019年10月18日周五 下午9:52写道:
> From: Hervé Poussineau <hpous...@reactos.org> > > Add ISA irqs as piix4 gpio in, and CPU interrupt request as piix4 gpio out. > Remove i8259 instanciated in malta board, to not have it twice. > > We can also remove the now unused piix4_init() function. > > Acked-by: Michael S. Tsirkin <m...@redhat.com> > Acked-by: Paolo Bonzini <pbonz...@redhat.com> > Signed-off-by: Hervé Poussineau <hpous...@reactos.org> > Message-Id: <20171216090228.28505-8-hpous...@reactos.org> > Reviewed-by: Aleksandar Markovic <amarko...@wavecomp.com> > [PMD: rebased, updated includes, use ISA_NUM_IRQS in for loop] > Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> > --- > hw/isa/piix4.c | 43 ++++++++++++++++++++++++++++++++----------- > hw/mips/mips_malta.c | 32 +++++++++++++------------------- > include/hw/i386/pc.h | 1 - > 3 files changed, 45 insertions(+), 31 deletions(-) > > diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c > index d0b18e0586..9c37c85ae2 100644 > --- a/hw/isa/piix4.c > +++ b/hw/isa/piix4.c > @@ -24,6 +24,7 @@ > */ > > #include "qemu/osdep.h" > +#include "hw/irq.h" > #include "hw/i386/pc.h" > #include "hw/pci/pci.h" > #include "hw/isa/isa.h" > @@ -36,6 +37,8 @@ PCIDevice *piix4_dev; > > typedef struct PIIX4State { > PCIDevice dev; > + qemu_irq cpu_intr; > + qemu_irq *isa; > > /* Reset Control Register */ > MemoryRegion rcr_mem; > @@ -94,6 +97,18 @@ static const VMStateDescription vmstate_piix4 = { > } > }; > > +static void piix4_request_i8259_irq(void *opaque, int irq, int level) > +{ > + PIIX4State *s = opaque; > + qemu_set_irq(s->cpu_intr, level); > +} > + > +static void piix4_set_i8259_irq(void *opaque, int irq, int level) > +{ > + PIIX4State *s = opaque; > + qemu_set_irq(s->isa[irq], level); > +} > + > static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val, > unsigned int len) > { > @@ -127,29 +142,35 @@ static const MemoryRegionOps piix4_rcr_ops = { > static void piix4_realize(PCIDevice *dev, Error **errp) > { > PIIX4State *s = PIIX4_PCI_DEVICE(dev); > + ISABus *isa_bus; > + qemu_irq *i8259_out_irq; > > - if (!isa_bus_new(DEVICE(dev), pci_address_space(dev), > - pci_address_space_io(dev), errp)) { > + isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev), > + pci_address_space_io(dev), errp); > + if (!isa_bus) { > return; > } > > + qdev_init_gpio_in_named(DEVICE(dev), piix4_set_i8259_irq, > + "isa", ISA_NUM_IRQS); > + qdev_init_gpio_out_named(DEVICE(dev), &s->cpu_intr, > + "intr", 1); > + > Does the piix4 hardware has the GPIO for interrupt? Seems not. > memory_region_init_io(&s->rcr_mem, OBJECT(dev), &piix4_rcr_ops, s, > "reset-control", 1); > memory_region_add_subregion_overlap(pci_address_space_io(dev), 0xcf9, > &s->rcr_mem, 1); > > + /* initialize i8259 pic */ > + i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1); > + s->isa = i8259_init(isa_bus, *i8259_out_irq); > In i8259_init, we also allocate 16 input line and 1 output line. Seems it is duplicated with the GPIO allocation in previous. Also Maybe here can uses i8259(isa_bus, qemu_allocate_irq(piix4_request_i8259_irq, s, 0)); > + > + /* initialize ISA irqs */ > + isa_bus_irqs(isa_bus, s->isa); > + > piix4_dev = dev; > } > > -int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn) > -{ > - PCIDevice *d; > - > - d = pci_create_simple_multifunction(bus, devfn, true, "PIIX4"); > - *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(d), "isa.0")); > - return d->devfn; > -} > - > static void piix4_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > index 4d9c64b36a..7d25ab6c23 100644 > --- a/hw/mips/mips_malta.c > +++ b/hw/mips/mips_malta.c > @@ -97,7 +97,7 @@ typedef struct { > SysBusDevice parent_obj; > > MIPSCPSState cps; > - qemu_irq *i8259; > + qemu_irq i8259[16]; > } MaltaState; > > static ISADevice *pit; > @@ -1235,8 +1235,8 @@ void mips_malta_init(MachineState *machine) > int64_t kernel_entry, bootloader_run_addr; > PCIBus *pci_bus; > ISABus *isa_bus; > - qemu_irq *isa_irq; > qemu_irq cbus_irq, i8259_irq; > + PCIDevice *pci; > int piix4_devfn; > I2CBus *smbus; > DriveInfo *dinfo; > @@ -1407,30 +1407,24 @@ void mips_malta_init(MachineState *machine) > /* Board ID = 0x420 (Malta Board with CoreLV) */ > stl_p(memory_region_get_ram_ptr(bios_copy) + 0x10, 0x00000420); > > - /* > - * We have a circular dependency problem: pci_bus depends on isa_irq, > - * isa_irq is provided by i8259, i8259 depends on ISA, ISA depends > - * on piix4, and piix4 depends on pci_bus. To stop the cycle we have > - * qemu_irq_proxy() adds an extra bit of indirection, allowing us > - * to resolve the isa_irq -> i8259 dependency after i8259 is > initialized. > - */ > - isa_irq = qemu_irq_proxy(&s->i8259, 16); > - > /* Northbridge */ > - pci_bus = gt64120_register(isa_irq); > + pci_bus = gt64120_register(s->i8259); > > /* Southbridge */ > ide_drive_get(hd, ARRAY_SIZE(hd)); > > - piix4_devfn = piix4_init(pci_bus, &isa_bus, 80); > + pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), > + true, "PIIX4"); > + dev = DEVICE(pci); > + isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0")); > + piix4_devfn = pci->devfn; > > - /* > - * Interrupt controller > - * The 8259 is attached to the MIPS CPU INT0 pin, ie interrupt 2 > - */ > - s->i8259 = i8259_init(isa_bus, i8259_irq); > + /* Interrupt controller */ > + qdev_connect_gpio_out_named(dev, "intr", 0, i8259_irq); > + for (int i = 0; i < ISA_NUM_IRQS; i++) { > + s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i); > + } > > Also here s->i8259 and the piix4 isa point to the same input line. Seems duplicated. I have come up with a more cleaner patch as following: Though 'i8259_init' is called in the mips_malta_init. But is uses the isa bus from piix4 device. And seems it's more clean. You can test it with more tests. Thanks, Li Qiang Author: Li Qiang <liq...@163.com> Date: Mon Oct 21 22:41:17 2019 +0800 piix4 diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c index d0b18e0586..66a041040a 100644 --- a/hw/isa/piix4.c +++ b/hw/isa/piix4.c @@ -24,6 +24,7 @@ */ #include "qemu/osdep.h" +#include "hw/irq.h" #include "hw/i386/pc.h" #include "hw/pci/pci.h" #include "hw/isa/isa.h" @@ -46,6 +47,7 @@ typedef struct PIIX4State { #define PIIX4_PCI_DEVICE(obj) \ OBJECT_CHECK(PIIX4State, (obj), TYPE_PIIX4_PCI_DEVICE) + static void piix4_isa_reset(DeviceState *dev) { PIIX4State *d = PIIX4_PCI_DEVICE(dev); @@ -141,14 +143,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp) piix4_dev = dev; } -int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn) -{ - PCIDevice *d; - - d = pci_create_simple_multifunction(bus, devfn, true, "PIIX4"); - *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(d), "isa.0")); - return d->devfn; -} static void piix4_class_init(ObjectClass *klass, void *data) { diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index 4d9c64b36a..420e0e9e80 100644 --- a/hw/mips/mips_malta.c +++ b/hw/mips/mips_malta.c @@ -28,6 +28,7 @@ #include "cpu.h" #include "hw/i386/pc.h" #include "hw/isa/superio.h" +//#include "hw/isa/piix4.h" #include "hw/dma/i8257.h" #include "hw/char/serial.h" #include "net/net.h" @@ -97,7 +98,7 @@ typedef struct { SysBusDevice parent_obj; MIPSCPSState cps; - qemu_irq *i8259; + qemu_irq i8259[ISA_NUM_IRQS]; } MaltaState; static ISADevice *pit; @@ -1235,8 +1236,9 @@ void mips_malta_init(MachineState *machine) int64_t kernel_entry, bootloader_run_addr; PCIBus *pci_bus; ISABus *isa_bus; - qemu_irq *isa_irq; qemu_irq cbus_irq, i8259_irq; + qemu_irq *i8259; + PCIDevice *pci; int piix4_devfn; I2CBus *smbus; DriveInfo *dinfo; @@ -1407,29 +1409,24 @@ void mips_malta_init(MachineState *machine) /* Board ID = 0x420 (Malta Board with CoreLV) */ stl_p(memory_region_get_ram_ptr(bios_copy) + 0x10, 0x00000420); - /* - * We have a circular dependency problem: pci_bus depends on isa_irq, - * isa_irq is provided by i8259, i8259 depends on ISA, ISA depends - * on piix4, and piix4 depends on pci_bus. To stop the cycle we have - * qemu_irq_proxy() adds an extra bit of indirection, allowing us - * to resolve the isa_irq -> i8259 dependency after i8259 is initialized. - */ - isa_irq = qemu_irq_proxy(&s->i8259, 16); - /* Northbridge */ - pci_bus = gt64120_register(isa_irq); + pci_bus = gt64120_register(s->i8259); /* Southbridge */ ide_drive_get(hd, ARRAY_SIZE(hd)); - piix4_devfn = piix4_init(pci_bus, &isa_bus, 80); + pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), + true, "PIIX4"); + dev = DEVICE(pci); + isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0")); + piix4_devfn = pci->devfn; - /* - * Interrupt controller - * The 8259 is attached to the MIPS CPU INT0 pin, ie interrupt 2 - */ - s->i8259 = i8259_init(isa_bus, i8259_irq); + i8259 = i8259_init(isa_bus, i8259_irq); + for (int i = 0; i < ISA_NUM_IRQS; i++) { + s->i8259[i] = i8259[i]; + } + g_free(i8259); isa_bus_irqs(isa_bus, s->i8259); pci_piix4_ide_init(pci_bus, hd, piix4_devfn + 1); pci_create_simple(pci_bus, piix4_devfn + 2, "piix4-usb-uhci"); > - isa_bus_irqs(isa_bus, s->i8259); > pci_piix4_ide_init(pci_bus, hd, piix4_devfn + 1); > pci_create_simple(pci_bus, piix4_devfn + 2, "piix4-usb-uhci"); > smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100, > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 37bfd95113..374f3e8835 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -286,7 +286,6 @@ PCIBus *i440fx_init(const char *host_type, const char > *pci_type, > PCIBus *find_i440fx(void); > /* piix4.c */ > extern PCIDevice *piix4_dev; > -int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn); > > /* pc_sysfw.c */ > void pc_system_flash_create(PCMachineState *pcms); > -- > 2.21.0 > > >
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel