QOMify IRQs
cover QOMify IRQs
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
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/timer/omap_gptimer.c | 4 ++-- include/hw/irq.h | 4 +++- 17 files changed, 62 insertions(+), 48 deletions(-)
Signature · 1 line
2.0.0
PATCH 1/4 sdhci: Fix misuse of qemu_free_irqs()
From: Andreas Färber <[email protected]>
It does a g_free() on the pointer.
---
hw/sd/sdhci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
hw/sd/sdhci.c +2 -2
@@ -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);
Signature · 1 line
2.0.0
PATCH 2/4 hw: Fix qemu_allocate_irqs() leaks
In reply to Peter CrosthwaiteFrom: Andreas Färber <[email protected]>
Replace qemu_allocate_irqs(foo, bar, 1)[0]
with qemu_allocate_irq(foo, bar, 0).
This avoids leaking the dereferenced qemu_irq *.
---
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/timer/omap_gptimer.c | 4 ++-- 13 files changed, 27 insertions(+), 27 deletions(-)
hw/arm/omap1.c +7 -7
@@ -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);
hw/arm/omap2.c +1 -1
@@ -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);
hw/arm/pxa2xx.c +2 -2
@@ -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);
hw/arm/spitz.c +2 -2
@@ -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);
hw/arm/z2.c +1 -1
@@ -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;
hw/core/irq.c +2 -2
@@ -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)
hw/dma/omap_dma.c +2 -2
@@ -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);
hw/ide/microdrive.c +1 -1
@@ -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)
hw/misc/cbus.c +3 -3
@@ -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 ;
hw/pcmcia/pxa2xx.c +1 -1
@@ -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,
hw/sd/omap_mmc.c +1 -1
@@ -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;
hw/sd/sdhci.c +2 -2
@@ -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);
hw/timer/omap_gptimer.c +2 -2
@@ -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);
Signature · 1 line
2.0.0
Re: [Qemu-devel] [PATCH qom v1 2/4] hw: Fix qemu_allocate_irqs() leaks
Quoted 5 lines · Peter Crosthwaite
From: Andreas Färber <[email protected]>
Replace
qemu_allocate_irqs(foo, bar, 1)[0]
withqemu_allocate_irq(foo, bar, 0).
You missed one occurrence in hw/sh4/sh7750.c in function sh7750_irl.
Signature · 1 line
Kirill
PATCH 3/4 irq: Allocate IRQs individually
In reply to Peter CrosthwaiteAllocate 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.
---
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(-)
hw/char/serial-pci.c +1 -1
@@ -148,7 +148,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 = {
hw/core/irq.c +7 -13
@@ -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); }
hw/core/qdev.c +1 -1
@@ -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
hw/ipack/ipack.c +1 -1
@@ -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[] = {
include/hw/irq.h +1 -1
@@ -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. */
Signature · 1 line
2.0.0
Re: [Qemu-devel] [PATCH qom v1 3/4] irq: Allocate IRQs individually
Quoted 11 lines · Peter Crosthwaite
---
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(-)
PATCH 4/4 irq: Slim conversion of qemu_irq to QOM
In reply to Peter CrosthwaiteFrom: Andreas Färber <[email protected]>
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.
[ PC Changes:
* Removed array-alloctor ref counting logic (limit changes just to
* single IRQ allocator)
* Removed WIP marking from subject line
]
---
hw/core/irq.c | 22 ++++++++++++++++++++-- include/hw/irq.h | 2 ++ 2 files changed, 22 insertions(+), 2 deletions(-)
hw/core/irq.c +20 -2
@@ -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 nqemu_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)
include/hw/irq.h +2 -0
@@ -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);
Signature · 1 line
2.0.0
Quoted 13 lines · 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.htmlRegards,
Peter
Quoted 7 lines
Andreas Färber (3):
sdhci: Fix misuse ofqemu_free_irqs()
hw: Fixqemu_allocate_irqs()leaks
irq: Slim conversion ofqemu_irqto QOMPeter Crosthwaite (1):
irq: Allocate IRQs individually
Possibly with QOM objects for boards most of the remaining qemu_allocate_irqs() calls could be turned into just having qemu_irq[] arrays directly in the board's state struct (eventually).
thanks
-- PMM