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)) {