Re: [Qemu-devel] [PATCH v1 06/13] pc: Move ioapic_init() from pc_piix.c to pc.c

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Oct 29, 2012 at 10:11:26PM -0400, Jason Baron wrote:
> From: Jason Baron <jbaron@redhat.com>
> 
> Move ioapic_init from pc_piix.c to pc.c, to make it a common function.
> Rename ioapic_init -> ioapic_init_gsi.
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jason Baron <jbaron@redhat.com>

I wonder whether this should be reworked using composition too?

> ---
>  hw/pc.c      |   24 ++++++++++++++++++++++++
>  hw/pc.h      |    2 ++
>  hw/pc_piix.c |   25 +------------------------
>  3 files changed, 27 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index cb7fa68..af1a076 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1129,3 +1129,27 @@ void pc_pci_device_init(PCIBus *pci_bus)
>          pci_create_simple(pci_bus, -1, "lsi53c895a");
>      }
>  }
> +
> +void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
> +{
> +    DeviceState *dev;
> +    SysBusDevice *d;
> +    unsigned int i;
> +
> +    if (kvm_irqchip_in_kernel()) {
> +        dev = qdev_create(NULL, "kvm-ioapic");
> +    } else {
> +        dev = qdev_create(NULL, "ioapic");
> +    }
> +    if (parent_name) {
> +        object_property_add_child(object_resolve_path(parent_name, NULL),
> +                                  "ioapic", OBJECT(dev), NULL);
> +    }
> +    qdev_init_nofail(dev);
> +    d = sysbus_from_qdev(dev);
> +    sysbus_mmio_map(d, 0, 0xfec00000);
> +
> +    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> +        gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
> +    }
> +}
> diff --git a/hw/pc.h b/hw/pc.h
> index d6639a6..2237e86 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -104,6 +104,8 @@ void pc_pci_device_init(PCIBus *pci_bus);
>  typedef void (*cpu_set_smm_t)(int smm, void *arg);
>  void cpu_smm_register(cpu_set_smm_t callback, void *arg);
>  
> +void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
> +
>  /* acpi.c */
>  extern int acpi_enabled;
>  extern char *acpi_tables;
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index acb1e92..7bcac87 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -95,29 +95,6 @@ static void kvm_piix3_gsi_handler(void *opaque, int n, int level)
>      }
>  }
>  
> -static void ioapic_init(GSIState *gsi_state)
> -{
> -    DeviceState *dev;
> -    SysBusDevice *d;
> -    unsigned int i;
> -
> -    if (kvm_irqchip_in_kernel()) {
> -        dev = qdev_create(NULL, "kvm-ioapic");
> -    } else {
> -        dev = qdev_create(NULL, "ioapic");
> -    }
> -    /* FIXME: this should be under the piix3.  */
> -    object_property_add_child(object_resolve_path("i440fx", NULL),
> -                              "ioapic", OBJECT(dev), NULL);
> -    qdev_init_nofail(dev);
> -    d = sysbus_from_qdev(dev);
> -    sysbus_mmio_map(d, 0, 0xfec00000);
> -
> -    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> -        gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
> -    }
> -}
> -
>  /* PC hardware initialisation */
>  static void pc_init1(MemoryRegion *system_memory,
>                       MemoryRegion *system_io,
> @@ -221,7 +198,7 @@ static void pc_init1(MemoryRegion *system_memory,
>          gsi_state->i8259_irq[i] = i8259[i];
>      }
>      if (pci_enabled) {
> -        ioapic_init(gsi_state);
> +        ioapic_init_gsi(gsi_state, "i440fx");
>      }
>  
>      pc_register_ferr_irq(gsi[13]);
> -- 
> 1.7.1