[PATCH v2 0/4] hw/isa: Remove unuseful qemu_allocate_irqs() call

[PATCH v2 0/4] hw/isa: Remove unuseful qemu_allocate_irqs() call

[Philippe Mathieu-Daudé]

I started to fix the LeakSanitizer error in piix4_realize(),
then looked for similar pattern and found 2 other places.
The older is i82378 (historically the first one) which then
spread.

Since v1:
- rebased
- removed vt82c686 patches

Philippe Mathieu-Daudé (4):
  hw/isa/i82378: Name output IRQ as 'intr'
  hw/isa/i82378: Simplify removing unuseful qemu_allocate_irqs() call
  hw/isa/i82378: Rename output IRQ variable
  hw/isa/piix4: Fix leak removing unuseful qemu_allocate_irqs() call

 hw/isa/i82378.c | 13 +++----------
 hw/isa/piix4.c  | 10 +---------
 hw/ppc/prep.c   |  4 ++--
 3 files changed, 6 insertions(+), 21 deletions(-)

-- 
2.26.3

[PATCH v2 1/4] hw/isa/i82378: Name output IRQ as 'intr'

[Philippe Mathieu-Daudé]

Named IRQs are easier to understand in the monitor.
Name the single output interrupt as 'intr'.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/isa/i82378.c | 2 +-
 hw/ppc/prep.c   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index 2a2ff05b937..fd296c8ed7a 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -113,7 +113,7 @@ static void i82378_init(Object *obj)
     DeviceState *dev = DEVICE(obj);
     I82378State *s = I82378(obj);
 
-    qdev_init_gpio_out(dev, s->out, 1);
+    qdev_init_gpio_out_named(dev, s->out, "intr", 1);
     qdev_init_gpio_in(dev, i82378_request_pic_irq, 16);
 }
 
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index acfc2a91d8e..f19ec612956 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -280,8 +280,8 @@ static void ibm_40p_init(MachineState *machine)
 
     /* PCI -> ISA bridge */
     i82378_dev = DEVICE(pci_create_simple(pci_bus, PCI_DEVFN(11, 0), "i82378"));
-    qdev_connect_gpio_out(i82378_dev, 0,
-                          cpu->env.irq_inputs[PPC6xx_INPUT_INT]);
+    qdev_connect_gpio_out_named(i82378_dev, "intr", 0,
+                                cpu->env.irq_inputs[PPC6xx_INPUT_INT]);
     sysbus_connect_irq(pcihost, 0, qdev_get_gpio_in(i82378_dev, 15));
     isa_bus = ISA_BUS(qdev_get_child_bus(i82378_dev, "isa.0"));
 
-- 
2.26.3

[PATCH v2 2/4] hw/isa/i82378: Simplify removing unuseful qemu_allocate_irqs() call

[Philippe Mathieu-Daudé]

When the i82378 model was added in commit a04ff940974 ("prep:
Add i82378 PCI-to-ISA bridge emulation") the i8259 model was
not yet QOM'ified. This happened later in commit 747c70af78f
("i8259: Convert to qdev").

Instead of creating an input IRQ with qemu_allocate_irqs()
to pass it as output IRQ of the PIC, with its handler simply
dispatching into the "intr" output IRQ, we can now simplify
and directly connect the PIC to the "intr" named output.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/isa/i82378.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index fd296c8ed7a..817eca47053 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -47,12 +47,6 @@ static const VMStateDescription vmstate_i82378 = {
     },
 };
 
-static void i82378_request_out0_irq(void *opaque, int irq, int level)
-{
-    I82378State *s = opaque;
-    qemu_set_irq(s->out[0], level);
-}
-
 static void i82378_request_pic_irq(void *opaque, int irq, int level)
 {
     DeviceState *dev = opaque;
@@ -94,8 +88,7 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
      */
 
     /* 2 82C59 (irq) */
-    s->i8259 = i8259_init(isabus,
-                          qemu_allocate_irq(i82378_request_out0_irq, s, 0));
+    s->i8259 = i8259_init(isabus, s->out[0]);
     isa_bus_irqs(isabus, s->i8259);
 
     /* 1 82C54 (pit) */
-- 
2.26.3

[PATCH v2 3/4] hw/isa/i82378: Rename output IRQ variable

[Philippe Mathieu-Daudé]

The i82378 has 2 output IRQs: "INT" and "NMI".
We do not model the NMI, so simplify I82378State by
removing the unused IRQ. To avoid keeping an array of
one element, remove the array and rename the variable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/isa/i82378.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index 817eca47053..164d6c65f64 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -32,7 +32,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(I82378State, I82378)
 struct I82378State {
     PCIDevice parent_obj;
 
-    qemu_irq out[2];
+    qemu_irq intr;
     qemu_irq *i8259;
     MemoryRegion io;
 };
@@ -88,7 +88,7 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
      */
 
     /* 2 82C59 (irq) */
-    s->i8259 = i8259_init(isabus, s->out[0]);
+    s->i8259 = i8259_init(isabus, s->intr);
     isa_bus_irqs(isabus, s->i8259);
 
     /* 1 82C54 (pit) */
@@ -106,7 +106,7 @@ static void i82378_init(Object *obj)
     DeviceState *dev = DEVICE(obj);
     I82378State *s = I82378(obj);
 
-    qdev_init_gpio_out_named(dev, s->out, "intr", 1);
+    qdev_init_gpio_out_named(dev, &s->intr, "intr", 1);
     qdev_init_gpio_in(dev, i82378_request_pic_irq, 16);
 }
 
-- 
2.26.3

[PATCH v2 4/4] hw/isa/piix4: Fix leak removing unuseful qemu_allocate_irqs() call

[Philippe Mathieu-Daudé]

We locally create an input IRQ with qemu_allocate_irqs() to
pass it as output IRQ of the PIC, but its handler simply dispatch
into another of our output IRQ ("intr" output).

Simplify by directly connecting the PIC output to our "intr"
output.

This fixes when using QEMU built with --enable-sanitizers:

  ==338425==ERROR: LeakSanitizer: detected memory leaks

  Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x5641b361e1df in malloc (qemu-system-mips+0x1b201df)
    #1 0x7f995e683958 in g_malloc (/lib64/libglib-2.0.so.0+0x58958)
    #2 0x5641b5558e36 in qemu_allocate_irqs hw/core/irq.c:66:12
    #3 0x5641b4161817 in piix4_realize hw/isa/piix4.c:171:21
    #4 0x5641b42f077a in pci_qdev_realize hw/pci/pci.c:2114:9
    #5 0x5641b554c802 in device_set_realized hw/core/qdev.c:761:13
    #6 0x5641b5578458 in property_set_bool qom/object.c:2257:5
    #7 0x5641b55709e2 in object_property_set qom/object.c:1402:5
    #8 0x5641b55861c9 in object_property_set_qobject qom/qom-qobject.c:28:10
    #9 0x5641b5571831 in object_property_set_bool qom/object.c:1472:15
   #10 0x5641b55410fd in qdev_realize hw/core/qdev.c:389:12

Fixes: 078778c5a55 ("piix4: Add an i8259 Interrupt Controller as specified in datasheet")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/isa/piix4.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 0fe7b69bc4c..a16f7a3ef94 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -115,12 +115,6 @@ static const VMStateDescription vmstate_piix4 = {
     }
 };
 
-static void piix4_request_i8259_irq(void *opaque, int irq, int level)
-{
-    PIIX4State *s = opaque;
-    qemu_set_irq(s->cpu_intr, level);
-}
-
 static void piix4_set_i8259_irq(void *opaque, int irq, int level)
 {
     PIIX4State *s = opaque;
@@ -161,7 +155,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
 {
     PIIX4State *s = PIIX4_PCI_DEVICE(dev);
     ISABus *isa_bus;
-    qemu_irq *i8259_out_irq;
 
     isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
                           pci_address_space_io(dev), errp);
@@ -180,8 +173,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
                                         PIIX_RCR_IOPORT, &s->rcr_mem, 1);
 
     /* initialize i8259 pic */
-    i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
-    s->isa = i8259_init(isa_bus, *i8259_out_irq);
+    s->isa = i8259_init(isa_bus, s->cpu_intr);
 
     /* initialize ISA irqs */
     isa_bus_irqs(isa_bus, s->isa);
-- 
2.26.3

Re: [PATCH v2 0/4] hw/isa: Remove unuseful qemu_allocate_irqs() call

[Philippe Mathieu-Daudé]

Cc'ing Zoltan

On 5/11/21 06:06, Philippe Mathieu-Daudé wrote:
> I started to fix the LeakSanitizer error in piix4_realize(),
> then looked for similar pattern and found 2 other places.
> The older is i82378 (historically the first one) which then
> spread.
> 
> Since v1:
> - rebased
> - removed vt82c686 patches
> 
> Philippe Mathieu-Daudé (4):
>   hw/isa/i82378: Name output IRQ as 'intr'
>   hw/isa/i82378: Simplify removing unuseful qemu_allocate_irqs() call
>   hw/isa/i82378: Rename output IRQ variable
>   hw/isa/piix4: Fix leak removing unuseful qemu_allocate_irqs() call
> 
>  hw/isa/i82378.c | 13 +++----------
>  hw/isa/piix4.c  | 10 +---------
>  hw/ppc/prep.c   |  4 ++--
>  3 files changed, 6 insertions(+), 21 deletions(-)
> 

Re: [PATCH v2 0/4] hw/isa: Remove unuseful qemu_allocate_irqs() call

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]

On Sat, 6 Nov 2021, Philippe Mathieu-Daudé wrote:
> Cc'ing Zoltan

What do I have to do with this? This was a while ago so I don't remember 
and it seems to not touch any of the devices I have some experience with 
so not sure what do you expect from me for this series.

Regards,
BALATON Zoltan

> On 5/11/21 06:06, Philippe Mathieu-Daudé wrote:
>> I started to fix the LeakSanitizer error in piix4_realize(),
>> then looked for similar pattern and found 2 other places.
>> The older is i82378 (historically the first one) which then
>> spread.
>>
>> Since v1:
>> - rebased
>> - removed vt82c686 patches
>>
>> Philippe Mathieu-Daudé (4):
>>   hw/isa/i82378: Name output IRQ as 'intr'
>>   hw/isa/i82378: Simplify removing unuseful qemu_allocate_irqs() call
>>   hw/isa/i82378: Rename output IRQ variable
>>   hw/isa/piix4: Fix leak removing unuseful qemu_allocate_irqs() call
>>
>>  hw/isa/i82378.c | 13 +++----------
>>  hw/isa/piix4.c  | 10 +---------
>>  hw/ppc/prep.c   |  4 ++--
>>  3 files changed, 6 insertions(+), 21 deletions(-)
>>
>
>