Re: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap

[Avi Kivity <avi@redhat.com>]

On 09/05/2012 08:41 AM, Li, Jiongxi wrote:
> With APICv virtual interrupt delivery feature, EOI write from non
> root mode doesn't cause VM-Exit unless set in EOI exit bitmap VMCS
> field. Basically there're two methods to manipulate EOI exit bitmap:

Should be folded into the previous patch, otherwise the previous patch
breaks level interrupts.

> 
> [Option 1]
> Ideally only level triggered irq requires a hook in vLAPIC EOI write,
> so that vIOAPIC EOI is triggered and emulated. So the simplest
> approach is to manipulate EOI exit bitmap when vLAPIC acks a new
> interrupt, based on value of TMR. There're several corner cases
> worthy of note though:
> 
>   - KVM has specific notifier hooks on vIOAPIC EOI path. So far two
>     sources use it: INT-based device passthrough and PIT pending
>     timers. For the former, it's virtually wired to vIOAPIC and
>     thus TMR already covers it. PIT is special here, which is an
>     edge triggered source. But since other timer sources like
>     vLAPIC timer don't require this notifier hook, possibly PIT
>     can be relaxed in the future too.

I would like to switch to changing the timer frequency when we need to
catch up.  But that can be done later.

> 
>   - posted interrupt will update TMR directly, w/o chance for KVM
>     to update EOI exit bitmap accordingly. This becomes a gap

Why not? we know what vector the PIT is wired to.

> 
> [Option 2]
> Indicate EOI exit bitmap requirement ('need_eoi') directly from
> every interrupt source device, and then check this requirement
> when vLAPIC acks a new pending interrupt. This requires more
> intrusive changes to current vLAPIC/vIOAPIC logic, so that the
> "irq_source_id" indicating source of interrupt is passed through
> from origination point to vLAPIC ack point. For natual requirement
> like vIOAPIC level triggered entries, it can be implicitly deduced.
> On the other hand for non-natural requirements like aformentioned
> PIT or posted interrupt, this approach can handle it efficiently.
> 
> For simplicity reason, now option 1 is used which should be
> enough to test MSI-based device passthrough.

You can change kvm_register_irq_ack_notifier() to call the ioapic and
pic to find out what vectors need EOI exits.

(alternatively, if we fix the PIT, then we only need ack notifiers for
level interrupts).

> 
> Signed-off-by: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> ---
>  arch/x86/include/asm/kvm_host.h |    1 +
>  arch/x86/kvm/lapic.c            |    7 ++++++-
>  arch/x86/kvm/vmx.c              |   37 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 44 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index ef74df5..4e06a82 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -671,6 +671,7 @@ struct kvm_x86_ops {
>  	void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
>  	int (*has_virtual_interrupt_delivery)(struct kvm_vcpu *vcpu);
>  	void (*update_irq)(struct kvm_vcpu *vcpu);
> +	void (*set_eoi_exitmap)(struct kvm_vcpu *vcpu, int vector, int need_eoi);
>  	int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
>  	int (*get_tdp_level)(void);
>  	u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index d203501..4058384 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -499,8 +499,13 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>  		if (trig_mode) {
>  			apic_debug("level trig mode for vector %d", vector);
>  			apic_set_vector(vector, apic->regs + APIC_TMR);
> -		} else
> +			if (kvm_apic_vid_enabled(vcpu))
> +				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 1);
> +		} else {
>  			apic_clear_vector(vector, apic->regs + APIC_TMR);
> +			if (kvm_apic_vid_enabled(vcpu))
> +				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 0);
> +		}

This is way too late.  The flow should come from the IOAPIC and PIC,
when setting up an irq, to the local APIC.



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