Re: [Qemu-devel] [PATCH for-2.7 v3 23/36] pc: keep gsi reference

[Markus Armbruster]

address@hidden writes:

> From: Marc-André Lureau <address@hidden>
>
> Further cleanup would need to call qemu_free_irq() at the appropriate
> time, but for now this silences ASAN about direct leaks.
>
> Signed-off-by: Marc-André Lureau <address@hidden>

I looked into cleaning up the qemu_irq allocation mess long ago, but
gave up after the work-in-progress had ballooned to a dozen commits or
so.

> ---
>  hw/i386/pc_piix.c    | 17 ++++++++---------
>  hw/i386/pc_q35.c     | 13 ++++++-------
>  include/hw/i386/pc.h |  1 +
>  3 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index a07dc81..2af8888 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -74,7 +74,6 @@ static void pc_init1(MachineState *machine,
>      ISABus *isa_bus;
>      PCII440FXState *i440fx_state;
>      int piix3_devfn = -1;
> -    qemu_irq *gsi;
>      qemu_irq *i8259;
>      qemu_irq smi_irq;
>      GSIState *gsi_state;
> @@ -185,16 +184,16 @@ static void pc_init1(MachineState *machine,
>      gsi_state = g_malloc0(sizeof(*gsi_state));
>      if (kvm_ioapic_in_kernel()) {
>          kvm_pc_setup_irq_routing(pcmc->pci_enabled);
> -        gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
> -                                 GSI_NUM_PINS);
> +        pcms->gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
> +                                       GSI_NUM_PINS);
>      } else {
> -        gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
> +        pcms->gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
>      }
>  
>      if (pcmc->pci_enabled) {
>          pci_bus = i440fx_init(host_type,
>                                pci_type,
> -                              &i440fx_state, &piix3_devfn, &isa_bus, gsi,
> +                              &i440fx_state, &piix3_devfn, &isa_bus, 
> pcms->gsi,
>                                system_memory, system_io, machine->ram_size,
>                                pcms->below_4g_mem_size,
>                                pcms->above_4g_mem_size,
> @@ -207,7 +206,7 @@ static void pc_init1(MachineState *machine,
>                                &error_abort);
>          no_hpet = 1;
>      }
> -    isa_bus_irqs(isa_bus, gsi);
> +    isa_bus_irqs(isa_bus, pcms->gsi);
>  
>      if (kvm_pic_in_kernel()) {
>          i8259 = kvm_i8259_init(isa_bus);
> @@ -225,7 +224,7 @@ static void pc_init1(MachineState *machine,
>          ioapic_init_gsi(gsi_state, "i440fx");
>      }
>  
> -    pc_register_ferr_irq(gsi[13]);
> +    pc_register_ferr_irq(pcms->gsi[13]);
>  
>      pc_vga_init(isa_bus, pcmc->pci_enabled ? pci_bus : NULL);
>  
> @@ -235,7 +234,7 @@ static void pc_init1(MachineState *machine,
>      }
>  
>      /* init basic PC hardware */
> -    pc_basic_device_init(isa_bus, gsi, &rtc_state, true,
> +    pc_basic_device_init(isa_bus, pcms->gsi, &rtc_state, true,
>                           (pcms->vmport != ON_OFF_AUTO_ON), 0x4);
>  
>      pc_nic_init(isa_bus, pci_bus);
> @@ -279,7 +278,7 @@ static void pc_init1(MachineState *machine,
>          smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
>          /* TODO: Populate SPD eeprom data.  */
>          smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
> -                              gsi[9], smi_irq,
> +                              pcms->gsi[9], smi_irq,
>                                pc_machine_is_smm_enabled(pcms),
>                                &piix4_pm);
>          smbus_eeprom_init(smbus, 8, NULL, 0);
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index c5e8367..3cbcbb0 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -69,7 +69,6 @@ static void pc_q35_init(MachineState *machine)
>      MemoryRegion *ram_memory;
>      GSIState *gsi_state;
>      ISABus *isa_bus;
> -    qemu_irq *gsi;
>      qemu_irq *i8259;
>      int i;
>      ICH9LPCState *ich9_lpc;
> @@ -153,10 +152,10 @@ static void pc_q35_init(MachineState *machine)
>      gsi_state = g_malloc0(sizeof(*gsi_state));
>      if (kvm_ioapic_in_kernel()) {
>          kvm_pc_setup_irq_routing(pcmc->pci_enabled);
> -        gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
> -                                 GSI_NUM_PINS);
> +        pcms->gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
> +                                       GSI_NUM_PINS);
>      } else {
> -        gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
> +        pcms->gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
>      }
>  
>      /* create pci host bus */
> @@ -195,7 +194,7 @@ static void pc_q35_init(MachineState *machine)
>      ich9_lpc = ICH9_LPC_DEVICE(lpc);
>      lpc_dev = DEVICE(lpc);
>      for (i = 0; i < GSI_NUM_PINS; i++) {
> -        qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, gsi[i]);
> +        qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, pcms->gsi[i]);
>      }
>      pci_bus_irqs(host_bus, ich9_lpc_set_irq, ich9_lpc_map_irq, ich9_lpc,
>                   ICH9_LPC_NB_PIRQS);
> @@ -219,7 +218,7 @@ static void pc_q35_init(MachineState *machine)
>          ioapic_init_gsi(gsi_state, "q35");
>      }
>  
> -    pc_register_ferr_irq(gsi[13]);
> +    pc_register_ferr_irq(pcms->gsi[13]);
>  
>      assert(pcms->vmport != ON_OFF_AUTO__MAX);
>      if (pcms->vmport == ON_OFF_AUTO_AUTO) {
> @@ -227,7 +226,7 @@ static void pc_q35_init(MachineState *machine)
>      }
>  
>      /* init basic PC hardware */
> -    pc_basic_device_init(isa_bus, gsi, &rtc_state, !mc->no_floppy,
> +    pc_basic_device_init(isa_bus, pcms->gsi, &rtc_state, !mc->no_floppy,
>                           (pcms->vmport != ON_OFF_AUTO_ON), 0xff0104);
>  
>      /* connect pm stuff to lpc */
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index afd025a..e97ccad 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -53,6 +53,7 @@ struct PCMachineState {
>      ISADevice *rtc;
>      PCIBus *bus;
>      FWCfgState *fw_cfg;
> +    qemu_irq *gsi;
>  
>      /* Configuration options: */
>      uint64_t max_ram_below_4g;

It's a start, I guess.

Reviewed-by: Markus Armbruster <address@hidden>