[snip]
>
> That's why all the paths where the level in device emulation can change has
> to be covered, so the information in piix3 stays accurate. What I'm saying
> is that the reset callback is not one of the points where you can call
> qemu_set_irq since the state of other devices is unknown at that point (for
> the purpose of simulating interrupt levels). When the reset callbacks are
from qemu_irq() edge (in piix3) is not a part of device emulation, so it
is not obvious to me that system reset should reset it and bring it to
unknown state. Line state change may reach IRQ chip before or after reset, but
this doesn't matter since IRQ chip will be in knows state in both cases.
Actually I want to bring it to a known state. It may not be equivalent to what happens in real HW, but it's compatible behavior and deterministic, which makes debugging easier.
> done with their work, all the simulated interrupt levels at all layers have
> to be restored to known state. This state is currently with all the levels
> set to zero. If you call qemu_set_irq during the reset callbacks the
> interrupt levels recorded in the device tree may end up in uncertain state.
That may happen on real HW too. OS will get spurious interrupt, that's it.
Agreed.
That's what I said earlier (or at least what I thought), you would get spurious interrupts. It may have other consequences which I can't think of, right now.
[snip]
> To change the level you first have to have the whole device tree topology in
> known state. In the reset callback that's not the case. If there was a
Level change (as seen by other device) can happen only after or before
reset, not during reset. In both cases the device state is known.
Hmm, makes sense. I suppose that if none of the bus/PIC/etc. devices altered their "interrupt level" state during the reset and each individual device brought it's IRQ line down it would work too. As long as the interrupts are communicated to the CPU after the reset in a correct way. I'm just not convinced that it's the right way. My rationale is that on power-up the interrupt levels in PIC/PCI/etc. are set to zero and they only change as the emulation is executed. So why should the reset be different? Doing it in a different way on reset sounds fishy to me, especially since it involves more code for no obvious benefit. Moreover APIC and lot of other devices clear the interrupt state now, so you'd have to change them too.
[snip]
> I'm suggesting not to call it at all and always reset the recorded
> "interrupt levels" all the way down the device tree on reset. If there is a
will work since assumption that irq level is zero after reset is usually
correct.
Yes, but the emulation is not running at that point, so it's harmless.
> device which sets the IRQ line high on reset then we should introduce the
> "late" reset/init callback.
>
Yes, calling set_irq(1) on reset may not achieve this since irq chip may
be reset after set_irq() was called.
Right, exactly my point.
[snip]
> If it's not clear then let's make it clear. Real HW communicates interrupt
> levels and detects edges, QEMU communicates edges and simulates levels, so
> it has to work differently than on real HW in this particular case.
>
> The system reset callback would reset device registers to a known state as
> specified by the HW documentation. The interrupt levels would be driven low
> at this point on the devices which simulate "interrupt levels" (buses, PIC,
> etc.) - as if "0" was sent to the copper wire ;)
>
> The "late" reset/init callback would drive the IRQs (and GPIOs) high if
> necessary once the system is in a known state, ie. after all reset callbacks
> have successfully completed.
>
some time after reset (like on real HW).
Sort of, but in a deterministic way.
> Hot-unplug callback doesn't need reset the registers of the device, it only
> has to drive the IRQ low on the bus - ie. call *_set_irq(0) for the specific
> bus. In theory that could be done at the bus level.
>
I should be done at bus level indeed. The device is disconnected at that
point.
Agreed.
> Feel free to disagree or correct me. Suggestions welcome.
>
Not disagreeing but this still is not enough, for instance there are
devices that have different state after power-up and reset.
Let's document it once we (including the maintainers) agree on something. At least that would set the precedent for future.
[snip]
>
> I don't remember the exact contents of 2/3, sorry.
>
http://lists.gnu.org/archive/html/qemu-devel/2009-06/msg00342.html
Looks good to me.