Re: [PATCH v5 2/7] Revert "hw/isa/vt82c686: Remove intermediate IRQ forwarder"

[BALATON Zoltan <balaton@eik.bme.hu>]

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

On Wed, 1 Mar 2023, Bernhard Beschow wrote:
> Am 1. März 2023 00:33:28 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> On Wed, 1 Mar 2023, BALATON Zoltan wrote:
>>> This partially reverts commit bb98e0f59cde846666d9fddc60ae74ef7ddfca17
>>> keeping the rename of a state field but reverting other cahanges which
>>> break interrupts on pegasos2.
>>
>> I've found this with just booting the MorphOS iso which now hangs without this revert when trying to read from the ide device. I think I've mentioned that I've also tried this way first but then ended up adding this because it was needed in a review of the patch earlier but I can't find that message now. For now it seems the easiest is to revert this and think about it later.
>
> It looks like Philippe's patch should work, at least in theory. Why does the indirection work while it doesn't without?

I have eno idea. I've found this before because I've also thought it 
should work in the simpler way but found it doesn't, that's why I've added 
the indirection which I saw was the way other similar devices also did it. 
We're now too close to the freeze to have another try so just chose to 
revert it for now then we can find out later what's going on.

Regards,
BALATON Zoltan
>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>> hw/isa/vt82c686.c | 10 +++++++++-
>>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>> index f4c40965cd..01e0148967 100644
>>> --- a/hw/isa/vt82c686.c
>>> +++ b/hw/isa/vt82c686.c
>>> @@ -598,15 +598,23 @@ void via_isa_set_irq(PCIDevice *d, int n, int level)
>>>     qemu_set_irq(s->isa_irqs_in[n], level);
>>> }
>>>
>>> +static void via_isa_request_i8259_irq(void *opaque, int irq, int level)
>>> +{
>>> +    ViaISAState *s = opaque;
>>> +    qemu_set_irq(s->cpu_intr, level);
>>> +}
>>> +
>>> 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;
>>>     ISABus *isa_bus;
>>>     int i;
>>>
>>>     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>>> +    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
>>>     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
>>>                           errp);
>>>
>>> @@ -614,7 +622,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>>>         return;
>>>     }
>>>
>>> -    s->isa_irqs_in = i8259_init(isa_bus, s->cpu_intr);
>>> +    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(isa_bus, 0);
>>>
>
>