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

Avi Kivity
2012-04-10T14:33:21Z

On 04/10/2012 05:26 PM, Michael S. Tsirkin wrote:
> > > u64 status;
> > > } osvw;
> > > +
> > > + struct {
> > > + u64 msr_val;
> > > + struct gfn_to_hva_cache data;
> > > + int vector;
> > > + } eoi;
> > > };
> >
> > Needs to be cleared on INIT.
>
> You mean kvm_arch_vcpu_reset?

Yes, or kvm_lapic_reset().

> > >
> > >
> > > @@ -307,6 +308,9 @@ void __cpuinit kvm_guest_cpu_init(void)
> > > smp_processor_id());
> > > }
> > >
> > > + wrmsrl(MSR_KVM_EOI_EN, __pa(this_cpu_ptr(apic_eoi)) |
> > > + MSR_KVM_EOI_ENABLED);
> > > +
> >
> > Clear on kexec.
>
> With register_reboot_notifier?

Yes, we already clear some kvm msrs there.

> > >
> > > - apic_set_vector(vector, apic->regs + APIC_ISR);
> > > + if (eoi_enabled(vcpu)) {
> > > + /* Anything pending? If yes disable eoi optimization. */
> > > + if (unlikely(apic_find_highest_isr(apic) >= 0)) {
> > > + int v = eoi_clr_pending_vector(vcpu);
> >
> > ISR != pending, that's IRR. If ISR vector has lower priority than the
> > new vector, then we don't need to disable eoi avoidance.
>
> 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'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.

> 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. Why do we care about
level-triggered interrupts? Everything uses MSI or edge-triggered
IOAPIC interrupts these days.

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