diff mbox series

[v2,03/15] hw/ide: Remove internal DMA qemu_irq

Message ID 20240627-san-v2-3-750bb0946dbd@daynix.com (mailing list archive)
State New
Headers show
Series Fix check-qtest-ppc64 sanitizer errors | expand

Commit Message

Akihiko Odaki June 27, 2024, 1:37 p.m. UTC 
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(-)

Comments

Mark Cave-Ayland June 28, 2024, 7:23 a.m. UTC | #1 
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.
diff mbox series

Patch

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)
 {