Re: [PATCHv1 dont apply] RFC: kvm eoi PV using shared memory

["Michael S. Tsirkin" <mst@redhat.com>]

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.