hw/audio/ac97.c | 43 +--- hw/audio/ac97.h | 65 ++++++ hw/audio/trace-events | 6 + hw/audio/via-ac97.c | 436 ++++++++++++++++++++++++++++++++++++- hw/ide/via.c | 2 +- hw/isa/vt82c686.c | 61 +++++- hw/pci-host/mv64361.c | 4 - hw/ppc/pegasos2.c | 26 ++- hw/usb/vt82c686-uhci-pci.c | 5 +- include/hw/isa/vt82c686.h | 39 +++- 10 files changed, 626 insertions(+), 61 deletions(-) create mode 100644 hw/audio/ac97.h
This series fixes PCI interrupts on the ppc/pegasos2 machine and adds partial implementation of the via-ac97 sound part enough to get audio output. I'd like this to be merged for QEMU 8.0. Regards, BALATON Zoltan BALATON Zoltan (5): hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq hw/isa/vt82c686: Implement PIRQ pins hw/ppc/pegasos2: Fix PCI interrupt routing hw/audio/ac97: Split off some definitions to a header hw/audio/via-ac97: Basic implementation of audio playback hw/audio/ac97.c | 43 +--- hw/audio/ac97.h | 65 ++++++ hw/audio/trace-events | 6 + hw/audio/via-ac97.c | 436 ++++++++++++++++++++++++++++++++++++- hw/ide/via.c | 2 +- hw/isa/vt82c686.c | 61 +++++- hw/pci-host/mv64361.c | 4 - hw/ppc/pegasos2.c | 26 ++- hw/usb/vt82c686-uhci-pci.c | 5 +- include/hw/isa/vt82c686.h | 39 +++- 10 files changed, 626 insertions(+), 61 deletions(-) create mode 100644 hw/audio/ac97.h -- 2.30.7
On Tue, Feb 21, 2023 at 7:44 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: > This series fixes PCI interrupts on the ppc/pegasos2 machine and adds > partial implementation of the via-ac97 sound part enough to get audio > output. I'd like this to be merged for QEMU 8.0. > > Regards, > BALATON Zoltan > > BALATON Zoltan (5): > hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq > hw/isa/vt82c686: Implement PIRQ pins > hw/ppc/pegasos2: Fix PCI interrupt routing > hw/audio/ac97: Split off some definitions to a header > hw/audio/via-ac97: Basic implementation of audio playback > > hw/audio/ac97.c | 43 +--- > hw/audio/ac97.h | 65 ++++++ > hw/audio/trace-events | 6 + > hw/audio/via-ac97.c | 436 ++++++++++++++++++++++++++++++++++++- > hw/ide/via.c | 2 +- > hw/isa/vt82c686.c | 61 +++++- > hw/pci-host/mv64361.c | 4 - > hw/ppc/pegasos2.c | 26 ++- > hw/usb/vt82c686-uhci-pci.c | 5 +- > include/hw/isa/vt82c686.h | 39 +++- > 10 files changed, 626 insertions(+), 61 deletions(-) > create mode 100644 hw/audio/ac97.h > > -- > 2.30.7 > > Wow, the MorphOS people paid attention to sound design. Thanks for presenting it to us, Zoltan! I've had a closer look at your series and I think it can be simplified: Patch 2 can be implemented quite straight-forward like I proposed in a private mail: https://github.com/shentok/qemu/commit/via-priq-routing. Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing" working, one can expose the PCI interrupts with a single line like you do in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq" isn't needed any longer and can be omitted. In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be used instead. pci_set_irq() internally takes care of all the ISA interrupt level tracking patch 1 attempted to address. I might have further comments but I think it's enough for now. Thanks again for making via-ac97 work! Best regards, Bernhard
On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote: > > > On Tue, Feb 21, 2023 at 7:44 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: > >> This series fixes PCI interrupts on the ppc/pegasos2 machine and adds >> partial implementation of the via-ac97 sound part enough to get audio >> output. I'd like this to be merged for QEMU 8.0. >> >> Regards, >> BALATON Zoltan >> >> BALATON Zoltan (5): >> hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq >> hw/isa/vt82c686: Implement PIRQ pins >> hw/ppc/pegasos2: Fix PCI interrupt routing >> hw/audio/ac97: Split off some definitions to a header >> hw/audio/via-ac97: Basic implementation of audio playback >> >> hw/audio/ac97.c | 43 +--- >> hw/audio/ac97.h | 65 ++++++ >> hw/audio/trace-events | 6 + >> hw/audio/via-ac97.c | 436 ++++++++++++++++++++++++++++++++++++- >> hw/ide/via.c | 2 +- >> hw/isa/vt82c686.c | 61 +++++- >> hw/pci-host/mv64361.c | 4 - >> hw/ppc/pegasos2.c | 26 ++- >> hw/usb/vt82c686-uhci-pci.c | 5 +- >> include/hw/isa/vt82c686.h | 39 +++- >> 10 files changed, 626 insertions(+), 61 deletions(-) >> create mode 100644 hw/audio/ac97.h >> >> -- >> 2.30.7 >> >> > Wow, the MorphOS people paid attention to sound design. Thanks for > presenting it to us, Zoltan! > > I've had a closer look at your series and I think it can be simplified: > Patch 2 can be implemented quite straight-forward like I proposed in a > private mail: https://github.com/shentok/qemu/commit/via-priq-routing. > Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing" > working, one can expose the PCI interrupts with a single line like you do > in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt > routing in via_isa_set_irq" isn't needed any longer and can be omitted. > > In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be > used instead. pci_set_irq() internally takes care of all the ISA interrupt > level tracking patch 1 attempted to address. > Here is a proof of concept branch to demonstrate that the simplification actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested with MorphOS with and without pegasos2.rom). Best regards, Bernhard > > I might have further comments but I think it's enough for now. > > Thanks again for making via-ac97 work! > > Best regards, > Bernhard >
On Wed, 22 Feb 2023, Bernhard Beschow wrote: > On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote: >> On Tue, Feb 21, 2023 at 7:44 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: >>> This series fixes PCI interrupts on the ppc/pegasos2 machine and adds >>> partial implementation of the via-ac97 sound part enough to get audio >>> output. I'd like this to be merged for QEMU 8.0. >>> >>> Regards, >>> BALATON Zoltan >>> >>> BALATON Zoltan (5): >>> hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq >>> hw/isa/vt82c686: Implement PIRQ pins >>> hw/ppc/pegasos2: Fix PCI interrupt routing >>> hw/audio/ac97: Split off some definitions to a header >>> hw/audio/via-ac97: Basic implementation of audio playback >>> >>> hw/audio/ac97.c | 43 +--- >>> hw/audio/ac97.h | 65 ++++++ >>> hw/audio/trace-events | 6 + >>> hw/audio/via-ac97.c | 436 ++++++++++++++++++++++++++++++++++++- >>> hw/ide/via.c | 2 +- >>> hw/isa/vt82c686.c | 61 +++++- >>> hw/pci-host/mv64361.c | 4 - >>> hw/ppc/pegasos2.c | 26 ++- >>> hw/usb/vt82c686-uhci-pci.c | 5 +- >>> include/hw/isa/vt82c686.h | 39 +++- >>> 10 files changed, 626 insertions(+), 61 deletions(-) >>> create mode 100644 hw/audio/ac97.h >>> >>> -- >>> 2.30.7 >>> >>> >> Wow, the MorphOS people paid attention to sound design. Thanks for >> presenting it to us, Zoltan! >> >> I've had a closer look at your series and I think it can be simplified: >> Patch 2 can be implemented quite straight-forward like I proposed in a >> private mail: https://github.com/shentok/qemu/commit/via-priq-routing. >> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing" >> working, one can expose the PCI interrupts with a single line like you do >> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt >> routing in via_isa_set_irq" isn't needed any longer and can be omitted. >> >> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be >> used instead. pci_set_irq() internally takes care of all the ISA interrupt >> level tracking patch 1 attempted to address. >> > > Here is a proof of concept branch to demonstrate that the simplification > actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested > with MorphOS with and without pegasos2.rom). Does this only work because both the via-ac97 and the PCI interrupts are mapped to the same ISA IRQ and you've only tested sound? The guest could configure each device to use a different IRQ, also mapping them so they share one ISA interrupt. What happens if multiple devices are mapped to IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all share this IRQ) and more than one such device wants to raise an interrupt at the same time? If you ack the ac97 interrupt but a PCI network card or the USB part still wants to get the CPUs attention the ISA IRQ should remain raised until all devices are serviced. I don't see a way to track the status of all devices in a single qemu_irq which can only be up or down so we need something to store the state of each source. My patch adds a state register to each ISA IRQ line for all possible sources which could probably be stored once but then for each change of ISA IRQ status all the mapped devices should be checked and combined so it's easier to store them for each IRQ. Does your approach still work if you play sound, and copy something from network to a USB device at the same time? (I'm not sure mine does not have remaining bugs but I don't think this can be simplified that way but if you can prove it would work I don't mind taking an alternative version but I'm not convinced yet.) Regards, BALATON Zoltan >> I might have further comments but I think it's enough for now. >> >> Thanks again for making via-ac97 work! >> >> Best regards, >> Bernhard >> >
Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >On Wed, 22 Feb 2023, Bernhard Beschow wrote: >> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote: >>> On Tue, Feb 21, 2023 at 7:44 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: >>>> This series fixes PCI interrupts on the ppc/pegasos2 machine and adds >>>> partial implementation of the via-ac97 sound part enough to get audio >>>> output. I'd like this to be merged for QEMU 8.0. >>>> >>>> Regards, >>>> BALATON Zoltan >>>> >>>> BALATON Zoltan (5): >>>> hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq >>>> hw/isa/vt82c686: Implement PIRQ pins >>>> hw/ppc/pegasos2: Fix PCI interrupt routing >>>> hw/audio/ac97: Split off some definitions to a header >>>> hw/audio/via-ac97: Basic implementation of audio playback >>>> >>>> hw/audio/ac97.c | 43 +--- >>>> hw/audio/ac97.h | 65 ++++++ >>>> hw/audio/trace-events | 6 + >>>> hw/audio/via-ac97.c | 436 ++++++++++++++++++++++++++++++++++++- >>>> hw/ide/via.c | 2 +- >>>> hw/isa/vt82c686.c | 61 +++++- >>>> hw/pci-host/mv64361.c | 4 - >>>> hw/ppc/pegasos2.c | 26 ++- >>>> hw/usb/vt82c686-uhci-pci.c | 5 +- >>>> include/hw/isa/vt82c686.h | 39 +++- >>>> 10 files changed, 626 insertions(+), 61 deletions(-) >>>> create mode 100644 hw/audio/ac97.h >>>> >>>> -- >>>> 2.30.7 >>>> >>>> >>> Wow, the MorphOS people paid attention to sound design. Thanks for >>> presenting it to us, Zoltan! >>> >>> I've had a closer look at your series and I think it can be simplified: >>> Patch 2 can be implemented quite straight-forward like I proposed in a >>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing. >>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing" >>> working, one can expose the PCI interrupts with a single line like you do >>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt >>> routing in via_isa_set_irq" isn't needed any longer and can be omitted. >>> >>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be >>> used instead. pci_set_irq() internally takes care of all the ISA interrupt >>> level tracking patch 1 attempted to address. >>> >> >> Here is a proof of concept branch to demonstrate that the simplification >> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested >> with MorphOS with and without pegasos2.rom). > >Does this only work because both the via-ac97 and the PCI interrupts are mapped to the same ISA IRQ and you've only tested sound? The guest could configure each device to use a different IRQ, also mapping them so they share one ISA interrupt. What happens if multiple devices are mapped to IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all share this IRQ) and more than one such device wants to raise an interrupt at the same time? If you ack the ac97 interrupt but a PCI network card or the USB part still wants to get the CPUs attention the ISA IRQ should remain raised until all devices are serviced. pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle exactly that case very well. >I don't see a way to track the status of all devices in a single qemu_irq which can only be up or down so we need something to store the state of each source. pci_set_irq() causes pci_bus_change_irq_level() to be called. pci_bus_change_irq_level() tracks the sum of all irq levels of all devices attached to a particular pin in irq_count. Have a look at pci_bus_change_irq_level() and you will understand better. >My patch adds a state register to each ISA IRQ line for all possible sources which could probably be stored once but then for each change of ISA IRQ status all the mapped devices should be checked and combined so it's easier to store them for each IRQ. Does your approach still work if you play sound, and copy something from network to a USB device at the same time? (I'm not sure mine does not have remaining bugs but I don't think this can be simplified that way but if you can prove it would work I don't mind taking an alternative version but I'm not convinced yet.) Well, I can't prove that my approach works but unfortunately I can prove that both our approaches cause a freeze :/ Try: 1. Start `qemu-system-ppc -M pegasos2 -bios pegasos2.rom -rtc base=localtime -device ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -device usb-mouse -device usb-kbd` 2. Move the mouse while sound is playing -> Observe the VM to freeze So there must be an issue somewhere else... Best regards, Bernhard > >Regards, >BALATON Zoltan > >>> I might have further comments but I think it's enough for now. >>> >>> Thanks again for making via-ac97 work! >>> >>> Best regards, >>> Bernhard >>> >>
On Wed, 22 Feb 2023, Bernhard Beschow wrote:
> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> On Wed, 22 Feb 2023, Bernhard Beschow wrote:
>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote:
>>>> On Tue, Feb 21, 2023 at 7:44 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>>>> This series fixes PCI interrupts on the ppc/pegasos2 machine and adds
>>>>> partial implementation of the via-ac97 sound part enough to get audio
>>>>> output. I'd like this to be merged for QEMU 8.0.
>>>>>
>>>>> Regards,
>>>>> BALATON Zoltan
>>>>>
>>>>> BALATON Zoltan (5):
>>>>> hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq
>>>>> hw/isa/vt82c686: Implement PIRQ pins
>>>>> hw/ppc/pegasos2: Fix PCI interrupt routing
>>>>> hw/audio/ac97: Split off some definitions to a header
>>>>> hw/audio/via-ac97: Basic implementation of audio playback
>>>>>
>>>>> hw/audio/ac97.c | 43 +---
>>>>> hw/audio/ac97.h | 65 ++++++
>>>>> hw/audio/trace-events | 6 +
>>>>> hw/audio/via-ac97.c | 436 ++++++++++++++++++++++++++++++++++++-
>>>>> hw/ide/via.c | 2 +-
>>>>> hw/isa/vt82c686.c | 61 +++++-
>>>>> hw/pci-host/mv64361.c | 4 -
>>>>> hw/ppc/pegasos2.c | 26 ++-
>>>>> hw/usb/vt82c686-uhci-pci.c | 5 +-
>>>>> include/hw/isa/vt82c686.h | 39 +++-
>>>>> 10 files changed, 626 insertions(+), 61 deletions(-)
>>>>> create mode 100644 hw/audio/ac97.h
>>>>>
>>>>> --
>>>>> 2.30.7
>>>>>
>>>>>
>>>> Wow, the MorphOS people paid attention to sound design. Thanks for
>>>> presenting it to us, Zoltan!
>>>>
>>>> I've had a closer look at your series and I think it can be simplified:
>>>> Patch 2 can be implemented quite straight-forward like I proposed in a
>>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing.
>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing"
>>>> working, one can expose the PCI interrupts with a single line like you do
>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt
>>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted.
>>>>
>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be
>>>> used instead. pci_set_irq() internally takes care of all the ISA interrupt
>>>> level tracking patch 1 attempted to address.
>>>>
>>>
>>> Here is a proof of concept branch to demonstrate that the simplification
>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested
>>> with MorphOS with and without pegasos2.rom).
>>
>> Does this only work because both the via-ac97 and the PCI interrupts are mapped to the same ISA IRQ and you've only tested sound? The guest could configure each device to use a different IRQ, also mapping them so they share one ISA interrupt. What happens if multiple devices are mapped to IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all share this IRQ) and more than one such device wants to raise an interrupt at the same time? If you ack the ac97 interrupt but a PCI network card or the USB part still wants to get the CPUs attention the ISA IRQ should remain raised until all devices are serviced.
>
> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle
> exactly that case very well.
>
>> I don't see a way to track the status of all devices in a single qemu_irq which can only be up or down so we need something to store the state of each source.
>
> pci_set_irq() causes pci_bus_change_irq_level() to be called.
> pci_bus_change_irq_level() tracks the sum of all irq levels of all
> devices attached to a particular pin in irq_count. Have a look at
> pci_bus_change_irq_level() and you will understand better.
>
>> My patch adds a state register to each ISA IRQ line for all possible sources which could probably be stored once but then for each change of ISA IRQ status all the mapped devices should be checked and combined so it's easier to store them for each IRQ. Does your approach still work if you play sound, and copy something from network to a USB device at the same time? (I'm not sure mine does not have remaining bugs but I don't think this can be simplified that way but if you can prove it would work I don't mind taking an alternative version but I'm not convinced yet.)
>
> Well, I can't prove that my approach works but unfortunately I can
> prove that both our approaches cause a freeze :/ Try:
> 1. Start `qemu-system-ppc -M pegasos2 -bios pegasos2.rom -rtc
> base=localtime -device ati-vga,guest_hwcursor=true,romfile="" -cdrom
> morphos-3.17.iso -device usb-mouse -device usb-kbd`
> 2. Move the mouse while sound is playing
> -> Observe the VM to freeze
Not quite sure why but it seems to happen when both the ac97 and USB raise
the interrupt and the guest driver seems to get confused. Adding some
debug logging:
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index b16620daf8..f840e5a8d0 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -636,12 +636,13 @@ void via_isa_set_irq(PCIDevice *d,
ViaISAIRQSourceBit n, int level)
if (!isa_irq) {
return;
}
-
+if (n > 1) fprintf(stderr, "%s: %d %d %d %x -> ", __func__, n, level, isa_irq, s->isa_irq_state[isa_irq]);
if (level) {
s->isa_irq_state[isa_irq] |= BIT(n);
} else {
s->isa_irq_state[isa_irq] &= ~BIT(n);
}
+if (n > 1) fprintf(stderr, "%x\n", s->isa_irq_state[isa_irq]);
qemu_set_irq(s->isa_irqs[isa_irq], !!s->isa_irq_state[isa_irq]);
}
I see in the normal case when there's only one interrupt for USB only:
via_isa_set_irq: 2 1 9 0 -> 4
usb_uhci_mmio_readw addr 0x0002, ret 0x0001
usb_uhci_mmio_writew addr 0x0002, val 0x0001
via_isa_set_irq: 2 0 9 4 -> 0
For sound only:
via_ac97_sgd_fetch addr=0x43b70bc --F len=3528
via_isa_set_irq: 8 1 9 0 -> 100
usb_uhci_mmio_readw addr 0x0002, ret 0x0000
usb_uhci_mmio_readw addr 0x0002, ret 0x0000
via_ac97_sgd_read 0x0 1 -> 0xc9
via_ac97_sgd_write 0x0 1 <- 0x1
via_isa_set_irq: 8 0 9 100 -> 0
via_ac97_sgd_read 0x4 4 -> 0x439cbe8
via_ac97_sgd_fetch addr=0x43c70bc -E- len=3528
via_isa_set_irq: 8 1 9 0 -> 100
via_ac97_sgd_read 0x4 4 -> 0x439cbe0
via_ac97_sgd_read 0x4 4 -> 0x439cbe0
via_ac97_sgd_read 0x10 1 -> 0x0
usb_uhci_mmio_readw addr 0x0002, ret 0x0000
usb_uhci_mmio_readw addr 0x0002, ret 0x0000
via_ac97_sgd_read 0x0 1 -> 0xca
via_ac97_sgd_write 0x0 1 <- 0x2
via_isa_set_irq: 8 0 9 100 -> 0
via_ac97_sgd_read 0x4 4 -> 0x439cbe0
but it stops acking irqs when both are raised or it seems USB IRQ is
raised while it's in the guest IRQ handler:
via_ac97_sgd_fetch addr=0x43c70bc -E- len=3528
via_isa_set_irq: 8 1 9 0 -> 100
usb_uhci_mmio_readw addr 0x0002, ret 0x0000
usb_uhci_mmio_readw addr 0x0002, ret 0x0000
via_isa_set_irq: 2 1 9 100 -> 104
via_ac97_sgd_read 0x0 1 -> 0xca
via_ac97_sgd_write 0x0 1 <- 0x2
via_isa_set_irq: 8 0 9 104 -> 4
via_ac97_sgd_read 0x4 4 -> 0x439cbe0
via_ac97_sgd_fetch addr=0x43b70bc --F len=3528
via_isa_set_irq: 8 1 9 4 -> 104
via_ac97_sgd_read 0x4 4 -> 0x439cbe8
via_ac97_sgd_read 0x4 4 -> 0x439cbe8
via_ac97_sgd_read 0x10 1 -> 0x0
usb_uhci_mmio_readw addr 0x0006, ret 0x06bf
usb_uhci_mmio_readw addr 0x0010, ret 0x0085
usb_uhci_mmio_writew addr 0x0010, val 0x0085
usb_uhci_mmio_readw addr 0x0012, ret 0x0085
usb_uhci_mmio_writew addr 0x0012, val 0x0085
usb_uhci_mmio_readw addr 0x0006, ret 0x06b7
usb_uhci_mmio_readw addr 0x0010, ret 0x0080
usb_uhci_mmio_writew addr 0x0010, val 0x0080
usb_uhci_mmio_readw addr 0x0012, ret 0x0080
usb_uhci_mmio_writew addr 0x0012, val 0x0080
usb_uhci_mmio_readw addr 0x0006, ret 0x0759
usb_uhci_mmio_readw addr 0x0010, ret 0x0085
usb_uhci_mmio_writew addr 0x0010, val 0x0085
usb_uhci_mmio_readw addr 0x0012, ret 0x0085
usb_uhci_mmio_writew addr 0x0012, val 0x0085
usb_uhci_mmio_readw addr 0x0006, ret 0x0752
usb_uhci_mmio_readw addr 0x0010, ret 0x0080
usb_uhci_mmio_writew addr 0x0010, val 0x0080
usb_uhci_mmio_readw addr 0x0012, ret 0x0080
usb_uhci_mmio_writew addr 0x0012, val 0x0080
via_isa_set_irq: 2 1 9 104 -> 104
usb_uhci_mmio_readw addr 0x0006, ret 0x07f1
usb_uhci_mmio_readw addr 0x0010, ret 0x0085
usb_uhci_mmio_writew addr 0x0010, val 0x0085
usb_uhci_mmio_readw addr 0x0012, ret 0x0085
usb_uhci_mmio_writew addr 0x0012, val 0x0085
usb_uhci_mmio_readw addr 0x0006, ret 0x07e9
It seems to not notice the USB interrupt any more after that although
sound playback stops but mouse still moves but otherwise does not work.
I'm not sure this is not a guest bug as it seems an interrupt handler
should disable interrupts to not get interrupted. Could this be reproduced
with Linux? I'd still go wit this patch series for 8.0 because the default
case works and this was also tested with two PCI cards on AmigaOS4 which
works not while it did not work at all before so this could be debugged
and fixed later but adding this series makes the machine generally usable
at least without USB devices. With -d unimp I also get these logs when
booting MorphOS:
ok boot cd boot.img
ISO-9660 filesystem: System-ID: "MORPHOS" Volume-ID: "MorphOSBoot"
Root dir: "" flags=0x2 extent=0x20 size=0x1800
31.127| Memory used before SYS_Init: 9MB
i8259: level sensitive irq not supported
i8259: level sensitive irq not supported
Could it be the PIC emulation should be fixed for this?
Regards,
BALATON Zoltan
On Wed, 22 Feb 2023, BALATON Zoltan wrote:
> On Wed, 22 Feb 2023, Bernhard Beschow wrote:
>> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan
>> <balaton@eik.bme.hu>:
>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote:
>>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com>
>>>> wrote:
>>>>> On Tue, Feb 21, 2023 at 7:44 PM BALATON Zoltan <balaton@eik.bme.hu>
>>>>> wrote:
>>>>>> This series fixes PCI interrupts on the ppc/pegasos2 machine and adds
>>>>>> partial implementation of the via-ac97 sound part enough to get audio
>>>>>> output. I'd like this to be merged for QEMU 8.0.
>>>>>>
>>>>>> Regards,
>>>>>> BALATON Zoltan
>>>>>>
>>>>>> BALATON Zoltan (5):
>>>>>> hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq
>>>>>> hw/isa/vt82c686: Implement PIRQ pins
>>>>>> hw/ppc/pegasos2: Fix PCI interrupt routing
>>>>>> hw/audio/ac97: Split off some definitions to a header
>>>>>> hw/audio/via-ac97: Basic implementation of audio playback
>>>>>>
>>>>>> hw/audio/ac97.c | 43 +---
>>>>>> hw/audio/ac97.h | 65 ++++++
>>>>>> hw/audio/trace-events | 6 +
>>>>>> hw/audio/via-ac97.c | 436 ++++++++++++++++++++++++++++++++++++-
>>>>>> hw/ide/via.c | 2 +-
>>>>>> hw/isa/vt82c686.c | 61 +++++-
>>>>>> hw/pci-host/mv64361.c | 4 -
>>>>>> hw/ppc/pegasos2.c | 26 ++-
>>>>>> hw/usb/vt82c686-uhci-pci.c | 5 +-
>>>>>> include/hw/isa/vt82c686.h | 39 +++-
>>>>>> 10 files changed, 626 insertions(+), 61 deletions(-)
>>>>>> create mode 100644 hw/audio/ac97.h
>>>>>>
>>>>>> --
>>>>>> 2.30.7
>>>>>>
>>>>>>
>>>>> Wow, the MorphOS people paid attention to sound design. Thanks for
>>>>> presenting it to us, Zoltan!
>>>>>
>>>>> I've had a closer look at your series and I think it can be simplified:
>>>>> Patch 2 can be implemented quite straight-forward like I proposed in a
>>>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing.
>>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt
>>>>> routing"
>>>>> working, one can expose the PCI interrupts with a single line like you
>>>>> do
>>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt
>>>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted.
>>>>>
>>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be
>>>>> used instead. pci_set_irq() internally takes care of all the ISA
>>>>> interrupt
>>>>> level tracking patch 1 attempted to address.
>>>>>
>>>>
>>>> Here is a proof of concept branch to demonstrate that the simplification
>>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested
>>>> with MorphOS with and without pegasos2.rom).
>>>
>>> Does this only work because both the via-ac97 and the PCI interrupts are
>>> mapped to the same ISA IRQ and you've only tested sound? The guest could
>>> configure each device to use a different IRQ, also mapping them so they
>>> share one ISA interrupt. What happens if multiple devices are mapped to
>>> IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all
>>> share this IRQ) and more than one such device wants to raise an interrupt
>>> at the same time? If you ack the ac97 interrupt but a PCI network card or
>>> the USB part still wants to get the CPUs attention the ISA IRQ should
>>> remain raised until all devices are serviced.
>>
>> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle
>> exactly that case very well.
>>
>>> I don't see a way to track the status of all devices in a single qemu_irq
>>> which can only be up or down so we need something to store the state of
>>> each source.
>>
>> pci_set_irq() causes pci_bus_change_irq_level() to be called.
>> pci_bus_change_irq_level() tracks the sum of all irq levels of all
>> devices attached to a particular pin in irq_count. Have a look at
>> pci_bus_change_irq_level() and you will understand better.
>>
>>> My patch adds a state register to each ISA IRQ line for all possible
>>> sources which could probably be stored once but then for each change of
>>> ISA IRQ status all the mapped devices should be checked and combined so
>>> it's easier to store them for each IRQ. Does your approach still work if
>>> you play sound, and copy something from network to a USB device at the
>>> same time? (I'm not sure mine does not have remaining bugs but I don't
>>> think this can be simplified that way but if you can prove it would work I
>>> don't mind taking an alternative version but I'm not convinced yet.)
>>
>> Well, I can't prove that my approach works but unfortunately I can
>> prove that both our approaches cause a freeze :/ Try:
>> 1. Start `qemu-system-ppc -M pegasos2 -bios pegasos2.rom -rtc
>> base=localtime -device ati-vga,guest_hwcursor=true,romfile="" -cdrom
>> morphos-3.17.iso -device usb-mouse -device usb-kbd`
>> 2. Move the mouse while sound is playing
>> -> Observe the VM to freeze
>
> Not quite sure why but it seems to happen when both the ac97 and USB raise
> the interrupt and the guest driver seems to get confused. Adding some debug
> logging:
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index b16620daf8..f840e5a8d0 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -636,12 +636,13 @@ void via_isa_set_irq(PCIDevice *d, ViaISAIRQSourceBit
> n, int level)
> if (!isa_irq) {
> return;
> }
> -
> +if (n > 1) fprintf(stderr, "%s: %d %d %d %x -> ", __func__, n, level,
> isa_irq, s->isa_irq_state[isa_irq]);
> if (level) {
> s->isa_irq_state[isa_irq] |= BIT(n);
> } else {
> s->isa_irq_state[isa_irq] &= ~BIT(n);
> }
> +if (n > 1) fprintf(stderr, "%x\n", s->isa_irq_state[isa_irq]);
> qemu_set_irq(s->isa_irqs[isa_irq], !!s->isa_irq_state[isa_irq]);
> }
>
> I see in the normal case when there's only one interrupt for USB only:
>
> via_isa_set_irq: 2 1 9 0 -> 4
> usb_uhci_mmio_readw addr 0x0002, ret 0x0001
> usb_uhci_mmio_writew addr 0x0002, val 0x0001
> via_isa_set_irq: 2 0 9 4 -> 0
>
> For sound only:
>
> via_ac97_sgd_fetch addr=0x43b70bc --F len=3528
> via_isa_set_irq: 8 1 9 0 -> 100
> usb_uhci_mmio_readw addr 0x0002, ret 0x0000
> usb_uhci_mmio_readw addr 0x0002, ret 0x0000
> via_ac97_sgd_read 0x0 1 -> 0xc9
> via_ac97_sgd_write 0x0 1 <- 0x1
> via_isa_set_irq: 8 0 9 100 -> 0
> via_ac97_sgd_read 0x4 4 -> 0x439cbe8
> via_ac97_sgd_fetch addr=0x43c70bc -E- len=3528
> via_isa_set_irq: 8 1 9 0 -> 100
> via_ac97_sgd_read 0x4 4 -> 0x439cbe0
> via_ac97_sgd_read 0x4 4 -> 0x439cbe0
> via_ac97_sgd_read 0x10 1 -> 0x0
> usb_uhci_mmio_readw addr 0x0002, ret 0x0000
> usb_uhci_mmio_readw addr 0x0002, ret 0x0000
> via_ac97_sgd_read 0x0 1 -> 0xca
> via_ac97_sgd_write 0x0 1 <- 0x2
> via_isa_set_irq: 8 0 9 100 -> 0
> via_ac97_sgd_read 0x4 4 -> 0x439cbe0
>
> but it stops acking irqs when both are raised or it seems USB IRQ is raised
> while it's in the guest IRQ handler:
>
> via_ac97_sgd_fetch addr=0x43c70bc -E- len=3528
> via_isa_set_irq: 8 1 9 0 -> 100
> usb_uhci_mmio_readw addr 0x0002, ret 0x0000
> usb_uhci_mmio_readw addr 0x0002, ret 0x0000
> via_isa_set_irq: 2 1 9 100 -> 104
> via_ac97_sgd_read 0x0 1 -> 0xca
> via_ac97_sgd_write 0x0 1 <- 0x2
> via_isa_set_irq: 8 0 9 104 -> 4
> via_ac97_sgd_read 0x4 4 -> 0x439cbe0
> via_ac97_sgd_fetch addr=0x43b70bc --F len=3528
> via_isa_set_irq: 8 1 9 4 -> 104
> via_ac97_sgd_read 0x4 4 -> 0x439cbe8
> via_ac97_sgd_read 0x4 4 -> 0x439cbe8
> via_ac97_sgd_read 0x10 1 -> 0x0
> usb_uhci_mmio_readw addr 0x0006, ret 0x06bf
> usb_uhci_mmio_readw addr 0x0010, ret 0x0085
> usb_uhci_mmio_writew addr 0x0010, val 0x0085
> usb_uhci_mmio_readw addr 0x0012, ret 0x0085
> usb_uhci_mmio_writew addr 0x0012, val 0x0085
> usb_uhci_mmio_readw addr 0x0006, ret 0x06b7
> usb_uhci_mmio_readw addr 0x0010, ret 0x0080
> usb_uhci_mmio_writew addr 0x0010, val 0x0080
> usb_uhci_mmio_readw addr 0x0012, ret 0x0080
> usb_uhci_mmio_writew addr 0x0012, val 0x0080
> usb_uhci_mmio_readw addr 0x0006, ret 0x0759
> usb_uhci_mmio_readw addr 0x0010, ret 0x0085
> usb_uhci_mmio_writew addr 0x0010, val 0x0085
> usb_uhci_mmio_readw addr 0x0012, ret 0x0085
> usb_uhci_mmio_writew addr 0x0012, val 0x0085
> usb_uhci_mmio_readw addr 0x0006, ret 0x0752
> usb_uhci_mmio_readw addr 0x0010, ret 0x0080
> usb_uhci_mmio_writew addr 0x0010, val 0x0080
> usb_uhci_mmio_readw addr 0x0012, ret 0x0080
> usb_uhci_mmio_writew addr 0x0012, val 0x0080
> via_isa_set_irq: 2 1 9 104 -> 104
> usb_uhci_mmio_readw addr 0x0006, ret 0x07f1
> usb_uhci_mmio_readw addr 0x0010, ret 0x0085
> usb_uhci_mmio_writew addr 0x0010, val 0x0085
> usb_uhci_mmio_readw addr 0x0012, ret 0x0085
> usb_uhci_mmio_writew addr 0x0012, val 0x0085
> usb_uhci_mmio_readw addr 0x0006, ret 0x07e9
>
> It seems to not notice the USB interrupt any more after that although sound
> playback stops but mouse still moves but otherwise does not work. I'm not
> sure this is not a guest bug as it seems an interrupt handler should disable
> interrupts to not get interrupted. Could this be reproduced with Linux? I'd
> still go wit this patch series for 8.0 because the default case works and
> this was also tested with two PCI cards on AmigaOS4 which works not while it
> did not work at all before so this could be debugged and fixed later but
> adding this series makes the machine generally usable at least without USB
> devices. With -d unimp I also get these logs when booting MorphOS:
>
> ok boot cd boot.img
> ISO-9660 filesystem: System-ID: "MORPHOS" Volume-ID: "MorphOSBoot"
> Root dir: "" flags=0x2 extent=0x20 size=0x1800
> 31.127| Memory used before SYS_Init: 9MB
> i8259: level sensitive irq not supported
> i8259: level sensitive irq not supported
>
> Could it be the PIC emulation should be fixed for this?
After thinking about that more I think this is the reason and this patch
just uncovered a defficiency in the PIC model. I would not care much it
this was only sound vs. USB but it's also sound vs. PCI cards e.g. network
so until that's fixed in i8259 I can hack around that here like this:
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index b16620daf8..a6cf55a632 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -597,6 +597,7 @@ void via_isa_set_irq(PCIDevice *d, ViaISAIRQSourceBit n, int level)
{
ViaISAState *s = VIA_ISA(pci_get_function_0(d));
uint8_t isa_irq = 0, max_irq = 15;
+ int old_level;
if (n == VIA_IRQ_USB0 && d == PCI_DEVICE(&s->uhci[1])) {
n++;
@@ -637,11 +638,16 @@ void via_isa_set_irq(PCIDevice *d, ViaISAIRQSourceBit n, int level)
return;
}
+ old_level = !!s->isa_irq_state[isa_irq];
if (level) {
s->isa_irq_state[isa_irq] |= BIT(n);
} else {
s->isa_irq_state[isa_irq] &= ~BIT(n);
}
+ if (old_level && !!s->isa_irq_state[isa_irq]) {
+ /* Only needed because i8259 model does not support level sensitive */
+ qemu_set_irq(s->isa_irqs[isa_irq], 0);
+ }
qemu_set_irq(s->isa_irqs[isa_irq], !!s->isa_irq_state[isa_irq]);
}
Unless somebody has a better idea I'll go with this for a v2 and let this
be cleaned up sometimes in the future when sombody gets around to improve
the PIC model.
Regards,
BALATON Zoltan
On 23/02/2023 00:43, BALATON Zoltan wrote:
> On Wed, 22 Feb 2023, BALATON Zoltan wrote:
>> On Wed, 22 Feb 2023, Bernhard Beschow wrote:
>>> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan
>>> <balaton@eik.bme.hu>:
>>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote:
>>>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com>
>>>>> wrote:
>>>>>> On Tue, Feb 21, 2023 at 7:44 PM BALATON Zoltan <balaton@eik.bme.hu>
>>>>>> wrote:
>>>>>>> This series fixes PCI interrupts on the ppc/pegasos2 machine and adds
>>>>>>> partial implementation of the via-ac97 sound part enough to get audio
>>>>>>> output. I'd like this to be merged for QEMU 8.0.
>>>>>>>
>>>>>>> Regards,
>>>>>>> BALATON Zoltan
>>>>>>>
>>>>>>> BALATON Zoltan (5):
>>>>>>> hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq
>>>>>>> hw/isa/vt82c686: Implement PIRQ pins
>>>>>>> hw/ppc/pegasos2: Fix PCI interrupt routing
>>>>>>> hw/audio/ac97: Split off some definitions to a header
>>>>>>> hw/audio/via-ac97: Basic implementation of audio playback
>>>>>>>
>>>>>>> hw/audio/ac97.c | 43 +---
>>>>>>> hw/audio/ac97.h | 65 ++++++
>>>>>>> hw/audio/trace-events | 6 +
>>>>>>> hw/audio/via-ac97.c | 436 ++++++++++++++++++++++++++++++++++++-
>>>>>>> hw/ide/via.c | 2 +-
>>>>>>> hw/isa/vt82c686.c | 61 +++++-
>>>>>>> hw/pci-host/mv64361.c | 4 -
>>>>>>> hw/ppc/pegasos2.c | 26 ++-
>>>>>>> hw/usb/vt82c686-uhci-pci.c | 5 +-
>>>>>>> include/hw/isa/vt82c686.h | 39 +++-
>>>>>>> 10 files changed, 626 insertions(+), 61 deletions(-)
>>>>>>> create mode 100644 hw/audio/ac97.h
>>>>>>>
>>>>>>> --
>>>>>>> 2.30.7
>>>>>>>
>>>>>>>
>>>>>> Wow, the MorphOS people paid attention to sound design. Thanks for
>>>>>> presenting it to us, Zoltan!
>>>>>>
>>>>>> I've had a closer look at your series and I think it can be simplified:
>>>>>> Patch 2 can be implemented quite straight-forward like I proposed in a
>>>>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing.
>>>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt
>>>>>> routing"
>>>>>> working, one can expose the PCI interrupts with a single line like you
>>>>>> do
>>>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt
>>>>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted.
>>>>>>
>>>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be
>>>>>> used instead. pci_set_irq() internally takes care of all the ISA
>>>>>> interrupt
>>>>>> level tracking patch 1 attempted to address.
>>>>>>
>>>>>
>>>>> Here is a proof of concept branch to demonstrate that the simplification
>>>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested
>>>>> with MorphOS with and without pegasos2.rom).
>>>>
>>>> Does this only work because both the via-ac97 and the PCI interrupts are
>>>> mapped to the same ISA IRQ and you've only tested sound? The guest could
>>>> configure each device to use a different IRQ, also mapping them so they
>>>> share one ISA interrupt. What happens if multiple devices are mapped to
>>>> IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all
>>>> share this IRQ) and more than one such device wants to raise an interrupt
>>>> at the same time? If you ack the ac97 interrupt but a PCI network card or
>>>> the USB part still wants to get the CPUs attention the ISA IRQ should
>>>> remain raised until all devices are serviced.
>>>
>>> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle
>>> exactly that case very well.
>>>
>>>> I don't see a way to track the status of all devices in a single qemu_irq
>>>> which can only be up or down so we need something to store the state of
>>>> each source.
>>>
>>> pci_set_irq() causes pci_bus_change_irq_level() to be called.
>>> pci_bus_change_irq_level() tracks the sum of all irq levels of all
>>> devices attached to a particular pin in irq_count. Have a look at
>>> pci_bus_change_irq_level() and you will understand better.
>>>
>>>> My patch adds a state register to each ISA IRQ line for all possible
>>>> sources which could probably be stored once but then for each change of
>>>> ISA IRQ status all the mapped devices should be checked and combined so
>>>> it's easier to store them for each IRQ. Does your approach still work if
>>>> you play sound, and copy something from network to a USB device at the
>>>> same time? (I'm not sure mine does not have remaining bugs but I don't
>>>> think this can be simplified that way but if you can prove it would work I
>>>> don't mind taking an alternative version but I'm not convinced yet.)
>>>
>>> Well, I can't prove that my approach works but unfortunately I can
>>> prove that both our approaches cause a freeze :/ Try:
>>> 1. Start `qemu-system-ppc -M pegasos2 -bios pegasos2.rom -rtc
>>> base=localtime -device ati-vga,guest_hwcursor=true,romfile="" -cdrom
>>> morphos-3.17.iso -device usb-mouse -device usb-kbd`
>>> 2. Move the mouse while sound is playing
>>> -> Observe the VM to freeze
>>
>> Not quite sure why but it seems to happen when both the ac97 and USB raise
>> the interrupt and the guest driver seems to get confused. Adding some debug
>> logging:
>>
>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>> index b16620daf8..f840e5a8d0 100644
>> --- a/hw/isa/vt82c686.c
>> +++ b/hw/isa/vt82c686.c
>> @@ -636,12 +636,13 @@ void via_isa_set_irq(PCIDevice *d, ViaISAIRQSourceBit
>> n, int level)
>> if (!isa_irq) {
>> return;
>> }
>> -
>> +if (n > 1) fprintf(stderr, "%s: %d %d %d %x -> ", __func__, n, level,
>> isa_irq, s->isa_irq_state[isa_irq]);
>> if (level) {
>> s->isa_irq_state[isa_irq] |= BIT(n);
>> } else {
>> s->isa_irq_state[isa_irq] &= ~BIT(n);
>> }
>> +if (n > 1) fprintf(stderr, "%x\n", s->isa_irq_state[isa_irq]);
>> qemu_set_irq(s->isa_irqs[isa_irq], !!s->isa_irq_state[isa_irq]);
>> }
>>
>> I see in the normal case when there's only one interrupt for USB only:
>>
>> via_isa_set_irq: 2 1 9 0 -> 4
>> usb_uhci_mmio_readw addr 0x0002, ret 0x0001
>> usb_uhci_mmio_writew addr 0x0002, val 0x0001
>> via_isa_set_irq: 2 0 9 4 -> 0
>>
>> For sound only:
>>
>> via_ac97_sgd_fetch addr=0x43b70bc --F len=3528
>> via_isa_set_irq: 8 1 9 0 -> 100
>> usb_uhci_mmio_readw addr 0x0002, ret 0x0000
>> usb_uhci_mmio_readw addr 0x0002, ret 0x0000
>> via_ac97_sgd_read 0x0 1 -> 0xc9
>> via_ac97_sgd_write 0x0 1 <- 0x1
>> via_isa_set_irq: 8 0 9 100 -> 0
>> via_ac97_sgd_read 0x4 4 -> 0x439cbe8
>> via_ac97_sgd_fetch addr=0x43c70bc -E- len=3528
>> via_isa_set_irq: 8 1 9 0 -> 100
>> via_ac97_sgd_read 0x4 4 -> 0x439cbe0
>> via_ac97_sgd_read 0x4 4 -> 0x439cbe0
>> via_ac97_sgd_read 0x10 1 -> 0x0
>> usb_uhci_mmio_readw addr 0x0002, ret 0x0000
>> usb_uhci_mmio_readw addr 0x0002, ret 0x0000
>> via_ac97_sgd_read 0x0 1 -> 0xca
>> via_ac97_sgd_write 0x0 1 <- 0x2
>> via_isa_set_irq: 8 0 9 100 -> 0
>> via_ac97_sgd_read 0x4 4 -> 0x439cbe0
>>
>> but it stops acking irqs when both are raised or it seems USB IRQ is raised
>> while it's in the guest IRQ handler:
>>
>> via_ac97_sgd_fetch addr=0x43c70bc -E- len=3528
>> via_isa_set_irq: 8 1 9 0 -> 100
>> usb_uhci_mmio_readw addr 0x0002, ret 0x0000
>> usb_uhci_mmio_readw addr 0x0002, ret 0x0000
>> via_isa_set_irq: 2 1 9 100 -> 104
>> via_ac97_sgd_read 0x0 1 -> 0xca
>> via_ac97_sgd_write 0x0 1 <- 0x2
>> via_isa_set_irq: 8 0 9 104 -> 4
>> via_ac97_sgd_read 0x4 4 -> 0x439cbe0
>> via_ac97_sgd_fetch addr=0x43b70bc --F len=3528
>> via_isa_set_irq: 8 1 9 4 -> 104
>> via_ac97_sgd_read 0x4 4 -> 0x439cbe8
>> via_ac97_sgd_read 0x4 4 -> 0x439cbe8
>> via_ac97_sgd_read 0x10 1 -> 0x0
>> usb_uhci_mmio_readw addr 0x0006, ret 0x06bf
>> usb_uhci_mmio_readw addr 0x0010, ret 0x0085
>> usb_uhci_mmio_writew addr 0x0010, val 0x0085
>> usb_uhci_mmio_readw addr 0x0012, ret 0x0085
>> usb_uhci_mmio_writew addr 0x0012, val 0x0085
>> usb_uhci_mmio_readw addr 0x0006, ret 0x06b7
>> usb_uhci_mmio_readw addr 0x0010, ret 0x0080
>> usb_uhci_mmio_writew addr 0x0010, val 0x0080
>> usb_uhci_mmio_readw addr 0x0012, ret 0x0080
>> usb_uhci_mmio_writew addr 0x0012, val 0x0080
>> usb_uhci_mmio_readw addr 0x0006, ret 0x0759
>> usb_uhci_mmio_readw addr 0x0010, ret 0x0085
>> usb_uhci_mmio_writew addr 0x0010, val 0x0085
>> usb_uhci_mmio_readw addr 0x0012, ret 0x0085
>> usb_uhci_mmio_writew addr 0x0012, val 0x0085
>> usb_uhci_mmio_readw addr 0x0006, ret 0x0752
>> usb_uhci_mmio_readw addr 0x0010, ret 0x0080
>> usb_uhci_mmio_writew addr 0x0010, val 0x0080
>> usb_uhci_mmio_readw addr 0x0012, ret 0x0080
>> usb_uhci_mmio_writew addr 0x0012, val 0x0080
>> via_isa_set_irq: 2 1 9 104 -> 104
>> usb_uhci_mmio_readw addr 0x0006, ret 0x07f1
>> usb_uhci_mmio_readw addr 0x0010, ret 0x0085
>> usb_uhci_mmio_writew addr 0x0010, val 0x0085
>> usb_uhci_mmio_readw addr 0x0012, ret 0x0085
>> usb_uhci_mmio_writew addr 0x0012, val 0x0085
>> usb_uhci_mmio_readw addr 0x0006, ret 0x07e9
>>
>> It seems to not notice the USB interrupt any more after that although sound
>> playback stops but mouse still moves but otherwise does not work. I'm not
>> sure this is not a guest bug as it seems an interrupt handler should disable
>> interrupts to not get interrupted. Could this be reproduced with Linux? I'd
>> still go wit this patch series for 8.0 because the default case works and
>> this was also tested with two PCI cards on AmigaOS4 which works not while it
>> did not work at all before so this could be debugged and fixed later but
>> adding this series makes the machine generally usable at least without USB
>> devices. With -d unimp I also get these logs when booting MorphOS:
>>
>> ok boot cd boot.img
>> ISO-9660 filesystem: System-ID: "MORPHOS" Volume-ID: "MorphOSBoot"
>> Root dir: "" flags=0x2 extent=0x20 size=0x1800
>> 31.127| Memory used before SYS_Init: 9MB
>> i8259: level sensitive irq not supported
>> i8259: level sensitive irq not supported
>>
>> Could it be the PIC emulation should be fixed for this?
>
> After thinking about that more I think this is the reason and this patch
> just uncovered a defficiency in the PIC model. I would not care much it
> this was only sound vs. USB but it's also sound vs. PCI cards e.g. network
> so until that's fixed in i8259 I can hack around that here like this:
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index b16620daf8..a6cf55a632 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -597,6 +597,7 @@ void via_isa_set_irq(PCIDevice *d, ViaISAIRQSourceBit n, int level)
> {
> ViaISAState *s = VIA_ISA(pci_get_function_0(d));
> uint8_t isa_irq = 0, max_irq = 15;
> + int old_level;
>
> if (n == VIA_IRQ_USB0 && d == PCI_DEVICE(&s->uhci[1])) {
> n++;
> @@ -637,11 +638,16 @@ void via_isa_set_irq(PCIDevice *d, ViaISAIRQSourceBit n, int level)
> return;
> }
>
> + old_level = !!s->isa_irq_state[isa_irq];
> if (level) {
> s->isa_irq_state[isa_irq] |= BIT(n);
> } else {
> s->isa_irq_state[isa_irq] &= ~BIT(n);
> }
> + if (old_level && !!s->isa_irq_state[isa_irq]) {
> + /* Only needed because i8259 model does not support level sensitive */
> + qemu_set_irq(s->isa_irqs[isa_irq], 0);
> + }
> qemu_set_irq(s->isa_irqs[isa_irq], !!s->isa_irq_state[isa_irq]);
> }
>
> Unless somebody has a better idea I'll go with this for a v2 and let this
> be cleaned up sometimes in the future when sombody gets around to improve
> the PIC model.
This still doesn't seem right: if this were required for ISA IRQs then it would also
be needed for 8259-based PICs, otherwise the various PC models would continually hang
in the same way. Again this suggests that multiple devices are sharing an ISA IRQ
which is likely incorrect.
ATB,
Mark.
On Wed, 22 Feb 2023, Bernhard Beschow wrote: > Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote: >>>> On Tue, Feb 21, 2023 at 7:44 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: >>>>> This series fixes PCI interrupts on the ppc/pegasos2 machine and adds >>>>> partial implementation of the via-ac97 sound part enough to get audio >>>>> output. I'd like this to be merged for QEMU 8.0. >>>>> >>>>> Regards, >>>>> BALATON Zoltan >>>>> >>>>> BALATON Zoltan (5): >>>>> hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq >>>>> hw/isa/vt82c686: Implement PIRQ pins >>>>> hw/ppc/pegasos2: Fix PCI interrupt routing >>>>> hw/audio/ac97: Split off some definitions to a header >>>>> hw/audio/via-ac97: Basic implementation of audio playback >>>>> >>>>> hw/audio/ac97.c | 43 +--- >>>>> hw/audio/ac97.h | 65 ++++++ >>>>> hw/audio/trace-events | 6 + >>>>> hw/audio/via-ac97.c | 436 ++++++++++++++++++++++++++++++++++++- >>>>> hw/ide/via.c | 2 +- >>>>> hw/isa/vt82c686.c | 61 +++++- >>>>> hw/pci-host/mv64361.c | 4 - >>>>> hw/ppc/pegasos2.c | 26 ++- >>>>> hw/usb/vt82c686-uhci-pci.c | 5 +- >>>>> include/hw/isa/vt82c686.h | 39 +++- >>>>> 10 files changed, 626 insertions(+), 61 deletions(-) >>>>> create mode 100644 hw/audio/ac97.h >>>>> >>>>> -- >>>>> 2.30.7 >>>>> >>>>> >>>> Wow, the MorphOS people paid attention to sound design. Thanks for >>>> presenting it to us, Zoltan! >>>> >>>> I've had a closer look at your series and I think it can be simplified: >>>> Patch 2 can be implemented quite straight-forward like I proposed in a >>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing. >>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing" >>>> working, one can expose the PCI interrupts with a single line like you do >>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt >>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted. >>>> >>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be >>>> used instead. pci_set_irq() internally takes care of all the ISA interrupt >>>> level tracking patch 1 attempted to address. >>>> >>> >>> Here is a proof of concept branch to demonstrate that the simplification >>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested >>> with MorphOS with and without pegasos2.rom). >> >> Does this only work because both the via-ac97 and the PCI interrupts are mapped to the same ISA IRQ and you've only tested sound? The guest could configure each device to use a different IRQ, also mapping them so they share one ISA interrupt. What happens if multiple devices are mapped to IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all share this IRQ) and more than one such device wants to raise an interrupt at the same time? If you ack the ac97 interrupt but a PCI network card or the USB part still wants to get the CPUs attention the ISA IRQ should remain raised until all devices are serviced. > > pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle > exactly that case very well. > >> I don't see a way to track the status of all devices in a single qemu_irq which can only be up or down so we need something to store the state of each source. > > pci_set_irq() causes pci_bus_change_irq_level() to be called. > pci_bus_change_irq_level() tracks the sum of all irq levels of all > devices attached to a particular pin in irq_count. Have a look at > pci_bus_change_irq_level() and you will understand better. I'm aware of that, we're using that in sam460ex which connects all PCI interrupt lines to a single IRQ and Peter explored and explained it in a comment there when that was discovered. First we had a patch with or-irq but due to this behaviot that's not needed for PCI interrupts. But the VT8132 could change what ISA IRQ you route the sub functions to. It happens that on pegasos2 by default all of those are routed to IRQ9 except IDE but what if a guest changes ac97 to use a different interrupt? Then it's not a PCI interrupt any more so you can't use pci_set_irq in via=ac97. There are only 4 PCI INT lines but the VIA components can be routed to 13 or 14 ISA IRQs. How do you keep track of that with only the PCI bus interrupts? I don't get your approach. >> My patch adds a state register to each ISA IRQ line for all possible sources which could probably be stored once but then for each change of ISA IRQ status all the mapped devices should be checked and combined so it's easier to store them for each IRQ. Does your approach still work if you play sound, and copy something from network to a USB device at the same time? (I'm not sure mine does not have remaining bugs but I don't think this can be simplified that way but if you can prove it would work I don't mind taking an alternative version but I'm not convinced yet.) > > Well, I can't prove that my approach works but unfortunately I can > prove that both our approaches cause a freeze :/ Try: > 1. Start `qemu-system-ppc -M pegasos2 -bios pegasos2.rom -rtc > base=localtime -device ati-vga,guest_hwcursor=true,romfile="" -cdrom > morphos-3.17.iso -device usb-mouse -device usb-kbd` > 2. Move the mouse while sound is playing > -> Observe the VM to freeze > > So there must be an issue somewhere else... I'll have a look later but my patch attempts to handle the USB controller interrupts. There may be another bug somewhere in USB emulation though, we have similar problem with mac99 with older MacOS guests. Considering that USB devices probably did not work at all before this patch it's at least still an imptovement. :-) Regards, BALATON Zoltan
On 22/02/2023 21:12, BALATON Zoltan wrote: > On Wed, 22 Feb 2023, Bernhard Beschow wrote: >> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote: >>>>> On Tue, Feb 21, 2023 at 7:44 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: >>>>>> This series fixes PCI interrupts on the ppc/pegasos2 machine and adds >>>>>> partial implementation of the via-ac97 sound part enough to get audio >>>>>> output. I'd like this to be merged for QEMU 8.0. >>>>>> >>>>>> Regards, >>>>>> BALATON Zoltan >>>>>> >>>>>> BALATON Zoltan (5): >>>>>> hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq >>>>>> hw/isa/vt82c686: Implement PIRQ pins >>>>>> hw/ppc/pegasos2: Fix PCI interrupt routing >>>>>> hw/audio/ac97: Split off some definitions to a header >>>>>> hw/audio/via-ac97: Basic implementation of audio playback >>>>>> >>>>>> hw/audio/ac97.c | 43 +--- >>>>>> hw/audio/ac97.h | 65 ++++++ >>>>>> hw/audio/trace-events | 6 + >>>>>> hw/audio/via-ac97.c | 436 ++++++++++++++++++++++++++++++++++++- >>>>>> hw/ide/via.c | 2 +- >>>>>> hw/isa/vt82c686.c | 61 +++++- >>>>>> hw/pci-host/mv64361.c | 4 - >>>>>> hw/ppc/pegasos2.c | 26 ++- >>>>>> hw/usb/vt82c686-uhci-pci.c | 5 +- >>>>>> include/hw/isa/vt82c686.h | 39 +++- >>>>>> 10 files changed, 626 insertions(+), 61 deletions(-) >>>>>> create mode 100644 hw/audio/ac97.h >>>>>> >>>>>> -- >>>>>> 2.30.7 >>>>>> >>>>>> >>>>> Wow, the MorphOS people paid attention to sound design. Thanks for >>>>> presenting it to us, Zoltan! >>>>> >>>>> I've had a closer look at your series and I think it can be simplified: >>>>> Patch 2 can be implemented quite straight-forward like I proposed in a >>>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing. >>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing" >>>>> working, one can expose the PCI interrupts with a single line like you do >>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt >>>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted. >>>>> >>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be >>>>> used instead. pci_set_irq() internally takes care of all the ISA interrupt >>>>> level tracking patch 1 attempted to address. >>>>> >>>> >>>> Here is a proof of concept branch to demonstrate that the simplification >>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested >>>> with MorphOS with and without pegasos2.rom). >>> >>> Does this only work because both the via-ac97 and the PCI interrupts are mapped to >>> the same ISA IRQ and you've only tested sound? The guest could configure each >>> device to use a different IRQ, also mapping them so they share one ISA interrupt. >>> What happens if multiple devices are mapped to IRQ 9 (which is the case on >>> pegasos2 where PCI cards, ac97 and USB all share this IRQ) and more than one such >>> device wants to raise an interrupt at the same time? If you ack the ac97 interrupt >>> but a PCI network card or the USB part still wants to get the CPUs attention the >>> ISA IRQ should remain raised until all devices are serviced. >> >> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle >> exactly that case very well. >> >>> I don't see a way to track the status of all devices in a single qemu_irq which >>> can only be up or down so we need something to store the state of each source. >> >> pci_set_irq() causes pci_bus_change_irq_level() to be called. >> pci_bus_change_irq_level() tracks the sum of all irq levels of all >> devices attached to a particular pin in irq_count. Have a look at >> pci_bus_change_irq_level() and you will understand better. > > I'm aware of that, we're using that in sam460ex which connects all PCI interrupt > lines to a single IRQ and Peter explored and explained it in a comment there when > that was discovered. First we had a patch with or-irq but due to this behaviot that's > not needed for PCI interrupts. But the VT8132 could change what ISA IRQ you route the > sub functions to. It happens that on pegasos2 by default all of those are routed to > IRQ9 except IDE but what if a guest changes ac97 to use a different interrupt? Then > it's not a PCI interrupt any more so you can't use pci_set_irq in via=ac97. There are > only 4 PCI INT lines but the VIA components can be routed to 13 or 14 ISA IRQs. How > do you keep track of that with only the PCI bus interrupts? I don't get your approach. It's up to the OS to setup the IRQ routing as required, so if the guest wants to route via an ISA IRQ then there should be a register to control this, much in the same way that legacy mode can be enabled for some PCI IDE controllers. >>> My patch adds a state register to each ISA IRQ line for all possible sources which >>> could probably be stored once but then for each change of ISA IRQ status all the >>> mapped devices should be checked and combined so it's easier to store them for >>> each IRQ. Does your approach still work if you play sound, and copy something from >>> network to a USB device at the same time? (I'm not sure mine does not have >>> remaining bugs but I don't think this can be simplified that way but if you can >>> prove it would work I don't mind taking an alternative version but I'm not >>> convinced yet.) This doesn't sound right at all: at least on old PCs an ISA card was wired to one or more IRQ lines, and it wasn't possible to share them. If you find you're sharing the same ISQ IRQ then it's likely something is wrong with the IRQ routing. >> Well, I can't prove that my approach works but unfortunately I can >> prove that both our approaches cause a freeze :/ Try: >> 1. Start `qemu-system-ppc -M pegasos2 -bios pegasos2.rom -rtc >> base=localtime -device ati-vga,guest_hwcursor=true,romfile="" -cdrom >> morphos-3.17.iso -device usb-mouse -device usb-kbd` >> 2. Move the mouse while sound is playing >> -> Observe the VM to freeze >> >> So there must be an issue somewhere else... > > I'll have a look later but my patch attempts to handle the USB controller interrupts. > There may be another bug somewhere in USB emulation though, we have similar problem > with mac99 with older MacOS guests. Considering that USB devices probably did not > work at all before this patch it's at least still an imptovement. :-) > > Regards, > BALATON Zoltan ATB, Mark.
On Wed, 1 Mar 2023, Mark Cave-Ayland wrote: > On 22/02/2023 21:12, BALATON Zoltan wrote: >> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan >>> <balaton@eik.bme.hu>: >>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> >>>>> wrote: >>>>>> On Tue, Feb 21, 2023 at 7:44 PM BALATON Zoltan <balaton@eik.bme.hu> >>>>>> wrote: >>>>>>> This series fixes PCI interrupts on the ppc/pegasos2 machine and adds >>>>>>> partial implementation of the via-ac97 sound part enough to get audio >>>>>>> output. I'd like this to be merged for QEMU 8.0. >>>>>>> >>>>>>> Regards, >>>>>>> BALATON Zoltan >>>>>>> >>>>>>> BALATON Zoltan (5): >>>>>>> hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq >>>>>>> hw/isa/vt82c686: Implement PIRQ pins >>>>>>> hw/ppc/pegasos2: Fix PCI interrupt routing >>>>>>> hw/audio/ac97: Split off some definitions to a header >>>>>>> hw/audio/via-ac97: Basic implementation of audio playback >>>>>>> >>>>>>> hw/audio/ac97.c | 43 +--- >>>>>>> hw/audio/ac97.h | 65 ++++++ >>>>>>> hw/audio/trace-events | 6 + >>>>>>> hw/audio/via-ac97.c | 436 >>>>>>> ++++++++++++++++++++++++++++++++++++- >>>>>>> hw/ide/via.c | 2 +- >>>>>>> hw/isa/vt82c686.c | 61 +++++- >>>>>>> hw/pci-host/mv64361.c | 4 - >>>>>>> hw/ppc/pegasos2.c | 26 ++- >>>>>>> hw/usb/vt82c686-uhci-pci.c | 5 +- >>>>>>> include/hw/isa/vt82c686.h | 39 +++- >>>>>>> 10 files changed, 626 insertions(+), 61 deletions(-) >>>>>>> create mode 100644 hw/audio/ac97.h >>>>>>> >>>>>>> -- >>>>>>> 2.30.7 >>>>>>> >>>>>>> >>>>>> Wow, the MorphOS people paid attention to sound design. Thanks for >>>>>> presenting it to us, Zoltan! >>>>>> >>>>>> I've had a closer look at your series and I think it can be simplified: >>>>>> Patch 2 can be implemented quite straight-forward like I proposed in a >>>>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing. >>>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt >>>>>> routing" >>>>>> working, one can expose the PCI interrupts with a single line like you >>>>>> do >>>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt >>>>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted. >>>>>> >>>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be >>>>>> used instead. pci_set_irq() internally takes care of all the ISA >>>>>> interrupt >>>>>> level tracking patch 1 attempted to address. >>>>>> >>>>> >>>>> Here is a proof of concept branch to demonstrate that the simplification >>>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested >>>>> with MorphOS with and without pegasos2.rom). >>>> >>>> Does this only work because both the via-ac97 and the PCI interrupts are >>>> mapped to the same ISA IRQ and you've only tested sound? The guest could >>>> configure each device to use a different IRQ, also mapping them so they >>>> share one ISA interrupt. What happens if multiple devices are mapped to >>>> IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all >>>> share this IRQ) and more than one such device wants to raise an interrupt >>>> at the same time? If you ack the ac97 interrupt but a PCI network card or >>>> the USB part still wants to get the CPUs attention the ISA IRQ should >>>> remain raised until all devices are serviced. >>> >>> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle >>> exactly that case very well. >>> >>>> I don't see a way to track the status of all devices in a single qemu_irq >>>> which can only be up or down so we need something to store the state of >>>> each source. >>> >>> pci_set_irq() causes pci_bus_change_irq_level() to be called. >>> pci_bus_change_irq_level() tracks the sum of all irq levels of all >>> devices attached to a particular pin in irq_count. Have a look at >>> pci_bus_change_irq_level() and you will understand better. >> >> I'm aware of that, we're using that in sam460ex which connects all PCI >> interrupt lines to a single IRQ and Peter explored and explained it in a >> comment there when that was discovered. First we had a patch with or-irq >> but due to this behaviot that's not needed for PCI interrupts. But the >> VT8132 could change what ISA IRQ you route the sub functions to. It happens >> that on pegasos2 by default all of those are routed to IRQ9 except IDE but >> what if a guest changes ac97 to use a different interrupt? Then it's not a >> PCI interrupt any more so you can't use pci_set_irq in via=ac97. There are >> only 4 PCI INT lines but the VIA components can be routed to 13 or 14 ISA >> IRQs. How do you keep track of that with only the PCI bus interrupts? I >> don't get your approach. > > It's up to the OS to setup the IRQ routing as required, so if the guest wants > to route via an ISA IRQ then there should be a register to control this, much > in the same way that legacy mode can be enabled for some PCI IDE controllers. The VT82C686B and VT8231 datasheets document PCI config reg 0x3c to do that which is what I implemented in the first version but then Bernhard said this does not confirm to PCI standard spec so let's model these internal functions of the VIA chip as PCI devices and only map the PCI interrupts with the other registers called PnP IRQ routing in the ISA bridge function for mapping PIRQ/PINT pins. This IMO isn't correctly modeling the chip as documented in the datasheet but may still work as long as guests program everything to share IRQ 9 which is what the pegasos2 firmware does and guests seem to not change it but deal with it one way or another (MorphOS using level sensitive mode of the ISA PICs, Linux and AmigaOS without that). So we can get away with this and make our model simpler until we find a guest that actually need the more fine grained control the chip appears to have. So after some debate and testing it still works with the guests I cate about I went with this simpification in the latest version, noting the above in the commit message. >>>> My patch adds a state register to each ISA IRQ line for all possible >>>> sources which could probably be stored once but then for each change of >>>> ISA IRQ status all the mapped devices should be checked and combined so >>>> it's easier to store them for each IRQ. Does your approach still work if >>>> you play sound, and copy something from network to a USB device at the >>>> same time? (I'm not sure mine does not have remaining bugs but I don't >>>> think this can be simplified that way but if you can prove it would work >>>> I don't mind taking an alternative version but I'm not convinced yet.) > > This doesn't sound right at all: at least on old PCs an ISA card was wired to > one or more IRQ lines, and it wasn't possible to share them. If you find > you're sharing the same ISQ IRQ then it's likely something is wrong with the > IRQ routing. The fimrmware definitely writes 9 to 0x3c config regs of all internal devices (including IDE which despite of that still uses IRQ 14/15 as we have found before) and also writes 9 to 0x55-57 which map the PIRQ or PINT (these are called differently in 686B and VT8231) which are connected to PCI interrupts as seen on an obscure part of the schematics that I've missed first. You can see this with -trace enable="pci*" and guests work accordingly: logs from Linux and BSD on real hardware I've found on-line all list USB and other parts using IRQ 9 and also PCI cards started working after this series with AmigaOS which previously expecting interrupt 9 for these bit did not get it because of the missing connection to PINT pins. I think your and Bernhard's assumption that internal functions of these VIA cups are PCI devices is wrong. There are multiple proofs for that: - The datasheet documents regs to work differently than PCI spec - The sound part ignores PCI bus master bit yet still can DMA (documented in datasheet and evidenced by AmigaOS driver which works on real hardware) - The IDE part is still using ISA IRQs even in native mode (this one is also contrary to the datasheet not just the PCI spec but this was confirmed before and seems to also be the case on fuloong2e so probably true for all VIA chips) So I think assuming these must work as normal PCI devices is clearly not what the datasheet says and likely not how real chip works so I'd rather believe the datasheet and what guests do than a PCI standard that may be later than this chip and nothing said the chip must confirm to it but you still want to assume that strongly. Yet due to the way guests use it, sharing the same interrupt for all PCI lines and all internal functions we can still model it that way and thus make the model simpler (albeit conceptually wrong). I can accept if you want it to be simpler but I can't accept it's really how the chip works. But then this debate is theoretical, as long as guests work I'm OK with that so just took Bernhard's patches and I really hope now you won't come up with a 3rd way you think this should be done instead which would be really late now. This was tested first with my patches then with Bernhard's approach and now is being tested with the current state after rebase. We may have time for only one more iteration left but I really don't want to break it with extensive changes at this point. There's always an oportunity to improve things later so my main goal now is to let people be able to use these guests on pegasos2 as soon as possible. For AmigaOS pegasos2 would really help as it's a lot faster than sam460ex and may also allow KVM to work but for that this needs to get out then we can improve it in future versions. Regards, BALATON Zoltan >>> Well, I can't prove that my approach works but unfortunately I can >>> prove that both our approaches cause a freeze :/ Try: >>> 1. Start `qemu-system-ppc -M pegasos2 -bios pegasos2.rom -rtc >>> base=localtime -device ati-vga,guest_hwcursor=true,romfile="" -cdrom >>> morphos-3.17.iso -device usb-mouse -device usb-kbd` >>> 2. Move the mouse while sound is playing >>> -> Observe the VM to freeze >>> >>> So there must be an issue somewhere else... >> >> I'll have a look later but my patch attempts to handle the USB controller >> interrupts. There may be another bug somewhere in USB emulation though, we >> have similar problem with mac99 with older MacOS guests. Considering that >> USB devices probably did not work at all before this patch it's at least >> still an imptovement. :-) >> >> Regards, >> BALATON Zoltan > > > ATB, > > Mark. > >
Am 22. Februar 2023 21:12:01 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >On Wed, 22 Feb 2023, Bernhard Beschow wrote: >> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote: >>>>> On Tue, Feb 21, 2023 at 7:44 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: >>>>>> This series fixes PCI interrupts on the ppc/pegasos2 machine and adds >>>>>> partial implementation of the via-ac97 sound part enough to get audio >>>>>> output. I'd like this to be merged for QEMU 8.0. >>>>>> >>>>>> Regards, >>>>>> BALATON Zoltan >>>>>> >>>>>> BALATON Zoltan (5): >>>>>> hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq >>>>>> hw/isa/vt82c686: Implement PIRQ pins >>>>>> hw/ppc/pegasos2: Fix PCI interrupt routing >>>>>> hw/audio/ac97: Split off some definitions to a header >>>>>> hw/audio/via-ac97: Basic implementation of audio playback >>>>>> >>>>>> hw/audio/ac97.c | 43 +--- >>>>>> hw/audio/ac97.h | 65 ++++++ >>>>>> hw/audio/trace-events | 6 + >>>>>> hw/audio/via-ac97.c | 436 ++++++++++++++++++++++++++++++++++++- >>>>>> hw/ide/via.c | 2 +- >>>>>> hw/isa/vt82c686.c | 61 +++++- >>>>>> hw/pci-host/mv64361.c | 4 - >>>>>> hw/ppc/pegasos2.c | 26 ++- >>>>>> hw/usb/vt82c686-uhci-pci.c | 5 +- >>>>>> include/hw/isa/vt82c686.h | 39 +++- >>>>>> 10 files changed, 626 insertions(+), 61 deletions(-) >>>>>> create mode 100644 hw/audio/ac97.h >>>>>> >>>>>> -- >>>>>> 2.30.7 >>>>>> >>>>>> >>>>> Wow, the MorphOS people paid attention to sound design. Thanks for >>>>> presenting it to us, Zoltan! >>>>> >>>>> I've had a closer look at your series and I think it can be simplified: >>>>> Patch 2 can be implemented quite straight-forward like I proposed in a >>>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing. >>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing" >>>>> working, one can expose the PCI interrupts with a single line like you do >>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt >>>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted. >>>>> >>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be >>>>> used instead. pci_set_irq() internally takes care of all the ISA interrupt >>>>> level tracking patch 1 attempted to address. >>>>> >>>> >>>> Here is a proof of concept branch to demonstrate that the simplification >>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested >>>> with MorphOS with and without pegasos2.rom). >>> >>> Does this only work because both the via-ac97 and the PCI interrupts are mapped to the same ISA IRQ and you've only tested sound? The guest could configure each device to use a different IRQ, also mapping them so they share one ISA interrupt. What happens if multiple devices are mapped to IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all share this IRQ) and more than one such device wants to raise an interrupt at the same time? If you ack the ac97 interrupt but a PCI network card or the USB part still wants to get the CPUs attention the ISA IRQ should remain raised until all devices are serviced. >> >> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle >> exactly that case very well. >> >>> I don't see a way to track the status of all devices in a single qemu_irq which can only be up or down so we need something to store the state of each source. >> >> pci_set_irq() causes pci_bus_change_irq_level() to be called. >> pci_bus_change_irq_level() tracks the sum of all irq levels of all >> devices attached to a particular pin in irq_count. Have a look at >> pci_bus_change_irq_level() and you will understand better. > >I'm aware of that, we're using that in sam460ex which connects all PCI interrupt lines to a single IRQ and Peter explored and explained it in a comment there when that was discovered. First we had a patch with or-irq but due to this behaviot that's not needed for PCI interrupts. But the VT8132 could change what ISA IRQ you route the sub functions to. That depends on the sub function if you can do that. And if so, then it depends on whether the function is still in PCI mode (see below). >It happens that on pegasos2 by default all of those are routed to IRQ9 except IDE All *PCI* interrupts are routed to IRQ9 while IDE legacy interrupts are routed to the compatible ISA IRQs. Note that the IDE function must only trigger the ISA IRQs if it is in legacy mode while it must only trigger the PCI IRQ in non-legacy mode. See https://www.bswd.com/pciide.pdf for more details on this particular topic. >but what if a guest changes ac97 to use a different interrupt? Then it's not a PCI interrupt any more so you can't use pci_set_irq in via=ac97. How would it do that? AFAICS there is no dedicated register to configure which IRQ to use. This means that it can only trigger an interrupt via its PCI intx pin which is subject to the PCI -> ISA IRQ router. > There are only 4 PCI INT lines but the VIA components can be routed to 13 or 14 ISA IRQs. Pure PCI components are only able to trigger one of the four PCI intx pins they are *hardwired* to. Each component has only one pin. Which ISA IRQ gets triggered through that pin can be selected from 13 or 14 ISA IRQs as you say by means of the three configuration registers of the PCI -> ISA IRQ router. >How do you keep track of that with only the PCI bus interrupts? Devices that operate in ISA mode such as the IDE function shall have their own, dedicated ISA IRQs assigned by the guest. Otherwise this causes a classic interrupt conflict, just like in the olden ISA days. If the function operates in PCI mode, it must not trigger the ISA IRQs, regardless of whether they are assigned or not. There is also the power management function whose ACPI interrupt (SCI) can be routed by means of a dedicated register. Again, a guest must make sure here to not configure interrupt conflicts. >I don't get your approach. I hope that I could help you get a better understanding. The linked .pdf is good and comprehensive reading material. Best regards, Bernhard > >>> My patch adds a state register to each ISA IRQ line for all possible sources which could probably be stored once but then for each change of ISA IRQ status all the mapped devices should be checked and combined so it's easier to store them for each IRQ. Does your approach still work if you play sound, and copy something from network to a USB device at the same time? (I'm not sure mine does not have remaining bugs but I don't think this can be simplified that way but if you can prove it would work I don't mind taking an alternative version but I'm not convinced yet.) >> >> Well, I can't prove that my approach works but unfortunately I can >> prove that both our approaches cause a freeze :/ Try: >> 1. Start `qemu-system-ppc -M pegasos2 -bios pegasos2.rom -rtc >> base=localtime -device ati-vga,guest_hwcursor=true,romfile="" -cdrom >> morphos-3.17.iso -device usb-mouse -device usb-kbd` >> 2. Move the mouse while sound is playing >> -> Observe the VM to freeze >> >> So there must be an issue somewhere else... > >I'll have a look later but my patch attempts to handle the USB controller interrupts. There may be another bug somewhere in USB emulation though, we have similar problem with mac99 with older MacOS guests. Considering that USB devices probably did not work at all before this patch it's at least still an imptovement. :-) > >Regards, >BALATON Zoltan
On Wed, 22 Feb 2023, Bernhard Beschow wrote: > Am 22. Februar 2023 21:12:01 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote: >>>>>> I've had a closer look at your series and I think it can be simplified: >>>>>> Patch 2 can be implemented quite straight-forward like I proposed in a >>>>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing. >>>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing" >>>>>> working, one can expose the PCI interrupts with a single line like you do >>>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt >>>>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted. >>>>>> >>>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be >>>>>> used instead. pci_set_irq() internally takes care of all the ISA interrupt >>>>>> level tracking patch 1 attempted to address. >>>>>> >>>>> >>>>> Here is a proof of concept branch to demonstrate that the simplification >>>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested >>>>> with MorphOS with and without pegasos2.rom). >>>> >>>> Does this only work because both the via-ac97 and the PCI interrupts are mapped to the same ISA IRQ and you've only tested sound? The guest could configure each device to use a different IRQ, also mapping them so they share one ISA interrupt. What happens if multiple devices are mapped to IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all share this IRQ) and more than one such device wants to raise an interrupt at the same time? If you ack the ac97 interrupt but a PCI network card or the USB part still wants to get the CPUs attention the ISA IRQ should remain raised until all devices are serviced. >>> >>> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle >>> exactly that case very well. >>> >>>> I don't see a way to track the status of all devices in a single qemu_irq which can only be up or down so we need something to store the state of each source. >>> >>> pci_set_irq() causes pci_bus_change_irq_level() to be called. >>> pci_bus_change_irq_level() tracks the sum of all irq levels of all >>> devices attached to a particular pin in irq_count. Have a look at >>> pci_bus_change_irq_level() and you will understand better. >> >> I'm aware of that, we're using that in sam460ex which connects all PCI >> interrupt lines to a single IRQ and Peter explored and explained it in >> a comment there when that was discovered. First we had a patch with >> or-irq but due to this behaviot that's not needed for PCI interrupts. >> But the VT8132 could change what ISA IRQ you route the sub functions >> to. > > That depends on the sub function if you can do that. And if so, then it > depends on whether the function is still in PCI mode (see below). > >> It happens that on pegasos2 by default all of those are routed to IRQ9 except IDE > > All *PCI* interrupts are routed to IRQ9 while IDE legacy interrupts are > routed to the compatible ISA IRQs. Note that the IDE function must only > trigger the ISA IRQs if it is in legacy mode while it must only trigger > the PCI IRQ in non-legacy mode. See https://www.bswd.com/pciide.pdf for > more details on this particular topic. The docs say so but based on what guests that work on real hardware do it does not work that way. Look up previous discussion on this on the list from around the time Mark changed via-ide about 4-5 years ago. That series was a result of his review of my proposed changes and gave resuled in an alternative appdroach. On pegasos2 (and probably also on fuloong2e based on same later findings, see patches to that, I can try to find these later if you can't find them) via-ide *always* uses IRQ 14/15 and the native mode only switches register addresses from legacy io ports to PCI io space so you can set it in with BAR regs but the IRQs don't change despite what the docs say. There are some hacks in Linux kernel and other guests to account for this but the comments for the reason are wrong in Linux, they say IDE is always in legacy mode but in fact if has a half-native mode which is what I called it where io addresses are set with BARs but IRQs are still the legacy ISA ones. You can find some references in previous discussion. Probably searching for via-ide half-native mode might find it. >> but what if a guest changes ac97 to use a different interrupt? Then >> it's not a PCI interrupt any more so you can't use pci_set_irq in >> via=ac97. > > How would it do that? AFAICS there is no dedicated register to configure > which IRQ to use. This means that it can only trigger an interrupt via > its PCI intx pin which is subject to the PCI -> ISA IRQ router. The VIA functions can use their PCI_INTERRUPT_LINE (0x3c) registers to set their ISA IRQ according to the docs (and unlike IDE in other functions like USB and sound this probably also works) and the PIRQA-D pins can be mapped to ISA IRQs by the 0x55-0x57 config registers of the isa bridge (function0). This is what I implemented in via_isa_set_irq() in this series. >> There are only 4 PCI INT lines but the VIA components can be routed to 13 or 14 ISA IRQs. > > Pure PCI components are only able to trigger one of the four PCI intx > pins they are *hardwired* to. This is true for PCI cards which can only use the 4 pins the slot they are in is wired to. These come in through the PIRQA-D pins and they are routed with the funstion 0 0x55-0x57 config registers. But I'm not sure about the internal functions. > Each component has only one pin. Which ISA > IRQ gets triggered through that pin can be selected from 13 or 14 ISA > IRQs as you say by means of the three configuration registers of the PCI > -> ISA IRQ router. So you say that internal functions are also wired to the same 4 lines like normal PCI cards? Then how can you route them to different interrupts setting their config reg 0x3c independent of function0 0x55-0x57 regs? >> How do you keep track of that with only the PCI bus interrupts? > > Devices that operate in ISA mode such as the IDE function shall have > their own, dedicated ISA IRQs assigned by the guest. Otherwise this > causes a classic interrupt conflict, just like in the olden ISA days. If > the function operates in PCI mode, it must not trigger the ISA IRQs, > regardless of whether they are assigned or not. This does not match with guests which clearly expect to get ISA IRQ9 for PCI cards and USB and sound which is where these are routed within the VIA bridge as the firmware programs it. > There is also the power management function whose ACPI interrupt (SCI) > can be routed by means of a dedicated register. Again, a guest must make > sure here to not configure interrupt conflicts. > >> I don't get your approach. > > I hope that I could help you get a better understanding. The linked .pdf > is good and comprehensive reading material. I'm not sure the via-ide confirms to that doc but it's also not any more a problem with via-ide now. That was discussed to death back then and "fixed" to work for the cases we want it to work with. We probably never agreed on how this really works but at least what we ended up with works with guests that run on real hardware. I'm OK with also making these cases work that we want now such as network and sound card under AmigaOS and sound under MorphOS (as long as you don't use USB) on pegasos2. This series does that so unless it breaks something that worked before I condider this moving forward and we can always improve adn fix it later. I'm not saying I'm not interested in your improvements just that let's that not hold this back now as we can fix and improve it later but otherwise users will have to wait until September to be able to use it. I know a few who want this and getting this out as it is would allow more people to test it and report problems so unless there are clearly wrong parts I'm OK with less than perfect but working solution as long as it's not too messy. Regards, BALATON Zoltan
Am 22. Februar 2023 23:00:02 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >On Wed, 22 Feb 2023, Bernhard Beschow wrote: >> Am 22. Februar 2023 21:12:01 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote: >>>>>>> I've had a closer look at your series and I think it can be simplified: >>>>>>> Patch 2 can be implemented quite straight-forward like I proposed in a >>>>>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing. >>>>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing" >>>>>>> working, one can expose the PCI interrupts with a single line like you do >>>>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt >>>>>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted. >>>>>>> >>>>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be >>>>>>> used instead. pci_set_irq() internally takes care of all the ISA interrupt >>>>>>> level tracking patch 1 attempted to address. >>>>>>> >>>>>> >>>>>> Here is a proof of concept branch to demonstrate that the simplification >>>>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested >>>>>> with MorphOS with and without pegasos2.rom). >>>>> >>>>> Does this only work because both the via-ac97 and the PCI interrupts are mapped to the same ISA IRQ and you've only tested sound? The guest could configure each device to use a different IRQ, also mapping them so they share one ISA interrupt. What happens if multiple devices are mapped to IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all share this IRQ) and more than one such device wants to raise an interrupt at the same time? If you ack the ac97 interrupt but a PCI network card or the USB part still wants to get the CPUs attention the ISA IRQ should remain raised until all devices are serviced. >>>> >>>> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle >>>> exactly that case very well. >>>> >>>>> I don't see a way to track the status of all devices in a single qemu_irq which can only be up or down so we need something to store the state of each source. >>>> >>>> pci_set_irq() causes pci_bus_change_irq_level() to be called. >>>> pci_bus_change_irq_level() tracks the sum of all irq levels of all >>>> devices attached to a particular pin in irq_count. Have a look at >>>> pci_bus_change_irq_level() and you will understand better. >>> >>> I'm aware of that, we're using that in sam460ex which connects all PCI interrupt lines to a single IRQ and Peter explored and explained it in a comment there when that was discovered. First we had a patch with or-irq but due to this behaviot that's not needed for PCI interrupts. But the VT8132 could change what ISA IRQ you route the sub functions to. >> >> That depends on the sub function if you can do that. And if so, then it depends on whether the function is still in PCI mode (see below). >> >>> It happens that on pegasos2 by default all of those are routed to IRQ9 except IDE >> >> All *PCI* interrupts are routed to IRQ9 while IDE legacy interrupts are routed to the compatible ISA IRQs. Note that the IDE function must only trigger the ISA IRQs if it is in legacy mode while it must only trigger the PCI IRQ in non-legacy mode. See https://www.bswd.com/pciide.pdf for more details on this particular topic. > >The docs say so but based on what guests that work on real hardware do it does not work that way. Look up previous discussion on this on the list from around the time Mark changed via-ide about 4-5 years ago. That series was a result of his review of my proposed changes and gave resuled in an alternative appdroach. On pegasos2 (and probably also on fuloong2e based on same later findings, see patches to that, I can try to find these later if you can't find them) via-ide *always* uses IRQ 14/15 and the native mode only switches register addresses from legacy io ports to PCI io space so you can set it in with BAR regs but the IRQs don't change despite what the docs say. There are some hacks in Linux kernel and other guests to account for this but the comments for the reason are wrong in Linux, they say IDE is always in legacy mode but in fact if has a half-native mode which is what I called it where io addresses are set with BARs but IRQs are still the legacy ISA ones. You can find some references in previous discussion. Probably searching for via-ide half-native mode might find it. > >>> but what if a guest changes ac97 to use a different interrupt? Then it's not a PCI interrupt any more so you can't use pci_set_irq in via=ac97. >> >> How would it do that? AFAICS there is no dedicated register to configure which IRQ to use. This means that it can only trigger an interrupt via its PCI intx pin which is subject to the PCI -> ISA IRQ router. > >The VIA functions can use their PCI_INTERRUPT_LINE (0x3c) registers to set their ISA IRQ according to the docs (and unlike IDE in other functions like USB and sound this probably also works) and the PIRQA-D pins can be mapped to ISA IRQs by the 0x55-0x57 config registers of the isa bridge (function0). This is what I implemented in via_isa_set_irq() in this series. > >>> There are only 4 PCI INT lines but the VIA components can be routed to 13 or 14 ISA IRQs. >> >> Pure PCI components are only able to trigger one of the four PCI intx pins they are *hardwired* to. > >This is true for PCI cards which can only use the 4 pins the slot they are in is wired to. These come in through the PIRQA-D pins and they are routed with the funstion 0 0x55-0x57 config registers. But I'm not sure about the internal functions. > >> Each component has only one pin. Which ISA IRQ gets triggered through that pin can be selected from 13 or 14 ISA IRQs as you say by means of the three configuration registers of the PCI -> ISA IRQ router. > >So you say that internal functions are also wired to the same 4 lines like normal PCI cards? Yes. > Then how can you route them to different interrupts setting their config reg 0x3c independent of function0 0x55-0x57 regs? 0x3c isn't supposed to be interpretet by hardware, and in general hardware can't: 0x3c is standardized for every PCI function which includes standalone PCI devices in particular. Standalone PCI devices don't have access to an IRQ router. So if they don't, how could they possibly configure the IRQ they are triggering? 0x3c is only information to the OS (populated by the BIOS). It merily indicates that the PCI device needs attention when the IRQ configured in 0x3c is raised. See comment 4 in https://community.osr.com/discussion/30399/read-only-pci-interrupt-line-register for another explanation. Even though the south bridge contains an interrupt router doesn't mean that its PCI functions can configure their IRQ through their 0x3c registers. That would change the semantics of standardized PCI registers which is surely not permitted by the standard. Instead, the PCI IRQs are configured through the device-specific 0x55-0x57 regs. I see that 0x3c is also used for the USB functions. They used to trigger the raw ISA IRQs before your series which seems wrong. I think 0x3c usage needs to be cleaned up in the VIA model. Otherwise this will likely cause problems elsewhere. > >>> How do you keep track of that with only the PCI bus interrupts? >> >> Devices that operate in ISA mode such as the IDE function shall have their own, dedicated ISA IRQs assigned by the guest. Otherwise this causes a classic interrupt conflict, just like in the olden ISA days. If the function operates in PCI mode, it must not trigger the ISA IRQs, regardless of whether they are assigned or not. > >This does not match with guests which clearly expect to get ISA IRQ9 for PCI cards and USB and sound which is where these are routed within the VIA bridge as the firmware programs it. What I meant was that a component able to operate in native/legacy/mixed mode such as IDE must not use both PCI and legacy ISA interrupts at the same time. Multiple PCI functions may of course share interrupts. > >> There is also the power management function whose ACPI interrupt (SCI) can be routed by means of a dedicated register. Again, a guest must make sure here to not configure interrupt conflicts. >> >>> I don't get your approach. >> >> I hope that I could help you get a better understanding. The linked .pdf is good and comprehensive reading material. > >I'm not sure the via-ide confirms to that doc but it's also not any more a problem with via-ide now. That was discussed to death back then and "fixed" to work for the cases we want it to work with. We probably never agreed on how this really works but at least what we ended up with works with guests that run on real hardware. I'm OK with also making these cases work that we want now such as network and sound card under AmigaOS and sound under MorphOS (as long as you don't use USB) on pegasos2. This series does that so unless it breaks something that worked before I condider this moving forward and we can always improve adn fix it later. I'm not saying I'm not interested in your improvements just that let's that not hold this back now as we can fix and improve it later but otherwise users will have to wait until September to be able to use it. I know a few who want this and getting this out as it is would allow more people to test it and report problems so unless there are clearly wrong parts I'm OK with less than perfect but working solution as long as it's not too messy. Patch 1 really seems like duplicating PCI code that already exists in QEMU. This is not needed and we should avoid that. Moreover, usage of the IRQ line register (0x3c) for interrupt routing should be switched to using the 0x55-0x57 regs to be PCI compliant. Thanks to your great work to make via-ac97 work we can confirm that both IRQ routing implementations basically work for now. Let's work out a solution that relies on existing code, sticks to the standard and hopefully works for i386 and MIPS, too. Best regards, Bernhard > >Regards, >BALATON Zoltan
On 23/02/2023 09:13, Bernhard Beschow wrote: > Am 22. Februar 2023 23:00:02 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>> Am 22. Februar 2023 21:12:01 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>>>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote: >>>>>>>> I've had a closer look at your series and I think it can be simplified: >>>>>>>> Patch 2 can be implemented quite straight-forward like I proposed in a >>>>>>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing. >>>>>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing" >>>>>>>> working, one can expose the PCI interrupts with a single line like you do >>>>>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt >>>>>>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted. >>>>>>>> >>>>>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be >>>>>>>> used instead. pci_set_irq() internally takes care of all the ISA interrupt >>>>>>>> level tracking patch 1 attempted to address. >>>>>>>> >>>>>>> >>>>>>> Here is a proof of concept branch to demonstrate that the simplification >>>>>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested >>>>>>> with MorphOS with and without pegasos2.rom). >>>>>> >>>>>> Does this only work because both the via-ac97 and the PCI interrupts are mapped to the same ISA IRQ and you've only tested sound? The guest could configure each device to use a different IRQ, also mapping them so they share one ISA interrupt. What happens if multiple devices are mapped to IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all share this IRQ) and more than one such device wants to raise an interrupt at the same time? If you ack the ac97 interrupt but a PCI network card or the USB part still wants to get the CPUs attention the ISA IRQ should remain raised until all devices are serviced. >>>>> >>>>> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle >>>>> exactly that case very well. >>>>> >>>>>> I don't see a way to track the status of all devices in a single qemu_irq which can only be up or down so we need something to store the state of each source. >>>>> >>>>> pci_set_irq() causes pci_bus_change_irq_level() to be called. >>>>> pci_bus_change_irq_level() tracks the sum of all irq levels of all >>>>> devices attached to a particular pin in irq_count. Have a look at >>>>> pci_bus_change_irq_level() and you will understand better. >>>> >>>> I'm aware of that, we're using that in sam460ex which connects all PCI interrupt lines to a single IRQ and Peter explored and explained it in a comment there when that was discovered. First we had a patch with or-irq but due to this behaviot that's not needed for PCI interrupts. But the VT8132 could change what ISA IRQ you route the sub functions to. >>> >>> That depends on the sub function if you can do that. And if so, then it depends on whether the function is still in PCI mode (see below). >>> >>>> It happens that on pegasos2 by default all of those are routed to IRQ9 except IDE >>> >>> All *PCI* interrupts are routed to IRQ9 while IDE legacy interrupts are routed to the compatible ISA IRQs. Note that the IDE function must only trigger the ISA IRQs if it is in legacy mode while it must only trigger the PCI IRQ in non-legacy mode. See https://www.bswd.com/pciide.pdf for more details on this particular topic. >> >> The docs say so but based on what guests that work on real hardware do it does not work that way. Look up previous discussion on this on the list from around the time Mark changed via-ide about 4-5 years ago. That series was a result of his review of my proposed changes and gave resuled in an alternative appdroach. On pegasos2 (and probably also on fuloong2e based on same later findings, see patches to that, I can try to find these later if you can't find them) via-ide *always* uses IRQ 14/15 and the native mode only switches register addresses from legacy io ports to PCI io space so you can set it in with BAR regs but the IRQs don't change despite what the docs say. There are some hacks in Linux kernel and other guests to account for this but the comments for the reason are wrong in Linux, they say IDE is always in legacy mode but in fact if has a half-native mode which is what I called it where io addresses are set with BARs but IRQs are still the legacy ISA ones. You can find some references in previous discussion. Probably searching for via-ide half-native mode might find it. >> >>>> but what if a guest changes ac97 to use a different interrupt? Then it's not a PCI interrupt any more so you can't use pci_set_irq in via=ac97. >>> >>> How would it do that? AFAICS there is no dedicated register to configure which IRQ to use. This means that it can only trigger an interrupt via its PCI intx pin which is subject to the PCI -> ISA IRQ router. >> >> The VIA functions can use their PCI_INTERRUPT_LINE (0x3c) registers to set their ISA IRQ according to the docs (and unlike IDE in other functions like USB and sound this probably also works) and the PIRQA-D pins can be mapped to ISA IRQs by the 0x55-0x57 config registers of the isa bridge (function0). This is what I implemented in via_isa_set_irq() in this series. >> >>>> There are only 4 PCI INT lines but the VIA components can be routed to 13 or 14 ISA IRQs. >>> >>> Pure PCI components are only able to trigger one of the four PCI intx pins they are *hardwired* to. >> >> This is true for PCI cards which can only use the 4 pins the slot they are in is wired to. These come in through the PIRQA-D pins and they are routed with the funstion 0 0x55-0x57 config registers. But I'm not sure about the internal functions. >> >>> Each component has only one pin. Which ISA IRQ gets triggered through that pin can be selected from 13 or 14 ISA IRQs as you say by means of the three configuration registers of the PCI -> ISA IRQ router. >> >> So you say that internal functions are also wired to the same 4 lines like normal PCI cards? > > Yes. > >> Then how can you route them to different interrupts setting their config reg 0x3c independent of function0 0x55-0x57 regs? > > 0x3c isn't supposed to be interpretet by hardware, and in general hardware can't: 0x3c is standardized for every PCI function which includes standalone PCI devices in particular. Standalone PCI devices don't have access to an IRQ router. So if they don't, how could they possibly configure the IRQ they are triggering? > > 0x3c is only information to the OS (populated by the BIOS). It merily indicates that the PCI device needs attention when the IRQ configured in 0x3c is raised. See comment 4 in https://community.osr.com/discussion/30399/read-only-pci-interrupt-line-register for another explanation. > > Even though the south bridge contains an interrupt router doesn't mean that its PCI functions can configure their IRQ through their 0x3c registers. That would change the semantics of standardized PCI registers which is surely not permitted by the standard. Instead, the PCI IRQs are configured through the device-specific 0x55-0x57 regs. > > I see that 0x3c is also used for the USB functions. They used to trigger the raw ISA IRQs before your series which seems wrong. I think 0x3c usage needs to be cleaned up in the VIA model. Otherwise this will likely cause problems elsewhere. > >> >>>> How do you keep track of that with only the PCI bus interrupts? >>> >>> Devices that operate in ISA mode such as the IDE function shall have their own, dedicated ISA IRQs assigned by the guest. Otherwise this causes a classic interrupt conflict, just like in the olden ISA days. If the function operates in PCI mode, it must not trigger the ISA IRQs, regardless of whether they are assigned or not. >> >> This does not match with guests which clearly expect to get ISA IRQ9 for PCI cards and USB and sound which is where these are routed within the VIA bridge as the firmware programs it. > > What I meant was that a component able to operate in native/legacy/mixed mode such as IDE must not use both PCI and legacy ISA interrupts at the same time. Multiple PCI functions may of course share interrupts. > >> >>> There is also the power management function whose ACPI interrupt (SCI) can be routed by means of a dedicated register. Again, a guest must make sure here to not configure interrupt conflicts. >>> >>>> I don't get your approach. >>> >>> I hope that I could help you get a better understanding. The linked .pdf is good and comprehensive reading material. >> >> I'm not sure the via-ide confirms to that doc but it's also not any more a problem with via-ide now. That was discussed to death back then and "fixed" to work for the cases we want it to work with. We probably never agreed on how this really works but at least what we ended up with works with guests that run on real hardware. I'm OK with also making these cases work that we want now such as network and sound card under AmigaOS and sound under MorphOS (as long as you don't use USB) on pegasos2. This series does that so unless it breaks something that worked before I condider this moving forward and we can always improve adn fix it later. I'm not saying I'm not interested in your improvements just that let's that not hold this back now as we can fix and improve it later but otherwise users will have to wait until September to be able to use it. I know a few who want this and getting this out as it is would allow more people to test it and report problems so unless there are clearly wrong parts I'm OK with less than perfect but working solution as long as it's not too messy. > > Patch 1 really seems like duplicating PCI code that already exists in QEMU. This is not needed and we should avoid that. > > Moreover, usage of the IRQ line register (0x3c) for interrupt routing should be switched to using the 0x55-0x57 regs to be PCI compliant. > > Thanks to your great work to make via-ac97 work we can confirm that both IRQ routing implementations basically work for now. Let's work out a solution that relies on existing code, sticks to the standard and hopefully works for i386 and MIPS, too. FWIW this analysis seems correct to me based upon my experience with older PCs and reading the various specifications. ATB, Mark.
On Wed, 1 Mar 2023, Mark Cave-Ayland wrote: > On 23/02/2023 09:13, Bernhard Beschow wrote: >> Am 22. Februar 2023 23:00:02 UTC schrieb BALATON Zoltan >> <balaton@eik.bme.hu>: >>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>> Am 22. Februar 2023 21:12:01 UTC schrieb BALATON Zoltan >>>> <balaton@eik.bme.hu>: >>>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>>> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan >>>>>> <balaton@eik.bme.hu>: >>>>>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>>>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> >>>>>>>> wrote: >>>>>>>>> I've had a closer look at your series and I think it can be >>>>>>>>> simplified: >>>>>>>>> Patch 2 can be implemented quite straight-forward like I proposed in >>>>>>>>> a >>>>>>>>> private mail: >>>>>>>>> https://github.com/shentok/qemu/commit/via-priq-routing. >>>>>>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt >>>>>>>>> routing" >>>>>>>>> working, one can expose the PCI interrupts with a single line like >>>>>>>>> you do >>>>>>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt >>>>>>>>> routing in via_isa_set_irq" isn't needed any longer and can be >>>>>>>>> omitted. >>>>>>>>> >>>>>>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can >>>>>>>>> be >>>>>>>>> used instead. pci_set_irq() internally takes care of all the ISA >>>>>>>>> interrupt >>>>>>>>> level tracking patch 1 attempted to address. >>>>>>>>> >>>>>>>> >>>>>>>> Here is a proof of concept branch to demonstrate that the >>>>>>>> simplification >>>>>>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 >>>>>>>> (Tested >>>>>>>> with MorphOS with and without pegasos2.rom). >>>>>>> >>>>>>> Does this only work because both the via-ac97 and the PCI interrupts >>>>>>> are mapped to the same ISA IRQ and you've only tested sound? The guest >>>>>>> could configure each device to use a different IRQ, also mapping them >>>>>>> so they share one ISA interrupt. What happens if multiple devices are >>>>>>> mapped to IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 >>>>>>> and USB all share this IRQ) and more than one such device wants to >>>>>>> raise an interrupt at the same time? If you ack the ac97 interrupt but >>>>>>> a PCI network card or the USB part still wants to get the CPUs >>>>>>> attention the ISA IRQ should remain raised until all devices are >>>>>>> serviced. >>>>>> >>>>>> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle >>>>>> exactly that case very well. >>>>>> >>>>>>> I don't see a way to track the status of all devices in a single >>>>>>> qemu_irq which can only be up or down so we need something to store >>>>>>> the state of each source. >>>>>> >>>>>> pci_set_irq() causes pci_bus_change_irq_level() to be called. >>>>>> pci_bus_change_irq_level() tracks the sum of all irq levels of all >>>>>> devices attached to a particular pin in irq_count. Have a look at >>>>>> pci_bus_change_irq_level() and you will understand better. >>>>> >>>>> I'm aware of that, we're using that in sam460ex which connects all PCI >>>>> interrupt lines to a single IRQ and Peter explored and explained it in a >>>>> comment there when that was discovered. First we had a patch with or-irq >>>>> but due to this behaviot that's not needed for PCI interrupts. But the >>>>> VT8132 could change what ISA IRQ you route the sub functions to. >>>> >>>> That depends on the sub function if you can do that. And if so, then it >>>> depends on whether the function is still in PCI mode (see below). >>>> >>>>> It happens that on pegasos2 by default all of those are routed to IRQ9 >>>>> except IDE >>>> >>>> All *PCI* interrupts are routed to IRQ9 while IDE legacy interrupts are >>>> routed to the compatible ISA IRQs. Note that the IDE function must only >>>> trigger the ISA IRQs if it is in legacy mode while it must only trigger >>>> the PCI IRQ in non-legacy mode. See https://www.bswd.com/pciide.pdf for >>>> more details on this particular topic. >>> >>> The docs say so but based on what guests that work on real hardware do it >>> does not work that way. Look up previous discussion on this on the list >>> from around the time Mark changed via-ide about 4-5 years ago. That series >>> was a result of his review of my proposed changes and gave resuled in an >>> alternative appdroach. On pegasos2 (and probably also on fuloong2e based >>> on same later findings, see patches to that, I can try to find these later >>> if you can't find them) via-ide *always* uses IRQ 14/15 and the native >>> mode only switches register addresses from legacy io ports to PCI io space >>> so you can set it in with BAR regs but the IRQs don't change despite what >>> the docs say. There are some hacks in Linux kernel and other guests to >>> account for this but the comments for the reason are wrong in Linux, they >>> say IDE is always in legacy mode but in fact if has a half-native mode >>> which is what I called it where io addresses are set with BARs but IRQs >>> are still the legacy ISA ones. You can find some references in previous >>> discussion. Probably searching for via-ide half-native mode might find it. >>> >>>>> but what if a guest changes ac97 to use a different interrupt? Then it's >>>>> not a PCI interrupt any more so you can't use pci_set_irq in via=ac97. >>>> >>>> How would it do that? AFAICS there is no dedicated register to configure >>>> which IRQ to use. This means that it can only trigger an interrupt via >>>> its PCI intx pin which is subject to the PCI -> ISA IRQ router. >>> >>> The VIA functions can use their PCI_INTERRUPT_LINE (0x3c) registers to set >>> their ISA IRQ according to the docs (and unlike IDE in other functions >>> like USB and sound this probably also works) and the PIRQA-D pins can be >>> mapped to ISA IRQs by the 0x55-0x57 config registers of the isa bridge >>> (function0). This is what I implemented in via_isa_set_irq() in this >>> series. >>> >>>>> There are only 4 PCI INT lines but the VIA components can be routed to >>>>> 13 or 14 ISA IRQs. >>>> >>>> Pure PCI components are only able to trigger one of the four PCI intx >>>> pins they are *hardwired* to. >>> >>> This is true for PCI cards which can only use the 4 pins the slot they are >>> in is wired to. These come in through the PIRQA-D pins and they are routed >>> with the funstion 0 0x55-0x57 config registers. But I'm not sure about the >>> internal functions. >>> >>>> Each component has only one pin. Which ISA IRQ gets triggered through >>>> that pin can be selected from 13 or 14 ISA IRQs as you say by means of >>>> the three configuration registers of the PCI -> ISA IRQ router. >>> >>> So you say that internal functions are also wired to the same 4 lines like >>> normal PCI cards? >> >> Yes. >> >>> Then how can you route them to different interrupts setting their config >>> reg 0x3c independent of function0 0x55-0x57 regs? >> >> 0x3c isn't supposed to be interpretet by hardware, and in general hardware >> can't: 0x3c is standardized for every PCI function which includes >> standalone PCI devices in particular. Standalone PCI devices don't have >> access to an IRQ router. So if they don't, how could they possibly >> configure the IRQ they are triggering? >> >> 0x3c is only information to the OS (populated by the BIOS). It merily >> indicates that the PCI device needs attention when the IRQ configured in >> 0x3c is raised. See comment 4 in >> https://community.osr.com/discussion/30399/read-only-pci-interrupt-line-register >> for another explanation. >> >> Even though the south bridge contains an interrupt router doesn't mean that >> its PCI functions can configure their IRQ through their 0x3c registers. >> That would change the semantics of standardized PCI registers which is >> surely not permitted by the standard. Instead, the PCI IRQs are configured >> through the device-specific 0x55-0x57 regs. >> >> I see that 0x3c is also used for the USB functions. They used to trigger >> the raw ISA IRQs before your series which seems wrong. I think 0x3c usage >> needs to be cleaned up in the VIA model. Otherwise this will likely cause >> problems elsewhere. >> >>> >>>>> How do you keep track of that with only the PCI bus interrupts? >>>> >>>> Devices that operate in ISA mode such as the IDE function shall have >>>> their own, dedicated ISA IRQs assigned by the guest. Otherwise this >>>> causes a classic interrupt conflict, just like in the olden ISA days. If >>>> the function operates in PCI mode, it must not trigger the ISA IRQs, >>>> regardless of whether they are assigned or not. >>> >>> This does not match with guests which clearly expect to get ISA IRQ9 for >>> PCI cards and USB and sound which is where these are routed within the VIA >>> bridge as the firmware programs it. >> >> What I meant was that a component able to operate in native/legacy/mixed >> mode such as IDE must not use both PCI and legacy ISA interrupts at the >> same time. Multiple PCI functions may of course share interrupts. >> >>> >>>> There is also the power management function whose ACPI interrupt (SCI) >>>> can be routed by means of a dedicated register. Again, a guest must make >>>> sure here to not configure interrupt conflicts. >>>> >>>>> I don't get your approach. >>>> >>>> I hope that I could help you get a better understanding. The linked .pdf >>>> is good and comprehensive reading material. >>> >>> I'm not sure the via-ide confirms to that doc but it's also not any more a >>> problem with via-ide now. That was discussed to death back then and >>> "fixed" to work for the cases we want it to work with. We probably never >>> agreed on how this really works but at least what we ended up with works >>> with guests that run on real hardware. I'm OK with also making these cases >>> work that we want now such as network and sound card under AmigaOS and >>> sound under MorphOS (as long as you don't use USB) on pegasos2. This >>> series does that so unless it breaks something that worked before I >>> condider this moving forward and we can always improve adn fix it later. >>> I'm not saying I'm not interested in your improvements just that let's >>> that not hold this back now as we can fix and improve it later but >>> otherwise users will have to wait until September to be able to use it. I >>> know a few who want this and getting this out as it is would allow more >>> people to test it and report problems so unless there are clearly wrong >>> parts I'm OK with less than perfect but working solution as long as it's >>> not too messy. >> >> Patch 1 really seems like duplicating PCI code that already exists in QEMU. >> This is not needed and we should avoid that. >> >> Moreover, usage of the IRQ line register (0x3c) for interrupt routing >> should be switched to using the 0x55-0x57 regs to be PCI compliant. >> >> Thanks to your great work to make via-ac97 work we can confirm that both >> IRQ routing implementations basically work for now. Let's work out a >> solution that relies on existing code, sticks to the standard and hopefully >> works for i386 and MIPS, too. > > FWIW this analysis seems correct to me based upon my experience with older > PCs and reading the various specifications. Have you read the VT82C686B and VT8231 spces which this is about? Other specs may not apply to these chips, they should confirm to their own docs. (But sometimes they may even fail to do that like in case of the IDE part.) Regards, BALATON Zoltan
Am 1. März 2023 19:24:20 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >On Wed, 1 Mar 2023, Mark Cave-Ayland wrote: >> On 23/02/2023 09:13, Bernhard Beschow wrote: >>> Am 22. Februar 2023 23:00:02 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>> Am 22. Februar 2023 21:12:01 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>>>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>>>> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>>>>>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>>>>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote: >>>>>>>>>> I've had a closer look at your series and I think it can be simplified: >>>>>>>>>> Patch 2 can be implemented quite straight-forward like I proposed in a >>>>>>>>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing. >>>>>>>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing" >>>>>>>>>> working, one can expose the PCI interrupts with a single line like you do >>>>>>>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt >>>>>>>>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted. >>>>>>>>>> >>>>>>>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be >>>>>>>>>> used instead. pci_set_irq() internally takes care of all the ISA interrupt >>>>>>>>>> level tracking patch 1 attempted to address. >>>>>>>>>> >>>>>>>>> >>>>>>>>> Here is a proof of concept branch to demonstrate that the simplification >>>>>>>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested >>>>>>>>> with MorphOS with and without pegasos2.rom). >>>>>>>> >>>>>>>> Does this only work because both the via-ac97 and the PCI interrupts are mapped to the same ISA IRQ and you've only tested sound? The guest could configure each device to use a different IRQ, also mapping them so they share one ISA interrupt. What happens if multiple devices are mapped to IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all share this IRQ) and more than one such device wants to raise an interrupt at the same time? If you ack the ac97 interrupt but a PCI network card or the USB part still wants to get the CPUs attention the ISA IRQ should remain raised until all devices are serviced. >>>>>>> >>>>>>> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle >>>>>>> exactly that case very well. >>>>>>> >>>>>>>> I don't see a way to track the status of all devices in a single qemu_irq which can only be up or down so we need something to store the state of each source. >>>>>>> >>>>>>> pci_set_irq() causes pci_bus_change_irq_level() to be called. >>>>>>> pci_bus_change_irq_level() tracks the sum of all irq levels of all >>>>>>> devices attached to a particular pin in irq_count. Have a look at >>>>>>> pci_bus_change_irq_level() and you will understand better. >>>>>> >>>>>> I'm aware of that, we're using that in sam460ex which connects all PCI interrupt lines to a single IRQ and Peter explored and explained it in a comment there when that was discovered. First we had a patch with or-irq but due to this behaviot that's not needed for PCI interrupts. But the VT8132 could change what ISA IRQ you route the sub functions to. >>>>> >>>>> That depends on the sub function if you can do that. And if so, then it depends on whether the function is still in PCI mode (see below). >>>>> >>>>>> It happens that on pegasos2 by default all of those are routed to IRQ9 except IDE >>>>> >>>>> All *PCI* interrupts are routed to IRQ9 while IDE legacy interrupts are routed to the compatible ISA IRQs. Note that the IDE function must only trigger the ISA IRQs if it is in legacy mode while it must only trigger the PCI IRQ in non-legacy mode. See https://www.bswd.com/pciide.pdf for more details on this particular topic. >>>> >>>> The docs say so but based on what guests that work on real hardware do it does not work that way. Look up previous discussion on this on the list from around the time Mark changed via-ide about 4-5 years ago. That series was a result of his review of my proposed changes and gave resuled in an alternative appdroach. On pegasos2 (and probably also on fuloong2e based on same later findings, see patches to that, I can try to find these later if you can't find them) via-ide *always* uses IRQ 14/15 and the native mode only switches register addresses from legacy io ports to PCI io space so you can set it in with BAR regs but the IRQs don't change despite what the docs say. There are some hacks in Linux kernel and other guests to account for this but the comments for the reason are wrong in Linux, they say IDE is always in legacy mode but in fact if has a half-native mode which is what I called it where io addresses are set with BARs but IRQs are still the legacy ISA ones. You can find some references in previous discussion. Probably searching for via-ide half-native mode might find it. >>>> >>>>>> but what if a guest changes ac97 to use a different interrupt? Then it's not a PCI interrupt any more so you can't use pci_set_irq in via=ac97. >>>>> >>>>> How would it do that? AFAICS there is no dedicated register to configure which IRQ to use. This means that it can only trigger an interrupt via its PCI intx pin which is subject to the PCI -> ISA IRQ router. >>>> >>>> The VIA functions can use their PCI_INTERRUPT_LINE (0x3c) registers to set their ISA IRQ according to the docs (and unlike IDE in other functions like USB and sound this probably also works) and the PIRQA-D pins can be mapped to ISA IRQs by the 0x55-0x57 config registers of the isa bridge (function0). This is what I implemented in via_isa_set_irq() in this series. >>>> >>>>>> There are only 4 PCI INT lines but the VIA components can be routed to 13 or 14 ISA IRQs. >>>>> >>>>> Pure PCI components are only able to trigger one of the four PCI intx pins they are *hardwired* to. >>>> >>>> This is true for PCI cards which can only use the 4 pins the slot they are in is wired to. These come in through the PIRQA-D pins and they are routed with the funstion 0 0x55-0x57 config registers. But I'm not sure about the internal functions. >>>> >>>>> Each component has only one pin. Which ISA IRQ gets triggered through that pin can be selected from 13 or 14 ISA IRQs as you say by means of the three configuration registers of the PCI -> ISA IRQ router. >>>> >>>> So you say that internal functions are also wired to the same 4 lines like normal PCI cards? >>> >>> Yes. >>> >>>> Then how can you route them to different interrupts setting their config reg 0x3c independent of function0 0x55-0x57 regs? >>> >>> 0x3c isn't supposed to be interpretet by hardware, and in general hardware can't: 0x3c is standardized for every PCI function which includes standalone PCI devices in particular. Standalone PCI devices don't have access to an IRQ router. So if they don't, how could they possibly configure the IRQ they are triggering? >>> >>> 0x3c is only information to the OS (populated by the BIOS). It merily indicates that the PCI device needs attention when the IRQ configured in 0x3c is raised. See comment 4 in https://community.osr.com/discussion/30399/read-only-pci-interrupt-line-register for another explanation. >>> >>> Even though the south bridge contains an interrupt router doesn't mean that its PCI functions can configure their IRQ through their 0x3c registers. That would change the semantics of standardized PCI registers which is surely not permitted by the standard. Instead, the PCI IRQs are configured through the device-specific 0x55-0x57 regs. >>> >>> I see that 0x3c is also used for the USB functions. They used to trigger the raw ISA IRQs before your series which seems wrong. I think 0x3c usage needs to be cleaned up in the VIA model. Otherwise this will likely cause problems elsewhere. >>> >>>> >>>>>> How do you keep track of that with only the PCI bus interrupts? >>>>> >>>>> Devices that operate in ISA mode such as the IDE function shall have their own, dedicated ISA IRQs assigned by the guest. Otherwise this causes a classic interrupt conflict, just like in the olden ISA days. If the function operates in PCI mode, it must not trigger the ISA IRQs, regardless of whether they are assigned or not. >>>> >>>> This does not match with guests which clearly expect to get ISA IRQ9 for PCI cards and USB and sound which is where these are routed within the VIA bridge as the firmware programs it. >>> >>> What I meant was that a component able to operate in native/legacy/mixed mode such as IDE must not use both PCI and legacy ISA interrupts at the same time. Multiple PCI functions may of course share interrupts. >>> >>>> >>>>> There is also the power management function whose ACPI interrupt (SCI) can be routed by means of a dedicated register. Again, a guest must make sure here to not configure interrupt conflicts. >>>>> >>>>>> I don't get your approach. >>>>> >>>>> I hope that I could help you get a better understanding. The linked .pdf is good and comprehensive reading material. >>>> >>>> I'm not sure the via-ide confirms to that doc but it's also not any more a problem with via-ide now. That was discussed to death back then and "fixed" to work for the cases we want it to work with. We probably never agreed on how this really works but at least what we ended up with works with guests that run on real hardware. I'm OK with also making these cases work that we want now such as network and sound card under AmigaOS and sound under MorphOS (as long as you don't use USB) on pegasos2. This series does that so unless it breaks something that worked before I condider this moving forward and we can always improve adn fix it later. I'm not saying I'm not interested in your improvements just that let's that not hold this back now as we can fix and improve it later but otherwise users will have to wait until September to be able to use it. I know a few who want this and getting this out as it is would allow more people to test it and report problems so unless there are clearly wrong parts I'm OK with less than perfect but working solution as long as it's not too messy. >>> >>> Patch 1 really seems like duplicating PCI code that already exists in QEMU. This is not needed and we should avoid that. >>> >>> Moreover, usage of the IRQ line register (0x3c) for interrupt routing should be switched to using the 0x55-0x57 regs to be PCI compliant. >>> >>> Thanks to your great work to make via-ac97 work we can confirm that both IRQ routing implementations basically work for now. Let's work out a solution that relies on existing code, sticks to the standard and hopefully works for i386 and MIPS, too. >> >> FWIW this analysis seems correct to me based upon my experience with older PCs and reading the various specifications. > >Have you read the VT82C686B and VT8231 spces which this is about? Yes, I did. For the USB function it specifies the interrupt line register (offset 0x3c) as follows: 7-4 Reserved........................................always reads 0 3-0 USB Interrupt Routing........................default = 16h However, 0x16 doesn't fit into four bits. Not even 16 (decimal) would fit because the biggest representable value is 15, i.e. 0xf. This makes me wonder if the datasheet is a reliable source describing these regiserts in the first place. Best regards, Bernhard > Other specs may not apply to these chips, they should confirm to their own docs. (But sometimes they may even fail to do that like in case of the IDE part.) > >Regards, >BALATON Zoltan
On Wed, 1 Mar 2023, Bernhard Beschow wrote: > Am 1. März 2023 19:24:20 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >> On Wed, 1 Mar 2023, Mark Cave-Ayland wrote: >>> On 23/02/2023 09:13, Bernhard Beschow wrote: >>>> Am 22. Februar 2023 23:00:02 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>>> Am 22. Februar 2023 21:12:01 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>>>>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>>>>> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>>>>>>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>>>>>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote: >>>>>>>>>>> I've had a closer look at your series and I think it can be simplified: >>>>>>>>>>> Patch 2 can be implemented quite straight-forward like I proposed in a >>>>>>>>>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing. >>>>>>>>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing" >>>>>>>>>>> working, one can expose the PCI interrupts with a single line like you do >>>>>>>>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt >>>>>>>>>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted. >>>>>>>>>>> >>>>>>>>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be >>>>>>>>>>> used instead. pci_set_irq() internally takes care of all the ISA interrupt >>>>>>>>>>> level tracking patch 1 attempted to address. >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Here is a proof of concept branch to demonstrate that the simplification >>>>>>>>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested >>>>>>>>>> with MorphOS with and without pegasos2.rom). >>>>>>>>> >>>>>>>>> Does this only work because both the via-ac97 and the PCI interrupts are mapped to the same ISA IRQ and you've only tested sound? The guest could configure each device to use a different IRQ, also mapping them so they share one ISA interrupt. What happens if multiple devices are mapped to IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all share this IRQ) and more than one such device wants to raise an interrupt at the same time? If you ack the ac97 interrupt but a PCI network card or the USB part still wants to get the CPUs attention the ISA IRQ should remain raised until all devices are serviced. >>>>>>>> >>>>>>>> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle >>>>>>>> exactly that case very well. >>>>>>>> >>>>>>>>> I don't see a way to track the status of all devices in a single qemu_irq which can only be up or down so we need something to store the state of each source. >>>>>>>> >>>>>>>> pci_set_irq() causes pci_bus_change_irq_level() to be called. >>>>>>>> pci_bus_change_irq_level() tracks the sum of all irq levels of all >>>>>>>> devices attached to a particular pin in irq_count. Have a look at >>>>>>>> pci_bus_change_irq_level() and you will understand better. >>>>>>> >>>>>>> I'm aware of that, we're using that in sam460ex which connects all PCI interrupt lines to a single IRQ and Peter explored and explained it in a comment there when that was discovered. First we had a patch with or-irq but due to this behaviot that's not needed for PCI interrupts. But the VT8132 could change what ISA IRQ you route the sub functions to. >>>>>> >>>>>> That depends on the sub function if you can do that. And if so, then it depends on whether the function is still in PCI mode (see below). >>>>>> >>>>>>> It happens that on pegasos2 by default all of those are routed to IRQ9 except IDE >>>>>> >>>>>> All *PCI* interrupts are routed to IRQ9 while IDE legacy interrupts are routed to the compatible ISA IRQs. Note that the IDE function must only trigger the ISA IRQs if it is in legacy mode while it must only trigger the PCI IRQ in non-legacy mode. See https://www.bswd.com/pciide.pdf for more details on this particular topic. >>>>> >>>>> The docs say so but based on what guests that work on real hardware do it does not work that way. Look up previous discussion on this on the list from around the time Mark changed via-ide about 4-5 years ago. That series was a result of his review of my proposed changes and gave resuled in an alternative appdroach. On pegasos2 (and probably also on fuloong2e based on same later findings, see patches to that, I can try to find these later if you can't find them) via-ide *always* uses IRQ 14/15 and the native mode only switches register addresses from legacy io ports to PCI io space so you can set it in with BAR regs but the IRQs don't change despite what the docs say. There are some hacks in Linux kernel and other guests to account for this but the comments for the reason are wrong in Linux, they say IDE is always in legacy mode but in fact if has a half-native mode which is what I called it where io addresses are set with BARs but IRQs are still the legacy ISA ones. You can fin d some references in previous discussion. Probably searching for via-ide half-native mode might find it. >>>>> >>>>>>> but what if a guest changes ac97 to use a different interrupt? Then it's not a PCI interrupt any more so you can't use pci_set_irq in via=ac97. >>>>>> >>>>>> How would it do that? AFAICS there is no dedicated register to configure which IRQ to use. This means that it can only trigger an interrupt via its PCI intx pin which is subject to the PCI -> ISA IRQ router. >>>>> >>>>> The VIA functions can use their PCI_INTERRUPT_LINE (0x3c) registers to set their ISA IRQ according to the docs (and unlike IDE in other functions like USB and sound this probably also works) and the PIRQA-D pins can be mapped to ISA IRQs by the 0x55-0x57 config registers of the isa bridge (function0). This is what I implemented in via_isa_set_irq() in this series. >>>>> >>>>>>> There are only 4 PCI INT lines but the VIA components can be routed to 13 or 14 ISA IRQs. >>>>>> >>>>>> Pure PCI components are only able to trigger one of the four PCI intx pins they are *hardwired* to. >>>>> >>>>> This is true for PCI cards which can only use the 4 pins the slot they are in is wired to. These come in through the PIRQA-D pins and they are routed with the funstion 0 0x55-0x57 config registers. But I'm not sure about the internal functions. >>>>> >>>>>> Each component has only one pin. Which ISA IRQ gets triggered through that pin can be selected from 13 or 14 ISA IRQs as you say by means of the three configuration registers of the PCI -> ISA IRQ router. >>>>> >>>>> So you say that internal functions are also wired to the same 4 lines like normal PCI cards? >>>> >>>> Yes. >>>> >>>>> Then how can you route them to different interrupts setting their config reg 0x3c independent of function0 0x55-0x57 regs? >>>> >>>> 0x3c isn't supposed to be interpretet by hardware, and in general hardware can't: 0x3c is standardized for every PCI function which includes standalone PCI devices in particular. Standalone PCI devices don't have access to an IRQ router. So if they don't, how could they possibly configure the IRQ they are triggering? >>>> >>>> 0x3c is only information to the OS (populated by the BIOS). It merily indicates that the PCI device needs attention when the IRQ configured in 0x3c is raised. See comment 4 in https://community.osr.com/discussion/30399/read-only-pci-interrupt-line-register for another explanation. >>>> >>>> Even though the south bridge contains an interrupt router doesn't mean that its PCI functions can configure their IRQ through their 0x3c registers. That would change the semantics of standardized PCI registers which is surely not permitted by the standard. Instead, the PCI IRQs are configured through the device-specific 0x55-0x57 regs. >>>> >>>> I see that 0x3c is also used for the USB functions. They used to trigger the raw ISA IRQs before your series which seems wrong. I think 0x3c usage needs to be cleaned up in the VIA model. Otherwise this will likely cause problems elsewhere. >>>> >>>>> >>>>>>> How do you keep track of that with only the PCI bus interrupts? >>>>>> >>>>>> Devices that operate in ISA mode such as the IDE function shall have their own, dedicated ISA IRQs assigned by the guest. Otherwise this causes a classic interrupt conflict, just like in the olden ISA days. If the function operates in PCI mode, it must not trigger the ISA IRQs, regardless of whether they are assigned or not. >>>>> >>>>> This does not match with guests which clearly expect to get ISA IRQ9 for PCI cards and USB and sound which is where these are routed within the VIA bridge as the firmware programs it. >>>> >>>> What I meant was that a component able to operate in native/legacy/mixed mode such as IDE must not use both PCI and legacy ISA interrupts at the same time. Multiple PCI functions may of course share interrupts. >>>> >>>>> >>>>>> There is also the power management function whose ACPI interrupt (SCI) can be routed by means of a dedicated register. Again, a guest must make sure here to not configure interrupt conflicts. >>>>>> >>>>>>> I don't get your approach. >>>>>> >>>>>> I hope that I could help you get a better understanding. The linked .pdf is good and comprehensive reading material. >>>>> >>>>> I'm not sure the via-ide confirms to that doc but it's also not any more a problem with via-ide now. That was discussed to death back then and "fixed" to work for the cases we want it to work with. We probably never agreed on how this really works but at least what we ended up with works with guests that run on real hardware. I'm OK with also making these cases work that we want now such as network and sound card under AmigaOS and sound under MorphOS (as long as you don't use USB) on pegasos2. This series does that so unless it breaks something that worked before I condider this moving forward and we can always improve adn fix it later. I'm not saying I'm not interested in your improvements just that let's that not hold this back now as we can fix and improve it later but otherwise users will have to wait until September to be able to use it. I know a few who want this and getting this out as it is would allow more people to test it and report problems so unless there are clear ly wrong parts I'm OK with less than perfect but working solution as long as it's not too messy. >>>> >>>> Patch 1 really seems like duplicating PCI code that already exists in QEMU. This is not needed and we should avoid that. >>>> >>>> Moreover, usage of the IRQ line register (0x3c) for interrupt routing should be switched to using the 0x55-0x57 regs to be PCI compliant. >>>> >>>> Thanks to your great work to make via-ac97 work we can confirm that both IRQ routing implementations basically work for now. Let's work out a solution that relies on existing code, sticks to the standard and hopefully works for i386 and MIPS, too. >>> >>> FWIW this analysis seems correct to me based upon my experience with older PCs and reading the various specifications. >> >> Have you read the VT82C686B and VT8231 spces which this is about? > > Yes, I did. > > For the USB function it specifies the interrupt line register (offset 0x3c) as follows: > > 7-4 Reserved........................................always reads 0 > 3-0 USB Interrupt Routing........................default = 16h > > However, 0x16 doesn't fit into four bits. Not even 16 (decimal) would fit because the biggest representable value is 15, i.e. 0xf. > > This makes me wonder if the datasheet is a reliable source describing these regiserts in the first place. It may clearly have errors but maybe still more likely to describe the actual chip than a PCI standard which may not even existed yet at the time this chip was produced. This is a part from the 90's I think when PCI was pretty new. Also firmware and guests appear to work the way the datasheet describes (except IDE which seems to have its IRQs tied to ISA ones irrespective or 0x3c reg (but there's some other reg in ISA func along PnP IRQ regs that also talks about IDE so that may also have something to do with it). But since it works now I'm less courious to find out what's behind it. Regards, BALATON Zoltan
On Thu, 23 Feb 2023, Bernhard Beschow wrote: > Am 22. Februar 2023 23:00:02 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>> Am 22. Februar 2023 21:12:01 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >>>>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: >>>>>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote: >>>>>>>> I've had a closer look at your series and I think it can be simplified: >>>>>>>> Patch 2 can be implemented quite straight-forward like I proposed in a >>>>>>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing. >>>>>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing" >>>>>>>> working, one can expose the PCI interrupts with a single line like you do >>>>>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt >>>>>>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted. >>>>>>>> >>>>>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be >>>>>>>> used instead. pci_set_irq() internally takes care of all the ISA interrupt >>>>>>>> level tracking patch 1 attempted to address. >>>>>>>> >>>>>>> >>>>>>> Here is a proof of concept branch to demonstrate that the simplification >>>>>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested >>>>>>> with MorphOS with and without pegasos2.rom). >>>>>> >>>>>> Does this only work because both the via-ac97 and the PCI interrupts are mapped to the same ISA IRQ and you've only tested sound? The guest could configure each device to use a different IRQ, also mapping them so they share one ISA interrupt. What happens if multiple devices are mapped to IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and USB all share this IRQ) and more than one such device wants to raise an interrupt at the same time? If you ack the ac97 interrupt but a PCI network card or the USB part still wants to get the CPUs attention the ISA IRQ should remain raised until all devices are serviced. >>>>> >>>>> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle >>>>> exactly that case very well. >>>>> >>>>>> I don't see a way to track the status of all devices in a single qemu_irq which can only be up or down so we need something to store the state of each source. >>>>> >>>>> pci_set_irq() causes pci_bus_change_irq_level() to be called. >>>>> pci_bus_change_irq_level() tracks the sum of all irq levels of all >>>>> devices attached to a particular pin in irq_count. Have a look at >>>>> pci_bus_change_irq_level() and you will understand better. >>>> >>>> I'm aware of that, we're using that in sam460ex which connects all PCI interrupt lines to a single IRQ and Peter explored and explained it in a comment there when that was discovered. First we had a patch with or-irq but due to this behaviot that's not needed for PCI interrupts. But the VT8132 could change what ISA IRQ you route the sub functions to. >>> >>> That depends on the sub function if you can do that. And if so, then it depends on whether the function is still in PCI mode (see below). >>> >>>> It happens that on pegasos2 by default all of those are routed to IRQ9 except IDE >>> >>> All *PCI* interrupts are routed to IRQ9 while IDE legacy interrupts are routed to the compatible ISA IRQs. Note that the IDE function must only trigger the ISA IRQs if it is in legacy mode while it must only trigger the PCI IRQ in non-legacy mode. See https://www.bswd.com/pciide.pdf for more details on this particular topic. >> >> The docs say so but based on what guests that work on real hardware do it does not work that way. Look up previous discussion on this on the list from around the time Mark changed via-ide about 4-5 years ago. That series was a result of his review of my proposed changes and gave resuled in an alternative appdroach. On pegasos2 (and probably also on fuloong2e based on same later findings, see patches to that, I can try to find these later if you can't find them) via-ide *always* uses IRQ 14/15 and the native mode only switches register addresses from legacy io ports to PCI io space so you can set it in with BAR regs but the IRQs don't change despite what the docs say. There are some hacks in Linux kernel and other guests to account for this but the comments for the reason are wrong in Linux, they say IDE is always in legacy mode but in fact if has a half-native mode which is what I called it where io addresses are set with BARs but IRQs are still the legacy ISA ones. You can find s ome references in previous discussion. Probably searching for via-ide half-native mode might find it. >> >>>> but what if a guest changes ac97 to use a different interrupt? Then it's not a PCI interrupt any more so you can't use pci_set_irq in via=ac97. >>> >>> How would it do that? AFAICS there is no dedicated register to configure which IRQ to use. This means that it can only trigger an interrupt via its PCI intx pin which is subject to the PCI -> ISA IRQ router. >> >> The VIA functions can use their PCI_INTERRUPT_LINE (0x3c) registers to set their ISA IRQ according to the docs (and unlike IDE in other functions like USB and sound this probably also works) and the PIRQA-D pins can be mapped to ISA IRQs by the 0x55-0x57 config registers of the isa bridge (function0). This is what I implemented in via_isa_set_irq() in this series. >> >>>> There are only 4 PCI INT lines but the VIA components can be routed to 13 or 14 ISA IRQs. >>> >>> Pure PCI components are only able to trigger one of the four PCI intx pins they are *hardwired* to. >> >> This is true for PCI cards which can only use the 4 pins the slot they are in is wired to. These come in through the PIRQA-D pins and they are routed with the funstion 0 0x55-0x57 config registers. But I'm not sure about the internal functions. >> >>> Each component has only one pin. Which ISA IRQ gets triggered through that pin can be selected from 13 or 14 ISA IRQs as you say by means of the three configuration registers of the PCI -> ISA IRQ router. >> >> So you say that internal functions are also wired to the same 4 lines like normal PCI cards? > > Yes. > >> Then how can you route them to different interrupts setting their config reg 0x3c independent of function0 0x55-0x57 regs? > > 0x3c isn't supposed to be interpretet by hardware, and in general > hardware can't: 0x3c is standardized for every PCI function which > includes standalone PCI devices in particular. Standalone PCI devices > don't have access to an IRQ router. So if they don't, how could they > possibly configure the IRQ they are triggering? > > 0x3c is only information to the OS (populated by the BIOS). It merily > indicates that the PCI device needs attention when the IRQ configured in > 0x3c is raised. See comment 4 in > https://community.osr.com/discussion/30399/read-only-pci-interrupt-line-register > for another explanation. But we're not talking about notmal PCI devices attached to a PCI slot here but internal functions of the VIA southbridges which are internally connected in some way inside the chip. You (and also Mark before, check the previous discussion on via-ide half-native mode) seem to want to assume these functions are normal PCI devices and force them in that model but that does not match with what the VIA datasheet says and what guests running on these behave so I don't think we want to (or can) assume these internal functions are normal PCI devices. > Even though the south bridge contains an interrupt router doesn't mean > that its PCI functions can configure their IRQ through their 0x3c > registers. That would change the semantics of standardized PCI registers > which is surely not permitted by the standard. Instead, the PCI IRQs are > configured through the device-specific 0x55-0x57 regs. But the datasheets (both 686B and 8231) say so and guests do expect IRQ 9 when the config reg of the AC97 and USB functions are set to that by the firmware. The 0x55-0x57 regs on function0 only configures the routing of the PIRQA-D pins which are external inputs to the chip while internal functions are routed by their 0x3c reg. At least that's how I understand the docs and how all the guests I've looked at seem to work. So I think this VIA device does not fully confirm to PCI standard with respect to its internal functions that are instead kind of mixed ISA/PCI things, probably for compatibility with contemporary OS drivers or trying to fit former ISA hardware to the then new PCI bus. These chips are from the 90's when these were new and maybe not that standardised or not everybody did everything in a standard way. > I see that 0x3c is also used for the USB functions. They used to trigger > the raw ISA IRQs before your series which seems wrong. I think 0x3c > usage needs to be cleaned up in the VIA model. Otherwise this will > likely cause problems elsewhere. Again, don't look at papers that this VIA chip might not follow. If anything, look at its datasheet and guests running on it for reference. The guests expect ISA IRQs as set in 0x3c of internal functions so these functions are not regular PCI devices. They are part of the chip and behave how the chip docs say which may not be fully match a normal PCI card. I think your proposed clesn up to make these functions PCI devices would break it becuase then you can't properly route IRQs the way the datasheet says. That's why I think this series is needed. >>>> How do you keep track of that with only the PCI bus interrupts? >>> >>> Devices that operate in ISA mode such as the IDE function shall have >>> their own, dedicated ISA IRQs assigned by the guest. Otherwise this >>> causes a classic interrupt conflict, just like in the olden ISA days. >>> If the function operates in PCI mode, it must not trigger the ISA >>> IRQs, regardless of whether they are assigned or not. >> >> This does not match with guests which clearly expect to get ISA IRQ9 >> for PCI cards and USB and sound which is where these are routed within >> the VIA bridge as the firmware programs it. > > What I meant was that a component able to operate in native/legacy/mixed > mode such as IDE must not use both PCI and legacy ISA interrupts at the > same time. Multiple PCI functions may of course share interrupts. The IDE part is even more complicated than other functions because of its "half-native" mode which is not quite what the datasheet suggests yet still both pegasos2 and fuloong2e guests seem to assume it works that way and we model it so now. So I think our model is correct we just doesn't model the startup state in legacy mode which is then immediately switched to native mode by the firmware anyway so we only model that, but a quirk in real hardware seems to be that contrary to the docs the IRQs are still tied to the legasy 14/15 even when in native mode and the full native mode IRQ routing via 0x3c is not implemented. The pegasos2 firmware sets the ide function to native mode and sets 0x3c to 9 but then guests still expect to get IRQs via 14/15. This is what the Linux kernel fixes up in the device tree for it's drivers to work, MorphOS just knows and does not care about the 0x3c setting yet it excpects to be able to set io addresses with BARs so it assumes native mode with legacy IRQs. This was what we were discussing long ago and now looks like we're back to that again just with you instead of Mark. >>> There is also the power management function whose ACPI interrupt (SCI) >>> can be routed by means of a dedicated register. Again, a guest must >>> make sure here to not configure interrupt conflicts. >>> >>>> I don't get your approach. >>> >>> I hope that I could help you get a better understanding. The linked >>> .pdf is good and comprehensive reading material. >> >> I'm not sure the via-ide confirms to that doc but it's also not any >> more a problem with via-ide now. That was discussed to death back then >> and "fixed" to work for the cases we want it to work with. We probably >> never agreed on how this really works but at least what we ended up >> with works with guests that run on real hardware. I'm OK with also >> making these cases work that we want now such as network and sound card >> under AmigaOS and sound under MorphOS (as long as you don't use USB) on >> pegasos2. This series does that so unless it breaks something that >> worked before I condider this moving forward and we can always improve >> adn fix it later. I'm not saying I'm not interested in your >> improvements just that let's that not hold this back now as we can fix >> and improve it later but otherwise users will have to wait until >> September to be able to use it. I know a few who want this and getting >> this out as it is would allow more people to test it and report >> problems so unless there are clearly wrong parts I'm OK with less than >> perfect but working solution as long as it's not too messy. > > Patch 1 really seems like duplicating PCI code that already exists in > QEMU. This is not needed and we should avoid that. > > Moreover, usage of the IRQ line register (0x3c) for interrupt routing > should be switched to using the 0x55-0x57 regs to be PCI compliant. That would not work because then guests were not able to separately configure IRQs for PCI interrupt lines and internal functions which is what the datasheet says should be possible. The internal functions' IRQs are not affeceted by 0x55-0x57 but routed by different registers. I think your series only work because pegasos2 firmware progeams everything to IRQ9 but if a guest decided to change that and route e.g. USB somewhere else then it would break. My series models that a bit better but may still break if a guest routes a function to an IRQ also controlled by some ISA device (like serial or ps2 keyboard) which are currently done within QEMU's ISA model so I can't easily channel those IRQs through the via-isa.for proper routing but it's unliikely guests would want to do that so in practice my series should work. We may duplicate PCI IRQ routing here but this chip does that and more so we need to implement it as it handles more than the 4 PCI interrupts so that implementation is not enough to handle all sources this chip has. This isn't a complex piece of code though so having a similar implementation is not a problem IMO. > Thanks to your great work to make via-ac97 work we can confirm that both > IRQ routing implementations basically work for now. Let's work out a > solution that relies on existing code, sticks to the standard and > hopefully works for i386 and MIPS, too. I'm still not convinced your implementation is correct so I would atick to my series with that, that this could be improved and changed in the future in follow up series but I don't want that to hold this back now. So unless there's a good reason against taking this series now (like it breaking something) I'd like this to be merged for 8.0. I could go with your version which might work too as long as guests share IRQ of internal functions with PCI interrupts but I don't think that's correct and I think my series better models the actual hardware and more clearly separates components. whereas your proposal reuses code from PCI that's not quite adequate for the job it's supposed to do. So I think in this case that should not be reused here. The VIA chip is more complex than a simple PCI IRQ router, that's just a part of its IRQ routing. It handles more sources than just the 4 PCI interrupt lines. Regards, BALATON Zoltan
On Thu, Feb 23, 2023 at 1:34 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: > On Thu, 23 Feb 2023, Bernhard Beschow wrote: > > Am 22. Februar 2023 23:00:02 UTC schrieb BALATON Zoltan < > balaton@eik.bme.hu>: > >> On Wed, 22 Feb 2023, Bernhard Beschow wrote: > >>> Am 22. Februar 2023 21:12:01 UTC schrieb BALATON Zoltan < > balaton@eik.bme.hu>: > >>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: > >>>>> Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan < > balaton@eik.bme.hu>: > >>>>>> On Wed, 22 Feb 2023, Bernhard Beschow wrote: > >>>>>>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow < > shentey@gmail.com> wrote: > >>>>>>>> I've had a closer look at your series and I think it can be > simplified: > >>>>>>>> Patch 2 can be implemented quite straight-forward like I proposed > in a > >>>>>>>> private mail: > https://github.com/shentok/qemu/commit/via-priq-routing. > >>>>>>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI > interrupt routing" > >>>>>>>> working, one can expose the PCI interrupts with a single line > like you do > >>>>>>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement > interrupt > >>>>>>>> routing in via_isa_set_irq" isn't needed any longer and can be > omitted. > >>>>>>>> > >>>>>>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() > can be > >>>>>>>> used instead. pci_set_irq() internally takes care of all the ISA > interrupt > >>>>>>>> level tracking patch 1 attempted to address. > >>>>>>>> > >>>>>>> > >>>>>>> Here is a proof of concept branch to demonstrate that the > simplification > >>>>>>> actually works: https://github.com/shentok/qemu/commits/pegasos2 > (Tested > >>>>>>> with MorphOS with and without pegasos2.rom). > >>>>>> > >>>>>> Does this only work because both the via-ac97 and the PCI > interrupts are mapped to the same ISA IRQ and you've only tested sound? The > guest could configure each device to use a different IRQ, also mapping them > so they share one ISA interrupt. What happens if multiple devices are > mapped to IRQ 9 (which is the case on pegasos2 where PCI cards, ac97 and > USB all share this IRQ) and more than one such device wants to raise an > interrupt at the same time? If you ack the ac97 interrupt but a PCI network > card or the USB part still wants to get the CPUs attention the ISA IRQ > should remain raised until all devices are serviced. > >>>>> > >>>>> pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle > >>>>> exactly that case very well. > >>>>> > >>>>>> I don't see a way to track the status of all devices in a single > qemu_irq which can only be up or down so we need something to store the > state of each source. > >>>>> > >>>>> pci_set_irq() causes pci_bus_change_irq_level() to be called. > >>>>> pci_bus_change_irq_level() tracks the sum of all irq levels of all > >>>>> devices attached to a particular pin in irq_count. Have a look at > >>>>> pci_bus_change_irq_level() and you will understand better. > >>>> > >>>> I'm aware of that, we're using that in sam460ex which connects all > PCI interrupt lines to a single IRQ and Peter explored and explained it in > a comment there when that was discovered. First we had a patch with or-irq > but due to this behaviot that's not needed for PCI interrupts. But the > VT8132 could change what ISA IRQ you route the sub functions to. > >>> > >>> That depends on the sub function if you can do that. And if so, then > it depends on whether the function is still in PCI mode (see below). > >>> > >>>> It happens that on pegasos2 by default all of those are routed to > IRQ9 except IDE > >>> > >>> All *PCI* interrupts are routed to IRQ9 while IDE legacy interrupts > are routed to the compatible ISA IRQs. Note that the IDE function must only > trigger the ISA IRQs if it is in legacy mode while it must only trigger the > PCI IRQ in non-legacy mode. See https://www.bswd.com/pciide.pdf for more > details on this particular topic. > >> > >> The docs say so but based on what guests that work on real hardware do > it does not work that way. Look up previous discussion on this on the list > from around the time Mark changed via-ide about 4-5 years ago. That series > was a result of his review of my proposed changes and gave resuled in an > alternative appdroach. On pegasos2 (and probably also on fuloong2e based on > same later findings, see patches to that, I can try to find these later if > you can't find them) via-ide *always* uses IRQ 14/15 and the native mode > only switches register addresses from legacy io ports to PCI io space so > you can set it in with BAR regs but the IRQs don't change despite what the > docs say. There are some hacks in Linux kernel and other guests to account > for this but the comments for the reason are wrong in Linux, they say IDE > is always in legacy mode but in fact if has a half-native mode which is > what I called it where io addresses are set with BARs but IRQs are still > the legacy ISA ones. You can find s > ome references in previous discussion. Probably searching for via-ide > half-native mode might find it. > >> > >>>> but what if a guest changes ac97 to use a different interrupt? Then > it's not a PCI interrupt any more so you can't use pci_set_irq in via=ac97. > >>> > >>> How would it do that? AFAICS there is no dedicated register to > configure which IRQ to use. This means that it can only trigger an > interrupt via its PCI intx pin which is subject to the PCI -> ISA IRQ > router. > >> > >> The VIA functions can use their PCI_INTERRUPT_LINE (0x3c) registers to > set their ISA IRQ according to the docs (and unlike IDE in other functions > like USB and sound this probably also works) and the PIRQA-D pins can be > mapped to ISA IRQs by the 0x55-0x57 config registers of the isa bridge > (function0). This is what I implemented in via_isa_set_irq() in this series. > >> > >>>> There are only 4 PCI INT lines but the VIA components can be routed > to 13 or 14 ISA IRQs. > >>> > >>> Pure PCI components are only able to trigger one of the four PCI intx > pins they are *hardwired* to. > >> > >> This is true for PCI cards which can only use the 4 pins the slot they > are in is wired to. These come in through the PIRQA-D pins and they are > routed with the funstion 0 0x55-0x57 config registers. But I'm not sure > about the internal functions. > >> > >>> Each component has only one pin. Which ISA IRQ gets triggered through > that pin can be selected from 13 or 14 ISA IRQs as you say by means of the > three configuration registers of the PCI -> ISA IRQ router. > >> > >> So you say that internal functions are also wired to the same 4 lines > like normal PCI cards? > > > > Yes. > > > >> Then how can you route them to different interrupts setting their > config reg 0x3c independent of function0 0x55-0x57 regs? > > > > 0x3c isn't supposed to be interpretet by hardware, and in general > > hardware can't: 0x3c is standardized for every PCI function which > > includes standalone PCI devices in particular. Standalone PCI devices > > don't have access to an IRQ router. So if they don't, how could they > > possibly configure the IRQ they are triggering? > > > > 0x3c is only information to the OS (populated by the BIOS). It merily > > indicates that the PCI device needs attention when the IRQ configured in > > 0x3c is raised. See comment 4 in > > > https://community.osr.com/discussion/30399/read-only-pci-interrupt-line-register > > for another explanation. > > But we're not talking about notmal PCI devices attached to a PCI slot here > but internal functions of the VIA southbridges which are internally > connected in some way inside the chip. You (and also Mark before, check > the previous discussion on via-ide half-native mode) seem to want to > assume these functions are normal PCI devices and force them in that model > but that does not match with what the VIA datasheet says and what guests > running on these behave so I don't think we want to (or can) assume these > internal functions are normal PCI devices. > > > Even though the south bridge contains an interrupt router doesn't mean > > that its PCI functions can configure their IRQ through their 0x3c > > registers. That would change the semantics of standardized PCI registers > > which is surely not permitted by the standard. Instead, the PCI IRQs are > > configured through the device-specific 0x55-0x57 regs. > > But the datasheets (both 686B and 8231) say so and guests do expect IRQ 9 > when the config reg of the AC97 and USB functions are set to that by the > firmware. The 0x55-0x57 regs on function0 only configures the routing of > the PIRQA-D pins which are external inputs to the chip while internal > functions are routed by their 0x3c reg. At least that's how I understand > the docs and how all the guests I've looked at seem to work. So I think > this VIA device does not fully confirm to PCI standard with respect to its > internal functions that are instead kind of mixed ISA/PCI things, probably > for compatibility with contemporary OS drivers or trying to fit former ISA > hardware to the then new PCI bus. These chips are from the 90's when these > were new and maybe not that standardised or not everybody did everything > in a standard way. > > > I see that 0x3c is also used for the USB functions. They used to trigger > > the raw ISA IRQs before your series which seems wrong. I think 0x3c > > usage needs to be cleaned up in the VIA model. Otherwise this will > > likely cause problems elsewhere. > > Again, don't look at papers that this VIA chip might not follow. If > anything, look at its datasheet and guests running on it for reference. > The guests expect ISA IRQs as set in 0x3c of internal functions so these > functions are not regular PCI devices. They are part of the chip and > behave how the chip docs say which may not be fully match a normal PCI > card. I think your proposed clesn up to make these functions PCI devices > would break it becuase then you can't properly route IRQs the way the > datasheet says. That's why I think this series is needed. > > >>>> How do you keep track of that with only the PCI bus interrupts? > >>> > >>> Devices that operate in ISA mode such as the IDE function shall have > >>> their own, dedicated ISA IRQs assigned by the guest. Otherwise this > >>> causes a classic interrupt conflict, just like in the olden ISA days. > >>> If the function operates in PCI mode, it must not trigger the ISA > >>> IRQs, regardless of whether they are assigned or not. > >> > >> This does not match with guests which clearly expect to get ISA IRQ9 > >> for PCI cards and USB and sound which is where these are routed within > >> the VIA bridge as the firmware programs it. > > > > What I meant was that a component able to operate in native/legacy/mixed > > mode such as IDE must not use both PCI and legacy ISA interrupts at the > > same time. Multiple PCI functions may of course share interrupts. > > The IDE part is even more complicated than other functions because of its > "half-native" mode which is not quite what the datasheet suggests yet > still both pegasos2 and fuloong2e guests seem to assume it works that way > and we model it so now. So I think our model is correct we just doesn't > model the startup state in legacy mode which is then immediately switched > to native mode by the firmware anyway so we only model that, but a quirk > in real hardware seems to be that contrary to the docs the IRQs are still > tied to the legasy 14/15 even when in native mode and the full native mode > IRQ routing via 0x3c is not implemented. The pegasos2 firmware sets the > ide function to native mode and sets 0x3c to 9 but then guests still > expect to get IRQs via 14/15. This is what the Linux kernel fixes up in > the device tree for it's drivers to work, MorphOS just knows and does not > care about the 0x3c setting yet it excpects to be able to set io addresses > with BARs so it assumes native mode with legacy IRQs. This was what we > were discussing long ago and now looks like we're back to that again just > with you instead of Mark. > > >>> There is also the power management function whose ACPI interrupt (SCI) > >>> can be routed by means of a dedicated register. Again, a guest must > >>> make sure here to not configure interrupt conflicts. > >>> > >>>> I don't get your approach. > >>> > >>> I hope that I could help you get a better understanding. The linked > >>> .pdf is good and comprehensive reading material. > >> > >> I'm not sure the via-ide confirms to that doc but it's also not any > >> more a problem with via-ide now. That was discussed to death back then > >> and "fixed" to work for the cases we want it to work with. We probably > >> never agreed on how this really works but at least what we ended up > >> with works with guests that run on real hardware. I'm OK with also > >> making these cases work that we want now such as network and sound card > >> under AmigaOS and sound under MorphOS (as long as you don't use USB) on > >> pegasos2. This series does that so unless it breaks something that > >> worked before I condider this moving forward and we can always improve > >> adn fix it later. I'm not saying I'm not interested in your > >> improvements just that let's that not hold this back now as we can fix > >> and improve it later but otherwise users will have to wait until > >> September to be able to use it. I know a few who want this and getting > >> this out as it is would allow more people to test it and report > >> problems so unless there are clearly wrong parts I'm OK with less than > >> perfect but working solution as long as it's not too messy. > > > > Patch 1 really seems like duplicating PCI code that already exists in > > QEMU. This is not needed and we should avoid that. > > > > Moreover, usage of the IRQ line register (0x3c) for interrupt routing > > should be switched to using the 0x55-0x57 regs to be PCI compliant. > > That would not work because then guests were not able to separately > configure IRQs for PCI interrupt lines and internal functions which is > what the datasheet says should be possible. The internal functions' IRQs > are not affeceted by 0x55-0x57 but routed by different registers. How do you know? I think > your series only work because pegasos2 firmware progeams everything to > IRQ9 but if a guest decided to change that and route e.g. USB somewhere > else then it would break. My series models that a bit better but may still > break if a guest routes a function to an IRQ also controlled by some ISA > device (like serial or ps2 keyboard) which are currently done within > QEMU's ISA model so I can't easily channel those IRQs through the > via-isa.for proper routing but it's unliikely guests would want to do that > so in practice my series should work. We may duplicate PCI IRQ routing > here but this chip does that and more so we need to implement it as it > handles more than the 4 PCI interrupts so that implementation is not > enough to handle all sources this chip has. This isn't a complex piece of > code though so having a similar implementation is not a problem IMO. > > > Thanks to your great work to make via-ac97 work we can confirm that both > > IRQ routing implementations basically work for now. Let's work out a > > solution that relies on existing code, sticks to the standard and > > hopefully works for i386 and MIPS, too. > > I'm still not convinced your implementation is correct It seems that Mark (cc'd), I, the commenter in https://community.osr.com/discussion/30399/read-only-pci-interrupt-line-register and the PCI specification agree that the 0x3c regs aren't supposed to be interpreted by hardware. I've provided a working example with no functional downsides to the 0x3c approach. I've provided the PCI-IDE reading material that Mark suggested for reading in https://lore.kernel.org/qemu-devel/b38987d5-5530-ecd9-2fd2-3a57e1a611dd@ilande.co.uk/ . I'm running out of ideas now on how to proceed. Best regards, Bernhard > so I would atick to > my series with that, that this could be improved and changed in the future > in follow up series but I don't want that to hold this back now. So unless > there's a good reason against taking this series now (like it breaking > something) I'd like this to be merged for 8.0. I could go with your > version which might work too as long as guests share IRQ of internal > functions with PCI interrupts but I don't think that's correct and I think > my series better models the actual hardware and more clearly separates > components. whereas your proposal reuses code from PCI that's not quite > adequate for the job it's supposed to do. So I think in this case that > should not be reused here. The VIA chip is more complex than a simple PCI > IRQ router, that's just a part of its IRQ routing. It handles more sources > than just the 4 PCI interrupt lines. > > Regards, > BALATON Zoltan
On Thu, 23 Feb 2023, Bernhard Beschow wrote: > On Thu, Feb 23, 2023 at 1:34 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: >>>>>> I don't get your approach. >>>>> >>>>> I hope that I could help you get a better understanding. The linked >>>>> .pdf is good and comprehensive reading material. >>>> >>>> I'm not sure the via-ide confirms to that doc but it's also not any >>>> more a problem with via-ide now. That was discussed to death back then >>>> and "fixed" to work for the cases we want it to work with. We probably >>>> never agreed on how this really works but at least what we ended up >>>> with works with guests that run on real hardware. I'm OK with also >>>> making these cases work that we want now such as network and sound card >>>> under AmigaOS and sound under MorphOS (as long as you don't use USB) on >>>> pegasos2. This series does that so unless it breaks something that >>>> worked before I condider this moving forward and we can always improve >>>> adn fix it later. I'm not saying I'm not interested in your >>>> improvements just that let's that not hold this back now as we can fix >>>> and improve it later but otherwise users will have to wait until >>>> September to be able to use it. I know a few who want this and getting >>>> this out as it is would allow more people to test it and report >>>> problems so unless there are clearly wrong parts I'm OK with less than >>>> perfect but working solution as long as it's not too messy. >>> >>> Patch 1 really seems like duplicating PCI code that already exists in >>> QEMU. This is not needed and we should avoid that. >>> >>> Moreover, usage of the IRQ line register (0x3c) for interrupt routing >>> should be switched to using the 0x55-0x57 regs to be PCI compliant. >> >> That would not work because then guests were not able to separately >> configure IRQs for PCI interrupt lines and internal functions which is >> what the datasheet says should be possible. The internal functions' IRQs >> are not affeceted by 0x55-0x57 but routed by different registers. > > How do you know? The datasheet says so. It says that 0x55-0x57 are controlling what ISA interrupts the PIRQA-D pins should raise while internal functions are documented to have 0x3c register to select what ISA IRQ they use. It's not said internal functions would use PCI interrupts that are separate and connected to the PIRQ pins. > I think >> your series only work because pegasos2 firmware progeams everything to >> IRQ9 but if a guest decided to change that and route e.g. USB somewhere >> else then it would break. My series models that a bit better but may still >> break if a guest routes a function to an IRQ also controlled by some ISA >> device (like serial or ps2 keyboard) which are currently done within >> QEMU's ISA model so I can't easily channel those IRQs through the >> via-isa.for proper routing but it's unliikely guests would want to do that >> so in practice my series should work. We may duplicate PCI IRQ routing >> here but this chip does that and more so we need to implement it as it >> handles more than the 4 PCI interrupts so that implementation is not >> enough to handle all sources this chip has. This isn't a complex piece of >> code though so having a similar implementation is not a problem IMO. >> >>> Thanks to your great work to make via-ac97 work we can confirm that both >>> IRQ routing implementations basically work for now. Let's work out a >>> solution that relies on existing code, sticks to the standard and >>> hopefully works for i386 and MIPS, too. >> >> I'm still not convinced your implementation is correct > > > It seems that Mark (cc'd), I, the commenter in > https://community.osr.com/discussion/30399/read-only-pci-interrupt-line-register > and the PCI specification agree that the 0x3c regs aren't supposed to be > interpreted by hardware. You could still all be wrong if the PCI spec does not apply to the internal functions of the VIA chip which is just an assumption you made but the docs and experience never proved that so I don't believe that's a valid assumption. According to the datasheet internal functions' interrupts are routed independently from PCI interrupts which is what I've tried to model. > I've provided a working example with no functional > downsides to the 0x3c approach. I've provided the PCI-IDE reading material > that Mark suggested for reading in > https://lore.kernel.org/qemu-devel/b38987d5-5530-ecd9-2fd2-3a57e1a611dd@ilande.co.uk/ > . I'm running out of ideas now on how to proceed. Please submit an alternative series that we can test and if it works with the guests that I want to run like mine we can take that instead even if I believe your way is wrong. I don't care about who's right as long as it works. But make sure it gets in for 8.0 as I do care that it should work in the next release. Regards, BALATON Zoltan
On Thu, Feb 23, 2023 at 3:23 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: > On Thu, 23 Feb 2023, Bernhard Beschow wrote: > > On Thu, Feb 23, 2023 at 1:34 PM BALATON Zoltan <balaton@eik.bme.hu> > wrote: > >>>>>> I don't get your approach. > >>>>> > >>>>> I hope that I could help you get a better understanding. The linked > >>>>> .pdf is good and comprehensive reading material. > >>>> > >>>> I'm not sure the via-ide confirms to that doc but it's also not any > >>>> more a problem with via-ide now. That was discussed to death back then > >>>> and "fixed" to work for the cases we want it to work with. We probably > >>>> never agreed on how this really works but at least what we ended up > >>>> with works with guests that run on real hardware. I'm OK with also > >>>> making these cases work that we want now such as network and sound > card > >>>> under AmigaOS and sound under MorphOS (as long as you don't use USB) > on > >>>> pegasos2. This series does that so unless it breaks something that > >>>> worked before I condider this moving forward and we can always improve > >>>> adn fix it later. I'm not saying I'm not interested in your > >>>> improvements just that let's that not hold this back now as we can fix > >>>> and improve it later but otherwise users will have to wait until > >>>> September to be able to use it. I know a few who want this and getting > >>>> this out as it is would allow more people to test it and report > >>>> problems so unless there are clearly wrong parts I'm OK with less than > >>>> perfect but working solution as long as it's not too messy. > >>> > >>> Patch 1 really seems like duplicating PCI code that already exists in > >>> QEMU. This is not needed and we should avoid that. > >>> > >>> Moreover, usage of the IRQ line register (0x3c) for interrupt routing > >>> should be switched to using the 0x55-0x57 regs to be PCI compliant. > >> > >> That would not work because then guests were not able to separately > >> configure IRQs for PCI interrupt lines and internal functions which is > >> what the datasheet says should be possible. The internal functions' IRQs > >> are not affeceted by 0x55-0x57 but routed by different registers. > > > > How do you know? > > The datasheet says so. It says that 0x55-0x57 are controlling what ISA > interrupts the PIRQA-D pins should raise while internal functions are > documented to have 0x3c register to select what ISA IRQ they use. It's not > said internal functions would use PCI interrupts that are separate and > connected to the PIRQ pins. > > > I think > >> your series only work because pegasos2 firmware progeams everything to > >> IRQ9 but if a guest decided to change that and route e.g. USB somewhere > >> else then it would break. My series models that a bit better but may > still > >> break if a guest routes a function to an IRQ also controlled by some ISA > >> device (like serial or ps2 keyboard) which are currently done within > >> QEMU's ISA model so I can't easily channel those IRQs through the > >> via-isa.for proper routing but it's unliikely guests would want to do > that > >> so in practice my series should work. We may duplicate PCI IRQ routing > >> here but this chip does that and more so we need to implement it as it > >> handles more than the 4 PCI interrupts so that implementation is not > >> enough to handle all sources this chip has. This isn't a complex piece > of > >> code though so having a similar implementation is not a problem IMO. > >> > >>> Thanks to your great work to make via-ac97 work we can confirm that > both > >>> IRQ routing implementations basically work for now. Let's work out a > >>> solution that relies on existing code, sticks to the standard and > >>> hopefully works for i386 and MIPS, too. > >> > >> I'm still not convinced your implementation is correct > > > > > > It seems that Mark (cc'd), I, the commenter in > > > https://community.osr.com/discussion/30399/read-only-pci-interrupt-line-register > > and the PCI specification agree that the 0x3c regs aren't supposed to be > > interpreted by hardware. > > You could still all be wrong if the PCI spec does not apply to the > internal functions of the VIA chip which is just an assumption you made > but the docs and experience never proved that so I don't believe that's a > valid assumption. According to the datasheet internal functions' > interrupts are routed independently from PCI interrupts which is what I've > tried to model. > > > I've provided a working example with no functional > > downsides to the 0x3c approach. I've provided the PCI-IDE reading > material > > that Mark suggested for reading in > > > https://lore.kernel.org/qemu-devel/b38987d5-5530-ecd9-2fd2-3a57e1a611dd@ilande.co.uk/ > > . I'm running out of ideas now on how to proceed. > > Please submit an alternative series that we can test and if it works with > the guests that I want to run like mine we can take that instead even if I > believe your way is wrong. I don't care about who's right as long as it > works. But make sure it gets in for 8.0 as I do care that it should work > in the next release. > Here we go: https://lore.kernel.org/qemu-devel/20230223202053.117050-1-shentey@gmail.com/ Please submit further iterations when audio changes are needed. Best regards, Bernhard > > Regards, > BALATON Zoltan >
© 2016 - 2024 Red Hat, Inc.