On Tue, Sep 09, 2014 at 02:30:08PM +0200, Paolo Bonzini wrote: > From: Pavel Dovgalyuk <[email protected]> > > This patch disables raising an irq while loading the state of PCI bridge. > The aim of this patch is preserving the same behavior while saving and > restoring the VM state. IRQ is not raised while saving the state of the > bridge. > That's why the behavior of the restored system will differ from > the original one.
Hmm I don't understand. You are removing call to piix3_update_irq_levels this call is supposed to just sync up bus to irq level. How can this change system state? Saved state is supposed to already be in sync. > This patch eliminates raising an irq and just restores > the calculated state fields in post_load function. > > Signed-off-by: Pavel Dovgalyuk <[email protected]> > Signed-off-by: Paolo Bonzini <[email protected]> > --- > hw/pci-host/piix.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > index e0e0946..cd50435 100644 > --- a/hw/pci-host/piix.c > +++ b/hw/pci-host/piix.c > @@ -409,7 +409,7 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int > pic_irq) > (pic_irq * PIIX_NUM_PIRQS)))); > } > > -static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level) > +static void piix3_set_irq_level_internal(PIIX3State *piix3, int pirq, int > level) > { > int pic_irq; > uint64_t mask; > @@ -422,6 +422,18 @@ static void piix3_set_irq_level(PIIX3State *piix3, int > pirq, int level) > mask = 1ULL << ((pic_irq * PIIX_NUM_PIRQS) + pirq); > piix3->pic_levels &= ~mask; > piix3->pic_levels |= mask * !!level; > +} > + > +static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level) > +{ > + int pic_irq; > + > + pic_irq = piix3->dev.config[PIIX_PIRQC + pirq]; > + if (pic_irq >= PIIX_NUM_PIC_IRQS) { > + return; > + } > + > + piix3_set_irq_level_internal(piix3, pirq, level); > > piix3_set_irq_pic(piix3, pic_irq); > } > @@ -527,7 +539,18 @@ static void piix3_reset(void *opaque) > static int piix3_post_load(void *opaque, int version_id) > { > PIIX3State *piix3 = opaque; > - piix3_update_irq_levels(piix3); > + int pirq; > + > + /* Update irq levels without raising an interrupt which could > + * happen in piix3_update_irq_levels. Raising an IRQ will > + * bring the system to a different state than the saved one. I don't like comments like this: don't discuss what could have been if code was different. Explain why code is like it is. > + * Interrupt state is serialized separately through the i8259. > + */ > + piix3->pic_levels = 0; > + for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) { > + piix3_set_irq_level_internal(piix3, pirq, > + pci_bus_get_irq_level(piix3->dev.bus, pirq)); > + } > return 0; > } > > -- > 2.1.0 > >
