[Qemu-devel] [PATCH qom v2 0/4] QOMify IRQs

[Qemu-devel] [PATCH qom v2 0/4] QOMify IRQs

[Peter Crosthwaite]

Hi Andreas and all,

I have done some cleanup of your WIP IRQ QOMification and have it in a
hopefully ready state. Its now link safe and the allocation/freeing
process is not as complex as before.

For fuller context of the motivation behind this series, please see:
http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03265.html

changed since v1:
Fixed sh4 instance of "[0]" bug (Kirill review)

Regards,
Peter


Andreas Färber (3):
  sdhci: Fix misuse of qemu_free_irqs()
  hw: Fix qemu_allocate_irqs() leaks
  irq: Slim conversion of qemu_irq to QOM

Peter Crosthwaite (1):
  irq: Allocate IRQs individually

 hw/arm/omap1.c          | 14 +++++++-------
 hw/arm/omap2.c          |  2 +-
 hw/arm/pxa2xx.c         |  4 ++--
 hw/arm/spitz.c          |  4 ++--
 hw/arm/z2.c             |  2 +-
 hw/char/serial-pci.c    |  2 +-
 hw/core/irq.c           | 46 +++++++++++++++++++++++++++++-----------------
 hw/core/qdev.c          |  2 +-
 hw/dma/omap_dma.c       |  4 ++--
 hw/ide/microdrive.c     |  2 +-
 hw/ipack/ipack.c        |  2 +-
 hw/misc/cbus.c          |  6 +++---
 hw/pcmcia/pxa2xx.c      |  2 +-
 hw/sd/omap_mmc.c        |  2 +-
 hw/sd/sdhci.c           |  8 ++++----
 hw/sh4/sh7750.c         |  3 +--
 hw/timer/omap_gptimer.c |  4 ++--
 include/hw/irq.h        |  4 +++-
 18 files changed, 63 insertions(+), 50 deletions(-)

-- 
2.0.0

[Qemu-devel] [PATCH qom v2 1/4] sdhci: Fix misuse of qemu_free_irqs()

[Peter Crosthwaite]

From: Andreas Färber <afaerber@suse.de>

It does a g_free() on the pointer.

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/sd/sdhci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index e2951e6..3e13d70 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1184,8 +1184,8 @@ static void sdhci_uninitfn(Object *obj)
     timer_free(s->insert_timer);
     timer_del(s->transfer_timer);
     timer_free(s->transfer_timer);
-    qemu_free_irqs(&s->eject_cb);
-    qemu_free_irqs(&s->ro_cb);
+    qemu_free_irq(s->eject_cb);
+    qemu_free_irq(s->ro_cb);
 
     if (s->fifo_buffer) {
         g_free(s->fifo_buffer);
-- 
2.0.0

Re: [Qemu-devel] [PATCH qom v2 1/4] sdhci: Fix misuse of qemu_free_irqs()

[Andreas Färber]

Am 18.06.2014 09:54, schrieb Peter Crosthwaite:
> From: Andreas Färber <afaerber@suse.de>
> 
> It does a g_free() on the pointer.
> 
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Thanks for picking this up and reviewing, applied to qom-next with
extended commit message:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH qom v2 2/4] hw: Fix qemu_allocate_irqs() leaks

[Peter Crosthwaite]

From: Andreas Färber <afaerber@suse.de>

Replace qemu_allocate_irqs(foo, bar, 1)[0]
with qemu_allocate_irq(foo, bar, 0).

This avoids leaking the dereferenced qemu_irq *.

Cc: Kirill Batuzov <batuzovk@ispras.ru>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Andreas Färber <afaerber@suse.de>
[PC Changes:
 * Applied change to instance in sh4/sh7750.c
]
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
Changed since 1:
Applied change to instance in sh4/sh7750.c (Kirill review)

 hw/arm/omap1.c          | 14 +++++++-------
 hw/arm/omap2.c          |  2 +-
 hw/arm/pxa2xx.c         |  4 ++--
 hw/arm/spitz.c          |  4 ++--
 hw/arm/z2.c             |  2 +-
 hw/core/irq.c           |  4 ++--
 hw/dma/omap_dma.c       |  4 ++--
 hw/ide/microdrive.c     |  2 +-
 hw/misc/cbus.c          |  6 +++---
 hw/pcmcia/pxa2xx.c      |  2 +-
 hw/sd/omap_mmc.c        |  2 +-
 hw/sd/sdhci.c           |  4 ++--
 hw/sh4/sh7750.c         |  3 +--
 hw/timer/omap_gptimer.c |  4 ++--
 14 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
index b28e052..e7cc5d7 100644
--- a/hw/arm/omap1.c
+++ b/hw/arm/omap1.c
@@ -172,7 +172,7 @@ static void omap_timer_clk_update(void *opaque, int line, int on)
 static void omap_timer_clk_setup(struct omap_mpu_timer_s *timer)
 {
     omap_clk_adduser(timer->clk,
-                    qemu_allocate_irqs(omap_timer_clk_update, timer, 1)[0]);
+                    qemu_allocate_irq(omap_timer_clk_update, timer, 0));
     timer->rate = omap_clk_getrate(timer->clk);
 }
 
@@ -2098,7 +2098,7 @@ static struct omap_mpuio_s *omap_mpuio_init(MemoryRegion *memory,
                           "omap-mpuio", 0x800);
     memory_region_add_subregion(memory, base, &s->iomem);
 
-    omap_clk_adduser(clk, qemu_allocate_irqs(omap_mpuio_onoff, s, 1)[0]);
+    omap_clk_adduser(clk, qemu_allocate_irq(omap_mpuio_onoff, s, 0));
 
     return s;
 }
@@ -2401,7 +2401,7 @@ static struct omap_pwl_s *omap_pwl_init(MemoryRegion *system_memory,
                           "omap-pwl", 0x800);
     memory_region_add_subregion(system_memory, base, &s->iomem);
 
-    omap_clk_adduser(clk, qemu_allocate_irqs(omap_pwl_clk_update, s, 1)[0]);
+    omap_clk_adduser(clk, qemu_allocate_irq(omap_pwl_clk_update, s, 0));
     return s;
 }
 
@@ -3485,8 +3485,8 @@ static void omap_mcbsp_i2s_start(void *opaque, int line, int level)
 void omap_mcbsp_i2s_attach(struct omap_mcbsp_s *s, I2SCodec *slave)
 {
     s->codec = slave;
-    slave->rx_swallow = qemu_allocate_irqs(omap_mcbsp_i2s_swallow, s, 1)[0];
-    slave->tx_start = qemu_allocate_irqs(omap_mcbsp_i2s_start, s, 1)[0];
+    slave->rx_swallow = qemu_allocate_irq(omap_mcbsp_i2s_swallow, s, 0);
+    slave->tx_start = qemu_allocate_irq(omap_mcbsp_i2s_start, s, 0);
 }
 
 /* LED Pulse Generators */
@@ -3634,7 +3634,7 @@ static struct omap_lpg_s *omap_lpg_init(MemoryRegion *system_memory,
     memory_region_init_io(&s->iomem, NULL, &omap_lpg_ops, s, "omap-lpg", 0x800);
     memory_region_add_subregion(system_memory, base, &s->iomem);
 
-    omap_clk_adduser(clk, qemu_allocate_irqs(omap_lpg_clk_update, s, 1)[0]);
+    omap_clk_adduser(clk, qemu_allocate_irq(omap_lpg_clk_update, s, 0));
 
     return s;
 }
@@ -3848,7 +3848,7 @@ struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *system_memory,
     s->sdram_size = sdram_size;
     s->sram_size = OMAP15XX_SRAM_SIZE;
 
-    s->wakeup = qemu_allocate_irqs(omap_mpu_wakeup, s, 1)[0];
+    s->wakeup = qemu_allocate_irq(omap_mpu_wakeup, s, 0);
 
     /* Clocks */
     omap_clk_init(s);
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index 36efde0..dc53a7a 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -2260,7 +2260,7 @@ struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion *sysmem,
     s->sdram_size = sdram_size;
     s->sram_size = OMAP242X_SRAM_SIZE;
 
-    s->wakeup = qemu_allocate_irqs(omap_mpu_wakeup, s, 1)[0];
+    s->wakeup = qemu_allocate_irq(omap_mpu_wakeup, s, 0);
 
     /* Clocks */
     omap_clk_init(s);
diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index 2d28a11..557e0f1 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -2052,7 +2052,7 @@ PXA2xxState *pxa270_init(MemoryRegion *address_space,
         fprintf(stderr, "Unable to find CPU definition\n");
         exit(1);
     }
-    s->reset = qemu_allocate_irqs(pxa2xx_reset, s, 1)[0];
+    s->reset = qemu_allocate_irq(pxa2xx_reset, s, 0);
 
     /* SDRAM & Internal Memory Storage */
     memory_region_init_ram(&s->sdram, NULL, "pxa270.sdram", sdram_size);
@@ -2183,7 +2183,7 @@ PXA2xxState *pxa255_init(MemoryRegion *address_space, unsigned int sdram_size)
         fprintf(stderr, "Unable to find CPU definition\n");
         exit(1);
     }
-    s->reset = qemu_allocate_irqs(pxa2xx_reset, s, 1)[0];
+    s->reset = qemu_allocate_irq(pxa2xx_reset, s, 0);
 
     /* SDRAM & Internal Memory Storage */
     memory_region_init_ram(&s->sdram, NULL, "pxa255.sdram", sdram_size);
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 5455dbf..e20d5b8 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -744,7 +744,7 @@ static void spitz_i2c_setup(PXA2xxState *cpu)
 
     spitz_wm8750_addr(wm, 0, 0);
     qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_WM,
-                    qemu_allocate_irqs(spitz_wm8750_addr, wm, 1)[0]);
+                          qemu_allocate_irq(spitz_wm8750_addr, wm, 0));
     /* .. and to the sound interface.  */
     cpu->i2s->opaque = wm;
     cpu->i2s->codec_out = wm8750_dac_dat;
@@ -850,7 +850,7 @@ static void spitz_gpio_setup(PXA2xxState *cpu, int slots)
      * wouldn't guarantee that a guest ever exits the loop.
      */
     spitz_hsync = 0;
-    lcd_hsync = qemu_allocate_irqs(spitz_lcd_hsync_handler, cpu, 1)[0];
+    lcd_hsync = qemu_allocate_irq(spitz_lcd_hsync_handler, cpu, 0);
     pxa2xx_gpio_read_notifier(cpu->gpio, lcd_hsync);
     pxa2xx_lcd_vsync_notifier(cpu->lcd, lcd_hsync);
 
diff --git a/hw/arm/z2.c b/hw/arm/z2.c
index ab9e4c9..36b3b50 100644
--- a/hw/arm/z2.c
+++ b/hw/arm/z2.c
@@ -363,7 +363,7 @@ static void z2_init(MachineState *machine)
     wm8750_data_req_set(wm, mpu->i2s->data_req, mpu->i2s);
 
     qdev_connect_gpio_out(mpu->gpio, Z2_GPIO_LCD_CS,
-        qemu_allocate_irqs(z2_lcd_cs, z2_lcd, 1)[0]);
+                          qemu_allocate_irq(z2_lcd_cs, z2_lcd, 0));
 
     z2_binfo.kernel_filename = kernel_filename;
     z2_binfo.kernel_cmdline = kernel_cmdline;
diff --git a/hw/core/irq.c b/hw/core/irq.c
index 03c8cb3..3d284c6 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -102,7 +102,7 @@ qemu_irq qemu_irq_invert(qemu_irq irq)
 {
     /* The default state for IRQs is low, so raise the output now.  */
     qemu_irq_raise(irq);
-    return qemu_allocate_irqs(qemu_notirq, irq, 1)[0];
+    return qemu_allocate_irq(qemu_notirq, irq, 0);
 }
 
 static void qemu_splitirq(void *opaque, int line, int level)
@@ -117,7 +117,7 @@ qemu_irq qemu_irq_split(qemu_irq irq1, qemu_irq irq2)
     qemu_irq *s = g_malloc0(2 * sizeof(qemu_irq));
     s[0] = irq1;
     s[1] = irq2;
-    return qemu_allocate_irqs(qemu_splitirq, s, 1)[0];
+    return qemu_allocate_irq(qemu_splitirq, s, 0);
 }
 
 static void proxy_irq_handler(void *opaque, int n, int level)
diff --git a/hw/dma/omap_dma.c b/hw/dma/omap_dma.c
index 0f35c42..756a87a 100644
--- a/hw/dma/omap_dma.c
+++ b/hw/dma/omap_dma.c
@@ -1660,7 +1660,7 @@ struct soc_dma_s *omap_dma_init(hwaddr base, qemu_irq *irqs,
     }
 
     omap_dma_setcaps(s);
-    omap_clk_adduser(s->clk, qemu_allocate_irqs(omap_dma_clk_update, s, 1)[0]);
+    omap_clk_adduser(s->clk, qemu_allocate_irq(omap_dma_clk_update, s, 0));
     omap_dma_reset(s->dma);
     omap_dma_clk_update(s, 0, 1);
 
@@ -2082,7 +2082,7 @@ struct soc_dma_s *omap_dma4_init(hwaddr base, qemu_irq *irqs,
     s->intr_update = omap_dma_interrupts_4_update;
 
     omap_dma_setcaps(s);
-    omap_clk_adduser(s->clk, qemu_allocate_irqs(omap_dma_clk_update, s, 1)[0]);
+    omap_clk_adduser(s->clk, qemu_allocate_irq(omap_dma_clk_update, s, 0));
     omap_dma_reset(s->dma);
     omap_dma_clk_update(s, 0, !!s->dma->freq);
 
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index f24946d..2d70ddb 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -593,7 +593,7 @@ static void microdrive_realize(DeviceState *dev, Error **errp)
 {
     MicroDriveState *md = MICRODRIVE(dev);
 
-    ide_init2(&md->bus, qemu_allocate_irqs(md_set_irq, md, 1)[0]);
+    ide_init2(&md->bus, qemu_allocate_irq(md_set_irq, md, 0));
 }
 
 static void microdrive_init(Object *obj)
diff --git a/hw/misc/cbus.c b/hw/misc/cbus.c
index 29b467b..495d507 100644
--- a/hw/misc/cbus.c
+++ b/hw/misc/cbus.c
@@ -135,9 +135,9 @@ CBus *cbus_init(qemu_irq dat)
     CBusPriv *s = (CBusPriv *) g_malloc0(sizeof(*s));
 
     s->dat_out = dat;
-    s->cbus.clk = qemu_allocate_irqs(cbus_clk, s, 1)[0];
-    s->cbus.dat = qemu_allocate_irqs(cbus_dat, s, 1)[0];
-    s->cbus.sel = qemu_allocate_irqs(cbus_sel, s, 1)[0];
+    s->cbus.clk = qemu_allocate_irq(cbus_clk, s, 0);
+    s->cbus.dat = qemu_allocate_irq(cbus_dat, s, 0);
+    s->cbus.sel = qemu_allocate_irq(cbus_sel, s, 0);
 
     s->sel = 1;
     s->clk = 0;
diff --git a/hw/pcmcia/pxa2xx.c b/hw/pcmcia/pxa2xx.c
index 96f3774..55e8a2a 100644
--- a/hw/pcmcia/pxa2xx.c
+++ b/hw/pcmcia/pxa2xx.c
@@ -195,7 +195,7 @@ static void pxa2xx_pcmcia_initfn(Object *obj)
     memory_region_add_subregion(&s->container_mem, 0x0c000000,
                                 &s->common_iomem);
 
-    s->slot.irq = qemu_allocate_irqs(pxa2xx_pcmcia_set_irq, s, 1)[0];
+    s->slot.irq = qemu_allocate_irq(pxa2xx_pcmcia_set_irq, s, 0);
 
     object_property_add_link(obj, "card", TYPE_PCMCIA_CARD,
                              (Object **)&s->card,
diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index 937a478..6c92149 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -625,7 +625,7 @@ struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta,
         exit(1);
     }
 
-    s->cdet = qemu_allocate_irqs(omap_mmc_cover_cb, s, 1)[0];
+    s->cdet = qemu_allocate_irq(omap_mmc_cover_cb, s, 0);
     sd_set_cb(s->card, NULL, s->cdet);
 
     return s;
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 3e13d70..b5a9eee 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1168,8 +1168,8 @@ static void sdhci_initfn(Object *obj)
     if (s->card == NULL) {
         exit(1);
     }
-    s->eject_cb = qemu_allocate_irqs(sdhci_insert_eject_cb, s, 1)[0];
-    s->ro_cb = qemu_allocate_irqs(sdhci_card_readonly_cb, s, 1)[0];
+    s->eject_cb = qemu_allocate_irq(sdhci_insert_eject_cb, s, 0);
+    s->ro_cb = qemu_allocate_irq(sdhci_card_readonly_cb, s, 0);
     sd_set_cb(s->card, s->ro_cb, s->eject_cb);
 
     s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_raise_insertion_irq, s);
diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
index 4a39357..9ccd770 100644
--- a/hw/sh4/sh7750.c
+++ b/hw/sh4/sh7750.c
@@ -838,6 +838,5 @@ SH7750State *sh7750_init(SuperHCPU *cpu, MemoryRegion *sysmem)
 qemu_irq sh7750_irl(SH7750State *s)
 {
     sh_intc_toggle_source(sh_intc_source(&s->intc, IRL), 1, 0); /* enable */
-    return qemu_allocate_irqs(sh_intc_set_irl, sh_intc_source(&s->intc, IRL),
-                               1)[0];
+    return qemu_allocate_irq(sh_intc_set_irl, sh_intc_source(&s->intc, IRL), 1);
 }
diff --git a/hw/timer/omap_gptimer.c b/hw/timer/omap_gptimer.c
index 016207f..b7f3d49 100644
--- a/hw/timer/omap_gptimer.c
+++ b/hw/timer/omap_gptimer.c
@@ -227,7 +227,7 @@ static void omap_gp_timer_clk_update(void *opaque, int line, int on)
 static void omap_gp_timer_clk_setup(struct omap_gp_timer_s *timer)
 {
     omap_clk_adduser(timer->clk,
-                    qemu_allocate_irqs(omap_gp_timer_clk_update, timer, 1)[0]);
+                     qemu_allocate_irq(omap_gp_timer_clk_update, timer, 0));
     timer->rate = omap_clk_getrate(timer->clk);
 }
 
@@ -476,7 +476,7 @@ struct omap_gp_timer_s *omap_gp_timer_init(struct omap_target_agent_s *ta,
     s->clk = fclk;
     s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, omap_gp_timer_tick, s);
     s->match = timer_new_ns(QEMU_CLOCK_VIRTUAL, omap_gp_timer_match, s);
-    s->in = qemu_allocate_irqs(omap_gp_timer_input, s, 1)[0];
+    s->in = qemu_allocate_irq(omap_gp_timer_input, s, 0);
     omap_gp_timer_reset(s);
     omap_gp_timer_clk_setup(s);
 
-- 
2.0.0

Re: [Qemu-devel] [PATCH qom v2 2/4] hw: Fix qemu_allocate_irqs() leaks

[Kirill Batuzov]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1441 bytes --]

On Wed, 18 Jun 2014, Peter Crosthwaite wrote:

> From: Andreas Färber <afaerber@suse.de>
> 
> Replace qemu_allocate_irqs(foo, bar, 1)[0]
> with qemu_allocate_irq(foo, bar, 0).
> 
> This avoids leaking the dereferenced qemu_irq *.
> 
> Cc: Kirill Batuzov <batuzovk@ispras.ru>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> [PC Changes:
>  * Applied change to instance in sh4/sh7750.c
> ]
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> Changed since 1:
> Applied change to instance in sh4/sh7750.c (Kirill review)
> 
>  hw/arm/omap1.c          | 14 +++++++-------
>  hw/arm/omap2.c          |  2 +-
>  hw/arm/pxa2xx.c         |  4 ++--
>  hw/arm/spitz.c          |  4 ++--
>  hw/arm/z2.c             |  2 +-
>  hw/core/irq.c           |  4 ++--
>  hw/dma/omap_dma.c       |  4 ++--
>  hw/ide/microdrive.c     |  2 +-
>  hw/misc/cbus.c          |  6 +++---
>  hw/pcmcia/pxa2xx.c      |  2 +-
>  hw/sd/omap_mmc.c        |  2 +-
>  hw/sd/sdhci.c           |  4 ++--
>  hw/sh4/sh7750.c         |  3 +--
>  hw/timer/omap_gptimer.c |  4 ++--
>  14 files changed, 28 insertions(+), 29 deletions(-)
>

Reviewed-by: Kirill Batuzov <batuzovk@ispras.ru>

-- 
Kirill

Re: [Qemu-devel] [PATCH qom v2 2/4] hw: Fix qemu_allocate_irqs() leaks

[Andreas Färber]

Am 18.06.2014 09:55, schrieb Peter Crosthwaite:
> From: Andreas Färber <afaerber@suse.de>
> 
> Replace qemu_allocate_irqs(foo, bar, 1)[0]
> with qemu_allocate_irq(foo, bar, 0).
> 
> This avoids leaking the dereferenced qemu_irq *.
> 
> Cc: Kirill Batuzov <batuzovk@ispras.ru>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> [PC Changes:
>  * Applied change to instance in sh4/sh7750.c
> ]
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> Changed since 1:
> Applied change to instance in sh4/sh7750.c (Kirill review)
[...]
> diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
> index 4a39357..9ccd770 100644
> --- a/hw/sh4/sh7750.c
> +++ b/hw/sh4/sh7750.c
> @@ -838,6 +838,5 @@ SH7750State *sh7750_init(SuperHCPU *cpu, MemoryRegion *sysmem)
>  qemu_irq sh7750_irl(SH7750State *s)
>  {
>      sh_intc_toggle_source(sh_intc_source(&s->intc, IRL), 1, 0); /* enable */
> -    return qemu_allocate_irqs(sh_intc_set_irl, sh_intc_source(&s->intc, IRL),
> -                               1)[0];
> +    return qemu_allocate_irq(sh_intc_set_irl, sh_intc_source(&s->intc, IRL), 1);

Thanks for catching this, my grep expression failed due to the line
break. But shouldn't this be 0 due to the zero-based index, as per my
commit message? Will fix up unless I hear objections.

Regards,
Andreas

>  }

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH qom v2 2/4] hw: Fix qemu_allocate_irqs() leaks

[Peter Crosthwaite]

On Fri, Jun 27, 2014 at 7:45 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 18.06.2014 09:55, schrieb Peter Crosthwaite:
>> From: Andreas Färber <afaerber@suse.de>
>>
>> Replace qemu_allocate_irqs(foo, bar, 1)[0]
>> with qemu_allocate_irq(foo, bar, 0).
>>
>> This avoids leaking the dereferenced qemu_irq *.
>>
>> Cc: Kirill Batuzov <batuzovk@ispras.ru>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> [PC Changes:
>>  * Applied change to instance in sh4/sh7750.c
>> ]
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>> Changed since 1:
>> Applied change to instance in sh4/sh7750.c (Kirill review)
> [...]
>> diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
>> index 4a39357..9ccd770 100644
>> --- a/hw/sh4/sh7750.c
>> +++ b/hw/sh4/sh7750.c
>> @@ -838,6 +838,5 @@ SH7750State *sh7750_init(SuperHCPU *cpu, MemoryRegion *sysmem)
>>  qemu_irq sh7750_irl(SH7750State *s)
>>  {
>>      sh_intc_toggle_source(sh_intc_source(&s->intc, IRL), 1, 0); /* enable */
>> -    return qemu_allocate_irqs(sh_intc_set_irl, sh_intc_source(&s->intc, IRL),
>> -                               1)[0];
>> +    return qemu_allocate_irq(sh_intc_set_irl, sh_intc_source(&s->intc, IRL), 1);
>
> Thanks for catching this, my grep expression failed due to the line
> break. But shouldn't this be 0 due to the zero-based index, as per my
> commit message? Will fix up unless I hear objections.
>

Yep, sorry.

Regards,
Peter

> Regards,
> Andreas
>
>>  }
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>

[Qemu-devel] [PATCH qom v2 3/4] irq: Allocate IRQs individually

[Peter Crosthwaite]

Allocate each IRQ individually on array allocations. This prepares for
QOMification of IRQs, where pointers to individual IRQs may be taken
and handed around for usage as QOM Links. The g_renew scheme used here
is too fragile and would break all existing links should an IRQ list
be extended.

We now have to pass the IRQ count to qemu_free_irqs(). We have so few
call sites however, so this change is reasonably trivial.

Cc: agarcia@igalia.com
Cc: mst@redhat.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Acked-by: Alberto Garcia <agarcia@igalia.com>
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/char/serial-pci.c |  2 +-
 hw/core/irq.c        | 20 +++++++-------------
 hw/core/qdev.c       |  2 +-
 hw/ipack/ipack.c     |  2 +-
 include/hw/irq.h     |  2 +-
 5 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
index 6c25296..f53bb9c 100644
--- a/hw/char/serial-pci.c
+++ b/hw/char/serial-pci.c
@@ -152,7 +152,7 @@ static void multi_serial_pci_exit(PCIDevice *dev)
         g_free(pci->name[i]);
     }
     memory_region_destroy(&pci->iobar);
-    qemu_free_irqs(pci->irqs);
+    qemu_free_irqs(pci->irqs, pci->ports);
 }
 
 static const VMStateDescription vmstate_pci_serial = {
diff --git a/hw/core/irq.c b/hw/core/irq.c
index 3d284c6..bc982a7 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -42,23 +42,14 @@ qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
                            void *opaque, int n)
 {
     qemu_irq *s;
-    struct IRQState *p;
     int i;
 
     if (!old) {
         n_old = 0;
     }
     s = old ? g_renew(qemu_irq, old, n + n_old) : g_new(qemu_irq, n);
-    p = old ? g_renew(struct IRQState, s[0], n + n_old) :
-                g_new(struct IRQState, n);
-    for (i = 0; i < n + n_old; i++) {
-        if (i >= n_old) {
-            p->handler = handler;
-            p->opaque = opaque;
-            p->n = i;
-        }
-        s[i] = p;
-        p++;
+    for (i = n_old; i < n + n_old; i++) {
+        s[i] = qemu_allocate_irq(handler, opaque, i);
     }
     return s;
 }
@@ -80,9 +71,12 @@ qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n)
     return irq;
 }
 
-void qemu_free_irqs(qemu_irq *s)
+void qemu_free_irqs(qemu_irq *s, int n)
 {
-    g_free(s[0]);
+    int i;
+    for (i = 0; i < n; i++) {
+        qemu_free_irq(s[i]);
+    }
     g_free(s);
 }
 
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index e65a5aa..02377c7 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -916,7 +916,7 @@ static void device_finalize(Object *obj)
 
     QLIST_FOREACH_SAFE(ngl, &dev->gpios, node, next) {
         QLIST_REMOVE(ngl, node);
-        qemu_free_irqs(ngl->in);
+        qemu_free_irqs(ngl->in, ngl->num_in);
         g_free(ngl->name);
         g_free(ngl);
         /* ngl->out irqs are owned by the other end and should not be freed
diff --git a/hw/ipack/ipack.c b/hw/ipack/ipack.c
index ef032e6..59bfe28 100644
--- a/hw/ipack/ipack.c
+++ b/hw/ipack/ipack.c
@@ -66,7 +66,7 @@ static void ipack_device_unrealize(DeviceState *dev, Error **errp)
         return;
     }
 
-    qemu_free_irqs(idev->irq);
+    qemu_free_irqs(idev->irq, 2);
 }
 
 static Property ipack_device_props[] = {
diff --git a/include/hw/irq.h b/include/hw/irq.h
index d08bc02..9f34c96 100644
--- a/include/hw/irq.h
+++ b/include/hw/irq.h
@@ -42,7 +42,7 @@ qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n);
 qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
                                 void *opaque, int n);
 
-void qemu_free_irqs(qemu_irq *s);
+void qemu_free_irqs(qemu_irq *s, int n);
 void qemu_free_irq(qemu_irq irq);
 
 /* Returns a new IRQ with opposite polarity.  */
-- 
2.0.0

[Qemu-devel] [PATCH qom v2 4/4] irq: Slim conversion of qemu_irq to QOM

[Peter Crosthwaite]

From: Andreas Färber <afaerber@suse.de>

As a prequel to any big Pin refactoring plans, do an in-place conversion
of qemu_irq to an Object, so that we can reference it in link<> properties.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Andreas Färber <afaerber@suse.de>
[ PC Changes:
 * Removed array-alloctor ref counting logic (limit changes just to
 * single IRQ allocator)
 * Removed WIP marking from subject line
]
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/irq.c    | 22 ++++++++++++++++++++--
 include/hw/irq.h |  2 ++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/hw/core/irq.c b/hw/core/irq.c
index bc982a7..cffced0 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -23,8 +23,13 @@
  */
 #include "qemu-common.h"
 #include "hw/irq.h"
+#include "qom/object.h"
+
+#define IRQ(obj) OBJECT_CHECK(struct IRQState, (obj), TYPE_IRQ)
 
 struct IRQState {
+    Object parent_obj;
+
     qemu_irq_handler handler;
     void *opaque;
     int n;
@@ -63,7 +68,7 @@ qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n)
 {
     struct IRQState *irq;
 
-    irq = g_new(struct IRQState, 1);
+    irq = IRQ(object_new(TYPE_IRQ));
     irq->handler = handler;
     irq->opaque = opaque;
     irq->n = n;
@@ -82,7 +87,7 @@ void qemu_free_irqs(qemu_irq *s, int n)
 
 void qemu_free_irq(qemu_irq irq)
 {
-    g_free(irq);
+    object_unref(OBJECT(irq));
 }
 
 static void qemu_notirq(void *opaque, int line, int level)
@@ -144,3 +149,16 @@ void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int n
     qemu_irq *old_irqs = *gpio_out;
     *gpio_out = qemu_allocate_irqs(handler, old_irqs, n);
 }
+
+static const TypeInfo irq_type_info = {
+   .name = TYPE_IRQ,
+   .parent = TYPE_OBJECT,
+   .instance_size = sizeof(struct IRQState),
+};
+
+static void irq_register_types(void)
+{
+    type_register_static(&irq_type_info);
+}
+
+type_init(irq_register_types)
diff --git a/include/hw/irq.h b/include/hw/irq.h
index 9f34c96..6f874f5 100644
--- a/include/hw/irq.h
+++ b/include/hw/irq.h
@@ -3,6 +3,8 @@
 
 /* Generic IRQ/GPIO pin infrastructure.  */
 
+#define TYPE_IRQ "irq"
+
 typedef struct IRQState *qemu_irq;
 
 typedef void (*qemu_irq_handler)(void *opaque, int n, int level);
-- 
2.0.0

Re: [Qemu-devel] [PATCH qom v2 4/4] irq: Slim conversion of qemu_irq to QOM

[Paolo Bonzini]

Il 18/06/2014 09:57, Peter Crosthwaite ha scritto:
> @@ -63,7 +68,7 @@ qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n)
>  {
>      struct IRQState *irq;
>
> -    irq = g_new(struct IRQState, 1);
> +    irq = IRQ(object_new(TYPE_IRQ));
>      irq->handler = handler;
>      irq->opaque = opaque;
>      irq->n = n;
> @@ -82,7 +87,7 @@ void qemu_free_irqs(qemu_irq *s, int n)
>
>  void qemu_free_irq(qemu_irq irq)
>  {
> -    g_free(irq);
> +    object_unref(OBJECT(irq));
>  }
>
>  static void qemu_notirq(void *opaque, int line, int level)

If the next step is to add an "owner" like the one in MemoryRegion, and 
change occurrences of qemu_free_irq to object_unparent, then

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [Qemu-devel] [PATCH qom v2 4/4] irq: Slim conversion of qemu_irq to QOM

[Peter Crosthwaite]

On Thu, Jun 19, 2014 at 12:40 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 18/06/2014 09:57, Peter Crosthwaite ha scritto:
>
>> @@ -63,7 +68,7 @@ qemu_irq qemu_allocate_irq(qemu_irq_handler handler,
>> void *opaque, int n)
>>  {
>>      struct IRQState *irq;
>>
>> -    irq = g_new(struct IRQState, 1);
>> +    irq = IRQ(object_new(TYPE_IRQ));
>>      irq->handler = handler;
>>      irq->opaque = opaque;
>>      irq->n = n;
>> @@ -82,7 +87,7 @@ void qemu_free_irqs(qemu_irq *s, int n)
>>
>>  void qemu_free_irq(qemu_irq irq)
>>  {
>> -    g_free(irq);
>> +    object_unref(OBJECT(irq));
>>  }
>>
>>  static void qemu_notirq(void *opaque, int line, int level)
>
>
> If the next step is to add an "owner" like the one in MemoryRegion, and
> change occurrences of qemu_free_irq to object_unparent,

Sure, I guess its a tree wide much like the one for Memory API though.
Can we do it as follow up though and sneak this through for 2.1?

Regards,
Peter

> then
>
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Paolo
>

Re: [Qemu-devel] [PATCH qom v2 4/4] irq: Slim conversion of qemu_irq to QOM

[Paolo Bonzini]

Il 19/06/2014 06:57, Peter Crosthwaite ha scritto:
>> >
>> > If the next step is to add an "owner" like the one in MemoryRegion, and
>> > change occurrences of qemu_free_irq to object_unparent,
> Sure, I guess its a tree wide much like the one for Memory API though.
> Can we do it as follow up though and sneak this through for 2.1?

Sure.

Paolo

Re: [Qemu-devel] [PATCH qom v2 0/4] QOMify IRQs

[Peter Crosthwaite]

Ping!

This is fully reviewed and should be rdy for a merge. I'd like to see
this through for 2.1.

Regards,
Peter

On Wed, Jun 18, 2014 at 5:53 PM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> Hi Andreas and all,
>
> I have done some cleanup of your WIP IRQ QOMification and have it in a
> hopefully ready state. Its now link safe and the allocation/freeing
> process is not as complex as before.
>
> For fuller context of the motivation behind this series, please see:
> http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03265.html
>
> changed since v1:
> Fixed sh4 instance of "[0]" bug (Kirill review)
>
> Regards,
> Peter
>
>
> Andreas Färber (3):
>   sdhci: Fix misuse of qemu_free_irqs()
>   hw: Fix qemu_allocate_irqs() leaks
>   irq: Slim conversion of qemu_irq to QOM
>
> Peter Crosthwaite (1):
>   irq: Allocate IRQs individually
>
>  hw/arm/omap1.c          | 14 +++++++-------
>  hw/arm/omap2.c          |  2 +-
>  hw/arm/pxa2xx.c         |  4 ++--
>  hw/arm/spitz.c          |  4 ++--
>  hw/arm/z2.c             |  2 +-
>  hw/char/serial-pci.c    |  2 +-
>  hw/core/irq.c           | 46 +++++++++++++++++++++++++++++-----------------
>  hw/core/qdev.c          |  2 +-
>  hw/dma/omap_dma.c       |  4 ++--
>  hw/ide/microdrive.c     |  2 +-
>  hw/ipack/ipack.c        |  2 +-
>  hw/misc/cbus.c          |  6 +++---
>  hw/pcmcia/pxa2xx.c      |  2 +-
>  hw/sd/omap_mmc.c        |  2 +-
>  hw/sd/sdhci.c           |  8 ++++----
>  hw/sh4/sh7750.c         |  3 +--
>  hw/timer/omap_gptimer.c |  4 ++--
>  include/hw/irq.h        |  4 +++-
>  18 files changed, 63 insertions(+), 50 deletions(-)
>
> --
> 2.0.0
>

Re: [Qemu-devel] [PATCH qom v2 0/4] QOMify IRQs

[Andreas Färber]

Am 25.06.2014 11:39, schrieb Peter Crosthwaite:
> Ping!
> 
> This is fully reviewed and should be rdy for a merge. I'd like to see
> this through for 2.1.

I have been very wary of applying the QOM conversion without full device
test coverage, similar to realization. People actually testing this
conversion would've been more reaffirming than a bit of review - the
hardfreeze can but does not necessarily uncover all corner cases. But
time is running out, so I intend to apply the series unless I discover
issues.

qtests for missing devices or statistics of how incomplete our coverage
actually is appreciated as always.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg