On 27/06/2024 14:37, Akihiko Odaki wrote:
> A function pointer is sufficient for internal usage. Replacing qemu_irq
> with one fixes the leak of qemu_irq.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> include/hw/ppc/mac_dbdma.h | 5 +++--
> hw/ide/macio.c | 11 +++++++----
> hw/misc/macio/mac_dbdma.c | 10 +++++-----
> 3 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/include/hw/ppc/mac_dbdma.h b/include/hw/ppc/mac_dbdma.h
> index 4a3f644516b3..1e1973c39580 100644
> --- a/include/hw/ppc/mac_dbdma.h
> +++ b/include/hw/ppc/mac_dbdma.h
> @@ -32,6 +32,7 @@
> typedef struct DBDMA_io DBDMA_io;
>
> typedef void (*DBDMA_flush)(DBDMA_io *io);
> +typedef void (*DBDMA_irq)(DBDMA_io *io);
> typedef void (*DBDMA_rw)(DBDMA_io *io);
> typedef void (*DBDMA_end)(DBDMA_io *io);
> struct DBDMA_io {
> @@ -154,7 +155,7 @@ typedef struct dbdma_cmd {
> typedef struct DBDMA_channel {
> int channel;
> uint32_t regs[DBDMA_REGS];
> - qemu_irq irq;
> + DBDMA_irq irq;
> DBDMA_io io;
> DBDMA_rw rw;
> DBDMA_flush flush;
> @@ -172,7 +173,7 @@ typedef struct DBDMAState DBDMAState;
>
> /* Externally callable functions */
>
> -void DBDMA_register_channel(void *dbdma, int nchan, qemu_irq irq,
> +void DBDMA_register_channel(void *dbdma, int nchan, DBDMA_irq irq,
> DBDMA_rw rw, DBDMA_flush flush,
> void *opaque);
> void DBDMA_kick(DBDMAState *dbdma);
> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> index 9c96a857a7c1..425b670a52a9 100644
> --- a/hw/ide/macio.c
> +++ b/hw/ide/macio.c
> @@ -427,9 +427,8 @@ static void macio_ide_realizefn(DeviceState *dev, Error **errp)
> s->bus.dma = &s->dma;
> }
>
> -static void pmac_irq(void *opaque, int n, int level)
> +static void pmac_irq(MACIOIDEState *s, int n, int level)
> {
> - MACIOIDEState *s = opaque;
> uint32_t mask = 0x80000000u >> n;
>
> /* We need to reflect the IRQ state in the irq register */
> @@ -446,6 +445,11 @@ static void pmac_irq(void *opaque, int n, int level)
> }
> }
>
> +static void pmac_dma_irq(DBDMA_io *io)
> +{
> + pmac_irq(io->opaque, 0, 1);
> +}
> +
> static void pmac_ide_irq(void *opaque, int n, int level)
> {
> pmac_irq(opaque, 1, level);
> @@ -461,7 +465,6 @@ static void macio_ide_initfn(Object *obj)
> sysbus_init_mmio(d, &s->mem);
> sysbus_init_irq(d, &s->real_ide_irq);
> sysbus_init_irq(d, &s->real_dma_irq);
> - s->dma_irq = qemu_allocate_irq(pmac_irq, s, 0);
> qdev_init_gpio_in_named_with_opaque(DEVICE(obj), pmac_ide_irq, s, NULL, 1);
>
> object_property_add_link(obj, "dbdma", TYPE_MAC_DBDMA,
> @@ -513,7 +516,7 @@ void macio_ide_init_drives(MACIOIDEState *s, DriveInfo **hd_table)
>
> void macio_ide_register_dma(MACIOIDEState *s)
> {
> - DBDMA_register_channel(s->dbdma, s->channel, s->dma_irq,
> + DBDMA_register_channel(s->dbdma, s->channel, pmac_dma_irq,
> pmac_ide_transfer, pmac_ide_flush, s);
> }
>
> diff --git a/hw/misc/macio/mac_dbdma.c b/hw/misc/macio/mac_dbdma.c
> index 2a528ea08caf..3450105ad851 100644
> --- a/hw/misc/macio/mac_dbdma.c
> +++ b/hw/misc/macio/mac_dbdma.c
> @@ -114,7 +114,7 @@ static void kill_channel(DBDMA_channel *ch)
> ch->regs[DBDMA_STATUS] |= DEAD;
> ch->regs[DBDMA_STATUS] &= ~ACTIVE;
>
> - qemu_irq_raise(ch->irq);
> + ch->irq(&ch->io);
> }
>
> static void conditional_interrupt(DBDMA_channel *ch)
> @@ -133,7 +133,7 @@ static void conditional_interrupt(DBDMA_channel *ch)
> case INTR_NEVER: /* don't interrupt */
> return;
> case INTR_ALWAYS: /* always interrupt */
> - qemu_irq_raise(ch->irq);
> + ch->irq(&ch->io);
> DBDMA_DPRINTFCH(ch, "%s: raise\n", __func__);
> return;
> }
> @@ -148,13 +148,13 @@ static void conditional_interrupt(DBDMA_channel *ch)
> switch(intr) {
> case INTR_IFSET: /* intr if condition bit is 1 */
> if (cond) {
> - qemu_irq_raise(ch->irq);
> + ch->irq(&ch->io);
> DBDMA_DPRINTFCH(ch, "%s: raise\n", __func__);
> }
> return;
> case INTR_IFCLR: /* intr if condition bit is 0 */
> if (!cond) {
> - qemu_irq_raise(ch->irq);
> + ch->irq(&ch->io);
> DBDMA_DPRINTFCH(ch, "%s: raise\n", __func__);
> }
> return;
> @@ -562,7 +562,7 @@ void DBDMA_kick(DBDMAState *dbdma)
> qemu_bh_schedule(dbdma->bh);
> }
>
> -void DBDMA_register_channel(void *dbdma, int nchan, qemu_irq irq,
> +void DBDMA_register_channel(void *dbdma, int nchan, DBDMA_irq irq,
> DBDMA_rw rw, DBDMA_flush flush,
> void *opaque)
> {
At first glance I can't say I'm keen on this: in general we should be moving towards
standardising on QEMU irqs or qdev gpios rather than introducing a custom function.
As per my previous email I suspect this is another symptom that something is wrong
with the modelling, so I will take a look.
ATB,
Mark.