[PATCH] hw/m68k/mcf_intc: Use qdev input gpios for input IRQs

Peter Maydell posted 1 patch 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260306190425.3047580-1-peter.maydell@linaro.org
Maintainers: Thomas Huth <huth@tuxfamily.org>
hw/m68k/mcf5208.c     | 24 +++++++++++-------------
hw/m68k/mcf_intc.c    |  8 +++-----
include/hw/m68k/mcf.h |  5 ++---
3 files changed, 16 insertions(+), 21 deletions(-)
[PATCH] hw/m68k/mcf_intc: Use qdev input gpios for input IRQs
Posted by Peter Maydell 1 month, 1 week ago
The m68k mcf_intc interrupt controller currently implements its
inbound IRQ lines by calling qemu_allocate_irqs() in mcf_intc_init().
This results in leaks like this:

Direct leak of 2944 byte(s) in 46 object(s) allocated from:
    #0 0x5cf95ec15323 in malloc (/home/pm215/qemu/build/san/qemu-system-m68k+0xf9e323) (BuildId: 18d55ef8ea9856e68ee30802078af5050b8b06c5)
    #1 0x7637c65c5ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #2 0x5cf95f6b2f27 in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:767:15
    #3 0x5cf95f6aa62e in qemu_allocate_irq /home/pm215/qemu/build/san/../../hw/core/irq.c:91:25
    #4 0x5cf95f6aa62e in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:79:16
    #5 0x5cf95f5f6d99 in mcf5208evb_init /home/pm215/qemu/build/san/../../hw/m68k/mcf5208.c:310:11

This isn't an important leak, as it is memory we allocate once at
QEMU startup and that has to stay live for the lifetime of the
system.  However it does point at a code improvement.

Modernise this to have the device itself create inbound GPIOs with
qdev_init_gpio_in() that the board can then refer to and wire up
individually.

As the device is used in only a single board, we can update device
and board in a single patch rather than having to try to figure out
some way to change the API more piecemeal.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
there's quite a bit more in this board which could be
modernised, but I stuck to just the thing getting reported
as a memory leak...
---
 hw/m68k/mcf5208.c     | 24 +++++++++++-------------
 hw/m68k/mcf_intc.c    |  8 +++-----
 include/hw/m68k/mcf.h |  5 ++---
 3 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index c6d1c5fae9..0e07aa45e9 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -227,7 +227,7 @@ static const MemoryRegionOps m5208_rcm_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
 };
 
-static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic,
+static void mcf5208_sys_init(MemoryRegion *address_space, DeviceState *intc,
                              M68kCPU *cpu)
 {
     MemoryRegion *iomem = g_new(MemoryRegion, 1);
@@ -250,11 +250,11 @@ static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic,
                               "m5208-timer", 0x00004000);
         memory_region_add_subregion(address_space, 0xfc080000 + 0x4000 * i,
                                     &s->iomem);
-        s->irq = pic[4 + i];
+        s->irq = qdev_get_gpio_in(intc, 4 + i);
     }
 }
 
-static void mcf_fec_init(MemoryRegion *sysmem, hwaddr base, qemu_irq *irqs)
+static void mcf_fec_init(MemoryRegion *sysmem, hwaddr base, DeviceState *intc)
 {
     DeviceState *dev;
     SysBusDevice *s;
@@ -268,7 +268,7 @@ static void mcf_fec_init(MemoryRegion *sysmem, hwaddr base, qemu_irq *irqs)
     s = SYS_BUS_DEVICE(dev);
     sysbus_realize_and_unref(s, &error_fatal);
     for (i = 0; i < FEC_NUM_IRQ; i++) {
-        sysbus_connect_irq(s, i, irqs[i]);
+        sysbus_connect_irq(s, i, qdev_get_gpio_in(intc, i + 36));
     }
 
     memory_region_add_subregion(sysmem, base, sysbus_mmio_get_region(s, 0));
@@ -283,10 +283,10 @@ static void mcf5208evb_init(MachineState *machine)
     int kernel_size;
     uint64_t elf_entry;
     hwaddr entry;
-    qemu_irq *pic;
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *rom = g_new(MemoryRegion, 1);
     MemoryRegion *sram = g_new(MemoryRegion, 1);
+    DeviceState *intc;
 
     cpu = M68K_CPU(cpu_create(machine->cpu_type));
     env = &cpu->env;
@@ -307,17 +307,15 @@ static void mcf5208evb_init(MachineState *machine)
     memory_region_add_subregion(address_space_mem, 0x80000000, sram);
 
     /* Internal peripherals.  */
-    pic = mcf_intc_init(address_space_mem, 0xfc048000, cpu);
+    intc = mcf_intc_init(address_space_mem, 0xfc048000, cpu);
 
-    mcf_uart_create_mmap(0xfc060000, pic[26], serial_hd(0));
-    mcf_uart_create_mmap(0xfc064000, pic[27], serial_hd(1));
-    mcf_uart_create_mmap(0xfc068000, pic[28], serial_hd(2));
+    mcf_uart_create_mmap(0xfc060000, qdev_get_gpio_in(intc, 26), serial_hd(0));
+    mcf_uart_create_mmap(0xfc064000, qdev_get_gpio_in(intc, 27), serial_hd(1));
+    mcf_uart_create_mmap(0xfc068000, qdev_get_gpio_in(intc, 28), serial_hd(2));
 
-    mcf5208_sys_init(address_space_mem, pic, cpu);
+    mcf5208_sys_init(address_space_mem, intc, cpu);
 
-    mcf_fec_init(address_space_mem, 0xfc030000, pic + 36);
-
-    g_free(pic);
+    mcf_fec_init(address_space_mem, 0xfc030000, intc);
 
     /*  0xfc000000 SCM.  */
     /*  0xfc004000 XBS.  */
diff --git a/hw/m68k/mcf_intc.c b/hw/m68k/mcf_intc.c
index 20112c94be..1014fe6fa5 100644
--- a/hw/m68k/mcf_intc.c
+++ b/hw/m68k/mcf_intc.c
@@ -175,6 +175,7 @@ static void mcf_intc_instance_init(Object *obj)
 
     memory_region_init_io(&s->iomem, obj, &mcf_intc_ops, s, "mcf", 0x100);
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+    qdev_init_gpio_in(DEVICE(s), mcf_intc_set_irq, 64);
 }
 
 static const Property mcf_intc_properties[] = {
@@ -206,9 +207,7 @@ static void mcf_intc_register_types(void)
 
 type_init(mcf_intc_register_types)
 
-qemu_irq *mcf_intc_init(MemoryRegion *sysmem,
-                        hwaddr base,
-                        M68kCPU *cpu)
+DeviceState *mcf_intc_init(MemoryRegion *sysmem, hwaddr base, M68kCPU *cpu)
 {
     DeviceState  *dev;
 
@@ -218,6 +217,5 @@ qemu_irq *mcf_intc_init(MemoryRegion *sysmem,
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
     memory_region_add_subregion(sysmem, base,
                                 sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
-
-    return qemu_allocate_irqs(mcf_intc_set_irq, dev, 64);
+    return dev;
 }
diff --git a/include/hw/m68k/mcf.h b/include/hw/m68k/mcf.h
index 5d9f876ffe..b2a599adaa 100644
--- a/include/hw/m68k/mcf.h
+++ b/include/hw/m68k/mcf.h
@@ -14,9 +14,8 @@ DeviceState *mcf_uart_create(qemu_irq irq, Chardev *chr);
 DeviceState *mcf_uart_create_mmap(hwaddr base, qemu_irq irq, Chardev *chr);
 
 /* mcf_intc.c */
-qemu_irq *mcf_intc_init(struct MemoryRegion *sysmem,
-                        hwaddr base,
-                        M68kCPU *cpu);
+DeviceState *mcf_intc_init(struct MemoryRegion *sysmem,
+                           hwaddr base, M68kCPU *cpu);
 
 /* mcf5206.c */
 #define TYPE_MCF5206_MBAR "mcf5206-mbar"
-- 
2.43.0
Re: [PATCH] hw/m68k/mcf_intc: Use qdev input gpios for input IRQs
Posted by Philippe Mathieu-Daudé 1 month ago
On 6/3/26 20:04, Peter Maydell wrote:
> The m68k mcf_intc interrupt controller currently implements its
> inbound IRQ lines by calling qemu_allocate_irqs() in mcf_intc_init().
> This results in leaks like this:
> 
> Direct leak of 2944 byte(s) in 46 object(s) allocated from:
>      #0 0x5cf95ec15323 in malloc (/home/pm215/qemu/build/san/qemu-system-m68k+0xf9e323) (BuildId: 18d55ef8ea9856e68ee30802078af5050b8b06c5)
>      #1 0x7637c65c5ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
>      #2 0x5cf95f6b2f27 in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:767:15
>      #3 0x5cf95f6aa62e in qemu_allocate_irq /home/pm215/qemu/build/san/../../hw/core/irq.c:91:25
>      #4 0x5cf95f6aa62e in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:79:16
>      #5 0x5cf95f5f6d99 in mcf5208evb_init /home/pm215/qemu/build/san/../../hw/m68k/mcf5208.c:310:11
> 
> This isn't an important leak, as it is memory we allocate once at
> QEMU startup and that has to stay live for the lifetime of the
> system.  However it does point at a code improvement.
> 
> Modernise this to have the device itself create inbound GPIOs with
> qdev_init_gpio_in() that the board can then refer to and wire up
> individually.
> 
> As the device is used in only a single board, we can update device
> and board in a single patch rather than having to try to figure out
> some way to change the API more piecemeal.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> there's quite a bit more in this board which could be
> modernised, but I stuck to just the thing getting reported
> as a memory leak...
> ---
>   hw/m68k/mcf5208.c     | 24 +++++++++++-------------
>   hw/m68k/mcf_intc.c    |  8 +++-----
>   include/hw/m68k/mcf.h |  5 ++---
>   3 files changed, 16 insertions(+), 21 deletions(-)


Queued via hw-misc, thanks.
Re: [PATCH] hw/m68k/mcf_intc: Use qdev input gpios for input IRQs
Posted by Thomas Huth 1 month ago
Am Fri,  6 Mar 2026 19:04:25 +0000
schrieb Peter Maydell <peter.maydell@linaro.org>:

> The m68k mcf_intc interrupt controller currently implements its
> inbound IRQ lines by calling qemu_allocate_irqs() in mcf_intc_init().
> This results in leaks like this:
> 
> Direct leak of 2944 byte(s) in 46 object(s) allocated from:
>     #0 0x5cf95ec15323 in malloc (/home/pm215/qemu/build/san/qemu-system-m68k+0xf9e323) (BuildId: 18d55ef8ea9856e68ee30802078af5050b8b06c5)
>     #1 0x7637c65c5ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
>     #2 0x5cf95f6b2f27 in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:767:15
>     #3 0x5cf95f6aa62e in qemu_allocate_irq /home/pm215/qemu/build/san/../../hw/core/irq.c:91:25
>     #4 0x5cf95f6aa62e in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:79:16
>     #5 0x5cf95f5f6d99 in mcf5208evb_init /home/pm215/qemu/build/san/../../hw/m68k/mcf5208.c:310:11
> 
> This isn't an important leak, as it is memory we allocate once at
> QEMU startup and that has to stay live for the lifetime of the
> system.  However it does point at a code improvement.
> 
> Modernise this to have the device itself create inbound GPIOs with
> qdev_init_gpio_in() that the board can then refer to and wire up
> individually.
> 
> As the device is used in only a single board, we can update device
> and board in a single patch rather than having to try to figure out
> some way to change the API more piecemeal.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> there's quite a bit more in this board which could be
> modernised, but I stuck to just the thing getting reported
> as a memory leak...
> ---
>  hw/m68k/mcf5208.c     | 24 +++++++++++-------------
>  hw/m68k/mcf_intc.c    |  8 +++-----
>  include/hw/m68k/mcf.h |  5 ++---
>  3 files changed, 16 insertions(+), 21 deletions(-)

Reviewed-by: Thomas Huth <th.huth+qemu@posteo.eu>
Re: [PATCH] hw/m68k/mcf_intc: Use qdev input gpios for input IRQs
Posted by Philippe Mathieu-Daudé 1 month ago
On 6/3/26 20:04, Peter Maydell wrote:
> The m68k mcf_intc interrupt controller currently implements its
> inbound IRQ lines by calling qemu_allocate_irqs() in mcf_intc_init().
> This results in leaks like this:
> 
> Direct leak of 2944 byte(s) in 46 object(s) allocated from:
>      #0 0x5cf95ec15323 in malloc (/home/pm215/qemu/build/san/qemu-system-m68k+0xf9e323) (BuildId: 18d55ef8ea9856e68ee30802078af5050b8b06c5)
>      #1 0x7637c65c5ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
>      #2 0x5cf95f6b2f27 in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:767:15
>      #3 0x5cf95f6aa62e in qemu_allocate_irq /home/pm215/qemu/build/san/../../hw/core/irq.c:91:25
>      #4 0x5cf95f6aa62e in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:79:16
>      #5 0x5cf95f5f6d99 in mcf5208evb_init /home/pm215/qemu/build/san/../../hw/m68k/mcf5208.c:310:11
> 
> This isn't an important leak, as it is memory we allocate once at
> QEMU startup and that has to stay live for the lifetime of the
> system.  However it does point at a code improvement.
> 
> Modernise this to have the device itself create inbound GPIOs with
> qdev_init_gpio_in() that the board can then refer to and wire up
> individually.
> 
> As the device is used in only a single board, we can update device
> and board in a single patch rather than having to try to figure out
> some way to change the API more piecemeal.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> there's quite a bit more in this board which could be
> modernised, but I stuck to just the thing getting reported
> as a memory leak...
> ---
>   hw/m68k/mcf5208.c     | 24 +++++++++++-------------
>   hw/m68k/mcf_intc.c    |  8 +++-----
>   include/hw/m68k/mcf.h |  5 ++---
>   3 files changed, 16 insertions(+), 21 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>