Re: [Qemu-devel] [PATCH 09/10] piix: do not raise irq while loading vmstate

[Paolo Bonzini]

Il 09/09/2014 22:51, Michael S. Tsirkin ha scritto:
> > i440FX/PIIX3 state is loaded before i8259, so the interrupt will never
> > be in the i8259 ISR.  I am not sure why it is a problem for
> > record/replay, but I think it's plausible to consider this a bug.  i8259
> > state should not be affected by the load of PIIX3 state, since i8259 is
> > migrated separately.
> 
> Sorry I still don't understand. Why do stuff from vmstate callback then?
> How is it different?

Reconstructing internal state from post_load is okay.

What is not okay (and I think it should be a rule) is to touch other
devices from post_load, unless you know that they are deserialized
first.  For example it's okay for a PCI device to talk to the parent
bridge in its post_load function.

In the case of PIIX3 vs. i8259, however, you know that i8259 is
deserialized _last_ because i8259 is an ISA device and PIIX3 provides
the ISA bus.  So it's incorrect, even though it's currently harmless, to
touch the i8259 before it's deserialized.

> I'd like to see a description of a scenario where this patch makes
> a difference.

Of course it would be nice to have testcases for this, but I guess one
case could be:

- LAPIC configured in ExtINT mode

- interrupts are masked in the i8259, but the i8259 doesn't know that
yet because it's not been loaded yet

- the PIIX3 loads the state and the interrupt is set.  pic_set_irq is
called, calls pic_update_irq

- pic_update_irq calls pic_get_irq, which uses IMR=0 and thus raises LINT0

- the APIC has been loaded already, so LINT0 is injected incorrectly


Another case could be:

- i8259 is processing IRQ0.  The lower-priority interrupt from PIIX3 is
in IRR.  Machine is migrated.

- the PIIX3 loads the state and sets the interrupt in the i8259.
pic_set_irq is called, calls pic_update_irq, calls pic_get_irq

- because i8259 has not been loaded yet, pic_get_irq sees ISR=0 and the
interrupt is injected even though IRQ0 (higher priority) is being serviced.


In both cases, the saved i8259 state will have the PIIX3 interrupt in
IRR, so the interrupt is not lost, just held (as it would have been on
the source machine).

Paolo