Re: [PATCH v2 04/15] hw/isa/vt82c686: Define a GPIO line between vt82c686 and i8259

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2024/06/29 16:38, Akihiko Odaki wrote:
> On 2024/06/28 16:27, Mark Cave-Ayland wrote:
>> On 27/06/2024 14:37, Akihiko Odaki wrote:
>>
>>> This fixes qemu_irq array leak.
>>>
>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> ---
>>>   hw/isa/vt82c686.c | 7 ++++---
>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>> index 8582ac0322eb..629d2d568137 100644
>>> --- a/hw/isa/vt82c686.c
>>> +++ b/hw/isa/vt82c686.c
>>> @@ -715,13 +715,14 @@ static void via_isa_realize(PCIDevice *d, Error 
>>> **errp)
>>>       ViaISAState *s = VIA_ISA(d);
>>>       DeviceState *dev = DEVICE(d);
>>>       PCIBus *pci_bus = pci_get_bus(d);
>>> -    qemu_irq *isa_irq;
>>> +    qemu_irq isa_irq;
>>>       ISABus *isa_bus;
>>>       int i;
>>>       qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>>>       qdev_init_gpio_in_named(dev, via_isa_pirq, "pirq", PCI_NUM_PINS);
>>> -    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
>>> +    qdev_init_gpio_in_named(dev, via_isa_request_i8259_irq, "i8259", 
>>> 1);
>>> +    isa_irq = qdev_get_gpio_in_named(dev, "i8259", 0);
>>>       isa_bus = isa_bus_new(dev, pci_address_space(d), 
>>> pci_address_space_io(d),
>>>                             errp);
>>> @@ -729,7 +730,7 @@ static void via_isa_realize(PCIDevice *d, Error 
>>> **errp)
>>>           return;
>>>       }
>>> -    s->isa_irqs_in = i8259_init(isa_bus, *isa_irq);
>>> +    s->isa_irqs_in = i8259_init(isa_bus, isa_irq);
>>>       isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in);
>>>       i8254_pit_init(isa_bus, 0x40, 0, NULL);
>>>       i8257_dma_init(OBJECT(d), isa_bus, 0);
>>
>> Have you confirmed that the machines using the VIA can still boot 
>> correctly with this change? I have a vague memory that Phil tried 
>> something like this, but due to legacy code poking around directly in 
>> the ISA IRQ array after realize it caused some things to break.
> 
> I tried to boot MorphOS on pegasos2 but it didn't boot even with QEMU 
> 9.0.1. The command line I used is:
> 
> qemu-system-ppc -M pegasos2 -rtc base=localtime -device 
> ati-vga,guest_hwcursor=true,romfile= -cdrom morphos-3.18.iso -kernel 
> boot.img -serial stdio

Apparently I failed to run it properly. I tried again now and it booted 
with this change.

Regards,
Akihiko Odaki

> 
> That said, I believe no such code remain now. The IRQ array is held in a 
> variable local in this function and nobody else can refer to it.