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
>
>
>
