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

[Avi Kivity <avi@redhat.com>]

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