On Wed, Aug 26, 2009 at 04:19:17PM +0300, Gleb Natapov wrote: > On Wed, Aug 26, 2009 at 09:43:48AM -0300, Marcelo Tosatti wrote: > > On Mon, Aug 24, 2009 at 09:19:05PM +0300, Gleb Natapov wrote: > > > > > Current code very fragile and relies on hacks to work. Lets take calling > > > > > of ack notifiers on pic reset as an example. Why is it needed? > > > > > > > > To signal the ack notifiers users that, in case of reset with pending > > > > IRR, the given interrupt has been "acked" (its an artificial ack event). > > > > > > > But IRR was not acked. The reason it is done is that otherwise the > > > current logic will prevent further interrupt injection. > > > > Or will keep the host irq disabled, for the assigned device case (in > > case you drop the hackish ack notification from pic_reset). > > > > I don't think it exists there because of PIT reinjection only, it seems > > a generic problem for users of ack notifiers (a reset notifier as you > > mentioned would also do it, and be cleaner). > > > Yes, I agree pic reset should be propagated to assigned devices somehow. > > > > > Is there a need to differentiate between actual interrupt ack and reset > > > > with pending IRR? At the time this code was written, there was no > > > > indication that differentation would be necessary. > > > This is two different things. Ack notifiers should be called when guest > > > acks interrupt. Calling it on reset is wrong (see below). We can add reset > > > notifiers, but we just build yet another infrastructure to support > > > current reinjection scheme. > > > > Its not specific to PIT reinjection. > > > > Anything that relies on ack notification to perform some action (either > > reinjection or host irq line enablement or some other use) suffers from > > the same thing. > > > > You might argue that a separate reset notification is more appropriate. > > > > > > > It is obviously wrong thing to do from assigned devices POV. > > > > > > > > Thats not entirely clear to me. So what happens if a guest with PIC > > > > assigned device resets with a pending IRR? The host interrupt line will > > > > be kept disabled, even though the guest is able to process further > > > > interrupts? > > > The host interrupt line will be enabled (assigned device ack notifier > > > does this) without clearing interrupt condition in assigned device > > > (guest hasn't acked irq so how can we be sure it ran device's irq > > > handler?). Host will hang. > > > > > > > > Why ioapic calls mask notifiers but pic doesn't? > > > > > > > > Because it is not implemented. > > > I see that. Why? Why it was important to implement for ioapic but not > > > for pic? > > > > 4780c65904f0fc4e312ee2da9383eacbe04e61ea > > > This commit and previous one adds infrastructure to fix a bug that is > there only because how we choose to do pit reinjection. Do it differently > and you can revert both of them. > > > > Do we know what doesn't work now? > > > > What you mean? > I mean that pit doesn't call mask notifier so similar bug to 4780c65 > hides somewhere out there. How can we test it? Program periodic PIT, mask irq0, wait a while, unmask irq0 ? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
