All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Fix IRQ routing in via south bridge
@ 2023-11-26 22:49 BALATON Zoltan
  2023-11-26 22:49 ` [PATCH v3 1/4] hw/isa/vt82c686: Bring back via_isa_set_irq() BALATON Zoltan
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: BALATON Zoltan @ 2023-11-26 22:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: philmd, Jiaxun Yang, Bernhard Beschow, Mark Cave-Ayland, vr_qemu

Philippe,

Could this be merged for 8.2 as it fixes USB on the amigaone machine?
This would be useful as usb-storage is the simplest way to share data
with the host with these machines.

This is a slight change from v2 adding more comments and improving
commit messages and clean things a bit but otherwise should be the
same as previous versions. Even v1 worked the same as this one and v2,
the additional check to avoid stuck bits is just paranoia, it does not
happen in practice as IRQ mappings are quite static, they are set once
at boot and don't change afterwards.

The rest is just some explanation on how we got here but can be
skipped if not interested in history.

This is going back to my original implementation of this IRQ routing
that I submitted already for 8.0 in the beginning of this year
(https://patchew.org/QEMU/cover.1677004414.git.balaton@eik.bme.hu/)
but Mark had concerns about that because he wanted to use PCI
interrupt routing instead. I've already told back then that won't work
but I could not convince reviewers about that. Now with the amigaone
machine this can also be seen and that's why this series is needed now.

The routing code in PCIBus cannot be used as that only considers the 4
PCI interrupts but there are about 12 interrupt sources in this chip
that need to be routed to ISA IRQs (the embedded chip functions and
the 4 PCI interrupts that are coming in through 4 pins of the chip).
Also the chip does not own the PCIBus, that's part of the north bridge
so it should not change the PCI interrupt routing of a bus it does not
own. (Piix calling pci_bus_irqs() I think is also wrong because the
PCI bus in that machine is also owned by the north bridge and piix
should not take over routing of IRQs on a bus it's connected to.)
Another concern from Mark was that this makes chip functions specific
to the chip and they cannot be used as individual PCI devices. Well,
yes, they are chip functions, are not user creatable and don't exist
as individual devices so it does not make sense to use them without
the actual VIA chip they are a function of so that's not a real
concern. These functions are also not actual PCI devices, they are
PCIDevice because they appear as PCI functions of the chip but are
connected internally and this series also models that correctly. This
seems to be supported by comments in Linux about how these VIA chip
function aren't following PCI standards and use ISA IRQs instead:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/pci/quirks.c?h=v6.5.6#n1172

Therefore I think Mark's proposals are not improving this model so I
went back to the original approach which was tested to work and is
also simpler and easier to understand than trying to reuse PCI
intrrupt routing which does not work and would be more complex anyway
for no good reason.

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          | 79 +++++++++++++++++++++++++-------------
 hw/usb/vt82c686-uhci-pci.c |  9 +++++
 include/hw/isa/vt82c686.h  |  2 +
 4 files changed, 67 insertions(+), 31 deletions(-)

-- 
2.30.9



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/4] hw/isa/vt82c686: Bring back via_isa_set_irq()
  2023-11-26 22:49 [PATCH v3 0/4] Fix IRQ routing in via south bridge BALATON Zoltan
@ 2023-11-26 22:49 ` BALATON Zoltan
  2023-11-26 22:49 ` [PATCH v3 2/4] hw/usb/vt82c686-uhci-pci: Use ISA instead of PCI interrupts BALATON Zoltan
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: BALATON Zoltan @ 2023-11-26 22:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: philmd, Jiaxun Yang, Bernhard Beschow, Mark Cave-Ayland, vr_qemu

The VIA integrated south bridge chips combine several functions and
allow routing their interrupts to any of the ISA IRQs also allowing
multiple sources to share the same ISA IRQ. E.g. pegasos2 firmware
configures everything to use IRQ 9 but amigaone routes them to
separate ISA IRQs so the current simplified routing does not work.
Bring back via_isa_set_irq() and change it to take the component that
wants to change an IRQ and keep track of interrupt status of each
source separately and do the mapping to ISA IRQ within the ISA bridge.

This may not handle cases when an ISA IRQ is 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
to be change by guests or share with other devices so 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         | 41 +++++++++++++++++++++++++++++++++++++++
 include/hw/isa/vt82c686.h |  2 ++
 2 files changed, 43 insertions(+)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 57bdfb4e78..6fad8293e6 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 irq_state[ISA_NUM_IRQS];
     ViaSuperIOState via_sio;
     MC146818RtcState rtc;
     PCIIDEState ide;
@@ -592,6 +593,46 @@ 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));
+    uint8_t irq = d->config[PCI_INTERRUPT_LINE], max_irq = 15;
+    int f = PCI_FUNC(d->devfn);
+    uint16_t mask = BIT(f);
+
+    switch (f) {
+    case 2: /* USB ports 0-1 */
+    case 3: /* USB ports 2-3 */
+        max_irq = 14;
+        break;
+    }
+
+    /* Keep track of the state of all sources */
+    if (level) {
+        s->irq_state[0] |= mask;
+    } else {
+        s->irq_state[0] &= ~mask;
+    }
+    if (irq == 0 || irq == 0xff) {
+        return; /* disabled */
+    }
+    if (unlikely(irq > max_irq || irq == 2)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "Invalid ISA IRQ routing %d for %d",
+                      irq, f);
+        return;
+    }
+    /* Record source state at mapped IRQ */
+    if (level) {
+        s->irq_state[irq] |= mask;
+    } else {
+        s->irq_state[irq] &= ~mask;
+    }
+    /* Make sure there are no stuck bits if mapping has changed */
+    s->irq_state[irq] &= s->irq_state[0];
+    /* ISA IRQ level is the OR of all sources routed to it */
+    qemu_set_irq(s->isa_irqs_in[irq], !!s->irq_state[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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/4] hw/usb/vt82c686-uhci-pci: Use ISA instead of PCI interrupts
  2023-11-26 22:49 [PATCH v3 0/4] Fix IRQ routing in via south bridge BALATON Zoltan
  2023-11-26 22:49 ` [PATCH v3 1/4] hw/isa/vt82c686: Bring back via_isa_set_irq() BALATON Zoltan
@ 2023-11-26 22:49 ` BALATON Zoltan
  2023-11-26 22:49 ` [PATCH v3 3/4] hw/isa/vt82c686: Route PIRQ inputs using via_isa_set_irq() BALATON Zoltan
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: BALATON Zoltan @ 2023-11-26 22:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: philmd, Jiaxun Yang, Bernhard Beschow, Mark Cave-Ayland, vr_qemu

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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 3/4] hw/isa/vt82c686: Route PIRQ inputs using via_isa_set_irq()
  2023-11-26 22:49 [PATCH v3 0/4] Fix IRQ routing in via south bridge BALATON Zoltan
  2023-11-26 22:49 ` [PATCH v3 1/4] hw/isa/vt82c686: Bring back via_isa_set_irq() BALATON Zoltan
  2023-11-26 22:49 ` [PATCH v3 2/4] hw/usb/vt82c686-uhci-pci: Use ISA instead of PCI interrupts BALATON Zoltan
@ 2023-11-26 22:49 ` BALATON Zoltan
  2023-11-26 22:49 ` [PATCH v3 4/4] hw/audio/via-ac97: Route interrupts " BALATON Zoltan
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: BALATON Zoltan @ 2023-11-26 22:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: philmd, Jiaxun Yang, Bernhard Beschow, Mark Cave-Ayland, vr_qemu

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 6fad8293e6..a3eb6769fc 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));
@@ -601,6 +616,10 @@ void via_isa_set_irq(PCIDevice *d, int pin, int level)
     uint16_t mask = BIT(f);
 
     switch (f) {
+    case 0: /* PIRQ/PINT inputs */
+        irq = via_isa_get_pci_irq(s, pin);
+        f = 8 + pin; /* Use function 8-11 for PCI interrupt inputs */
+        break;
     case 2: /* USB ports 0-1 */
     case 3: /* USB ports 2-3 */
         max_irq = 14;
@@ -633,50 +652,15 @@ void via_isa_set_irq(PCIDevice *d, int pin, int level)
     qemu_set_irq(s->isa_irqs_in[irq], !!s->irq_state[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)
@@ -689,6 +673,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);
@@ -702,8 +687,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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 4/4] hw/audio/via-ac97: Route interrupts using via_isa_set_irq()
  2023-11-26 22:49 [PATCH v3 0/4] Fix IRQ routing in via south bridge BALATON Zoltan
                   ` (2 preceding siblings ...)
  2023-11-26 22:49 ` [PATCH v3 3/4] hw/isa/vt82c686: Route PIRQ inputs using via_isa_set_irq() BALATON Zoltan
@ 2023-11-26 22:49 ` BALATON Zoltan
  2023-11-28 12:47 ` [PATCH v3 0/4] Fix IRQ routing in via south bridge BALATON Zoltan
  2023-11-29 12:13 ` Michael Tokarev
  5 siblings, 0 replies; 10+ messages in thread
From: BALATON Zoltan @ 2023-11-26 22:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: philmd, Jiaxun Yang, Bernhard Beschow, Mark Cave-Ayland, vr_qemu

This device is a function of VIA south bridge and should allow setting
interrupt routing within that chip. This is implemented in
via_isa_set_irq().

Fixes: eb604411a78b82c468e2b8d81a9401eb8b9c7658
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 a3eb6769fc..9c2333a277 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -622,6 +622,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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/4] Fix IRQ routing in via south bridge
  2023-11-26 22:49 [PATCH v3 0/4] Fix IRQ routing in via south bridge BALATON Zoltan
                   ` (3 preceding siblings ...)
  2023-11-26 22:49 ` [PATCH v3 4/4] hw/audio/via-ac97: Route interrupts " BALATON Zoltan
@ 2023-11-28 12:47 ` BALATON Zoltan
  2023-11-28 13:26   ` Philippe Mathieu-Daudé
  2023-11-29 12:13 ` Michael Tokarev
  5 siblings, 1 reply; 10+ messages in thread
From: BALATON Zoltan @ 2023-11-28 12:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: philmd, Jiaxun Yang, Bernhard Beschow, Mark Cave-Ayland, vr_qemu

On Sun, 26 Nov 2023, BALATON Zoltan wrote:
> Philippe,
>
> Could this be merged for 8.2 as it fixes USB on the amigaone machine?
> This would be useful as usb-storage is the simplest way to share data
> with the host with these machines.

Philippe, do you have some time to look at this now for 8.2 please? I 
still hope this could be fixed for the amigaone machine on release and 
dont' have to wait until the next one for USB to work on that machine.

Regards,
BALATON Zoltan

> This is a slight change from v2 adding more comments and improving
> commit messages and clean things a bit but otherwise should be the
> same as previous versions. Even v1 worked the same as this one and v2,
> the additional check to avoid stuck bits is just paranoia, it does not
> happen in practice as IRQ mappings are quite static, they are set once
> at boot and don't change afterwards.
>
> The rest is just some explanation on how we got here but can be
> skipped if not interested in history.
>
> This is going back to my original implementation of this IRQ routing
> that I submitted already for 8.0 in the beginning of this year
> (https://patchew.org/QEMU/cover.1677004414.git.balaton@eik.bme.hu/)
> but Mark had concerns about that because he wanted to use PCI
> interrupt routing instead. I've already told back then that won't work
> but I could not convince reviewers about that. Now with the amigaone
> machine this can also be seen and that's why this series is needed now.
>
> The routing code in PCIBus cannot be used as that only considers the 4
> PCI interrupts but there are about 12 interrupt sources in this chip
> that need to be routed to ISA IRQs (the embedded chip functions and
> the 4 PCI interrupts that are coming in through 4 pins of the chip).
> Also the chip does not own the PCIBus, that's part of the north bridge
> so it should not change the PCI interrupt routing of a bus it does not
> own. (Piix calling pci_bus_irqs() I think is also wrong because the
> PCI bus in that machine is also owned by the north bridge and piix
> should not take over routing of IRQs on a bus it's connected to.)
> Another concern from Mark was that this makes chip functions specific
> to the chip and they cannot be used as individual PCI devices. Well,
> yes, they are chip functions, are not user creatable and don't exist
> as individual devices so it does not make sense to use them without
> the actual VIA chip they are a function of so that's not a real
> concern. These functions are also not actual PCI devices, they are
> PCIDevice because they appear as PCI functions of the chip but are
> connected internally and this series also models that correctly. This
> seems to be supported by comments in Linux about how these VIA chip
> function aren't following PCI standards and use ISA IRQs instead:
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/pci/quirks.c?h=v6.5.6#n1172
>
> Therefore I think Mark's proposals are not improving this model so I
> went back to the original approach which was tested to work and is
> also simpler and easier to understand than trying to reuse PCI
> intrrupt routing which does not work and would be more complex anyway
> for no good reason.
>
> 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          | 79 +++++++++++++++++++++++++-------------
> hw/usb/vt82c686-uhci-pci.c |  9 +++++
> include/hw/isa/vt82c686.h  |  2 +
> 4 files changed, 67 insertions(+), 31 deletions(-)
>
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/4] Fix IRQ routing in via south bridge
  2023-11-28 12:47 ` [PATCH v3 0/4] Fix IRQ routing in via south bridge BALATON Zoltan
@ 2023-11-28 13:26   ` Philippe Mathieu-Daudé
  2023-11-28 14:18     ` BALATON Zoltan
  0 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-28 13:26 UTC (permalink / raw)
  To: BALATON Zoltan, qemu-devel
  Cc: Jiaxun Yang, Bernhard Beschow, Mark Cave-Ayland, vr_qemu

Hi Zoltan,

On 28/11/23 13:47, BALATON Zoltan wrote:
> On Sun, 26 Nov 2023, BALATON Zoltan wrote:
>> Philippe,
>>
>> Could this be merged for 8.2 as it fixes USB on the amigaone machine?
>> This would be useful as usb-storage is the simplest way to share data
>> with the host with these machines.
> 
> Philippe, do you have some time to look at this now for 8.2 please? I 
> still hope this could be fixed for the amigaone machine on release and 
> dont' have to wait until the next one for USB to work on that machine.

Thanks for your detailed cover and patch descriptions.

I just finished to run my tests and they all passed.

I couldn't spend much time reviewing the patches, but having a quick
look I don't think the way you model it is correct. This is a tricky
setup and apparently we don't fully understand it (I understand what
you explained, but some pieces don't make sense to me). That said,
I understand it help you and the AmigaOne users, and nobody objected.
So, while being a bit reluctant, I am queuing this series; and will
send a PR in a few. We'll have time to improve this model later.

Regards,

Phil.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/4] Fix IRQ routing in via south bridge
  2023-11-28 13:26   ` Philippe Mathieu-Daudé
@ 2023-11-28 14:18     ` BALATON Zoltan
  0 siblings, 0 replies; 10+ messages in thread
From: BALATON Zoltan @ 2023-11-28 14:18 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Jiaxun Yang, Bernhard Beschow, Mark Cave-Ayland, vr_qemu

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

On Tue, 28 Nov 2023, Philippe Mathieu-Daudé wrote:
> On 28/11/23 13:47, BALATON Zoltan wrote:
>> On Sun, 26 Nov 2023, BALATON Zoltan wrote:
>>> Philippe,
>>> 
>>> Could this be merged for 8.2 as it fixes USB on the amigaone machine?
>>> This would be useful as usb-storage is the simplest way to share data
>>> with the host with these machines.
>> 
>> Philippe, do you have some time to look at this now for 8.2 please? I still 
>> hope this could be fixed for the amigaone machine on release and dont' have 
>> to wait until the next one for USB to work on that machine.
>
> Thanks for your detailed cover and patch descriptions.
>
> I just finished to run my tests and they all passed.
>
> I couldn't spend much time reviewing the patches, but having a quick
> look I don't think the way you model it is correct. This is a tricky
> setup and apparently we don't fully understand it (I understand what
> you explained, but some pieces don't make sense to me). That said,
> I understand it help you and the AmigaOne users, and nobody objected.
> So, while being a bit reluctant, I am queuing this series; and will
> send a PR in a few. We'll have time to improve this model later.

Thanks very much. I'm open to further discussion and improving this model, 
just wanted to have something working in master now. The discussion about 
this seemed never ending, it started before 8.0 and still could not get to 
a conclusion yet so until then this should work for now and allow users to 
use it and does not prevent improving it later. So I'm still interested in 
your review and why do you think this is not modelling it correctly but we 
have more time for that now and can change this further as a follow up.

I think the current way makes it easier to add Bernhard's SCI interrupt as 
well for which I had a review proposal before. The main disagreement 
seemsd to be if the chip functions should be PCI devices or not. I think 
they aren't like regular PCI devices and clearly don't use PCI interrupts 
so Mark's and Bernhard's idea to use PCI bus interrupt routing for these 
does not work because they need to be independently routable to ISA 
interrupts. So whatever we do we'll need to distinguish the interrupt 
sources and keep track of their state individually because more than one 
of them can control a single ISA IRQ. Doing this in the ISA bridge seems 
like the best place because that already owns the ISA interrupts so no 
other component will need access to them and it can keep track of state of 
IRQ sources at one place.

Regards,
BALATON Zoltan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/4] Fix IRQ routing in via south bridge
  2023-11-26 22:49 [PATCH v3 0/4] Fix IRQ routing in via south bridge BALATON Zoltan
                   ` (4 preceding siblings ...)
  2023-11-28 12:47 ` [PATCH v3 0/4] Fix IRQ routing in via south bridge BALATON Zoltan
@ 2023-11-29 12:13 ` Michael Tokarev
  2023-11-29 12:40   ` BALATON Zoltan
  5 siblings, 1 reply; 10+ messages in thread
From: Michael Tokarev @ 2023-11-29 12:13 UTC (permalink / raw)
  To: BALATON Zoltan, qemu-devel
  Cc: philmd, Jiaxun Yang, Bernhard Beschow, Mark Cave-Ayland, vr_qemu

27.11.2023 01:49, BALATON Zoltan:
> Philippe,
> 
> Could this be merged for 8.2 as it fixes USB on the amigaone machine?
> This would be useful as usb-storage is the simplest way to share data
> with the host with these machines.
> 
> This is a slight change from v2 adding more comments and improving
> commit messages and clean things a bit but otherwise should be the
> same as previous versions. Even v1 worked the same as this one and v2,
> the additional check to avoid stuck bits is just paranoia, it does not
> happen in practice as IRQ mappings are quite static, they are set once
> at boot and don't change afterwards.

Should this patchset be picked up for stable-8.1?


Thanks,

/mjt


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/4] Fix IRQ routing in via south bridge
  2023-11-29 12:13 ` Michael Tokarev
@ 2023-11-29 12:40   ` BALATON Zoltan
  0 siblings, 0 replies; 10+ messages in thread
From: BALATON Zoltan @ 2023-11-29 12:40 UTC (permalink / raw)
  To: Michael Tokarev
  Cc: qemu-devel, philmd, Jiaxun Yang, Bernhard Beschow,
	Mark Cave-Ayland, vr_qemu

On Wed, 29 Nov 2023, Michael Tokarev wrote:
> 27.11.2023 01:49, BALATON Zoltan:
>> Philippe,
>> 
>> Could this be merged for 8.2 as it fixes USB on the amigaone machine?
>> This would be useful as usb-storage is the simplest way to share data
>> with the host with these machines.
>> 
>> This is a slight change from v2 adding more comments and improving
>> commit messages and clean things a bit but otherwise should be the
>> same as previous versions. Even v1 worked the same as this one and v2,
>> the additional check to avoid stuck bits is just paranoia, it does not
>> happen in practice as IRQ mappings are quite static, they are set once
>> at boot and don't change afterwards.
>
> Should this patchset be picked up for stable-8.1?

The amigaone machine was added now in 8.2 so the problem does not occur in 
8.1 yet (the different usage of this chip by amigaone firmware uncovered 
the issue that made this fix necessary). So 8.1 should still work without 
this therefore I don't think this is needed in stable.

Regards,
BALATON Zoltan


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-11-29 12:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-26 22:49 [PATCH v3 0/4] Fix IRQ routing in via south bridge BALATON Zoltan
2023-11-26 22:49 ` [PATCH v3 1/4] hw/isa/vt82c686: Bring back via_isa_set_irq() BALATON Zoltan
2023-11-26 22:49 ` [PATCH v3 2/4] hw/usb/vt82c686-uhci-pci: Use ISA instead of PCI interrupts BALATON Zoltan
2023-11-26 22:49 ` [PATCH v3 3/4] hw/isa/vt82c686: Route PIRQ inputs using via_isa_set_irq() BALATON Zoltan
2023-11-26 22:49 ` [PATCH v3 4/4] hw/audio/via-ac97: Route interrupts " BALATON Zoltan
2023-11-28 12:47 ` [PATCH v3 0/4] Fix IRQ routing in via south bridge BALATON Zoltan
2023-11-28 13:26   ` Philippe Mathieu-Daudé
2023-11-28 14:18     ` BALATON Zoltan
2023-11-29 12:13 ` Michael Tokarev
2023-11-29 12:40   ` BALATON Zoltan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.