On Sun, 29 Oct 2023, Bernhard Beschow wrote:
Am 28. Oktober 2023 17:47:15 UTC schrieb BALATON Zoltan <bala...@eik.bme.hu>:
On Sat, 28 Oct 2023, Bernhard Beschow wrote:
Am 28. Oktober 2023 12:58:32 UTC schrieb BALATON Zoltan <bala...@eik.bme.hu>:
Hello,
On Sat, 28 Oct 2023, Bernhard Beschow wrote:
This series is part of my work to bring the VIA south bridges to the PC machine
[1]. It implements missing ACPI functionality which ACPI-aware x86 guests
expect for a smooth experience. The implementation is heavily inspired by PIIX4.
I think first the interrupt routing should be fixed because that may change a
few things in this series so that should be the next step and then rebase this
series on top of that.
What I mean by fixing interrupt routing? You may remember this discussion:
https://patchew.org/QEMU/cover.1677004414.git.bala...@eik.bme.hu/
With pegasos2 your (over)simplification worked only because the firmware of
that machine maps everythnig to one ISA IRQ and guests were happy with that. I
told you that back then but could not convince you and Mark about that. Now
with the amigaone machine the firmware maps VIA devices and PCI interuupt pins
to different ISA IRQs so we need to go back either to my otiginal
implementation or something else you come up with. You can test this trying to
use USB devices with amigaone machine which only works after reverting
4e5a20b6da9b1 and 422a6e8075752. So please either propose something to fix that
first or wait with this series until I can update my pathches to solve
interrupt routing. I think this series should wait until after that because it
adds more interrupt handling which should follow whatever way we come up with
for that so it's too early fir this series yet. (If you want to try fixing it
keep in mind that in both amigaone and pegasos2 the PCI buses are in the north
br
id
ge not in the VIA south bridge so don't try to force the IRQ mapping into the
PCI bus. All the VIA chip needs to do is mapping its PIRQ/PINT pins to ISA IRQs
as the VIA is only handling ISA IRQs and all other pci stuff is handled in the
north bridge. So I think we need a via_set_isa_irq function but we could change
it according to Mark's idea to pass the PCI device and use its function number
to select itq source instead of the enum I had in my original series.)
I have some other comments that I'll add in reply to individual patches for the
future/
Hi Zoltan,
The interrupt handling introduced in this series is not related to PCI
interrupt routing: The SMI is a dedicated pin on the device and the SCI is
mapped internally to an ISA interrupt (note how the power management function
lacks the registers for PCI interrupt information). Hence, PCI interrupt
routing isn't changed in this series and therefore seems off-topic to me.
Moreover, the SMI is a new interrupt which is therefore not used in any machine
yet. The SCI is deactivated if set to IRQ 0 which is the default even. If a
guest configures it, it shall be aware to receive an *ISA* interrupt.
So I think this series shouldn't conflict with any previous work and should not
be blocked by the PCI IRQ routing topic.
The topic I've raised is not about routing PCI interrupts but routing different
IRQ sources in VIA chip (such as different functions plus the PIRQ/PINT pins)
to ISA interrupts so that would conflict with how the PM func interrupts are
routed. I think only the isa function should have qemu_irqs and it should
handle mapping of the different sources to the appropriate ISA IRQ so the
different sources (functions) should not have their own qemu_irqs or gpios but
they would just call via_isa_set_irq with their PCIDevice, pun and level and
then the isa model would do the routing. I plan to do this eventually but it
you're adding more things that would need to be reverted then it becomes more
difficult.
We've had lenghty discussions about this topic before and we -- together
-- ended up with the current solution.
Which does not work as demonstrated by the amigaone machine now. I've told
you that before but you did not accept my arguments so for the sake of
being able to merge pegasos2 in time for release I've accepted your
alternative that was still OK for pegasos2 and let this be fixed later
when we need it. That is about now that we have the amigaone machine. The
amigaone still boots and usable without fixing this IRQ mapping but some
devices like USB does not work due to not getting the expected interrupt
correctly. But if you want to change the via device further I'd like to
fix it before that to avoid having to rewrtite what you add now.
This series adds the last missing
feature to the VIA south bridges before they can be integrated into the
PC machine. Delaying progress by reopening the same topics over and over
again really seems unfair to me. Instead, let's be optimistic that we'll
end up with a solution that suits all needs well.
I'm sorry if it seems unfair to you but I did not bring this up tp delay
your work but to avoid more work in the future and fix something that is
broken to improve the device model. If you PC machine wants to map
internal functions to different IRQs then you will also get this problem
so it will need to be fixed and fixing it will also simplify your ACPI
patches. To help with this I've spent some time now to do the fix I think
would work so you could just rebase your series on top of that.
That said, I've ran both the pegasos2.rom and the u-boot.bin for
amigaone that is used in the Avocado test. I've traced both with '-trace
"pci_cfg_*"'. The result is that neither BIOS pokes the SCI routing
register in the ISA function which means that the interrupt stays
deactivated. Hence, it is very unlikely that the changes introduced in
this series would interfer with guests on these machines.
It does not interfere with guests, it inteferes with fixing probelms with
interrupt routing that amigaone (and liekly your PC machine) has so it
makes sense to fix that first because it changes the way SMI should be
added.
In summary, I don't see any blockers so far for merging this series for
the upcoming release.
I hope I could explain above.
Regards,
BALATON Zoltan
