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?