Re: [PATCH v5 5/7] hw/isa/vt82c686: Work around missing level sensitive irq in i8259 model

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

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

On Wed, 1 Mar 2023, Bernhard Beschow wrote:
> Am 1. März 2023 00:17:11 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> MorphOS sets the ISA PIC to level sensitive mode but QEMU does not
>> support that so this causes a freeze if multiple devices try to raise
>> a shared interrupt. Work around it by lowering the interrupt before
>> raising it again if it is already raised. This could be reverted when
>> the i8259 model is fixed.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> hw/isa/vt82c686.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>> index 018a119964..3e44a51f92 100644
>> --- a/hw/isa/vt82c686.c
>> +++ b/hw/isa/vt82c686.c
>> @@ -549,6 +549,7 @@ struct ViaISAState {
>>     PCIDevice dev;
>>     qemu_irq cpu_intr;
>>     qemu_irq *isa_irqs_in;
>> +    uint16_t isa_irqs_state;
>>     ViaSuperIOState via_sio;
>>     MC146818RtcState rtc;
>>     PCIIDEState ide;
>> @@ -636,6 +637,14 @@ static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>>             pic_level |= pci_bus_get_irq_level(bus, i);
>>         }
>>     }
>> +    /* FIXME: workaround for i8259: level sensitive irq not supported */
>> +    if ((s->isa_irqs_state & BIT(pic_irq)) && pic_level) {
>> +        qemu_irq_lower(s->isa_irqs_in[pic_irq]);
>> +    } else if (pic_level) {
>> +        s->isa_irqs_state |= BIT(pic_irq);
>> +    } else {
>> +        s->isa_irqs_state &= ~BIT(pic_irq);
>> +    }
>
> Let's not clutter the device model with workarounds which quickly snowball into unmaintainable code. Please fix the i8259 instead.

Do you have an idea how? I don't know PC hardware well so it's not likely 
I can do that in one day and breaking PIC model would affect a lot of 
machines among them some production critical ones. Adding this workaround 
here only affects pegasos2 and the only partially modeled likely not used 
fuloong2e board (which is mostly there because it was there before and 
good to keep it to be able to test this device model with another machine) 
and I can test these two but not all the others using i8259. So I think 
for this release this is the best we can do and if somebody more 
knowledgeable about PC hardware fixes the i8259 PIC model later this can 
be easily reverted. It's not a big clutter so unless you can show this 
breaks something I'd rather have it to keep MorphOS usable on pegasos2 
with sound and network or USB. If you can prove this breaks something we 
could drop it but not based on opinion or feelengs only some evidence.

Regards,
BALATON Zoltan

>>     /* Now we change the pic irq level according to the via irq mappings. */
>>     qemu_set_irq(s->isa_irqs_in[pic_irq], pic_level);
>> }
>
>