[PATCH v3 0/8] Use eoi to track RTC interrupt delivery status

[PATCH v3 0/8] Use eoi to track RTC interrupt delivery status

[Yang Zhang]

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.

Changes from v2 to v3:
* Remove unused irq_ack_notifier.
* Acquire ioapic->lock before calculte destination vcpu map.
* Copy vcpu_map to expected_eoi_timap on each RTC irq and clear it on eoi.

Changes from v1 to v2:
* Don't register a dumy ack notifier for RTC. Check it directly when calculating
  eoi exit bitmap.
* Use kvm_apic_pending_eoi() instead call apic_test_vector() directly.
* Check coalesced info before set ioapic->irr
* Calculate destination vcpu map of RTC when ioapic entry or apic(id,ldr/dfr) changed.
  And only set need_eoi when delivering RTC interrupt.

Yang Zhang (8):
  KVM: Parse ioapic entry to get destination vcpu
  KVM: Rename kvm_ioapic_make_eoibitmap_request to
    kvm_scan_ioapic_entry
  KVM: Add vcpu info to ioapic_update_eoi()
  KVM: Introduce struct rtc_status
  KVM: Recalculate destination vcpu map
  KVM: Add reset/restore rtc_status support
  KVM: Add rtc irq to eoi exit bitmap
  KVM: Use eoi to track RTC interrupt delivery status

 arch/x86/kvm/lapic.c |   52 ++++++++++------
 arch/x86/kvm/lapic.h |    4 +
 virt/kvm/ioapic.c    |  165 ++++++++++++++++++++++++++++++++++++++++++++++----
 virt/kvm/ioapic.h    |   14 ++++-
 virt/kvm/irq_comm.c  |    4 +-
 5 files changed, 205 insertions(+), 34 deletions(-)

[PATCH v3 1/8] KVM: Parse ioapic entry to get destination vcpu

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

Get destination vcpu map from one ioapic entry.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 arch/x86/kvm/lapic.c |   40 ++++++++++++++++++++++++----------------
 arch/x86/kvm/lapic.h |    3 +++
 2 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 02b51dd..b4339a5 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -145,28 +145,26 @@ static inline int kvm_apic_id(struct kvm_lapic *apic)
 	return (kvm_apic_get_reg(apic, APIC_ID) >> 24) & 0xff;
 }
 
-void kvm_calculate_eoi_exitmap(struct kvm_vcpu *vcpu,
-				struct kvm_lapic_irq *irq,
-				u64 *eoi_exit_bitmap)
+void kvm_get_dest_vcpu(struct kvm *kvm, struct kvm_lapic_irq *irq,
+				unsigned long *vcpu_map)
 {
 	struct kvm_lapic **dst;
 	struct kvm_apic_map *map;
 	unsigned long bitmap = 1;
+	struct kvm_vcpu *vcpu;
 	int i;
 
 	rcu_read_lock();
-	map = rcu_dereference(vcpu->kvm->arch.apic_map);
+	map = rcu_dereference(kvm->arch.apic_map);
 
-	if (unlikely(!map)) {
-		__set_bit(irq->vector, (unsigned long *)eoi_exit_bitmap);
+	if (unlikely(!map))
 		goto out;
-	}
 
 	if (irq->dest_mode == 0) { /* physical mode */
-		if (irq->delivery_mode == APIC_DM_LOWEST ||
-				irq->dest_id == 0xff) {
-			__set_bit(irq->vector,
-				  (unsigned long *)eoi_exit_bitmap);
+		if (irq->dest_id == 0xff) {
+			kvm_for_each_vcpu(i, vcpu, kvm)
+				if (apic_enabled(vcpu->arch.apic))
+					set_bit(vcpu->vcpu_id, vcpu_map);
 			goto out;
 		}
 		dst = &map->phys_map[irq->dest_id & 0xff];
@@ -181,17 +179,27 @@ void kvm_calculate_eoi_exitmap(struct kvm_vcpu *vcpu,
 	for_each_set_bit(i, &bitmap, 16) {
 		if (!dst[i])
 			continue;
-		if (dst[i]->vcpu == vcpu) {
-			__set_bit(irq->vector,
-				  (unsigned long *)eoi_exit_bitmap);
-			break;
-		}
+		set_bit(dst[i]->vcpu->vcpu_id, vcpu_map);
 	}
 
 out:
 	rcu_read_unlock();
 }
 
+void kvm_calculate_eoi_exitmap(struct kvm_vcpu *vcpu,
+				struct kvm_lapic_irq *irq,
+				u64 *eoi_exit_bitmap)
+{
+	DECLARE_BITMAP(vcpu_map, KVM_MAX_VCPUS);
+
+	memset(vcpu_map, 0, sizeof(vcpu_map));
+
+	kvm_get_dest_vcpu(vcpu->kvm, irq, vcpu_map);
+	if (test_bit(vcpu->vcpu_id, vcpu_map) ||
+			bitmap_empty(vcpu_map, sizeof(vcpu_map)))
+		__set_bit(irq->vector, (unsigned long *)eoi_exit_bitmap);
+}
+
 static void recalculate_apic_map(struct kvm *kvm)
 {
 	struct kvm_apic_map *new, *old = NULL;
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 1676d34..3a0f9d8 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -158,4 +158,7 @@ void kvm_calculate_eoi_exitmap(struct kvm_vcpu *vcpu,
 				struct kvm_lapic_irq *irq,
 				u64 *eoi_bitmap);
 
+void kvm_get_dest_vcpu(struct kvm *kvm, struct kvm_lapic_irq *irq,
+				unsigned long *vcpu_map);
+
 #endif
-- 
1.7.1

[PATCH v3 2/8] KVM: Rename kvm_ioapic_make_eoibitmap_request to kvm_scan_ioapic_entry

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

RTC interrupt coalesced need to parse ioapic entry to get destionation
vcpu too. Rename it to more common name.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 arch/x86/kvm/lapic.c |    2 +-
 virt/kvm/ioapic.c    |    6 +++---
 virt/kvm/ioapic.h    |    2 +-
 virt/kvm/irq_comm.c  |    4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index b4339a5..12179b3 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -264,7 +264,7 @@ out:
 	if (old)
 		kfree_rcu(old, rcu);
 
-	kvm_ioapic_make_eoibitmap_request(kvm);
+	kvm_scan_ioapic_entry(kvm);
 }
 
 static inline void kvm_apic_set_id(struct kvm_lapic *apic, u8 id)
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index ce82b94..d087e07 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -143,7 +143,7 @@ void kvm_ioapic_calculate_eoi_exitmap(struct kvm_vcpu *vcpu,
 }
 EXPORT_SYMBOL_GPL(kvm_ioapic_calculate_eoi_exitmap);
 
-void kvm_ioapic_make_eoibitmap_request(struct kvm *kvm)
+void kvm_scan_ioapic_entry(struct kvm *kvm)
 {
 	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
 
@@ -193,7 +193,7 @@ static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val)
 		if (e->fields.trig_mode == IOAPIC_LEVEL_TRIG
 		    && ioapic->irr & (1 << index))
 			ioapic_service(ioapic, index);
-		kvm_ioapic_make_eoibitmap_request(ioapic->kvm);
+		kvm_scan_ioapic_entry(ioapic->kvm);
 		break;
 	}
 }
@@ -493,7 +493,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);
-	kvm_ioapic_make_eoibitmap_request(kvm);
+	kvm_scan_ioapic_entry(kvm);
 	spin_unlock(&ioapic->lock);
 	return 0;
 }
diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index 0400a46..cebc123 100644
--- a/virt/kvm/ioapic.h
+++ b/virt/kvm/ioapic.h
@@ -82,7 +82,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 		struct kvm_lapic_irq *irq);
 int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
 int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
-void kvm_ioapic_make_eoibitmap_request(struct kvm *kvm);
+void kvm_scan_ioapic_entry(struct kvm *kvm);
 void kvm_ioapic_calculate_eoi_exitmap(struct kvm_vcpu *vcpu,
 					u64 *eoi_exit_bitmap);
 
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index ff6d40e..15de3b7 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -284,7 +284,7 @@ void kvm_register_irq_ack_notifier(struct kvm *kvm,
 	mutex_lock(&kvm->irq_lock);
 	hlist_add_head_rcu(&kian->link, &kvm->irq_ack_notifier_list);
 	mutex_unlock(&kvm->irq_lock);
-	kvm_ioapic_make_eoibitmap_request(kvm);
+	kvm_scan_ioapic_entry(kvm);
 }
 
 void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
@@ -294,7 +294,7 @@ void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 	hlist_del_init_rcu(&kian->link);
 	mutex_unlock(&kvm->irq_lock);
 	synchronize_rcu();
-	kvm_ioapic_make_eoibitmap_request(kvm);
+	kvm_scan_ioapic_entry(kvm);
 }
 
 int kvm_request_irq_source_id(struct kvm *kvm)
-- 
1.7.1

[PATCH v3 3/8] KVM: Add vcpu info to ioapic_update_eoi()

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

Need to know which vcpu writes EOI.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 arch/x86/kvm/lapic.c |    2 +-
 virt/kvm/ioapic.c    |   12 ++++++------
 virt/kvm/ioapic.h    |    3 ++-
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 12179b3..6fb22e3 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -790,7 +790,7 @@ static void kvm_ioapic_send_eoi(struct kvm_lapic *apic, int vector)
 			trigger_mode = IOAPIC_LEVEL_TRIG;
 		else
 			trigger_mode = IOAPIC_EDGE_TRIG;
-		kvm_ioapic_update_eoi(apic->vcpu->kvm, vector, trigger_mode);
+		kvm_ioapic_update_eoi(apic->vcpu, vector, trigger_mode);
 	}
 }
 
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index d087e07..4296116 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -264,8 +264,8 @@ void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id)
 	spin_unlock(&ioapic->lock);
 }
 
-static void __kvm_ioapic_update_eoi(struct kvm_ioapic *ioapic, int vector,
-				     int trigger_mode)
+static void __kvm_ioapic_update_eoi(struct kvm_vcpu *vcpu,
+			struct kvm_ioapic *ioapic, int vector, int trigger_mode)
 {
 	int i;
 
@@ -304,12 +304,12 @@ bool kvm_ioapic_handles_vector(struct kvm *kvm, int vector)
 	return test_bit(vector, ioapic->handled_vectors);
 }
 
-void kvm_ioapic_update_eoi(struct kvm *kvm, int vector, int trigger_mode)
+void kvm_ioapic_update_eoi(struct kvm_vcpu *vcpu, int vector, int trigger_mode)
 {
-	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
+	struct kvm_ioapic *ioapic = vcpu->kvm->arch.vioapic;
 
 	spin_lock(&ioapic->lock);
-	__kvm_ioapic_update_eoi(ioapic, vector, trigger_mode);
+	__kvm_ioapic_update_eoi(vcpu, ioapic, vector, trigger_mode);
 	spin_unlock(&ioapic->lock);
 }
 
@@ -407,7 +407,7 @@ static int ioapic_mmio_write(struct kvm_io_device *this, gpa_t addr, int len,
 		break;
 #ifdef	CONFIG_IA64
 	case IOAPIC_REG_EOI:
-		__kvm_ioapic_update_eoi(ioapic, data, IOAPIC_LEVEL_TRIG);
+		__kvm_ioapic_update_eoi(NULL, ioapic, data, IOAPIC_LEVEL_TRIG);
 		break;
 #endif
 
diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index cebc123..2001b61 100644
--- a/virt/kvm/ioapic.h
+++ b/virt/kvm/ioapic.h
@@ -70,7 +70,8 @@ static inline struct kvm_ioapic *ioapic_irqchip(struct kvm *kvm)
 int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
 		int short_hand, int dest, int dest_mode);
 int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2);
-void kvm_ioapic_update_eoi(struct kvm *kvm, int vector, int trigger_mode);
+void kvm_ioapic_update_eoi(struct kvm_vcpu *vcpu, int vector,
+			int trigger_mode);
 bool kvm_ioapic_handles_vector(struct kvm *kvm, int vector);
 int kvm_ioapic_init(struct kvm *kvm);
 void kvm_ioapic_destroy(struct kvm *kvm);
-- 
1.7.1

[PATCH v3 4/8] KVM: Introduce struct rtc_status

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 virt/kvm/ioapic.h |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index 2001b61..20715d0 100644
--- a/virt/kvm/ioapic.h
+++ b/virt/kvm/ioapic.h
@@ -34,6 +34,12 @@ struct kvm_vcpu;
 #define	IOAPIC_INIT			0x5
 #define	IOAPIC_EXTINT			0x7
 
+struct rtc_status {
+	int need_eoi;
+	DECLARE_BITMAP(vcpu_map, KVM_MAX_VCPUS);
+	DECLARE_BITMAP(expected_eoi_bitmap, KVM_MAX_VCPUS);
+};
+
 struct kvm_ioapic {
 	u64 base_address;
 	u32 ioregsel;
@@ -47,6 +53,9 @@ struct kvm_ioapic {
 	void (*ack_notifier)(void *opaque, int irq);
 	spinlock_t lock;
 	DECLARE_BITMAP(handled_vectors, 256);
+#ifdef CONFIG_X86
+	struct rtc_status rtc_status;
+#endif
 };
 
 #ifdef DEBUG
-- 
1.7.1

[PATCH v3 5/8] KVM: Recalculate destination vcpu map

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

Update destination vcpu map when ioapic entry or apic(id, ldr, dfr) is changed

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 virt/kvm/ioapic.c |   40 ++++++++++++++++++++++++++++++++++++++--
 1 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 4296116..329efe1 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -87,6 +87,38 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic,
 	return result;
 }
 
+#ifdef CONFIG_X86
+static void rtc_irq_get_dest_vcpu(struct kvm_ioapic *ioapic, int irq)
+{
+	union kvm_ioapic_redirect_entry *entry = &ioapic->redirtbl[irq];
+	struct kvm_lapic_irq irqe;
+
+	if (irq != 8 || entry->fields.mask)
+		return;
+
+	spin_lock(&ioapic->lock);
+	irqe.dest_id = entry->fields.dest_id;
+	irqe.vector = entry->fields.vector;
+	irqe.dest_mode = entry->fields.dest_mode;
+	irqe.trig_mode = entry->fields.trig_mode;
+	irqe.delivery_mode = entry->fields.delivery_mode << 8;
+	irqe.level = 1;
+	irqe.shorthand = 0;
+
+	bitmap_zero(ioapic->rtc_status.vcpu_map, KVM_MAX_VCPUS);
+
+	kvm_get_dest_vcpu(ioapic->kvm, &irqe, ioapic->rtc_status.vcpu_map);
+	spin_unlock(&ioapic->lock);
+}
+
+#else
+
+static void rtc_irq_get_dest_vcpu(struct kvm_ioapic *ioapic, int irq)
+{
+	return;
+}
+#endif
+
 static int ioapic_service(struct kvm_ioapic *ioapic, unsigned int idx)
 {
 	union kvm_ioapic_redirect_entry *pent;
@@ -147,9 +179,13 @@ void kvm_scan_ioapic_entry(struct kvm *kvm)
 {
 	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
 
-	if (!kvm_apic_vid_enabled(kvm) || !ioapic)
+	if (!ioapic)
 		return;
-	kvm_make_update_eoibitmap_request(kvm);
+
+	rtc_irq_get_dest_vcpu(ioapic, 8);
+
+	if (kvm_apic_vid_enabled(kvm))
+		kvm_make_update_eoibitmap_request(kvm);
 }
 
 static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val)
-- 
1.7.1

Re: [PATCH v3 5/8] KVM: Recalculate destination vcpu map

[Gleb Natapov]

On Mon, Mar 18, 2013 at 08:47:19PM +0800, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
> 
> Update destination vcpu map when ioapic entry or apic(id, ldr, dfr) is changed
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> ---
>  virt/kvm/ioapic.c |   40 ++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index 4296116..329efe1 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -87,6 +87,38 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic,
>  	return result;
>  }
>  
> +#ifdef CONFIG_X86
> +static void rtc_irq_get_dest_vcpu(struct kvm_ioapic *ioapic, int irq)
> +{
> +	union kvm_ioapic_redirect_entry *entry = &ioapic->redirtbl[irq];
> +	struct kvm_lapic_irq irqe;
> +
> +	if (irq != 8 || entry->fields.mask)
> +		return;
> +
> +	spin_lock(&ioapic->lock);
How does this not deadlock? The is called from kvm_scan_ioapic_entry()
and kvm_scan_ioapic_entry() is called from ioapic_write_indirect() with
the lock already taken. You should handle that the same way we handle
eoibitmap recalculation: signal vcpu and calculate there.

> +	irqe.dest_id = entry->fields.dest_id;
> +	irqe.vector = entry->fields.vector;
> +	irqe.dest_mode = entry->fields.dest_mode;
> +	irqe.trig_mode = entry->fields.trig_mode;
> +	irqe.delivery_mode = entry->fields.delivery_mode << 8;
> +	irqe.level = 1;
> +	irqe.shorthand = 0;
> +
> +	bitmap_zero(ioapic->rtc_status.vcpu_map, KVM_MAX_VCPUS);
> +
> +	kvm_get_dest_vcpu(ioapic->kvm, &irqe, ioapic->rtc_status.vcpu_map);
> +	spin_unlock(&ioapic->lock);
> +}
> +
> +#else
> +
> +static void rtc_irq_get_dest_vcpu(struct kvm_ioapic *ioapic, int irq)
> +{
> +	return;
> +}
> +#endif
> +
>  static int ioapic_service(struct kvm_ioapic *ioapic, unsigned int idx)
>  {
>  	union kvm_ioapic_redirect_entry *pent;
> @@ -147,9 +179,13 @@ void kvm_scan_ioapic_entry(struct kvm *kvm)
>  {
>  	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
>  
> -	if (!kvm_apic_vid_enabled(kvm) || !ioapic)
> +	if (!ioapic)
>  		return;
> -	kvm_make_update_eoibitmap_request(kvm);
> +
> +	rtc_irq_get_dest_vcpu(ioapic, 8);
> +
> +	if (kvm_apic_vid_enabled(kvm))
> +		kvm_make_update_eoibitmap_request(kvm);
>  }
>  
>  static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val)
> -- 
> 1.7.1

--
			Gleb.

RE: [PATCH v3 5/8] KVM: Recalculate destination vcpu map

[Zhang, Yang Z]

Gleb Natapov wrote on 2013-03-19:
> On Mon, Mar 18, 2013 at 08:47:19PM +0800, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> Update destination vcpu map when ioapic entry or apic(id, ldr, dfr) is changed
>> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>> ---
>>  virt/kvm/ioapic.c |   40 ++++++++++++++++++++++++++++++++++++++--
>>  1 files changed, 38 insertions(+), 2 deletions(-)
>> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
>> index 4296116..329efe1 100644
>> --- a/virt/kvm/ioapic.c
>> +++ b/virt/kvm/ioapic.c
>> @@ -87,6 +87,38 @@ static unsigned long ioapic_read_indirect(struct
> kvm_ioapic *ioapic,
>>  	return result;
>>  }
>> +#ifdef CONFIG_X86
>> +static void rtc_irq_get_dest_vcpu(struct kvm_ioapic *ioapic, int irq)
>> +{
>> +	union kvm_ioapic_redirect_entry *entry = &ioapic->redirtbl[irq];
>> +	struct kvm_lapic_irq irqe;
>> +
>> +	if (irq != 8 || entry->fields.mask)
>> +		return;
>> +
>> +	spin_lock(&ioapic->lock);
> How does this not deadlock? The is called from kvm_scan_ioapic_entry()
> and kvm_scan_ioapic_entry() is called from ioapic_write_indirect() with
I removed the lock before call kvm_scan_ioapic_entry() in ioapic_write_indirect().

> the lock already taken. You should handle that the same way we handle
> eoibitmap recalculation: signal vcpu and calculate there.
Sure.

>> +	irqe.dest_id = entry->fields.dest_id;
>> +	irqe.vector = entry->fields.vector;
>> +	irqe.dest_mode = entry->fields.dest_mode;
>> +	irqe.trig_mode = entry->fields.trig_mode;
>> +	irqe.delivery_mode = entry->fields.delivery_mode << 8;
>> +	irqe.level = 1;
>> +	irqe.shorthand = 0;
>> +
>> +	bitmap_zero(ioapic->rtc_status.vcpu_map, KVM_MAX_VCPUS);
>> +
>> +	kvm_get_dest_vcpu(ioapic->kvm, &irqe, ioapic->rtc_status.vcpu_map);
>> +	spin_unlock(&ioapic->lock);
>> +}
>> +
>> +#else
>> +
>> +static void rtc_irq_get_dest_vcpu(struct kvm_ioapic *ioapic, int irq)
>> +{
>> +	return;
>> +}
>> +#endif
>> +
>>  static int ioapic_service(struct kvm_ioapic *ioapic, unsigned int idx)
>>  { 	union kvm_ioapic_redirect_entry *pent; @@ -147,9 +179,13 @@ void
>>  kvm_scan_ioapic_entry(struct kvm *kvm) { 	struct kvm_ioapic *ioapic =
>>  kvm->arch.vioapic;
>> -	if (!kvm_apic_vid_enabled(kvm) || !ioapic)
>> +	if (!ioapic)
>>  		return;
>> -	kvm_make_update_eoibitmap_request(kvm);
>> +
>> +	rtc_irq_get_dest_vcpu(ioapic, 8);
>> +
>> +	if (kvm_apic_vid_enabled(kvm))
>> +		kvm_make_update_eoibitmap_request(kvm);
>>  }
>>  
>>  static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val)
>> --
>> 1.7.1
> 
> --
> 			Gleb.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Best regards,
Yang

[PATCH v3 6/8] KVM: Add reset/restore rtc_status support

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

reset/restore rtc_status when ioapic reset/restore.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 arch/x86/kvm/lapic.c |    8 ++++++++
 arch/x86/kvm/lapic.h |    1 +
 virt/kvm/ioapic.c    |   33 +++++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 6fb22e3..a223170 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -94,6 +94,14 @@ static inline int apic_test_vector(int vec, void *bitmap)
 	return test_bit(VEC_POS(vec), (bitmap) + REG_POS(vec));
 }
 
+bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector)
+{
+	struct kvm_lapic *apic = vcpu->arch.apic;
+
+	return apic_test_vector(vector, apic->regs + APIC_ISR) ||
+		apic_test_vector(vector, apic->regs + APIC_IRR);
+}
+
 static inline void apic_set_vector(int vec, void *bitmap)
 {
 	set_bit(VEC_POS(vec), (bitmap) + REG_POS(vec));
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 3a0f9d8..e2a03d1 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -160,5 +160,6 @@ void kvm_calculate_eoi_exitmap(struct kvm_vcpu *vcpu,
 
 void kvm_get_dest_vcpu(struct kvm *kvm, struct kvm_lapic_irq *irq,
 				unsigned long *vcpu_map);
+bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector);
 
 #endif
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 329efe1..b74c73f 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -88,6 +88,27 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic,
 }
 
 #ifdef CONFIG_X86
+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;
+	int vector, i, need_eoi = 0, rtc_pin = 8;
+
+	vector = ioapic->redirtbl[rtc_pin].fields.vector;
+	kvm_for_each_vcpu(i, vcpu, ioapic->kvm) {
+		if (kvm_apic_pending_eoi(vcpu, vector)) {
+			need_eoi++;
+			set_bit(vcpu->vcpu_id, ioapic->rtc_status.vcpu_map);
+		}
+	}
+	ioapic->rtc_status.need_eoi = need_eoi;
+}
+
 static void rtc_irq_get_dest_vcpu(struct kvm_ioapic *ioapic, int irq)
 {
 	union kvm_ioapic_redirect_entry *entry = &ioapic->redirtbl[irq];
@@ -113,6 +134,16 @@ static void rtc_irq_get_dest_vcpu(struct kvm_ioapic *ioapic, int irq)
 
 #else
 
+static void rtc_irq_reset(struct kvm_ioapic *ioapic)
+{
+	return;
+}
+
+static void rtc_irq_restore(struct kvm_ioapic *ioapic)
+{
+	return;
+}
+
 static void rtc_irq_get_dest_vcpu(struct kvm_ioapic *ioapic, int irq)
 {
 	return;
@@ -464,6 +495,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);
 }
 
@@ -529,6 +561,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_scan_ioapic_entry(kvm);
 	spin_unlock(&ioapic->lock);
 	return 0;
-- 
1.7.1

[PATCH v3 7/8] KVM: Add rtc irq to eoi exit bitmap

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

Add rtc irq to eoi exit bitmap to force vmexit when virtual interrupt
delivery is enabled.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 virt/kvm/ioapic.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index b74c73f..4ddaa07 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -194,7 +194,7 @@ void kvm_ioapic_calculate_eoi_exitmap(struct kvm_vcpu *vcpu,
 		if (!e->fields.mask &&
 			(e->fields.trig_mode == IOAPIC_LEVEL_TRIG ||
 			 kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC,
-				 index))) {
+				 index) || index == 8)) {
 			irqe.dest_id = e->fields.dest_id;
 			irqe.vector = e->fields.vector;
 			irqe.dest_mode = e->fields.dest_mode;
-- 
1.7.1

[PATCH v3 8/8] KVM: Use eoi to track RTC interrupt delivery status

[Yang Zhang]

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>
---
 virt/kvm/ioapic.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 4ddaa07..35b3d8c 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -103,7 +103,8 @@ static void rtc_irq_restore(struct kvm_ioapic *ioapic)
 	kvm_for_each_vcpu(i, vcpu, ioapic->kvm) {
 		if (kvm_apic_pending_eoi(vcpu, vector)) {
 			need_eoi++;
-			set_bit(vcpu->vcpu_id, ioapic->rtc_status.vcpu_map);
+			set_bit(vcpu->vcpu_id,
+					ioapic->rtc_status.expected_eoi_bitmap);
 		}
 	}
 	ioapic->rtc_status.need_eoi = need_eoi;
@@ -132,6 +133,50 @@ static void rtc_irq_get_dest_vcpu(struct kvm_ioapic *ioapic, int irq)
 	spin_unlock(&ioapic->lock);
 }
 
+static void rtc_irq_set_eoi(struct kvm_ioapic *ioapic, int irq)
+{
+	union kvm_ioapic_redirect_entry *entry = &ioapic->redirtbl[irq];
+
+	if (irq != 8)
+		return;
+
+	if (likely(!bitmap_empty(ioapic->rtc_status.vcpu_map, KVM_MAX_VCPUS))) {
+		if (entry->fields.delivery_mode == APIC_DM_LOWEST)
+			ioapic->rtc_status.need_eoi = 1;
+		else {
+			int weight;
+			weight = bitmap_weight(ioapic->rtc_status.vcpu_map,
+					sizeof(ioapic->rtc_status.vcpu_map));
+			ioapic->rtc_status.need_eoi = weight;
+		}
+		bitmap_copy(ioapic->rtc_status.expected_eoi_bitmap,
+				ioapic->rtc_status.vcpu_map, KVM_MAX_VCPUS);
+	}
+}
+
+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->expected_eoi_bitmap))
+		--rtc_status->need_eoi;
+
+	WARN_ON(rtc_status->need_eoi < 0);
+}
+
+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_irq_reset(struct kvm_ioapic *ioapic)
@@ -148,6 +193,22 @@ static void rtc_irq_get_dest_vcpu(struct kvm_ioapic *ioapic, int irq)
 {
 	return;
 }
+
+static void rtc_irq_set_eoi(struct kvm_ioapic *ioapic, 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)
@@ -260,7 +321,9 @@ static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val)
 		if (e->fields.trig_mode == IOAPIC_LEVEL_TRIG
 		    && ioapic->irr & (1 << index))
 			ioapic_service(ioapic, index);
+		spin_unlock(&ioapic->lock);
 		kvm_scan_ioapic_entry(ioapic->kvm);
+		spin_lock(&ioapic->lock);
 		break;
 	}
 }
@@ -284,6 +347,8 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
 	irqe.level = 1;
 	irqe.shorthand = 0;
 
+	rtc_irq_set_eoi(ioapic, irq);
+
 	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
 }
 
@@ -308,6 +373,11 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
 		ret = 1;
 	} else {
 		int edge = (entry.fields.trig_mode == IOAPIC_EDGE_TRIG);
+
+		if (rtc_irq_check(ioapic, irq)) {
+			ret = 0; /* coalesced */
+			goto out;
+		}
 		ioapic->irr |= mask;
 		if ((edge && old_irr != ioapic->irr) ||
 		    (!edge && !entry.fields.remote_irr))
@@ -315,6 +385,7 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
 		else
 			ret = 0; /* report coalesced interrupt */
 	}
+out:
 	trace_kvm_ioapic_set_irq(entry.bits, irq, ret == 0);
 	spin_unlock(&ioapic->lock);
 
@@ -342,6 +413,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
@@ -562,7 +634,7 @@ int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
 	memcpy(ioapic, state, sizeof(struct kvm_ioapic_state));
 	update_handled_vectors(ioapic);
 	rtc_irq_restore(ioapic);
-	kvm_scan_ioapic_entry(kvm);
 	spin_unlock(&ioapic->lock);
+	kvm_scan_ioapic_entry(kvm);
 	return 0;
 }
-- 
1.7.1