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

[Avi Kivity <avi@redhat.com>]

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.

> > > I'll add a trace to make sure.
> > >
> > > > > +			if (v != -1)
> > > > > +				apic_set_vector(v, apic->regs + APIC_ISR);
> > > > > +		} else {
> > > > > +			eoi_set_pending_vector(vcpu, vector);
> > > > > +			set_isr = false;
> > > > 
> > > > Weird.  Just set it normally.  Remember that reading the ISR needs to
> > > > return the correct value.
> > >
> > > Marcelo said linux does not normally read ISR - not true?
> > 
> > It's true and it's irrelevant.  We aren't coding a feature to what linux
> > does now, but for what linux or another guest may do in the future.
>
> Right. So you think reading ISR has value
> in combination with PV EOI for future guests?
> I'm not arguing either way just curious.

I don't.  But we need to preserve the same interface the APIC has
presented for thousands of years (well, almost).

>
> > > Note this has no effect if the PV optimization is not enabled.
> > >
> > > > We need to process the avoided EOI before any APIC read/writes, to be
> > > > sure the guest sees the updated values.  Same for IOAPIC, EOI affects
> > > > remote_irr.  That may been we need to sample it after every exit, or
> > > > perhaps disable the feature for level-triggered interrupts.
> > >
> > > Disabling would be very sad.  Can we sample on remote irr read?
> > 
> > That can be done from another vcpu.
>
> We still can handle it, right? Where's the code that handles that read?

Better to keep everything per-cpu.  The code is in virt/kvm/ioapic.c

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

>
> Besides, kvm_get_apic_interrupt
> simply doesn't know about the triggering mode at the moment.
>


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