On Mon, Apr 16, 2012 at 08:37:37PM +0300, Gleb Natapov wrote: > On Mon, Apr 16, 2012 at 08:24:16PM +0300, Michael S. Tsirkin wrote: > > On Mon, Apr 16, 2012 at 06:10:11PM +0300, Gleb Natapov wrote: > > > > > lapic changes should be minimal. > > > > > > > > Exactly my motivation. > > > > > > > My patch removes 13 lines more :) > > > > Haven't checked whether your patch is correct yet > > but I see it's checking the eoi register on each exit. > > > Only if eoi.pending == true on exit. it's checking eoi.pending always and eoi register when that is true. > > I think it's clear this would make code a bit shorter > > (not necessarily clearer as we are relying on ) but > > as I said even though the extra overhead is likely > > negligeable I have a feeling it's a wrong approach since > > this won't scale as we add more features. > > > > Let's see what do others think. > > > What I do not like about not calling eoi here is that we > have to reason about all the paths into apic and check that > we clear isr on all of them. Not at all. Instead, check each time before we read isr or ppr. > And that is not all. eoi handler > set event request, is it OK to skip it? May be, or may be not. I think it's for reinjection of lower prio interrupt. Since eoi optimization is disabled in that case we don't need to set event request. > > > > I also find the logic easier to follow as is - > > > > it is contained in lapic.c without relying > > > > on being called from x86.c as just the right moment. > > > > > > > See the patch. It change nothing outside of lapic.c. > > > > True but you rely on x86.c to call kvm_sync_lapic_from_vapic > > at the right time before injecting interrupts. > Not before injecting interrupts, but on vmexit. sync is called on entry, not on exit, no? > > I haven't checked whether that is always the case but > > to me, this makes the code less clear and more fragile. > > > We call a lot of apic functions from x86.c. That's were interrupt > injection happens. > > > Again, it appears to be a matter of taste. > > > > -- > > MST > > -- > 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
