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

[Gleb Natapov <gleb@redhat.com>]

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.

> 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. And that is not all. eoi handler
set event request, is it OK to skip it? May be, or may be not.
 
> > > 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.

> 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.