On Fri, 6 Mar 2020, Mark Cave-Ayland wrote:
On 06/03/2020 00:21, BALATON Zoltan wrote:
On Fri, 6 Mar 2020, BALATON Zoltan wrote:
On Thu, 5 Mar 2020, Mark Cave-Ayland wrote:
On 04/03/2020 22:33, BALATON Zoltan wrote:
another possibility: PCI configuration space register 0x3d (Interrupt pin) is
documented as having value 0 == Legacy IRQ routing which should be the initial
value
on reset, but QEMU incorrectly sets it to 1 which indicates PCI IRQ routing.
The VT8231 docs say this should always read 1 but may be this is somehow set to 0
on the Pegasos2. What does that mean? Should we use this value instead of the
feature bit to force using legacy interrupts? We'd still need a property in
via-ide
to set this reg or is it possible to set it from board code overriding the
default
after device is created? That would allow to drop patch 1. I can try this.
This seemed like it could simplify patches a bit but it does not work. Setting
this
reg to 0 breaks Linux and MorphOS which then think the device does not have an
interrupt at all and fail as before waiting for the irq. So we still need the
feature
bit, cant use this reg to force legacy interrupts. I've spent considerable time
testing different OSes before I've ended up with this patch series I've
submitted and
I could not find a simpler way that works with everything.
I appreciate that testing these things can take a lot of time, but what is
important
thing to ask here is whether these hacks are attempting to work around
something in
QEMU that doesn't match the hardware specification, and to me it feels that
this is
what is happening here.
It may be we need to work around some incomplete modelling of devices in
QEMU, e.g. we only model the native mode of these IDE interfaces so
anything involving legacy mode is out of scope. To also emulate legacy
mode we'd need changing common ISA code and maybe PIC code as well. As
those parts are also used by other more commonly used machine models I'd
avoid breaking those and rather implement it confined to these machines
that are not yet finished or complete anyway than try to change all
dependent devices that would need even more testing. These "hacks" could
be cleaned up later and this would not be the only hack in QEMU, I don't
have time to fix everything and it's unreasonable to demand it I think.
I'd suggest to take this patch as it is now and if you don't like it you
can submit patches that clean it up the way you think is correct or submit
an alternative patch now that shows how do you think it can be done in a
cleaner way because I don't see it and don't have more time for it now.
Obviously this thread has become quite long (and even I'm struggling to find
previous
discussions) but here is my summary below:
- I don't think the patch in its current form is the right way to do this.
Instead of
adding a feature bit to fudge the existing IRQ routing when the existing IRQ
routing
is wrong, let's fix the existing IRQ routing instead.
I think that would involve changing parts which could break other machines
so I'd rather go with a featute bit only affecting pegasos2 and fulonge2
than touch i8259 or ISA emulation basing that on some guess how the real
chip may be implemented. Is it possible to implement what you propose
without changing common IDE, ISA and PIC emulation only in via-ide and
fulong2e code?
- There is no mention of "non-100%" native mode in the 8231 or 686B datasheet:
this
is simply a term used within the Linux patches. The controller is either in
native
mode, or legacy mode. It may be that guests are making use of some undefined
behaviour here.
Yes, this is a Linux term and Linux also uses a feature bit to enable this
workaround. If that's good enough for Linux why isn't it good enough for
you?
- The code that uses the value of PCI_INTERRUPT_LINE in via-ide is incorrect
(as your
patch comment points out, some guests ignore it anyway).
You're misunderstanding the comment. The via_ide_config_read function is
needed to restore value in interrupt line that common PCI reset code
deletes. Linux depends on this value to be the same as on real hardware so
this is needed to work around QEMU and Linux pecularities.
I've tried using PCI_INTERRUPT_PIN in place of the feature bit but setting
that to 0 breaks Linux and MorphOS on pegasos2 because these apparently
expect this to be set to 1 corresponding to native mode. (Firmware only
sets native mode enable bits in prog-if but datasheet says this reg should
be 1 by default and other PCI docs say 0 here means no interrupt used so
maybe that's why Linux and MorphOS don't like it.)
- There is different behaviour here between the 8231 and 686B in this area,
despite
having the same vendor/device id.
The first thing you need to fix is the PCI_INTERRUPT_LINE part; I would start by
removing the existing code and instead expose a qdev named gpio "legacy-irq" and
wiring that up to your interrupt controller. Note you'd have to do the same for
fulong2e, but that is reasonably trivial.
Please go ahead and do it but if you don't submit an alternative patch
before the freeze I'd ask John and Peter to make a judgement here and tell
if my series is acceptable or not in its current form and if it is then
please merge it and leave clean ups for subsequent patches. This is
blocking my further patches to implement pegasos2 emulation.
Regards,
BALATON Zoltan
