On Mon, Jan 07, 2013 at 06:17:22PM -0600, mmogilvi@xxxxxxxxxxxx wrote: > On Mon, 7 Jan 2013 11:39:18 +0200, Gleb Natapov <gleb@xxxxxxxxxx> wrote: > > On Wed, Dec 26, 2012 at 10:39:53PM -0700, Matthew Ogilvie wrote: > >> Reading the spec, it is clear that most modes normally leave the IRQ > >> output line high, and only pulse it low to generate a leading edge. > >> Especially the most commonly used mode 2. > >> > >> The KVM i8254 model does not try to emulate the duration of the pulse at > >> all, so just swap the high/low settings it to leave it high most of > >> the time. > >> > >> This fix is a prerequisite to improving the i8259 model to handle > >> the trailing edge of an interupt request as indicated in its spec: > >> If it gets a trailing edge of an IRQ line before it starts to service > >> the interrupt, the request should be canceled. > >> > >> See http://bochs.sourceforge.net/techspec/intel-82c54-timer.pdf.gz > >> or search the net for 23124406.pdf. > >> > >> Risks: > >> > >> There is a risk that migrating a running guest between versions > >> with and without this patch will lose or gain a single timer > >> interrupt during the migration process. The only case where > > Can you elaborate on how exactly this can happen? Do not see it. > > > > KVM 8254: In the corrected model, when the count expires, the model > briefly pulses output low and then high again, with the low to high > transition being what triggers the interrupt. In the old model, > when the count expires, the model expects the output line > to already be low, and briefly pulses it high (triggering the > interrupt) and then low again. But if the line was already > high (because it migrated from the corrected model), > this won't generate a new leading edge (low to high) and won't > trigger a new interrupt (the first post-back-migration pulse turns > into a simple trailing edge instead of a pulse). > > Unless there is something I'm missing? > No, I missed that pic->last_irr/ioapic->irr will be migrated as 1. But this means that next interrupt after migration from new to old will always be lost. What about clearing pit bit from last_irr/irr before migration? Should not affect new->new migration and should fix new->old one. The only problem is that we may need to consult irq routing table to know how pit is connected to ioapic. Still do not see how can we gain one interrupt. > The qemu 8254 model actually models each edge at independent > clock ticks instead of combining both into a very brief pulse at one time. > I've found it handy to draw out old and new timing diagrams on paper > (for each mode), and then carefully think about what happens with respect > to levels and edges when you transition back and forth between old and > new models at various points in the timing cycle. (Note I've spent more > time examining the qemu models rather than the kvm models.) > Yes, drawing it definitely helps :) > >> this is likely to be serious is probably losing a single-shot (mode 4) > >> interrupt, but if my understanding of how things work is good, then > >> that should only be possible if a whole slew of conditions are > >> all met: > >> > >> 1. The guest is configured to run in a "tickless" mode (like > >> modern Linux). > >> 2. The guest is for some reason still using the i8254 rather > >> than something more modern like an HPET. (The combination > >> of 1 and 2 should be rare.) > > This is not so rare. For performance reason it is better to not have > > HPET at all. In fact -no-hpet is how I would advice anyone to run qemu. > > In a later email you mention that Linux prefers a timer in the APIC. > I don't know much about the APIC (advanced interrupt controller), and > wasn't > even aware had it's own timer. > > The big question is if we can safely just fix the i825* models, or if > we need something more subtle to avoid breaking commonly used guests > like modern Linux (support both corrected and old models, > or only fix IRQ2 instead of all IRQs, or similar subtlety). Migration may happen while guest is running firmaware. Who knows what those are doing. If the fix is as easy as I described above we should go for it. > > > > >> 3. The migration is going from a fixed version back to the > >> old version. (Not sure how common this is, but it should > >> be rarer than migrating from old to new.) > >> 4. There are not going to be any "timely" events/interrupts > >> (keyboard, network, process sleeps, etc) that cause the guest > >> to reset the PIT mode 4 one-shot counter "soon enough". > >> > >> This combination should be rare enough that more complicated > >> solutions are not worth the effort. > >> > >> Signed-off-by: Matthew Ogilvie <mmogilvi_qemu@xxxxxxxxxxxx> > >> --- > >> arch/x86/kvm/i8254.c | 6 +++++- > >> 1 file changed, 5 insertions(+), 1 deletion(-) > >> > >> diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c > >> index c1d30b2..cd4ec60 100644 > >> --- a/arch/x86/kvm/i8254.c > >> +++ b/arch/x86/kvm/i8254.c > >> @@ -290,8 +290,12 @@ static void pit_do_work(struct kthread_work *work) > >> } > >> spin_unlock(&ps->inject_lock); > >> if (inject) { > >> - kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1); > >> + /* Clear previous interrupt, then create a rising > >> + * edge to request another interupt, and leave it at > >> + * level=1 until time to inject another one. > >> + */ > >> kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0); > >> + kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1); > >> > >> /* > >> * Provides NMI watchdog support via Virtual Wire mode. > >> -- > >> 1.7.10.2.484.gcd07cc5 > > > > -- > > Gleb. -- Gleb. -- 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
