Re: [Qemu-block] [PATCH 10/13] ICH9 LPC: handle PIC and I/O APIC IRQs as qdev GPIO

[Paolo Bonzini]

On 17/06/2016 15:11, Efimov Vasily wrote:
> The ICH9 LPC bridge has 24 output IRQs connected to I/O APIC and 16 output 
> IRQs
> connected through GSI to both PIC and I/O APIC. Currently the IRQs are
> referenced by pointers. The pointers are initialized at startup by direct
> access to the structure fields. This violates Qemu device model.
> 
> The patch makes the IRQs handling to use GPIO model.
> 
> Signed-off-by: Efimov Vasily <address@hidden>
> ---
>  hw/i386/pc_q35.c       | 11 +++++++++--
>  hw/isa/lpc_ich9.c      |  4 ++++
>  include/hw/i386/ich9.h |  4 ++--
>  3 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index d7dc23a..a1fad2b 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -60,6 +60,7 @@ static void pc_q35_init(MachineState *machine)
>      PCIHostState *phb;
>      PCIBus *host_bus;
>      PCIDevice *lpc;
> +    DeviceState *lpc_dev;
>      BusState *idebus[MAX_SATA_PORTS];
>      ISADevice *rtc_state;
>      MemoryRegion *system_io = get_system_io();
> @@ -190,8 +191,10 @@ static void pc_q35_init(MachineState *machine)
>                               PC_MACHINE_ACPI_DEVICE_PROP, &error_abort);
>  
>      ich9_lpc = ICH9_LPC_DEVICE(lpc);
> -    ich9_lpc->pic = gsi;
> -    ich9_lpc->ioapic = gsi_state->ioapic_irq;
> +    lpc_dev = DEVICE(lpc);
> +    for (i = 0; i < ISA_NUM_IRQS; i++) {
> +        qdev_connect_gpio_out(lpc_dev, i, gsi[i]);
> +    }
>      pci_bus_irqs(host_bus, ich9_lpc_set_irq, ich9_lpc_map_irq, ich9_lpc,
>                   ICH9_LPC_NB_PIRQS);
>      pci_bus_set_route_irq_fn(host_bus, ich9_route_intx_pin_to_irq);
> @@ -213,6 +216,10 @@ static void pc_q35_init(MachineState *machine)
>      }
>      if (pcmc->pci_enabled) {
>          ioapic_init_gsi(gsi_state, "q35");
> +        for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> +            qdev_connect_gpio_out(lpc_dev, ISA_NUM_IRQS + i,
> +                                  gsi_state->ioapic_irq[i]);
> +        }
>      }
>  
>      pc_register_ferr_irq(gsi[13]);
> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> index 213741b..1e8e0e4 100644
> --- a/hw/isa/lpc_ich9.c
> +++ b/hw/isa/lpc_ich9.c
> @@ -608,6 +608,7 @@ static void ich9_lpc_initfn(Object *obj)
>  static void ich9_lpc_realize(PCIDevice *d, Error **errp)
>  {
>      ICH9LPCState *lpc = ICH9_LPC_DEVICE(d);
> +    DeviceState *dev = DEVICE(d);
>      ISABus *isa_bus;
>  
>      isa_bus = isa_bus_new(DEVICE(d), get_system_memory(), get_system_io(),
> @@ -635,6 +636,9 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
>      memory_region_add_subregion_overlap(pci_address_space_io(d),
>                                          ICH9_RST_CNT_IOPORT, 
> &lpc->rst_cnt_mem,
>                                          1);
> +
> +    qdev_init_gpio_out(dev, lpc->pic, ISA_NUM_IRQS);
> +    qdev_init_gpio_out(dev, lpc->ioapic, IOAPIC_NUM_PINS);
>  }
>  
>  static bool ich9_rst_cnt_needed(void *opaque)
> diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
> index f1294bc..e800e68 100644
> --- a/include/hw/i386/ich9.h
> +++ b/include/hw/i386/ich9.h
> @@ -68,8 +68,8 @@ typedef struct ICH9LPCState {
>      MemoryRegion rcrb_mem; /* root complex register block */
>      Notifier machine_ready;
>  
> -    qemu_irq *pic;
> -    qemu_irq *ioapic;
> +    qemu_irq pic[ISA_NUM_IRQS];
> +    qemu_irq ioapic[IOAPIC_NUM_PINS];

Please use named GPIO pins, otherwise looks good.

Paolo

>  } ICH9LPCState;
>  
>  Object *ich9_lpc_find(void);
>