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

Michael S. Tsirkin
2012-04-10T17:06:46Z

On Tue, Apr 10, 2012 at 07:08:26PM +0300, Avi Kivity wrote:
> On 04/10/2012 06:14 PM, Michael S. Tsirkin wrote:
> > On Tue, Apr 10, 2012 at 06:00:51PM +0300, Avi Kivity wrote:
> > > On 04/10/2012 05:53 PM, Michael S. Tsirkin wrote:
> > > > > >
> > > > > > Yes. But we can and it's easier than figuring out priorities.
> > > > > > I am guessing such collisions are rare, right?
> > > > >
> > > > > It's pretty easy, if there is something in IRR but
> > > > > kvm_lapic_has_interrupt() returns -1, then we need to disable eoi avoidance.
> > > >
> > > > I only see kvm_apic_has_interrupt - is this what you mean?
> > >
> > > Yes, sorry.
> > >
> > > It's not clear whether to do the check in kvm_apic_has_interrupt() or
> > > kvm_apic_get_interrupt() - the latter is called only after interrupts
> > > are enabled, so it looks like a better place (EOIs while interrupts are
> > > disabled have no effect). But need to make sure those functions are
> > > actually called, since they're protected by KVM_REQ_EVENT.
> >
> > Sorry not sure what you mean by "make sure" - read the code carefully?
>
> Yes. And I mean, get called at the right time.

OK, Review will help here.

> > >
> > > Better to keep everything per-cpu. The code is in virt/kvm/ioapic.c
> >
> > Hmm. Disabling for level handles the ack notifiers
> > issue as well, which I forgot about.
> > It's a tough call. You think looking at
> > TMR in kvm_get_apic_interrupt is safe?
>
> Yes, it's read only from the guest point of view IIRC.
>
> > > >
> > > > > Why do we care about
> > > > > level-triggered interrupts? Everything uses MSI or edge-triggered
> > > > > IOAPIC interrupts these days.
> > > >
> > > > Well lots of emulated devices don't yet.
> > > > They probably should but it's nice to be able to
> > > > test with e.g. e1000 emulation not just virtio.
> > >
> > >
> > > e1000 doesn't support msi?
> >
> > qemu emulation doesn't.
> >
>
> Can be changed if someone's really interested. But really, avoiding
> EOIs for e1000 won't help it much.

It will help test EOI avoidance.

> --
> error compiling committee.c: too many arguments to function