[PATCH 0/4] Fix IRQ routing in via south bridge

[PATCH 0/4] Fix IRQ routing in via south bridge

[BALATON Zoltan]

This is going back to my otiginal proposal in
https://patchew.org/QEMU/cover.1677004414.git.balaton@eik.bme.hu/
implementing routing of interrupts from device functions and PCI
devices to ISA interrupts. On pegasos2 the firmware sets evertyhing to
share IRQ 9 so the current simpified version worked for taht but with
the amigaone machine its firmware makes use of this feature and
assigns different interrupts to functions and PCI devices so we need
to properly impelent this. Since any ISA interrupt can be controlled
by any interrupt source (different functions of the multifunction
device plus the 4 input pins from PCI devices) there are more than 4
possible sources so this can't be handled by just the 4 PCI interrupt
lines. We need to keep track of the state of each interrupt source to
be able to determine the level of the ISA interrupt and avoid one
device clearing it while other still has an interrupt.

This fixes USB on amigaone and maybe other bugs not discovered yet.

Regards,
BALATON Zoltan

BALATON Zoltan (4):
  hw/isa/vt82c686: Bring back via_isa_set_irq()
  hw/usb/vt82c686-uhci-pci: Use ISA instead of PCI interrupts
  hw/isa/vt82c686: Route PIRQ inputs using via_isa_set_irq()
  hw/audio/via-ac97: Route interrupts using via_isa_set_irq()

 hw/audio/via-ac97.c        |  8 ++---
 hw/isa/vt82c686.c          | 67 +++++++++++++++++++++++---------------
 hw/usb/vt82c686-uhci-pci.c |  9 +++++
 include/hw/isa/vt82c686.h  |  2 ++
 4 files changed, 56 insertions(+), 30 deletions(-)

-- 
2.30.9

[PATCH 1/4] hw/isa/vt82c686: Bring back via_isa_set_irq()

[BALATON Zoltan]

The VIA intergrated south bridge chips combine several functions and
allow routing their interrupts to any of the ISA IRQs (also allowing
multiple components to share the same ISA IRQ, e.g. pegasos2 firmware
configures USB, sound and PCI to use IRQ 9). Bring back
via_isa_set_irq() that takes the component that wants to change an IRQ
and keeps track of interrupt status of each source separately and does
the mapping to ISA IRQ within the ISA bridge to allow different
sources to control the same ISA IRQ lines.

This may not handle cases when the ISA IRQ is also controlled by
devices directly, not going through via_isa_set_irq() such as serial,
parallel or keyboard but these IRQs being conventionally fixed are not
likely for guests to change or share with other devices so hopefully
this does not cause a problem in practice.

This reverts commit 4e5a20b6da9b1f6d2e9621ed7eb8b239560104ae.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/isa/vt82c686.c         | 31 +++++++++++++++++++++++++++++++
 include/hw/isa/vt82c686.h |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 57bdfb4e78..07ae8d73bc 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_irq_state[ISA_NUM_IRQS];
     ViaSuperIOState via_sio;
     MC146818RtcState rtc;
     PCIIDEState ide;
@@ -592,6 +593,36 @@ static const TypeInfo via_isa_info = {
     },
 };
 
+void via_isa_set_irq(PCIDevice *d, int pin, int level)
+{
+    ViaISAState *s = VIA_ISA(pci_get_function_0(d));
+    int n = PCI_FUNC(d->devfn);
+    uint8_t isa_irq = d->config[PCI_INTERRUPT_LINE], max_irq = 15;
+
+    switch (n) {
+    case 2: /* USB ports 0-1 */
+    case 3: /* USB ports 2-3 */
+        max_irq = 14;
+        break;
+    }
+
+    if (unlikely(isa_irq > max_irq || isa_irq == 2)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "Invalid ISA IRQ routing %d for %d",
+                      isa_irq, n);
+        return;
+    }
+    if (!isa_irq) {
+        return;
+    }
+
+    if (level) {
+        s->isa_irq_state[isa_irq] |= BIT(n);
+    } else {
+        s->isa_irq_state[isa_irq] &= ~BIT(n);
+    }
+    qemu_set_irq(s->isa_irqs_in[isa_irq], !!s->isa_irq_state[isa_irq]);
+}
+
 static void via_isa_request_i8259_irq(void *opaque, int irq, int level)
 {
     ViaISAState *s = opaque;
diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index b6e95b2851..da1722daf2 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -34,4 +34,6 @@ struct ViaAC97State {
     uint32_t ac97_cmd;
 };
 
+void via_isa_set_irq(PCIDevice *d, int n, int level);
+
 #endif
-- 
2.30.9

[PATCH 2/4] hw/usb/vt82c686-uhci-pci: Use ISA instead of PCI interrupts

[BALATON Zoltan]

This device is part of a superio/ISA bridge chip and IRQs from it are
routed to an ISA interrupt. Use via_isa_set_irq() function to implement
this in a vt82c686-uhci-pci specific irq handler.

This reverts commit 422a6e8075752bc5342afd3eace23a4990dd7d98.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/usb/vt82c686-uhci-pci.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/usb/vt82c686-uhci-pci.c b/hw/usb/vt82c686-uhci-pci.c
index b4884c9011..6162806172 100644
--- a/hw/usb/vt82c686-uhci-pci.c
+++ b/hw/usb/vt82c686-uhci-pci.c
@@ -1,7 +1,14 @@
 #include "qemu/osdep.h"
+#include "hw/irq.h"
 #include "hw/isa/vt82c686.h"
 #include "hcd-uhci.h"
 
+static void uhci_isa_set_irq(void *opaque, int irq_num, int level)
+{
+    UHCIState *s = opaque;
+    via_isa_set_irq(&s->dev, 0, level);
+}
+
 static void usb_uhci_vt82c686b_realize(PCIDevice *dev, Error **errp)
 {
     UHCIState *s = UHCI(dev);
@@ -15,6 +22,8 @@ static void usb_uhci_vt82c686b_realize(PCIDevice *dev, Error **errp)
     pci_set_long(pci_conf + 0xc0, 0x00002000);
 
     usb_uhci_common_realize(dev, errp);
+    object_unref(s->irq);
+    s->irq = qemu_allocate_irq(uhci_isa_set_irq, s, 0);
 }
 
 static UHCIInfo uhci_info[] = {
-- 
2.30.9

[PATCH 3/4] hw/isa/vt82c686: Route PIRQ inputs using via_isa_set_irq()

[BALATON Zoltan]

The chip has 4 pins (called PIRQA-D in VT82C686B and PINTA-D in
VT8231) that are meant to be connected to PCI IRQ lines and allow
routing PCI interrupts to the ISA PIC. Route these in
via_isa_set_irq() to make it possible to share them with internal
functions that can also be routed to the same ISA IRQs.

Fixes: 2fdadd02e675caca4aba4ae26317701fe2c4c901
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/isa/vt82c686.c | 65 +++++++++++++++++------------------------------
 1 file changed, 24 insertions(+), 41 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 07ae8d73bc..18cc2f653d 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -593,6 +593,21 @@ static const TypeInfo via_isa_info = {
     },
 };
 
+static int via_isa_get_pci_irq(const ViaISAState *s, int pin)
+{
+    switch (pin) {
+    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;
+}
+
 void via_isa_set_irq(PCIDevice *d, int pin, int level)
 {
     ViaISAState *s = VIA_ISA(pci_get_function_0(d));
@@ -600,6 +615,10 @@ void via_isa_set_irq(PCIDevice *d, int pin, int level)
     uint8_t isa_irq = d->config[PCI_INTERRUPT_LINE], max_irq = 15;
 
     switch (n) {
+    case 0: /* PIRQ/PINT inputs */
+        isa_irq = via_isa_get_pci_irq(s, pin);
+        n = 12 + pin;
+        break;
     case 2: /* USB ports 0-1 */
     case 3: /* USB ports 2-3 */
         max_irq = 14;
@@ -623,50 +642,15 @@ void via_isa_set_irq(PCIDevice *d, int pin, int level)
     qemu_set_irq(s->isa_irqs_in[isa_irq], !!s->isa_irq_state[isa_irq]);
 }
 
-static void via_isa_request_i8259_irq(void *opaque, int irq, int level)
-{
-    ViaISAState *s = opaque;
-    qemu_set_irq(s->cpu_intr, level);
-}
-
-static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
+static void via_isa_pirq(void *opaque, int pin, int level)
 {
-    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;
+    via_isa_set_irq(opaque, pin, level);
 }
 
-static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
+static void via_isa_request_i8259_irq(void *opaque, int irq, 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);
-
-    /* IRQ 0: disabled, IRQ 2,8,13: reserved */
-    if (!pic_irq) {
-        return;
-    }
-    if (unlikely(pic_irq == 2 || pic_irq == 8 || pic_irq == 13)) {
-        qemu_log_mask(LOG_GUEST_ERROR, "Invalid ISA IRQ routing");
-    }
-
-    /* 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);
+    qemu_set_irq(s->cpu_intr, level);
 }
 
 static void via_isa_realize(PCIDevice *d, Error **errp)
@@ -679,6 +663,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
     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);
     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
                           errp);
@@ -692,8 +677,6 @@ 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)) {
-- 
2.30.9

[PATCH 4/4] hw/audio/via-ac97: Route interrupts using via_isa_set_irq()

[BALATON Zoltan]

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/audio/via-ac97.c | 8 ++++----
 hw/isa/vt82c686.c   | 1 +
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/audio/via-ac97.c b/hw/audio/via-ac97.c
index 30095a4c7a..4c127a1def 100644
--- a/hw/audio/via-ac97.c
+++ b/hw/audio/via-ac97.c
@@ -211,14 +211,14 @@ static void out_cb(void *opaque, int avail)
                     AUD_set_active_out(s->vo, 0);
                 }
                 if (c->type & STAT_EOL) {
-                    pci_set_irq(&s->dev, 1);
+                    via_isa_set_irq(&s->dev, 0, 1);
                 }
             }
             if (CLEN_IS_FLAG(c)) {
                 c->stat |= STAT_FLAG;
                 c->stat |= STAT_PAUSED;
                 if (c->type & STAT_FLAG) {
-                    pci_set_irq(&s->dev, 1);
+                    via_isa_set_irq(&s->dev, 0, 1);
                 }
             }
             if (CLEN_IS_STOP(c)) {
@@ -305,13 +305,13 @@ static void sgd_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
         if (val & STAT_EOL) {
             s->aur.stat &= ~(STAT_EOL | STAT_PAUSED);
             if (s->aur.type & STAT_EOL) {
-                pci_set_irq(&s->dev, 0);
+                via_isa_set_irq(&s->dev, 0, 0);
             }
         }
         if (val & STAT_FLAG) {
             s->aur.stat &= ~(STAT_FLAG | STAT_PAUSED);
             if (s->aur.type & STAT_FLAG) {
-                pci_set_irq(&s->dev, 0);
+                via_isa_set_irq(&s->dev, 0, 0);
             }
         }
         break;
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 18cc2f653d..830aac7670 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -621,6 +621,7 @@ void via_isa_set_irq(PCIDevice *d, int pin, int level)
         break;
     case 2: /* USB ports 0-1 */
     case 3: /* USB ports 2-3 */
+    case 5: /* AC97 audio */
         max_irq = 14;
         break;
     }
-- 
2.30.9

Re: [PATCH 0/4] Fix IRQ routing in via south bridge

[Bernhard Beschow]

Am 28. Oktober 2023 23:56:21 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>This is going back to my otiginal proposal in
>https://patchew.org/QEMU/cover.1677004414.git.balaton@eik.bme.hu/
>implementing routing of interrupts from device functions and PCI
>devices to ISA interrupts. On pegasos2 the firmware sets evertyhing to
>share IRQ 9 so the current simpified version worked for taht but with
>the amigaone machine its firmware makes use of this feature and
>assigns different interrupts to functions and PCI devices so we need
>to properly impelent this. Since any ISA interrupt can be controlled
>by any interrupt source (different functions of the multifunction
>device plus the 4 input pins from PCI devices) there are more than 4
>possible sources so this can't be handled by just the 4 PCI interrupt
>lines. We need to keep track of the state of each interrupt source to
>be able to determine the level of the ISA interrupt and avoid one
>device clearing it while other still has an interrupt.
>
>This fixes USB on amigaone and maybe other bugs not discovered yet.

Amigaone's U-Boot maps the PCI IRQ pins to PIC IRQs 7,9,10,11. IRQ 7 seems to be the parallel port on ISA machines. The VIA hardware disables it by default (see index e2 in superio configuration registers) while it is enabled by default in our device models. Does this maybe cause an IRQ conflict, making the USB function unusable?

Best regards,
Bernhard

>
>Regards,
>BALATON Zoltan
>
>BALATON Zoltan (4):
>  hw/isa/vt82c686: Bring back via_isa_set_irq()
>  hw/usb/vt82c686-uhci-pci: Use ISA instead of PCI interrupts
>  hw/isa/vt82c686: Route PIRQ inputs using via_isa_set_irq()
>  hw/audio/via-ac97: Route interrupts using via_isa_set_irq()
>
> hw/audio/via-ac97.c        |  8 ++---
> hw/isa/vt82c686.c          | 67 +++++++++++++++++++++++---------------
> hw/usb/vt82c686-uhci-pci.c |  9 +++++
> include/hw/isa/vt82c686.h  |  2 ++
> 4 files changed, 56 insertions(+), 30 deletions(-)
>

Re: [PATCH 0/4] Fix IRQ routing in via south bridge

[BALATON Zoltan]

On Sun, 29 Oct 2023, Bernhard Beschow wrote:
> Am 28. Oktober 2023 23:56:21 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> This is going back to my otiginal proposal in
>> https://patchew.org/QEMU/cover.1677004414.git.balaton@eik.bme.hu/
>> implementing routing of interrupts from device functions and PCI
>> devices to ISA interrupts. On pegasos2 the firmware sets evertyhing to
>> share IRQ 9 so the current simpified version worked for taht but with
>> the amigaone machine its firmware makes use of this feature and
>> assigns different interrupts to functions and PCI devices so we need
>> to properly impelent this. Since any ISA interrupt can be controlled
>> by any interrupt source (different functions of the multifunction
>> device plus the 4 input pins from PCI devices) there are more than 4
>> possible sources so this can't be handled by just the 4 PCI interrupt
>> lines. We need to keep track of the state of each interrupt source to
>> be able to determine the level of the ISA interrupt and avoid one
>> device clearing it while other still has an interrupt.
>>
>> This fixes USB on amigaone and maybe other bugs not discovered yet.
>
> Amigaone's U-Boot maps the PCI IRQ pins to PIC IRQs 7,9,10,11. IRQ 7 
> seems to be the parallel port on ISA machines. The VIA hardware disables 
> it by default (see index e2 in superio configuration registers) while it 
> is enabled by default in our device models. Does this maybe cause an IRQ 
> conflict, making the USB function unusable?

Not likely because parellel port is not used and does not generate 
interrupts. It's just your current patch in master only maps PCI 
interrupts and does not correctly route interrupts from chip functions so 
the USB interrupts end up at the wrong ISA IRQ.

> Best regards,
> Bernhard
>
>>
>> Regards,
>> BALATON Zoltan
>>
>> BALATON Zoltan (4):
>>  hw/isa/vt82c686: Bring back via_isa_set_irq()
>>  hw/usb/vt82c686-uhci-pci: Use ISA instead of PCI interrupts
>>  hw/isa/vt82c686: Route PIRQ inputs using via_isa_set_irq()
>>  hw/audio/via-ac97: Route interrupts using via_isa_set_irq()
>>
>> hw/audio/via-ac97.c        |  8 ++---
>> hw/isa/vt82c686.c          | 67 +++++++++++++++++++++++---------------
>> hw/usb/vt82c686-uhci-pci.c |  9 +++++
>> include/hw/isa/vt82c686.h  |  2 ++
>> 4 files changed, 56 insertions(+), 30 deletions(-)
>>
>
>

Re: [PATCH 0/4] Fix IRQ routing in via south bridge

[Mark Cave-Ayland]

On 29/10/2023 00:56, BALATON Zoltan wrote:

> This is going back to my otiginal proposal in
> https://patchew.org/QEMU/cover.1677004414.git.balaton@eik.bme.hu/
> implementing routing of interrupts from device functions and PCI
> devices to ISA interrupts. On pegasos2 the firmware sets evertyhing to
> share IRQ 9 so the current simpified version worked for taht but with
> the amigaone machine its firmware makes use of this feature and
> assigns different interrupts to functions and PCI devices so we need
> to properly impelent this.

<quote>
> Since any ISA interrupt can be controlled
> by any interrupt source (different functions of the multifunction
> device plus the 4 input pins from PCI devices) there are more than 4
> possible sources so this can't be handled by just the 4 PCI interrupt
> lines. We need to keep track of the state of each interrupt source to
> be able to determine the level of the ISA interrupt and avoid one
> device clearing it while other still has an interrupt.
</quote>

This here is the important bit, since what you're describing here is exactly how PCI 
interrupts in QEMU work, and so is already handled by the existing PCI IRQ routing 
code. It seems to me that what you're doing here is creating an incomplete 
re-implementation of part of the PCI interrupt logic in isa_irq_state, which is a 
strong hint that this is the wrong approach and that you should be making use of PCI 
IRQ routing.

> This fixes USB on amigaone and maybe other bugs not discovered yet.

Given that the AmigaOne machine is new, can you explain in a bit more detail what the 
difference is between the Pegasos2 and AmigaOne machines, particularly with respect 
to where the existing IRQ routing logic is getting this wrong?


ATB,

Mark.

Re: [PATCH 0/4] Fix IRQ routing in via south bridge

[BALATON Zoltan]

On Sun, 29 Oct 2023, Mark Cave-Ayland wrote:
> On 29/10/2023 00:56, BALATON Zoltan wrote:
>
>> This is going back to my otiginal proposal in
>> https://patchew.org/QEMU/cover.1677004414.git.balaton@eik.bme.hu/
>> implementing routing of interrupts from device functions and PCI
>> devices to ISA interrupts. On pegasos2 the firmware sets evertyhing to
>> share IRQ 9 so the current simpified version worked for taht but with
>> the amigaone machine its firmware makes use of this feature and
>> assigns different interrupts to functions and PCI devices so we need
>> to properly impelent this.
>
> <quote>
>> Since any ISA interrupt can be controlled
>> by any interrupt source (different functions of the multifunction
>> device plus the 4 input pins from PCI devices) there are more than 4
>> possible sources so this can't be handled by just the 4 PCI interrupt
>> lines. We need to keep track of the state of each interrupt source to
>> be able to determine the level of the ISA interrupt and avoid one
>> device clearing it while other still has an interrupt.
> </quote>
>
> This here is the important bit, since what you're describing here is exactly 
> how PCI interrupts in QEMU work, and so is already handled by the existing 
> PCI IRQ routing code. It seems to me that what you're doing here is creating 
> an incomplete re-implementation of part of the PCI interrupt logic in 
> isa_irq_state, which is a strong hint that this is the wrong approach and 
> that you should be making use of PCI IRQ routing.

I don't see how this can be handled by the PCI interrupt routing which 
only considers 4 lines while in VIA we have more sources than that which 
are the chip functions (some even with more than one IRQ like IDE) and the 
4 PCI interrupt inputs and these can be routed to any of the ISA IRQs 
independently (there's a register for each of them, the funcs use thi 
interrupt line config reg and the PCI pins the regs in the ISA func). So 
how would PCI handle this when it only sees the 4 PCI interrupt lines but 
not the chip functions as separate sources? You've assumed that those 
functions must be PCI devices too but their interrupts are routable 
separately from the PCI interrupts so you can have PCI INTA-D mapped to 
IRQ 7,8,9,10 and USB func mapped to IRQ 5 (like amigaone does) so you 
can't use PCI interrupt for the USB too but have to consider all of these 
separately and route them in the ISA bridge.

>> This fixes USB on amigaone and maybe other bugs not discovered yet.
>
> Given that the AmigaOne machine is new, can you explain in a bit more detail 
> what the difference is between the Pegasos2 and AmigaOne machines, 
> particularly with respect to where the existing IRQ routing logic is getting 
> this wrong?

The pegasos2 firmware sets all chip functions and PCI devices (except IDE 
which is hard coded to legacy interrupts) to IRQ 9 so it worked with 
mixing PCI interrupts with chip functions but the amigaone does not do 
that and sets different ISA interrupts to chip functions and PCI 
interrupts so the current simplified version cannot work with that any 
more and we need to allow separate routing of all the interrupt sources. 
(Additionally we need to keep interrupt state for each source to allow 
multiple sources to control the same ISA interrupt.) I could not think of 
any simpler way than my patch to correctly implement this.

Regards,
BALATON Zoltan

Re: [PATCH 0/4] Fix IRQ routing in via south bridge

[Bernhard Beschow]

Am 29. Oktober 2023 11:35:27 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Sun, 29 Oct 2023, Bernhard Beschow wrote:
>> Am 28. Oktober 2023 23:56:21 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>>> This is going back to my otiginal proposal in
>>> https://patchew.org/QEMU/cover.1677004414.git.balaton@eik.bme.hu/
>>> implementing routing of interrupts from device functions and PCI
>>> devices to ISA interrupts. On pegasos2 the firmware sets evertyhing to
>>> share IRQ 9 so the current simpified version worked for taht but with
>>> the amigaone machine its firmware makes use of this feature and
>>> assigns different interrupts to functions and PCI devices so we need
>>> to properly impelent this. Since any ISA interrupt can be controlled
>>> by any interrupt source (different functions of the multifunction
>>> device plus the 4 input pins from PCI devices) there are more than 4
>>> possible sources so this can't be handled by just the 4 PCI interrupt
>>> lines. We need to keep track of the state of each interrupt source to
>>> be able to determine the level of the ISA interrupt and avoid one
>>> device clearing it while other still has an interrupt.
>>> 
>>> This fixes USB on amigaone and maybe other bugs not discovered yet.
>> 
>> Amigaone's U-Boot maps the PCI IRQ pins to PIC IRQs 7,9,10,11. IRQ 7 seems to be the parallel port on ISA machines. The VIA hardware disables it by default (see index e2 in superio configuration registers) while it is enabled by default in our device models. Does this maybe cause an IRQ conflict, making the USB function unusable?
>
>Not likely because parellel port is not used and does not generate interrupts. It's just your current patch in master only maps PCI interrupts and does not correctly route interrupts from chip functions so the USB interrupts end up at the wrong ISA IRQ.

Indeed. Even booting into a Linux guest doesn't generate "parallel*" trace logs.

Best regards,
Bernhard

>
>> Best regards,
>> Bernhard
>> 
>>> 
>>> Regards,
>>> BALATON Zoltan
>>> 
>>> BALATON Zoltan (4):
>>>  hw/isa/vt82c686: Bring back via_isa_set_irq()
>>>  hw/usb/vt82c686-uhci-pci: Use ISA instead of PCI interrupts
>>>  hw/isa/vt82c686: Route PIRQ inputs using via_isa_set_irq()
>>>  hw/audio/via-ac97: Route interrupts using via_isa_set_irq()
>>> 
>>> hw/audio/via-ac97.c        |  8 ++---
>>> hw/isa/vt82c686.c          | 67 +++++++++++++++++++++++---------------
>>> hw/usb/vt82c686-uhci-pci.c |  9 +++++
>>> include/hw/isa/vt82c686.h  |  2 ++
>>> 4 files changed, 56 insertions(+), 30 deletions(-)
>>> 
>> 
>> 

Re: [PATCH 0/4] Fix IRQ routing in via south bridge

[Mark Cave-Ayland]

On 29/10/2023 13:45, BALATON Zoltan wrote:

> On Sun, 29 Oct 2023, Mark Cave-Ayland wrote:
>> On 29/10/2023 00:56, BALATON Zoltan wrote:
>>
>>> This is going back to my otiginal proposal in
>>> https://patchew.org/QEMU/cover.1677004414.git.balaton@eik.bme.hu/
>>> implementing routing of interrupts from device functions and PCI
>>> devices to ISA interrupts. On pegasos2 the firmware sets evertyhing to
>>> share IRQ 9 so the current simpified version worked for taht but with
>>> the amigaone machine its firmware makes use of this feature and
>>> assigns different interrupts to functions and PCI devices so we need
>>> to properly impelent this.
>>
>> <quote>
>>> Since any ISA interrupt can be controlled
>>> by any interrupt source (different functions of the multifunction
>>> device plus the 4 input pins from PCI devices) there are more than 4
>>> possible sources so this can't be handled by just the 4 PCI interrupt
>>> lines. We need to keep track of the state of each interrupt source to
>>> be able to determine the level of the ISA interrupt and avoid one
>>> device clearing it while other still has an interrupt.
>> </quote>
>>
>> This here is the important bit, since what you're describing here is exactly how 
>> PCI interrupts in QEMU work, and so is already handled by the existing PCI IRQ 
>> routing code. It seems to me that what you're doing here is creating an incomplete 
>> re-implementation of part of the PCI interrupt logic in isa_irq_state, which is a 
>> strong hint that this is the wrong approach and that you should be making use of 
>> PCI IRQ routing.
> 
> I don't see how this can be handled by the PCI interrupt routing which only considers 
> 4 lines while in VIA we have more sources than that which are the chip functions 
> (some even with more than one IRQ like IDE) and the 4 PCI interrupt inputs and these 
> can be routed to any of the ISA IRQs independently (there's a register for each of 
> them, the funcs use thi interrupt line config reg and the PCI pins the regs in the 
> ISA func). So how would PCI handle this when it only sees the 4 PCI interrupt lines 
> but not the chip functions as separate sources? You've assumed that those functions 
> must be PCI devices too but their interrupts are routable separately from the PCI 
> interrupts so you can have PCI INTA-D mapped to IRQ 7,8,9,10 and USB func mapped to 
> IRQ 5 (like amigaone does) so you can't use PCI interrupt for the USB too but have to 
> consider all of these separately and route them in the ISA bridge.

Ah so the restriction here is the number of PCI interrupt lines? That can be done by 
increasing the number of PCI bus IRQs to 4 + N, where 0-3 represent INTA-D and the N 
others represent individual functions on the in-built devices. You can then determine 
the slot/function in the PCI map IRQ function to route to the appropriate N IRQ.

>>> This fixes USB on amigaone and maybe other bugs not discovered yet.
>>
>> Given that the AmigaOne machine is new, can you explain in a bit more detail what 
>> the difference is between the Pegasos2 and AmigaOne machines, particularly with 
>> respect to where the existing IRQ routing logic is getting this wrong?
> 
> The pegasos2 firmware sets all chip functions and PCI devices (except IDE which is 
> hard coded to legacy interrupts) to IRQ 9 so it worked with mixing PCI interrupts 
> with chip functions but the amigaone does not do that and sets different ISA 
> interrupts to chip functions and PCI interrupts so the current simplified version 
> cannot work with that any more and we need to allow separate routing of all the 
> interrupt sources. (Additionally we need to keep interrupt state for each source to 
> allow multiple sources to control the same ISA interrupt.) I could not think of any 
> simpler way than my patch to correctly implement this.

The key point of interest is that we have PIIX that basically already works using the 
existing PCI IRQ routing APIs: the aim is to do something similar with VIA, or to 
tweak the existing APIs if needed to make it possible. Otherwise you end up with the 
situation in this series in which you're effectively inventing a parallel form of PCI 
IRQ routing just for the VIA ISA bridge and hardcoding it into the in-built VIA devices.

The benefit of using the PCI IRQ routing APIs is that it is also possible to plug in 
the individual PCI device/functions using -device into any PCI bus for testing, which 
is something that is already done with PIIX-IDE.


ATB,

Mark.

Re: [PATCH 0/4] Fix IRQ routing in via south bridge

[BALATON Zoltan]

On Mon, 30 Oct 2023, Mark Cave-Ayland wrote:
> On 29/10/2023 13:45, BALATON Zoltan wrote:
>> On Sun, 29 Oct 2023, Mark Cave-Ayland wrote:
>>> On 29/10/2023 00:56, BALATON Zoltan wrote:
>>> 
>>>> This is going back to my otiginal proposal in
>>>> https://patchew.org/QEMU/cover.1677004414.git.balaton@eik.bme.hu/
>>>> implementing routing of interrupts from device functions and PCI
>>>> devices to ISA interrupts. On pegasos2 the firmware sets evertyhing to
>>>> share IRQ 9 so the current simpified version worked for taht but with
>>>> the amigaone machine its firmware makes use of this feature and
>>>> assigns different interrupts to functions and PCI devices so we need
>>>> to properly impelent this.
>>> 
>>> <quote>
>>>> Since any ISA interrupt can be controlled
>>>> by any interrupt source (different functions of the multifunction
>>>> device plus the 4 input pins from PCI devices) there are more than 4
>>>> possible sources so this can't be handled by just the 4 PCI interrupt
>>>> lines. We need to keep track of the state of each interrupt source to
>>>> be able to determine the level of the ISA interrupt and avoid one
>>>> device clearing it while other still has an interrupt.
>>> </quote>
>>> 
>>> This here is the important bit, since what you're describing here is 
>>> exactly how PCI interrupts in QEMU work, and so is already handled by the 
>>> existing PCI IRQ routing code. It seems to me that what you're doing here 
>>> is creating an incomplete re-implementation of part of the PCI interrupt 
>>> logic in isa_irq_state, which is a strong hint that this is the wrong 
>>> approach and that you should be making use of PCI IRQ routing.
>> 
>> I don't see how this can be handled by the PCI interrupt routing which only 
>> considers 4 lines while in VIA we have more sources than that which are the 
>> chip functions (some even with more than one IRQ like IDE) and the 4 PCI 
>> interrupt inputs and these can be routed to any of the ISA IRQs 
>> independently (there's a register for each of them, the funcs use thi 
>> interrupt line config reg and the PCI pins the regs in the ISA func). So 
>> how would PCI handle this when it only sees the 4 PCI interrupt lines but 
>> not the chip functions as separate sources? You've assumed that those 
>> functions must be PCI devices too but their interrupts are routable 
>> separately from the PCI interrupts so you can have PCI INTA-D mapped to IRQ 
>> 7,8,9,10 and USB func mapped to IRQ 5 (like amigaone does) so you can't use 
>> PCI interrupt for the USB too but have to consider all of these separately 
>> and route them in the ISA bridge.
>
> Ah so the restriction here is the number of PCI interrupt lines? That can be 
> done by increasing the number of PCI bus IRQs to 4 + N, where 0-3 represent 
> INTA-D and the N others represent individual functions on the in-built 
> devices. You can then determine the slot/function in the PCI map IRQ function 
> to route to the appropriate N IRQ.

I can't because the PCI bus is in the north bridge. This VIA south bridge 
is just a PCIDevice connected to a bus so it should not take over 
interrupt handling of that bus which it does not own like the piix seems 
to do. That seems much more hacky than my solution to model what the chip 
does and map internal interrupt sources to ISA interrupts. The PCI 
interrupts are just additional input pins on this chip it does not handle 
the PCI bus itself, that's in the north bridge outside of this device.

>>>> This fixes USB on amigaone and maybe other bugs not discovered yet.
>>> 
>>> Given that the AmigaOne machine is new, can you explain in a bit more 
>>> detail what the difference is between the Pegasos2 and AmigaOne machines, 
>>> particularly with respect to where the existing IRQ routing logic is 
>>> getting this wrong?
>> 
>> The pegasos2 firmware sets all chip functions and PCI devices (except IDE 
>> which is hard coded to legacy interrupts) to IRQ 9 so it worked with mixing 
>> PCI interrupts with chip functions but the amigaone does not do that and 
>> sets different ISA interrupts to chip functions and PCI interrupts so the 
>> current simplified version cannot work with that any more and we need to 
>> allow separate routing of all the interrupt sources. (Additionally we need 
>> to keep interrupt state for each source to allow multiple sources to 
>> control the same ISA interrupt.) I could not think of any simpler way than 
>> my patch to correctly implement this.
>
> The key point of interest is that we have PIIX that basically already works 
> using the existing PCI IRQ routing APIs: the aim is to do something similar 
> with VIA, or to tweak the existing APIs if needed to make it possible. 
> Otherwise you end up with the situation in this series in which you're 
> effectively inventing a parallel form of PCI IRQ routing just for the VIA ISA 
> bridge and hardcoding it into the in-built VIA devices.

I've looked at piix now but that seems to have less functions and those 
are probably PCI devices that only use PCI interrutps so you can just use 
PCI intrrupts there. It srill needs to keep track of interrupt state 
separately so piix also has code for that and piix replaces the interrupt 
callbacks of the bus the chip is connected to so it takes over that from 
the north bridge or whatever the pci bus is part of. That does not seem 
right to me and this may break the bus piix is connected to. A PCIDevice 
should not call pci_bus_irqs() IMO, only the part that owns the PCI bus or 
board code should set this but not a device. The pegasos2 already uses it 
to connect to PCI interrupts so VIA can't do that and should not do that. 
What I have is self contained and models the chip correctly. Why not 
change piix to do similarly or why do you insist these have to be the same 
when they are different chips with different quirks of their own?

> The benefit of using the PCI IRQ routing APIs is that it is also possible to 
> plug in the individual PCI device/functions using -device into any PCI bus 
> for testing, which is something that is already done with PIIX-IDE.

That does not seem like a useful goal to me. These VIA ide, usb and ac97 
parts are functions of the same chip so their models are part of the chip 
model. They may be different QOM objects to separate them but they aren't 
user creatable and should not be as these are part of the chip so it does 
not make sense to instantiate them separately. Then it's also not a 
problem to use VIA specific irq routing in these as they are part of the 
same model.

I think trying to force using the PCI irq mapping and setting code from 
PCIBus would not make the via model any better just make it more complex 
for no gain so I don't think that's a goal that should be followed. 
Besides not being possible to cleanly do that it would also make it more 
difficult to understand and debug interrupt routing later so I don't see 
what advantage would that have over having a self contained model of the 
chip even if piix manages to do it with PCIBus. But piix has only 4 
interrupt lines and I don't think what it does taking over PCI interrupts 
is a good idea so I don't want to go that direction. Having via and piix 
models work slightly differently is not a problem IMO as long as both are 
working and correct in themselves. There are other device models which 
implement similar devices yet do it differently and in this case piix and 
via are different in that via has more functions and their interrupts are 
routed separately so it does not fit well with what we have in PCIBus. 
Forcing it in that would just make it more complicated. These chips are 
also from different vendors so while it may make sense to model piix3 and 
piix4 similarly and sharing code (like vt82c686b and vt8231 are sharing 
code) it may not make that much sense to try to make piix and via similar 
when they may not be in reality other than both are collecting similar 
functions but their inner working may be different. The via chips seem 
more like an ISA superio chip that has some PCI support added while piix 
may more like a collection of separate PCI functions so their models may 
also be different in that.

Regards,
BALATON Zoltan

Re: [PATCH 0/4] Fix IRQ routing in via south bridge

[Mark Cave-Ayland]

On 30/10/2023 21:57, BALATON Zoltan wrote:

> On Mon, 30 Oct 2023, Mark Cave-Ayland wrote:
>> On 29/10/2023 13:45, BALATON Zoltan wrote:
>>> On Sun, 29 Oct 2023, Mark Cave-Ayland wrote:
>>>> On 29/10/2023 00:56, BALATON Zoltan wrote:
>>>>
>>>>> This is going back to my otiginal proposal in
>>>>> https://patchew.org/QEMU/cover.1677004414.git.balaton@eik.bme.hu/
>>>>> implementing routing of interrupts from device functions and PCI
>>>>> devices to ISA interrupts. On pegasos2 the firmware sets evertyhing to
>>>>> share IRQ 9 so the current simpified version worked for taht but with
>>>>> the amigaone machine its firmware makes use of this feature and
>>>>> assigns different interrupts to functions and PCI devices so we need
>>>>> to properly impelent this.
>>>>
>>>> <quote>
>>>>> Since any ISA interrupt can be controlled
>>>>> by any interrupt source (different functions of the multifunction
>>>>> device plus the 4 input pins from PCI devices) there are more than 4
>>>>> possible sources so this can't be handled by just the 4 PCI interrupt
>>>>> lines. We need to keep track of the state of each interrupt source to
>>>>> be able to determine the level of the ISA interrupt and avoid one
>>>>> device clearing it while other still has an interrupt.
>>>> </quote>
>>>>
>>>> This here is the important bit, since what you're describing here is exactly how 
>>>> PCI interrupts in QEMU work, and so is already handled by the existing PCI IRQ 
>>>> routing code. It seems to me that what you're doing here is creating an 
>>>> incomplete re-implementation of part of the PCI interrupt logic in isa_irq_state, 
>>>> which is a strong hint that this is the wrong approach and that you should be 
>>>> making use of PCI IRQ routing.
>>>
>>> I don't see how this can be handled by the PCI interrupt routing which only 
>>> considers 4 lines while in VIA we have more sources than that which are the chip 
>>> functions (some even with more than one IRQ like IDE) and the 4 PCI interrupt 
>>> inputs and these can be routed to any of the ISA IRQs independently (there's a 
>>> register for each of them, the funcs use thi interrupt line config reg and the PCI 
>>> pins the regs in the ISA func). So how would PCI handle this when it only sees the 
>>> 4 PCI interrupt lines but not the chip functions as separate sources? You've 
>>> assumed that those functions must be PCI devices too but their interrupts are 
>>> routable separately from the PCI interrupts so you can have PCI INTA-D mapped to 
>>> IRQ 7,8,9,10 and USB func mapped to IRQ 5 (like amigaone does) so you can't use 
>>> PCI interrupt for the USB too but have to consider all of these separately and 
>>> route them in the ISA bridge.
>>
>> Ah so the restriction here is the number of PCI interrupt lines? That can be done 
>> by increasing the number of PCI bus IRQs to 4 + N, where 0-3 represent INTA-D and 
>> the N others represent individual functions on the in-built devices. You can then 
>> determine the slot/function in the PCI map IRQ function to route to the appropriate 
>> N IRQ.
> 
> I can't because the PCI bus is in the north bridge. This VIA south bridge is just a 
> PCIDevice connected to a bus so it should not take over interrupt handling of that 
> bus which it does not own like the piix seems to do. That seems much more hacky than 
> my solution to model what the chip does and map internal interrupt sources to ISA 
> interrupts. The PCI interrupts are just additional input pins on this chip it does 
> not handle the PCI bus itself, that's in the north bridge outside of this device.

The PCI bus is only in the north bridge in your particular machine because you felt 
in your opinion it should be there, no? Certainly PIIX and ICH9 both use the south 
bridge because it allows PCI IRQ routing with existing APIs. Maybe that is the main 
reason why it is done this way?

I've added both the PCI and x86 maintainers to this email, since it clear we need 
input from developers with a deeper understanding of the PCI IRQ routing API and how 
it should work on x86. I'll attempt to summarise for them below:

   - Normally a PCIDevice uses pci_set_irq() to set its IRQ, and the routing is
     handled by a pci_map_irq_fn() with the ISA interrupt controller being driven
     through pci_route_irq_fn() e.g. piix_route_intx_pin_to_irq(). That is to say
     that the PCIDevice is simply responsible for asserting its own IRQ, and does
     not involve itself with IRQ routing.

   - The VIA PCI-ISA bridge needs a way to implement routing of PCI pins to ISA IRQs
     based upon the PCI devfn of the integrated device that raises its PCI IRQ. It
     doesn't appear there is currently an obvious way to do this.

   - Zoltan has a series that attempts to implement this, but it bypasses the
     existing PCI IRQ routing mechanism and uses a custom routing function that
     needs to added to each integrated VIA device. This instinctively feels
     wrong to me because it bypasses the existing PCI routing infrastructure, requires
     the VIA PCIDevices to be aware of their own routing, and prevents e.g.
     instantiating via-ide as a separate -device on the command line for testing, as
     already works with piix3-ide.

Questions for PCI and x86 maintainers:

    1) Is there a way to implement routing of PCI pins to ISA IRQs based upon the PCI
       devfn of the PCIDevice that asserts its IRQ?

    2) If not, how can it be implemented in a way that best complements/integrates
       with the existing PCI IRQ routing infrastructure? PIIX and ICH9 implementations
       show how it is possible to get very close to what is needed by VIA, but there
       may be maintainers with better ideas for implementing this in modern QEMU.

Zoltan: I suggest that you discuss this with the PCI/x86 maintainers in order to 
determine a suitable way forward.

>>>>> This fixes USB on amigaone and maybe other bugs not discovered yet.
>>>>
>>>> Given that the AmigaOne machine is new, can you explain in a bit more detail what 
>>>> the difference is between the Pegasos2 and AmigaOne machines, particularly with 
>>>> respect to where the existing IRQ routing logic is getting this wrong?
>>>
>>> The pegasos2 firmware sets all chip functions and PCI devices (except IDE which is 
>>> hard coded to legacy interrupts) to IRQ 9 so it worked with mixing PCI interrupts 
>>> with chip functions but the amigaone does not do that and sets different ISA 
>>> interrupts to chip functions and PCI interrupts so the current simplified version 
>>> cannot work with that any more and we need to allow separate routing of all the 
>>> interrupt sources. (Additionally we need to keep interrupt state for each source 
>>> to allow multiple sources to control the same ISA interrupt.) I could not think of 
>>> any simpler way than my patch to correctly implement this.
>>
>> The key point of interest is that we have PIIX that basically already works using 
>> the existing PCI IRQ routing APIs: the aim is to do something similar with VIA, or 
>> to tweak the existing APIs if needed to make it possible. Otherwise you end up with 
>> the situation in this series in which you're effectively inventing a parallel form 
>> of PCI IRQ routing just for the VIA ISA bridge and hardcoding it into the in-built 
>> VIA devices.
> 
> I've looked at piix now but that seems to have less functions and those are probably 
> PCI devices that only use PCI interrutps so you can just use PCI intrrupts there. It 
> srill needs to keep track of interrupt state separately so piix also has code for 
> that and piix replaces the interrupt callbacks of the bus the chip is connected to so 
> it takes over that from the north bridge or whatever the pci bus is part of. That 
> does not seem right to me and this may break the bus piix is connected to. A 
> PCIDevice should not call pci_bus_irqs() IMO, only the part that owns the PCI bus or 
> board code should set this but not a device. The pegasos2 already uses it to connect 
> to PCI interrupts so VIA can't do that and should not do that. What I have is self 
> contained and models the chip correctly. Why not change piix to do similarly or why 
> do you insist these have to be the same when they are different chips with different 
> quirks of their own?
> 
>> The benefit of using the PCI IRQ routing APIs is that it is also possible to plug 
>> in the individual PCI device/functions using -device into any PCI bus for testing, 
>> which is something that is already done with PIIX-IDE.
> 
> That does not seem like a useful goal to me. These VIA ide, usb and ac97 parts are 
> functions of the same chip so their models are part of the chip model. They may be 
> different QOM objects to separate them but they aren't user creatable and should not 
> be as these are part of the chip so it does not make sense to instantiate them 
> separately. Then it's also not a problem to use VIA specific irq routing in these as 
> they are part of the same model.
> 
> I think trying to force using the PCI irq mapping and setting code from PCIBus would 
> not make the via model any better just make it more complex for no gain so I don't 
> think that's a goal that should be followed. Besides not being possible to cleanly do 
> that it would also make it more difficult to understand and debug interrupt routing 
> later so I don't see what advantage would that have over having a self contained 
> model of the chip even if piix manages to do it with PCIBus. But piix has only 4 
> interrupt lines and I don't think what it does taking over PCI interrupts is a good 
> idea so I don't want to go that direction. Having via and piix models work slightly 
> differently is not a problem IMO as long as both are working and correct in 
> themselves. There are other device models which implement similar devices yet do it 
> differently and in this case piix and via are different in that via has more 
> functions and their interrupts are routed separately so it does not fit well with 
> what we have in PCIBus. Forcing it in that would just make it more complicated. These 
> chips are also from different vendors so while it may make sense to model piix3 and 
> piix4 similarly and sharing code (like vt82c686b and vt8231 are sharing code) it may 
> not make that much sense to try to make piix and via similar when they may not be in 
> reality other than both are collecting similar functions but their inner working may 
> be different. The via chips seem more like an ISA superio chip that has some PCI 
> support added while piix may more like a collection of separate PCI functions so 
> their models may also be different in that.


ATB,

Mark.

Re: [PATCH 0/4] Fix IRQ routing in via south bridge

[BALATON Zoltan]

On Tue, 31 Oct 2023, Mark Cave-Ayland wrote:
> On 30/10/2023 21:57, BALATON Zoltan wrote:
>> On Mon, 30 Oct 2023, Mark Cave-Ayland wrote:
>>> On 29/10/2023 13:45, BALATON Zoltan wrote:
>>>> On Sun, 29 Oct 2023, Mark Cave-Ayland wrote:
>>>>> On 29/10/2023 00:56, BALATON Zoltan wrote:
>>>>> 
>>>>>> This is going back to my otiginal proposal in
>>>>>> https://patchew.org/QEMU/cover.1677004414.git.balaton@eik.bme.hu/
>>>>>> implementing routing of interrupts from device functions and PCI
>>>>>> devices to ISA interrupts. On pegasos2 the firmware sets evertyhing to
>>>>>> share IRQ 9 so the current simpified version worked for taht but with
>>>>>> the amigaone machine its firmware makes use of this feature and
>>>>>> assigns different interrupts to functions and PCI devices so we need
>>>>>> to properly impelent this.
>>>>> 
>>>>> <quote>
>>>>>> Since any ISA interrupt can be controlled
>>>>>> by any interrupt source (different functions of the multifunction
>>>>>> device plus the 4 input pins from PCI devices) there are more than 4
>>>>>> possible sources so this can't be handled by just the 4 PCI interrupt
>>>>>> lines. We need to keep track of the state of each interrupt source to
>>>>>> be able to determine the level of the ISA interrupt and avoid one
>>>>>> device clearing it while other still has an interrupt.
>>>>> </quote>
>>>>> 
>>>>> This here is the important bit, since what you're describing here is 
>>>>> exactly how PCI interrupts in QEMU work, and so is already handled by 
>>>>> the existing PCI IRQ routing code. It seems to me that what you're doing 
>>>>> here is creating an incomplete re-implementation of part of the PCI 
>>>>> interrupt logic in isa_irq_state, which is a strong hint that this is 
>>>>> the wrong approach and that you should be making use of PCI IRQ routing.
>>>> 
>>>> I don't see how this can be handled by the PCI interrupt routing which 
>>>> only considers 4 lines while in VIA we have more sources than that which 
>>>> are the chip functions (some even with more than one IRQ like IDE) and 
>>>> the 4 PCI interrupt inputs and these can be routed to any of the ISA IRQs 
>>>> independently (there's a register for each of them, the funcs use thi 
>>>> interrupt line config reg and the PCI pins the regs in the ISA func). So 
>>>> how would PCI handle this when it only sees the 4 PCI interrupt lines but 
>>>> not the chip functions as separate sources? You've assumed that those 
>>>> functions must be PCI devices too but their interrupts are routable 
>>>> separately from the PCI interrupts so you can have PCI INTA-D mapped to 
>>>> IRQ 7,8,9,10 and USB func mapped to IRQ 5 (like amigaone does) so you 
>>>> can't use PCI interrupt for the USB too but have to consider all of these 
>>>> separately and route them in the ISA bridge.
>>> 
>>> Ah so the restriction here is the number of PCI interrupt lines? That can 
>>> be done by increasing the number of PCI bus IRQs to 4 + N, where 0-3 
>>> represent INTA-D and the N others represent individual functions on the 
>>> in-built devices. You can then determine the slot/function in the PCI map 
>>> IRQ function to route to the appropriate N IRQ.
>> 
>> I can't because the PCI bus is in the north bridge. This VIA south bridge 
>> is just a PCIDevice connected to a bus so it should not take over interrupt 
>> handling of that bus which it does not own like the piix seems to do. That 
>> seems much more hacky than my solution to model what the chip does and map 
>> internal interrupt sources to ISA interrupts. The PCI interrupts are just 
>> additional input pins on this chip it does not handle the PCI bus itself, 
>> that's in the north bridge outside of this device.
>
> The PCI bus is only in the north bridge in your particular machine because 
> you felt in your opinion it should be there, no?

No. It's there because in real hardware the PCI busses are provided by the 
north bridge which is the PCI host so it should register and own the PCI 
busses. The south bridge is a PCIDevice that's connected to one of these 
buses. It's only involved in PCI interrupt handling in that it contains 
the ISA PICs which already handles the internal functions and has 4 inputs 
for PCI interrupts that it can also route to ISA interrupts. In pegasos2 
the PCU interrupts are connected to both these inputs on the VIA south 
bridge and to CPIO intputs on the north bridge and both can be programmed 
to generate interrupts to CPU with their own registers. Some guests may 
use one way while others the other way. All of this is now modelled 
correctly.

> Certainly PIIX and ICH9 both 
> use the south bridge because it allows PCI IRQ routing with existing APIs. 
> Maybe that is the main reason why it is done this way?

I'm not sure what you mean but piix seems to call pci_bus_irqs() in its 
realize method thus replacing IRQ handling of the bus it is connected to 
which I think is wrong. If you plug this in a machine that has its 
interrupt routing this will break that so I think PCI devices should not 
do that, only the pci host or board code should wire this up.

> I've added both the PCI and x86 maintainers to this email, since it clear we 
> need input from developers with a deeper understanding of the PCI IRQ routing 
> API and how it should work on x86. I'll attempt to summarise for them below:
>
>  - Normally a PCIDevice uses pci_set_irq() to set its IRQ, and the routing 
> is
>    handled by a pci_map_irq_fn() with the ISA interrupt controller being 
> driven
>    through pci_route_irq_fn() e.g. piix_route_intx_pin_to_irq(). That is to 
> say
>    that the PCIDevice is simply responsible for asserting its own IRQ, and 
> does
>    not involve itself with IRQ routing.
>
>  - The VIA PCI-ISA bridge needs a way to implement routing of PCI pins to 
> ISA IRQs
>    based upon the PCI devfn of the integrated device that raises its PCI 
> IRQ. It
>    doesn't appear there is currently an obvious way to do this.
>
>  - Zoltan has a series that attempts to implement this, but it bypasses the
>    existing PCI IRQ routing mechanism and uses a custom routing function 
> that
>    needs to added to each integrated VIA device. This instinctively feels
>    wrong to me because it bypasses the existing PCI routing infrastructure, 
> requires
>    the VIA PCIDevices to be aware of their own routing, and prevents e.g.
>    instantiating via-ide as a separate -device on the command line for 
> testing, as
>    already works with piix3-ide.
>
> Questions for PCI and x86 maintainers:
>
>   1) Is there a way to implement routing of PCI pins to ISA IRQs based upon 
> the PCI
>      devfn of the PCIDevice that asserts its IRQ?
>
>   2) If not, how can it be implemented in a way that best 
> complements/integrates
>      with the existing PCI IRQ routing infrastructure? PIIX and ICH9 
> implementations
>      show how it is possible to get very close to what is needed by VIA, but 
> there
>      may be maintainers with better ideas for implementing this in modern 
> QEMU.
>
> Zoltan: I suggest that you discuss this with the PCI/x86 maintainers in order 
> to determine a suitable way forward.

We could discuss this but I think what you propose is not only unnecessary 
but also wrong:

- PCIDevice should not replace IRQ handling of the bus it's plugged in.

- There are more IRQ sources to route here than the 4 PCI lines (we have 
each chip function (IDE, 2xUSB, PM, AC97 + the 4 PCI IRQ inputs and maybe 
some more we don't model yet) so this does not fit in the PCIBus interrupt 
handling that only concerns the 4 PCU interrupt lines. Your proposal to 
increase number of PCI interrupts to overcome this deviates from real 
hardware and tries to shoehorn ISA interrupt routing into PCIBus where it 
does not belong. Your rationale is code reuse but you're trying to reuse 
here something that's not doing what is needed.

- Modelling it in the ISA bridge is straightforward and easier to 
understand and matches the hardware, no tricks is needed to model 
something with PCI that isn't a PCI device.

- The internal functions aren't independent PCI devices, they are part of 
the chip and only modelled as PCIDevice because they appear as functions 
of the south bridge chip but they are connected internally and clearly use 
registers in a non-standard way, e.g. using PCI_INTERRUPT_LINE to decide 
which ISA interrupt they should raise. These devices aren't user creatable 
as they are part of the south bdidge chip model (only split off to break 
up the device model into objects) so then using via_isa_set_irq instead of 
pci_set_irq is also not a problem as these functions only work as part of 
the VIA south bridge so no point in trying to make them standalone PCI 
devices when they are not. That just unnecessarily complicates code 
without any useful advantage. These devices don't exist standalone so 
there's no point in allowing them to be instantiated without the south 
bridge they are part of.

Lastly, accepting my solution now does not make it impossible to change it 
later when this discussion is finished. If we find another way to make it 
simpler we could do that but so far I don't think what you're pushing here 
would make it simpler or better so I'm yet to be convinced about that.

Regerds,
BALATON Zoltan

>>>>>> This fixes USB on amigaone and maybe other bugs not discovered yet.
>>>>> 
>>>>> Given that the AmigaOne machine is new, can you explain in a bit more 
>>>>> detail what the difference is between the Pegasos2 and AmigaOne 
>>>>> machines, particularly with respect to where the existing IRQ routing 
>>>>> logic is getting this wrong?
>>>> 
>>>> The pegasos2 firmware sets all chip functions and PCI devices (except IDE 
>>>> which is hard coded to legacy interrupts) to IRQ 9 so it worked with 
>>>> mixing PCI interrupts with chip functions but the amigaone does not do 
>>>> that and sets different ISA interrupts to chip functions and PCI 
>>>> interrupts so the current simplified version cannot work with that any 
>>>> more and we need to allow separate routing of all the interrupt sources. 
>>>> (Additionally we need to keep interrupt state for each source to allow 
>>>> multiple sources to control the same ISA interrupt.) I could not think of 
>>>> any simpler way than my patch to correctly implement this.
>>> 
>>> The key point of interest is that we have PIIX that basically already 
>>> works using the existing PCI IRQ routing APIs: the aim is to do something 
>>> similar with VIA, or to tweak the existing APIs if needed to make it 
>>> possible. Otherwise you end up with the situation in this series in which 
>>> you're effectively inventing a parallel form of PCI IRQ routing just for 
>>> the VIA ISA bridge and hardcoding it into the in-built VIA devices.
>> 
>> I've looked at piix now but that seems to have less functions and those are 
>> probably PCI devices that only use PCI interrutps so you can just use PCI 
>> intrrupts there. It srill needs to keep track of interrupt state separately 
>> so piix also has code for that and piix replaces the interrupt callbacks of 
>> the bus the chip is connected to so it takes over that from the north 
>> bridge or whatever the pci bus is part of. That does not seem right to me 
>> and this may break the bus piix is connected to. A PCIDevice should not 
>> call pci_bus_irqs() IMO, only the part that owns the PCI bus or board code 
>> should set this but not a device. The pegasos2 already uses it to connect 
>> to PCI interrupts so VIA can't do that and should not do that. What I have 
>> is self contained and models the chip correctly. Why not change piix to do 
>> similarly or why do you insist these have to be the same when they are 
>> different chips with different quirks of their own?
>> 
>>> The benefit of using the PCI IRQ routing APIs is that it is also possible 
>>> to plug in the individual PCI device/functions using -device into any PCI 
>>> bus for testing, which is something that is already done with PIIX-IDE.
>> 
>> That does not seem like a useful goal to me. These VIA ide, usb and ac97 
>> parts are functions of the same chip so their models are part of the chip 
>> model. They may be different QOM objects to separate them but they aren't 
>> user creatable and should not be as these are part of the chip so it does 
>> not make sense to instantiate them separately. Then it's also not a problem 
>> to use VIA specific irq routing in these as they are part of the same 
>> model.
>> 
>> I think trying to force using the PCI irq mapping and setting code from 
>> PCIBus would not make the via model any better just make it more complex 
>> for no gain so I don't think that's a goal that should be followed. Besides 
>> not being possible to cleanly do that it would also make it more difficult 
>> to understand and debug interrupt routing later so I don't see what 
>> advantage would that have over having a self contained model of the chip 
>> even if piix manages to do it with PCIBus. But piix has only 4 interrupt 
>> lines and I don't think what it does taking over PCI interrupts is a good 
>> idea so I don't want to go that direction. Having via and piix models work 
>> slightly differently is not a problem IMO as long as both are working and 
>> correct in themselves. There are other device models which implement 
>> similar devices yet do it differently and in this case piix and via are 
>> different in that via has more functions and their interrupts are routed 
>> separately so it does not fit well with what we have in PCIBus. Forcing it 
>> in that would just make it more complicated. These chips are also from 
>> different vendors so while it may make sense to model piix3 and piix4 
>> similarly and sharing code (like vt82c686b and vt8231 are sharing code) it 
>> may not make that much sense to try to make piix and via similar when they 
>> may not be in reality other than both are collecting similar functions but 
>> their inner working may be different. The via chips seem more like an ISA 
>> superio chip that has some PCI support added while piix may more like a 
>> collection of separate PCI functions so their models may also be different 
>> in that.
>
>
> ATB,
>
> Mark.
>
>

Re: [PATCH 0/4] Fix IRQ routing in via south bridge

[BALATON Zoltan]

On Tue, 31 Oct 2023, BALATON Zoltan wrote:
> On Tue, 31 Oct 2023, Mark Cave-Ayland wrote:
>> On 30/10/2023 21:57, BALATON Zoltan wrote:
>>> On Mon, 30 Oct 2023, Mark Cave-Ayland wrote:
>>>> On 29/10/2023 13:45, BALATON Zoltan wrote:
>>>>> On Sun, 29 Oct 2023, Mark Cave-Ayland wrote:
>>>>>> On 29/10/2023 00:56, BALATON Zoltan wrote:
>>>>>> 
>>>>>>> This is going back to my otiginal proposal in
>>>>>>> https://patchew.org/QEMU/cover.1677004414.git.balaton@eik.bme.hu/
>>>>>>> implementing routing of interrupts from device functions and PCI
>>>>>>> devices to ISA interrupts. On pegasos2 the firmware sets evertyhing to
>>>>>>> share IRQ 9 so the current simpified version worked for taht but with
>>>>>>> the amigaone machine its firmware makes use of this feature and
>>>>>>> assigns different interrupts to functions and PCI devices so we need
>>>>>>> to properly impelent this.
>>>>>> 
>>>>>> <quote>
>>>>>>> Since any ISA interrupt can be controlled
>>>>>>> by any interrupt source (different functions of the multifunction
>>>>>>> device plus the 4 input pins from PCI devices) there are more than 4
>>>>>>> possible sources so this can't be handled by just the 4 PCI interrupt
>>>>>>> lines. We need to keep track of the state of each interrupt source to
>>>>>>> be able to determine the level of the ISA interrupt and avoid one
>>>>>>> device clearing it while other still has an interrupt.
>>>>>> </quote>
>>>>>> 
>>>>>> This here is the important bit, since what you're describing here is 
>>>>>> exactly how PCI interrupts in QEMU work, and so is already handled by 
>>>>>> the existing PCI IRQ routing code. It seems to me that what you're 
>>>>>> doing here is creating an incomplete re-implementation of part of the 
>>>>>> PCI interrupt logic in isa_irq_state, which is a strong hint that this 
>>>>>> is the wrong approach and that you should be making use of PCI IRQ 
>>>>>> routing.
>>>>> 
>>>>> I don't see how this can be handled by the PCI interrupt routing which 
>>>>> only considers 4 lines while in VIA we have more sources than that which 
>>>>> are the chip functions (some even with more than one IRQ like IDE) and 
>>>>> the 4 PCI interrupt inputs and these can be routed to any of the ISA 
>>>>> IRQs independently (there's a register for each of them, the funcs use 
>>>>> thi interrupt line config reg and the PCI pins the regs in the ISA 
>>>>> func). So how would PCI handle this when it only sees the 4 PCI 
>>>>> interrupt lines but not the chip functions as separate sources? You've 
>>>>> assumed that those functions must be PCI devices too but their 
>>>>> interrupts are routable separately from the PCI interrupts so you can 
>>>>> have PCI INTA-D mapped to IRQ 7,8,9,10 and USB func mapped to IRQ 5 
>>>>> (like amigaone does) so you can't use PCI interrupt for the USB too but 
>>>>> have to consider all of these separately and route them in the ISA 
>>>>> bridge.
>>>> 
>>>> Ah so the restriction here is the number of PCI interrupt lines? That can 
>>>> be done by increasing the number of PCI bus IRQs to 4 + N, where 0-3 
>>>> represent INTA-D and the N others represent individual functions on the 
>>>> in-built devices. You can then determine the slot/function in the PCI map 
>>>> IRQ function to route to the appropriate N IRQ.
>>> 
>>> I can't because the PCI bus is in the north bridge. This VIA south bridge 
>>> is just a PCIDevice connected to a bus so it should not take over 
>>> interrupt handling of that bus which it does not own like the piix seems 
>>> to do. That seems much more hacky than my solution to model what the chip 
>>> does and map internal interrupt sources to ISA interrupts. The PCI 
>>> interrupts are just additional input pins on this chip it does not handle 
>>> the PCI bus itself, that's in the north bridge outside of this device.
>> 
>> The PCI bus is only in the north bridge in your particular machine because 
>> you felt in your opinion it should be there, no?
>
> No. It's there because in real hardware the PCI busses are provided by the 
> north bridge which is the PCI host so it should register and own the PCI 
> busses. The south bridge is a PCIDevice that's connected to one of these 
> buses. It's only involved in PCI interrupt handling in that it contains the 
> ISA PICs which already handles the internal functions and has 4 inputs for 
> PCI interrupts that it can also route to ISA interrupts. In pegasos2 the PCU 
> interrupts are connected to both these inputs on the VIA south bridge and to 
> CPIO intputs on the north bridge and both can be programmed to generate 
> interrupts to CPU with their own registers. Some guests may use one way while 
> others the other way. All of this is now modelled correctly.
>
>> Certainly PIIX and ICH9 both use the south bridge because it allows PCI IRQ 
>> routing with existing APIs. Maybe that is the main reason why it is done 
>> this way?
>
> I'm not sure what you mean but piix seems to call pci_bus_irqs() in its 
> realize method thus replacing IRQ handling of the bus it is connected to 
> which I think is wrong. If you plug this in a machine that has its interrupt 
> routing this will break that so I think PCI devices should not do that, only 
> the pci host or board code should wire this up.
>
>> I've added both the PCI and x86 maintainers to this email, since it clear 
>> we need input from developers with a deeper understanding of the PCI IRQ 
>> routing API and how it should work on x86. I'll attempt to summarise for

The machines discussed here (amigaone and pegasos2) aren't x86 and they 
may work differently than a PC using PIIX or ICH9 therefore I'm not sure 
what those do is relevant here. Here are some docs for reference about the 
north bridges these machines use where you can see that PCI is part of 
those and not the sourh bridge which may be the case on PCs where the 
south bridge may be more involved in PCI and north bridge is only memory 
controller there but that's not the case here so south bridge should not 
do PCI handling.

http://obligement.free.fr/files/articia_s.pdf
https://static6.arrow.com/aropdfconversion/2e73cb3a884e8a2fe165974fb7fefef556d12c18/disciipower.pdf

Regards,
BALATON Zoltan

>> them below:
>>
>>  - Normally a PCIDevice uses pci_set_irq() to set its IRQ, and the routing 
>> is
>>    handled by a pci_map_irq_fn() with the ISA interrupt controller being 
>> driven
>>    through pci_route_irq_fn() e.g. piix_route_intx_pin_to_irq(). That is to 
>> say
>>    that the PCIDevice is simply responsible for asserting its own IRQ, and 
>> does
>>    not involve itself with IRQ routing.
>>
>>  - The VIA PCI-ISA bridge needs a way to implement routing of PCI pins to 
>> ISA IRQs
>>    based upon the PCI devfn of the integrated device that raises its PCI 
>> IRQ. It
>>    doesn't appear there is currently an obvious way to do this.
>>
>>  - Zoltan has a series that attempts to implement this, but it bypasses the
>>    existing PCI IRQ routing mechanism and uses a custom routing function 
>> that
>>    needs to added to each integrated VIA device. This instinctively feels
>>    wrong to me because it bypasses the existing PCI routing infrastructure, 
>> requires
>>    the VIA PCIDevices to be aware of their own routing, and prevents e.g.
>>    instantiating via-ide as a separate -device on the command line for 
>> testing, as
>>    already works with piix3-ide.
>> 
>> Questions for PCI and x86 maintainers:
>>
>>   1) Is there a way to implement routing of PCI pins to ISA IRQs based upon 
>> the PCI
>>      devfn of the PCIDevice that asserts its IRQ?
>>
>>   2) If not, how can it be implemented in a way that best 
>> complements/integrates
>>      with the existing PCI IRQ routing infrastructure? PIIX and ICH9 
>> implementations
>>      show how it is possible to get very close to what is needed by VIA, 
>> but there
>>      may be maintainers with better ideas for implementing this in modern 
>> QEMU.
>> 
>> Zoltan: I suggest that you discuss this with the PCI/x86 maintainers in 
>> order to determine a suitable way forward.
>
> We could discuss this but I think what you propose is not only unnecessary 
> but also wrong:
>
> - PCIDevice should not replace IRQ handling of the bus it's plugged in.
>
> - There are more IRQ sources to route here than the 4 PCI lines (we have each 
> chip function (IDE, 2xUSB, PM, AC97 + the 4 PCI IRQ inputs and maybe some 
> more we don't model yet) so this does not fit in the PCIBus interrupt 
> handling that only concerns the 4 PCU interrupt lines. Your proposal to 
> increase number of PCI interrupts to overcome this deviates from real 
> hardware and tries to shoehorn ISA interrupt routing into PCIBus where it 
> does not belong. Your rationale is code reuse but you're trying to reuse here 
> something that's not doing what is needed.
>
> - Modelling it in the ISA bridge is straightforward and easier to understand 
> and matches the hardware, no tricks is needed to model something with PCI 
> that isn't a PCI device.
>
> - The internal functions aren't independent PCI devices, they are part of the 
> chip and only modelled as PCIDevice because they appear as functions of the 
> south bridge chip but they are connected internally and clearly use registers 
> in a non-standard way, e.g. using PCI_INTERRUPT_LINE to decide which ISA 
> interrupt they should raise. These devices aren't user creatable as they are 
> part of the south bdidge chip model (only split off to break up the device 
> model into objects) so then using via_isa_set_irq instead of pci_set_irq is 
> also not a problem as these functions only work as part of the VIA south 
> bridge so no point in trying to make them standalone PCI devices when they 
> are not. That just unnecessarily complicates code without any useful 
> advantage. These devices don't exist standalone so there's no point in 
> allowing them to be instantiated without the south bridge they are part of.
>
> Lastly, accepting my solution now does not make it impossible to change it 
> later when this discussion is finished. If we find another way to make it 
> simpler we could do that but so far I don't think what you're pushing here 
> would make it simpler or better so I'm yet to be convinced about that.
>
> Regerds,
> BALATON Zoltan
>
>>>>>>> This fixes USB on amigaone and maybe other bugs not discovered yet.
>>>>>> 
>>>>>> Given that the AmigaOne machine is new, can you explain in a bit more 
>>>>>> detail what the difference is between the Pegasos2 and AmigaOne 
>>>>>> machines, particularly with respect to where the existing IRQ routing 
>>>>>> logic is getting this wrong?
>>>>> 
>>>>> The pegasos2 firmware sets all chip functions and PCI devices (except 
>>>>> IDE which is hard coded to legacy interrupts) to IRQ 9 so it worked with 
>>>>> mixing PCI interrupts with chip functions but the amigaone does not do 
>>>>> that and sets different ISA interrupts to chip functions and PCI 
>>>>> interrupts so the current simplified version cannot work with that any 
>>>>> more and we need to allow separate routing of all the interrupt sources. 
>>>>> (Additionally we need to keep interrupt state for each source to allow 
>>>>> multiple sources to control the same ISA interrupt.) I could not think 
>>>>> of any simpler way than my patch to correctly implement this.
>>>> 
>>>> The key point of interest is that we have PIIX that basically already 
>>>> works using the existing PCI IRQ routing APIs: the aim is to do something 
>>>> similar with VIA, or to tweak the existing APIs if needed to make it 
>>>> possible. Otherwise you end up with the situation in this series in which 
>>>> you're effectively inventing a parallel form of PCI IRQ routing just for 
>>>> the VIA ISA bridge and hardcoding it into the in-built VIA devices.
>>> 
>>> I've looked at piix now but that seems to have less functions and those 
>>> are probably PCI devices that only use PCI interrutps so you can just use 
>>> PCI intrrupts there. It srill needs to keep track of interrupt state 
>>> separately so piix also has code for that and piix replaces the interrupt 
>>> callbacks of the bus the chip is connected to so it takes over that from 
>>> the north bridge or whatever the pci bus is part of. That does not seem 
>>> right to me and this may break the bus piix is connected to. A PCIDevice 
>>> should not call pci_bus_irqs() IMO, only the part that owns the PCI bus or 
>>> board code should set this but not a device. The pegasos2 already uses it 
>>> to connect to PCI interrupts so VIA can't do that and should not do that. 
>>> What I have is self contained and models the chip correctly. Why not 
>>> change piix to do similarly or why do you insist these have to be the same 
>>> when they are different chips with different quirks of their own?
>>> 
>>>> The benefit of using the PCI IRQ routing APIs is that it is also possible 
>>>> to plug in the individual PCI device/functions using -device into any PCI 
>>>> bus for testing, which is something that is already done with PIIX-IDE.
>>> 
>>> That does not seem like a useful goal to me. These VIA ide, usb and ac97 
>>> parts are functions of the same chip so their models are part of the chip 
>>> model. They may be different QOM objects to separate them but they aren't 
>>> user creatable and should not be as these are part of the chip so it does 
>>> not make sense to instantiate them separately. Then it's also not a 
>>> problem to use VIA specific irq routing in these as they are part of the 
>>> same model.
>>> 
>>> I think trying to force using the PCI irq mapping and setting code from 
>>> PCIBus would not make the via model any better just make it more complex 
>>> for no gain so I don't think that's a goal that should be followed. 
>>> Besides not being possible to cleanly do that it would also make it more 
>>> difficult to understand and debug interrupt routing later so I don't see 
>>> what advantage would that have over having a self contained model of the 
>>> chip even if piix manages to do it with PCIBus. But piix has only 4 
>>> interrupt lines and I don't think what it does taking over PCI interrupts 
>>> is a good idea so I don't want to go that direction. Having via and piix 
>>> models work slightly differently is not a problem IMO as long as both are 
>>> working and correct in themselves. There are other device models which 
>>> implement similar devices yet do it differently and in this case piix and 
>>> via are different in that via has more functions and their interrupts are 
>>> routed separately so it does not fit well with what we have in PCIBus. 
>>> Forcing it in that would just make it more complicated. These chips are 
>>> also from different vendors so while it may make sense to model piix3 and 
>>> piix4 similarly and sharing code (like vt82c686b and vt8231 are sharing 
>>> code) it may not make that much sense to try to make piix and via similar 
>>> when they may not be in reality other than both are collecting similar 
>>> functions but their inner working may be different. The via chips seem 
>>> more like an ISA superio chip that has some PCI support added while piix 
>>> may more like a collection of separate PCI functions so their models may 
>>> also be different in that.
>> 
>> 
>> ATB,
>> 
>> Mark.
>> 
>> 
>
>