On Mon, Mar 21, 2011 at 02:31:11PM +0200, Michael S. Tsirkin wrote: > On Mon, Mar 21, 2011 at 09:10:32PM +0900, Isaku Yamahata wrote: > > On Mon, Mar 21, 2011 at 01:37:07PM +0200, Michael S. Tsirkin wrote: > > > > +static int piix3_post_load(void *opaque, int version_id) > > > > +{ > > > > + PIIX3State *piix3 = opaque; > > > > + piix3_update_irq_levels(piix3); > > > > > > Couldn't figure out why would we not want to > > > propagate the interrupts here. > > > Could you explain please? > > > What happens if we do propagate them? > > > Nothing bad, right? > > > > I wanted to be just conservative. > > If you are brave enough to change the behavior, I'm fine with propagating > > interrupts. > > > > If we propagate the interrupts, guest OS may see interrupts > > unnecessarily/spuriously injected after load. > > Probably such interrupts doesn't harm OSes, so there is nothing > > bad in theory as you said. > > On the other hand, I hesitated to change the existing behavior because > > it would be very difficult to debug it and to test many OSes. > > I expect it won't change the behaviour because the interrupts > are level: at the moment e.g. pci devices already reassert > interrupts on load. > > But I agree it better be a separate patch, and needs a lot of testing.
Like this? diff --git a/hw/piix_pci.c b/hw/piix_pci.c index f07e19d..8052c1e 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -280,8 +280,7 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) ((PIIX_NUM_PIRQS - 1) << (pic_irq * PIIX_NUM_PIRQS)))); } -static void piix3_set_irq_level(PIIX3State *piix3, int irq_num, int level, - bool propagate) +static void piix3_set_irq_level(PIIX3State *piix3, int irq_num, int level) { int pic_irq; uint64_t mask; @@ -295,15 +294,13 @@ static void piix3_set_irq_level(PIIX3State *piix3, int irq_num, int level, piix3->pic_levels &= ~mask; piix3->pic_levels |= mask * !!level; - if (propagate) { - piix3_set_irq_pic(piix3, pic_irq); - } + piix3_set_irq_pic(piix3, pic_irq); } static void piix3_set_irq(void *opaque, int irq_num, int level) { PIIX3State *piix3 = opaque; - piix3_set_irq_level(piix3, irq_num, level, true); + piix3_set_irq_level(piix3, irq_num, level); } /* irq routing is changed. so rebuild bitmap */ @@ -314,8 +311,7 @@ static void piix3_update_irq_levels(PIIX3State *piix3) piix3->pic_levels = 0; for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) { piix3_set_irq_level(piix3, pirq, - pci_bus_get_irq_level(piix3->dev.bus, pirq), - false); + pci_bus_get_irq_level(piix3->dev.bus, pirq)); } } -- yamahata