[v6,3/7] hw/isa/vt82c686: Implement PCI IRQ routing

The real VIA south bridges implement a PCI IRQ router which is configured
by the BIOS or the OS. In order to respect these configurations, QEMU
needs to implement it as well. The real chip may allow routing IRQs from
internal functions independently of PCI interrupts but since guests
usually configute it to a single shared interrupt we don't model that
here for simplicity.

Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.

Suggested-by: Bernhard Beschow <shentey@gmail.com>
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Tested-by: Rene Engel <ReneEngel80@emailn.de>
---
 hw/isa/vt82c686.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

Am 4. März 2023 14:48:20 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>The real VIA south bridges implement a PCI IRQ router which is configured
>by the BIOS or the OS. In order to respect these configurations, QEMU
>needs to implement it as well. The real chip may allow routing IRQs from
>internal functions independently of PCI interrupts but since guests
>usually configute it to a single shared interrupt we don't model that
>here for simplicity.
>
>Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>
>Suggested-by: Bernhard Beschow <shentey@gmail.com>
>Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>Tested-by: Rene Engel <ReneEngel80@emailn.de>
>---
> hw/isa/vt82c686.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
>diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>index f4c40965cd..51c0dd4c41 100644
>--- a/hw/isa/vt82c686.c
>+++ b/hw/isa/vt82c686.c
>@@ -598,6 +598,42 @@ void via_isa_set_irq(PCIDevice *d, int n, int level)
>     qemu_set_irq(s->isa_irqs_in[n], level);
> }
> 
>+static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>+{
>+    switch (irq_num) {
>+    case 0:
>+        return s->dev.config[0x55] >> 4;
>+    case 1:
>+        return s->dev.config[0x56] & 0xf;
>+    case 2:
>+        return s->dev.config[0x56] >> 4;
>+    case 3:
>+        return s->dev.config[0x57] >> 4;
>+    }
>+    return 0;
>+}
>+
>+static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>+{
>+    ViaISAState *s = opaque;
>+    PCIBus *bus = pci_get_bus(&s->dev);
>+    int i, pic_level, pic_irq = via_isa_get_pci_irq(s, irq_num);
>+
>+    if (unlikely(pic_irq == 0 || pic_irq == 2 || pic_irq > 14)) {

In the previous iteration I already mentioned this: Why "pic_irq > 14"? Please either remove or put a comment in the code since the datasheet allows it. Otherwise this leads to hard to comprehend and therefore hard to maintain code.

Moreover, "pic_irq == 2" is reserved which we should log a guest error for. Otherwise, misbehaving guests will go unnoticed, which is exactly what guest errors should prevent. Also, logging a guest error here makes the code more self documenting.

Note that excess logging can always be filtered out using grep.

Best regards,
Bernhard

>+        return;
>+    }
>+
>+    /* The pic level is the logical OR of all the PCI irqs mapped to it. */
>+    pic_level = 0;
>+    for (i = 0; i < PCI_NUM_PINS; i++) {
>+        if (pic_irq == via_isa_get_pci_irq(s, i)) {
>+            pic_level |= pci_bus_get_irq_level(bus, i);
>+        }
>+    }
>+    /* Now we change the pic irq level according to the via irq mappings. */
>+    qemu_set_irq(s->isa_irqs_in[pic_irq], pic_level);
>+}
>+
> static void via_isa_realize(PCIDevice *d, Error **errp)
> {
>     ViaISAState *s = VIA_ISA(d);
>@@ -619,6 +655,8 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>     i8254_pit_init(isa_bus, 0x40, 0, NULL);
>     i8257_dma_init(isa_bus, 0);
> 
>+    qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq", PCI_NUM_PINS);
>+
>     /* RTC */
>     qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
>     if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {