Am 9. März 2026 17:12:58 UTC schrieb Peter Maydell <[email protected]>:
>The pci_piix_realize() function's use of qemu_allocate_irqs()
>results in a memory leak:
>
>Direct leak of 8 byte(s) in 1 object(s) allocated from:
> #0 0x61045c7a1a43 in malloc
> (/home/pm215/qemu/build/san/qemu-system-mips+0x16f8a43) (BuildId:
> aa43d3865e0f1991b1fc04422b5570fe522b6fa7)
> #1 0x724cc3095ac9 in g_malloc
> (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId:
> 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
> #2 0x61045db72134 in qemu_extend_irqs
> /home/pm215/qemu/build/san/../../hw/core/irq.c:77:51
> #3 0x61045cd7bf49 in pci_piix_realize
> /home/pm215/qemu/build/san/../../hw/isa/piix.c:318:35
> #4 0x61045cf4533e in pci_qdev_realize
> /home/pm215/qemu/build/san/../../hw/pci/pci.c:2308:9
> #5 0x61045db6cbca in device_set_realized
> /home/pm215/qemu/build/san/../../hw/core/qdev.c:523:13
> #6 0x61045db86bd9 in property_set_bool
> /home/pm215/qemu/build/san/../../qom/object.c:2376:5
> #7 0x61045db81c5e in object_property_set
> /home/pm215/qemu/build/san/../../qom/object.c:1450:5
> #8 0x61045db8e2fc in object_property_set_qobject
> /home/pm215/qemu/build/san/../../qom/qom-qobject.c:28:10
> #9 0x61045db8258f in object_property_set_bool
> /home/pm215/qemu/build/san/../../qom/object.c:1520:15
> #10 0x61045db687aa in qdev_realize_and_unref
> /home/pm215/qemu/build/san/../../hw/core/qdev.c:283:11
> #11 0x61045d892e21 in mips_malta_init
> /home/pm215/qemu/build/san/../../hw/mips/malta.c:1239:5
>
>(The i386 PC sets the has-pic property to 'false', so this only
>affects the MIPS Malta board.)
>
>Fix this by embedding the i8259 irq in the device state instead of
>allocating it. This is a similar fix to the one we used for vt82c686
>in commit 2225dc562a93dc, except that we use qemu_init_irq_child()
>instead of qemu_init_irq(). The behaviour is identical except that
>the _child() version avoids what would be a leak if we ever
>unrealized the device.
>
>Signed-off-by: Peter Maydell <[email protected]>
>---
>The handling of IRQs in i8259_init() is rather convoluted and forces
>its callers into having an IRQ that just forwards the set operation
>outwards. It could in theory be cleaned up, but it's used in too
>many places for that to seem worthwhile just now.
>---
> hw/isa/piix.c | 11 ++++++-----
> include/hw/southbridge/piix.h | 3 +++
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
>diff --git a/hw/isa/piix.c b/hw/isa/piix.c
>index 04b2be2cc3..31fa53e6a4 100644
>--- a/hw/isa/piix.c
>+++ b/hw/isa/piix.c
>@@ -315,12 +315,13 @@ static void pci_piix_realize(PCIDevice *dev, const char
>*uhci_type,
>
> /* PIC */
> if (d->has_pic) {
>- qemu_irq *i8259_out_irq = qemu_allocate_irqs(piix_request_i8259_irq,
>d,
>- 1);
>- qemu_irq *i8259 = i8259_init(isa_bus, *i8259_out_irq);
>- size_t i;
>+ qemu_irq *i8259;
>
>- for (i = 0; i < ISA_NUM_IRQS; i++) {
>+ qemu_init_irq_child(OBJECT(dev), "i8259-irq", &d->i8259_irq,
>+ piix_request_i8259_irq, d, 0);
>+ i8259 = i8259_init(isa_bus, &d->i8259_irq);
>+
>+ for (size_t i = 0; i < ISA_NUM_IRQS; i++) {
> d->isa_irqs_in[i] = i8259[i];
> }
>
>diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
>index 86709ba2e4..a296b1205a 100644
>--- a/include/hw/southbridge/piix.h
>+++ b/include/hw/southbridge/piix.h
>@@ -17,6 +17,7 @@
> #include "hw/ide/pci.h"
> #include "hw/rtc/mc146818rtc.h"
> #include "hw/usb/hcd-uhci.h"
>+#include "hw/core/irq.h"
>
> /* PIRQRC[A:D]: PIRQx Route Control Registers */
> #define PIIX_PIRQCA 0x60
>@@ -52,6 +53,8 @@ struct PIIXState {
> qemu_irq cpu_intr;
> qemu_irq isa_irqs_in[ISA_NUM_IRQS];
>
>+ IRQState i8259_irq;
>+
> /* This member isn't used. Just for save/load compatibility */
> int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
Reviewed-by: Bernhard Beschow <[email protected]>
