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?
> > > > 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).
Talk about overstatements :)
> >
> > > > 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
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?
> >
> > > 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.
> >
> > 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
--
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
