Re: [PATCH 5/5] KVM: use eoi to track RTC interrupt delivery status

[Gleb Natapov <gleb@redhat.com>]

On Fri, Mar 15, 2013 at 04:05:00PM +0800, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
> 
> Current interrupt coalescing logci which only used by RTC has conflict
> with Posted Interrupt.
> This patch introduces a new mechinism to use eoi to track interrupt:
> When delivering an interrupt to vcpu, the need_eoi set to number of
> vcpu that received the interrupt. And decrease it when each vcpu writing
> eoi. No subsequent RTC interrupt can deliver to vcpu until all vcpus
> write eoi.
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> ---
>  arch/x86/kvm/lapic.c |    2 +-
>  arch/x86/kvm/lapic.h |    1 +
>  virt/kvm/ioapic.c    |  105 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 107 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index ad97f1f..bf2d208 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -89,7 +89,7 @@ static inline int apic_test_and_clear_vector(int vec, void *bitmap)
>  	return test_and_clear_bit(VEC_POS(vec), (bitmap) + REG_POS(vec));
>  }
>  
> -static inline int apic_test_vector(int vec, void *bitmap)
> +int apic_test_vector(int vec, void *bitmap)
>  {
>  	return test_bit(VEC_POS(vec), (bitmap) + REG_POS(vec));
>  }
That's too low level to call from IOAPIC. Put kvm_apic_pending_eoi()
here instead.

> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index 3a0f9d8..02da8b8 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -84,6 +84,7 @@ static inline bool kvm_hv_vapic_assist_page_enabled(struct kvm_vcpu *vcpu)
>  
>  int kvm_lapic_enable_pv_eoi(struct kvm_vcpu *vcpu, u64 data);
>  void kvm_lapic_init(void);
> +int apic_test_vector(int vec, void *bitmap);
>  
>  static inline u32 kvm_apic_get_reg(struct kvm_lapic *apic, int reg_off)
>  {
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index 2c6235c..46cb8ed 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -103,12 +103,110 @@ static void rtc_register_notifier(struct kvm_ioapic *ioapic)
>  	kvm_register_irq_ack_notifier(ioapic->kvm,
>  				&ioapic->rtc_status.irq_ack_notifier);
>  }
> +
> +static void rtc_irq_reset(struct kvm_ioapic *ioapic)
> +{
> +	ioapic->rtc_status.need_eoi = 0;
> +	bitmap_zero(ioapic->rtc_status.vcpu_map, KVM_MAX_VCPUS);
> +}
> +
> +static void rtc_irq_restore(struct kvm_ioapic *ioapic)
> +{
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_lapic *apic;
> +	int vector, i, need_eoi = 0, rtc_pin = 8;
> +
> +	vector = ioapic->redirtbl[rtc_pin].fields.vector;
> +	kvm_for_each_vcpu(i, vcpu, ioapic->kvm) {
> +		apic = vcpu->arch.apic;
> +		if (apic_test_vector(vector, apic->regs + APIC_ISR) ||
> +			    apic_test_vector(vector, apic->regs + APIC_IRR)) {
> +			need_eoi++;
> +			set_bit(vcpu->vcpu_id, ioapic->rtc_status.vcpu_map);
> +		}
> +	}
> +	ioapic->rtc_status.need_eoi = need_eoi;
> +}
> +
> +static void rtc_irq_update(struct kvm_ioapic *ioapic,
> +		struct kvm_lapic_irq *irqe, int irq)
> +{
> +	int weight;
> +
> +	if (irq != 8)
> +		return;
> +
> +	rtc_irq_reset(ioapic);
> +
> +	kvm_get_dest_vcpu(ioapic->kvm, irqe, ioapic->rtc_status.vcpu_map);
> +	if (likely(!bitmap_empty(ioapic->rtc_status.vcpu_map, KVM_MAX_VCPUS))) {
> +		if (irqe->delivery_mode == APIC_DM_LOWEST)
> +			ioapic->rtc_status.need_eoi = 1;
> +		else {
> +			weight = bitmap_weight(ioapic->rtc_status.vcpu_map,
> +					sizeof(ioapic->rtc_status.vcpu_map));
> +			ioapic->rtc_status.need_eoi = weight;
> +		}
> +	}
> +}
> +
> +static void rtc_irq_ack_eoi(struct kvm_vcpu *vcpu,
> +			struct rtc_status *rtc_status, int irq)
> +{
> +	if (irq != 8)
> +		return;
> +
> +	if (test_and_clear_bit(vcpu->vcpu_id, rtc_status->vcpu_map)) {
> +		if (!(--rtc_status->need_eoi))
WARN_ON(need_eoi < 0)?

> +			/* Clear irr to accept subsequent RTC interrupt */
> +			vcpu->kvm->arch.vioapic->irr &= ~(1 << 8);
This is not needed if you do not set irr if irq is coalesced.

> +	}
> +}
> +
> +static bool rtc_irq_check(struct kvm_ioapic *ioapic, int irq)
> +{
> +	if (irq != 8)
> +		return false;
> +
> +	if (ioapic->rtc_status.need_eoi > 0)
> +		return true; /* coalesced */
> +
> +	return false;
> +}
> +
>  #else
>  
>  static void rtc_register_notifier(struct kvm_ioapic *ioapic)
>  {
>  	return;
>  }
> +
> +static void rtc_irq_reset(struct kvm_ioapic *ioapic)
> +{
> +	return;
> +}
> +
> +static void rtc_irq_restore(struct kvm_ioapic *ioapic)
> +{
> +	return;
> +}
> +
> +static void rtc_irq_update(struct kvm_ioapic *ioapic,
> +		struct kvm_lapic_irq *irqe, int irq)
> +{
> +	return;
> +}
> +
> +static void rtc_irq_ack_eoi(struct kvm_vcpu *vcpu,
> +			struct rtc_status *rtc_status, int irq)
> +{
> +	return;
> +}
> +
> +static bool rtc_irq_check(struct kvm_ioapic *ioapic, int irq)
> +{
> +	return false;
> +}
>  #endif
>  
>  static int ioapic_service(struct kvm_ioapic *ioapic, unsigned int idx)
> @@ -119,6 +217,8 @@ static int ioapic_service(struct kvm_ioapic *ioapic, unsigned int idx)
>  	pent = &ioapic->redirtbl[idx];
>  
>  	if (!pent->fields.mask) {
> +		if (rtc_irq_check(ioapic, idx))
> +			return 0; /* coalesced */
>  		injected = ioapic_deliver(ioapic, idx);
>  		if (injected && pent->fields.trig_mode == IOAPIC_LEVEL_TRIG)
>  			pent->fields.remote_irr = 1;
> @@ -241,6 +341,8 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
>  	irqe.level = 1;
>  	irqe.shorthand = 0;
>  
> +	rtc_irq_update(ioapic, &irqe, irq);
> +
Do it once when GSI is configured.

>  	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
>  }
>  
> @@ -299,6 +401,7 @@ static void __kvm_ioapic_update_eoi(struct kvm_vcpu *vcpu,
>  		if (ent->fields.vector != vector)
>  			continue;
>  
> +		rtc_irq_ack_eoi(vcpu, &ioapic->rtc_status, i);
>  		/*
>  		 * We are dropping lock while calling ack notifiers because ack
>  		 * notifier callbacks for assigned devices call into IOAPIC
> @@ -452,6 +555,7 @@ void kvm_ioapic_reset(struct kvm_ioapic *ioapic)
>  	ioapic->ioregsel = 0;
>  	ioapic->irr = 0;
>  	ioapic->id = 0;
> +	rtc_irq_reset(ioapic);
>  	update_handled_vectors(ioapic);
>  }
>  
> @@ -519,6 +623,7 @@ int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
>  	spin_lock(&ioapic->lock);
>  	memcpy(ioapic, state, sizeof(struct kvm_ioapic_state));
>  	update_handled_vectors(ioapic);
> +	rtc_irq_restore(ioapic);
>  	kvm_ioapic_make_eoibitmap_request(kvm);
>  	spin_unlock(&ioapic->lock);
>  	return 0;
> -- 
> 1.7.1

--
			Gleb.