[PATCH v5 0/6] Use eoi to track RTC interrupt delivery status

[PATCH v5 0/6] 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 pending_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 v4 to v5
* Calculate destination vcpu on interrupt injection not hook into ioapic 
  modification.
* Rebase on top of KVM.

Changes from v3 to v4
* Call kvm_apic_match_dest() to check destination vcpu.
* Update RTC interrrupt's destination vcpu map when ioapic entry of RTC
  or apic register (id, ldr, dfr) is changed.

Changes from v2 to v3:
* Remove unused viarable 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.

Yang Zhang (6):
  KVM: Add vcpu info to ioapic_update_eoi()
  KVM: Introduce struct rtc_status
  KVM : Return destination vcpu on interrupt injection
  KVM: Add reset/restore rtc_status support
  KVM : Force vmexit with virtual interrupt delivery
  KVM: Use eoi to track RTC interrupt delivery status

 arch/x86/kvm/lapic.c |   31 ++++++++++++---
 arch/x86/kvm/lapic.h |    7 ++-
 virt/kvm/ioapic.c    |  103 ++++++++++++++++++++++++++++++++++++++++++++++----
 virt/kvm/ioapic.h    |   13 +++++-
 virt/kvm/irq_comm.c  |   12 +++---
 5 files changed, 142 insertions(+), 24 deletions(-)

[PATCH v5 1/6] KVM: Add vcpu info to ioapic_update_eoi()

[Yang Zhang]

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

Add vcpu info to ioapic_update_eoi, so we can know which vcpu
issued this 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 a8e9369..d3e322a 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -786,7 +786,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 ce82b94..ed6f111 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 0400a46..2fc61a5 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 v5 2/6] 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 |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index 2fc61a5..6e5c88f 100644
--- a/virt/kvm/ioapic.h
+++ b/virt/kvm/ioapic.h
@@ -34,6 +34,11 @@ struct kvm_vcpu;
 #define	IOAPIC_INIT			0x5
 #define	IOAPIC_EXTINT			0x7
 
+struct rtc_status {
+	int pending_eoi;
+	DECLARE_BITMAP(dest_map, KVM_MAX_VCPUS);
+};
+
 struct kvm_ioapic {
 	u64 base_address;
 	u32 ioregsel;
@@ -47,6 +52,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 v5 3/6] KVM : Calculate destination vcpu on interrupt injection

[Yang Zhang]

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

Add a new parameter to know vcpus who received the interrupt.

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

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index d3e322a..5f6b1d0 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu)
 static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 			     int vector, int level, int trig_mode);
 
-int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
+static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long *dest_map)
+{
+	if (!kvm_lapic_enabled(vcpu))
+		return;
+	__set_bit(vcpu->vcpu_id, dest_map);
+}
+
+int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
+		unsigned long *dest_map)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
+	if (dest_map)
+		kvm_set_irq_dest_map(vcpu, dest_map);
+
 	return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
 			irq->level, irq->trig_mode);
 }
@@ -611,7 +622,7 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
 }
 
 bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
-		struct kvm_lapic_irq *irq, int *r)
+		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
 {
 	struct kvm_apic_map *map;
 	unsigned long bitmap = 1;
@@ -622,7 +633,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
 	*r = -1;
 
 	if (irq->shorthand == APIC_DEST_SELF) {
-		*r = kvm_apic_set_irq(src->vcpu, irq);
+		*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
 		return true;
 	}
 
@@ -667,7 +678,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
 			continue;
 		if (*r < 0)
 			*r = 0;
-		*r += kvm_apic_set_irq(dst[i]->vcpu, irq);
+		*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
 	}
 
 	ret = true;
@@ -852,7 +863,7 @@ static void apic_send_ipi(struct kvm_lapic *apic)
 		   irq.trig_mode, irq.level, irq.dest_mode, irq.delivery_mode,
 		   irq.vector);
 
-	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq);
+	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
 }
 
 static u32 apic_get_tmcct(struct kvm_lapic *apic)
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 2c721b9..967519c 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -55,11 +55,12 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
 
 int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
 int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
-int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq);
+int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
+		unsigned long *dest_map);
 int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
 
 bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
-		struct kvm_lapic_irq *irq, int *r);
+		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
 
 u64 kvm_get_apic_base(struct kvm_vcpu *vcpu);
 void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index ed6f111..4767fa6 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -217,7 +217,7 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
 	irqe.level = 1;
 	irqe.shorthand = 0;
 
-	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
+	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
 }
 
 int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index 6e5c88f..14e5289 100644
--- a/virt/kvm/ioapic.h
+++ b/virt/kvm/ioapic.h
@@ -88,7 +88,7 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
 void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
 void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
 int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
-		struct kvm_lapic_irq *irq);
+		struct kvm_lapic_irq *irq, unsigned long *dest_map);
 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);
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index e9073cf..2f07d2e 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -63,7 +63,7 @@ inline static bool kvm_is_dm_lowest_prio(struct kvm_lapic_irq *irq)
 }
 
 int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
-		struct kvm_lapic_irq *irq)
+		struct kvm_lapic_irq *irq, unsigned long *dest_map)
 {
 	int i, r = -1;
 	struct kvm_vcpu *vcpu, *lowest = NULL;
@@ -74,7 +74,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 		irq->delivery_mode = APIC_DM_FIXED;
 	}
 
-	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r))
+	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r, dest_map))
 		return r;
 
 	kvm_for_each_vcpu(i, vcpu, kvm) {
@@ -88,7 +88,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 		if (!kvm_is_dm_lowest_prio(irq)) {
 			if (r < 0)
 				r = 0;
-			r += kvm_apic_set_irq(vcpu, irq);
+			r += kvm_apic_set_irq(vcpu, irq, dest_map);
 		} else if (kvm_lapic_enabled(vcpu)) {
 			if (!lowest)
 				lowest = vcpu;
@@ -98,7 +98,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 	}
 
 	if (lowest)
-		r = kvm_apic_set_irq(lowest, irq);
+		r = kvm_apic_set_irq(lowest, irq, dest_map);
 
 	return r;
 }
@@ -130,7 +130,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 
 	kvm_set_msi_irq(e, &irq);
 
-	return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
+	return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
 }
 
 
@@ -142,7 +142,7 @@ static int kvm_set_msi_inatomic(struct kvm_kernel_irq_routing_entry *e,
 
 	kvm_set_msi_irq(e, &irq);
 
-	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r))
+	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL))
 		return r;
 	else
 		return -EWOULDBLOCK;
-- 
1.7.1

[PATCH v5 4/6] KVM: Add reset/restore rtc_status support

[Yang Zhang]

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

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

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 5f6b1d0..158e0a3 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 967519c..004d2ad 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -170,4 +170,6 @@ static inline bool kvm_apic_has_events(struct kvm_vcpu *vcpu)
 	return vcpu->arch.apic->pending_events;
 }
 
+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 4767fa6..8f9c62b 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -87,6 +87,39 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic,
 	return result;
 }
 
+#ifdef CONFIG_X86
+static void rtc_irq_reset(struct kvm_ioapic *ioapic)
+{
+	ioapic->rtc_status.pending_eoi = 0;
+	bitmap_zero(ioapic->rtc_status.dest_map, KVM_MAX_VCPUS);
+}
+
+static void rtc_irq_restore(struct kvm_ioapic *ioapic)
+{
+	struct kvm_vcpu *vcpu;
+	int vector, i, pending_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)) {
+			pending_eoi++;
+			set_bit(vcpu->vcpu_id, ioapic->rtc_status.dest_map);
+		}
+	}
+	ioapic->rtc_status.pending_eoi = pending_eoi;
+}
+#else
+static void rtc_irq_reset(struct kvm_ioapic *ioapic)
+{
+	return;
+}
+
+static void rtc_irq_restore(struct kvm_ioapic *ioapic)
+{
+	return;
+}
+#endif
+
 static int ioapic_service(struct kvm_ioapic *ioapic, unsigned int idx)
 {
 	union kvm_ioapic_redirect_entry *pent;
@@ -428,6 +461,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);
 }
 
@@ -493,6 +527,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

[PATCH v5 5/6] KVM: Force vmexit with virtual interrupt delivery

[Yang Zhang]

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

Need the EOI to track interrupt deliver status, so force vmexit
on EOI for rtc interrupt when enabling virtual interrupt delivery.

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 4c77832..a5ee8bf 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -165,7 +165,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 v5 6/6] 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 pending_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 |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 52 insertions(+), 1 deletions(-)

diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index a5ee8bf..3aec0a2 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -109,6 +109,29 @@ static void rtc_irq_restore(struct kvm_ioapic *ioapic)
 	}
 	ioapic->rtc_status.pending_eoi = pending_eoi;
 }
+
+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->dest_map))
+		--rtc_status->pending_eoi;
+
+	WARN_ON(rtc_status->pending_eoi < 0);
+}
+
+static bool rtc_irq_check(struct kvm_ioapic *ioapic, int irq)
+{
+	if (irq != 8)
+		return false;
+
+	if (ioapic->rtc_status.pending_eoi > 0)
+		return true; /* coalesced */
+
+	return false;
+}
 #else
 static void rtc_irq_reset(struct kvm_ioapic *ioapic)
 {
@@ -119,6 +142,17 @@ static void rtc_irq_restore(struct kvm_ioapic *ioapic)
 {
 	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)
@@ -236,6 +270,7 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
 {
 	union kvm_ioapic_redirect_entry *entry = &ioapic->redirtbl[irq];
 	struct kvm_lapic_irq irqe;
+	int ret;
 
 	ioapic_debug("dest=%x dest_mode=%x delivery_mode=%x "
 		     "vector=%x trig_mode=%x\n",
@@ -251,7 +286,16 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
 	irqe.level = 1;
 	irqe.shorthand = 0;
 
-	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
+#ifdef CONFIG_X86
+	if (irq == 8) {
+		ret = kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe,
+				ioapic->rtc_status.dest_map);
+		ioapic->rtc_status.pending_eoi = ret;
+	} else
+#endif
+		ret = kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
+
+	return ret;
 }
 
 int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
@@ -275,6 +319,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))
@@ -282,6 +331,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);
 
@@ -309,6 +359,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
-- 
1.7.1

Re: [PATCH v5 3/6] KVM : Calculate destination vcpu on interrupt injection

[Gleb Natapov]

On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
> 
> Add a new parameter to know vcpus who received the interrupt.
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> ---
>  arch/x86/kvm/lapic.c |   21 ++++++++++++++++-----
>  arch/x86/kvm/lapic.h |    5 +++--
>  virt/kvm/ioapic.c    |    2 +-
>  virt/kvm/ioapic.h    |    2 +-
>  virt/kvm/irq_comm.c  |   12 ++++++------
>  5 files changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index d3e322a..5f6b1d0 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu)
>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>  			     int vector, int level, int trig_mode);
>  
> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
> +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long *dest_map)
> +{
> +	if (!kvm_lapic_enabled(vcpu))
> +		return;
Why this check here?

> +	__set_bit(vcpu->vcpu_id, dest_map);
> +}
> +
> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> +		unsigned long *dest_map)
>  {
>  	struct kvm_lapic *apic = vcpu->arch.apic;
>  
> +	if (dest_map)
> +		kvm_set_irq_dest_map(vcpu, dest_map);
> +
>  	return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
>  			irq->level, irq->trig_mode);
>  }
> @@ -611,7 +622,7 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
>  }
>  
>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> -		struct kvm_lapic_irq *irq, int *r)
> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
>  {
>  	struct kvm_apic_map *map;
>  	unsigned long bitmap = 1;
> @@ -622,7 +633,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
>  	*r = -1;
>  
>  	if (irq->shorthand == APIC_DEST_SELF) {
> -		*r = kvm_apic_set_irq(src->vcpu, irq);
> +		*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
>  		return true;
>  	}
>  
> @@ -667,7 +678,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
>  			continue;
>  		if (*r < 0)
>  			*r = 0;
> -		*r += kvm_apic_set_irq(dst[i]->vcpu, irq);
> +		*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
>  	}
>  
>  	ret = true;
> @@ -852,7 +863,7 @@ static void apic_send_ipi(struct kvm_lapic *apic)
>  		   irq.trig_mode, irq.level, irq.dest_mode, irq.delivery_mode,
>  		   irq.vector);
>  
> -	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq);
> +	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
>  }
>  
>  static u32 apic_get_tmcct(struct kvm_lapic *apic)
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index 2c721b9..967519c 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -55,11 +55,12 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
>  
>  int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
>  int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq);
> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> +		unsigned long *dest_map);
>  int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
>  
>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> -		struct kvm_lapic_irq *irq, int *r);
> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
>  
>  u64 kvm_get_apic_base(struct kvm_vcpu *vcpu);
>  void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index ed6f111..4767fa6 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -217,7 +217,7 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
>  	irqe.level = 1;
>  	irqe.shorthand = 0;
>  
> -	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
> +	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
>  }
>  
>  int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
> diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
> index 6e5c88f..14e5289 100644
> --- a/virt/kvm/ioapic.h
> +++ b/virt/kvm/ioapic.h
> @@ -88,7 +88,7 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
>  void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
>  void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> -		struct kvm_lapic_irq *irq);
> +		struct kvm_lapic_irq *irq, unsigned long *dest_map);
>  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);
> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
> index e9073cf..2f07d2e 100644
> --- a/virt/kvm/irq_comm.c
> +++ b/virt/kvm/irq_comm.c
> @@ -63,7 +63,7 @@ inline static bool kvm_is_dm_lowest_prio(struct kvm_lapic_irq *irq)
>  }
>  
>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> -		struct kvm_lapic_irq *irq)
> +		struct kvm_lapic_irq *irq, unsigned long *dest_map)
>  {
>  	int i, r = -1;
>  	struct kvm_vcpu *vcpu, *lowest = NULL;
> @@ -74,7 +74,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>  		irq->delivery_mode = APIC_DM_FIXED;
>  	}
>  
> -	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r))
> +	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r, dest_map))
>  		return r;
>  
>  	kvm_for_each_vcpu(i, vcpu, kvm) {
> @@ -88,7 +88,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>  		if (!kvm_is_dm_lowest_prio(irq)) {
>  			if (r < 0)
>  				r = 0;
> -			r += kvm_apic_set_irq(vcpu, irq);
> +			r += kvm_apic_set_irq(vcpu, irq, dest_map);
>  		} else if (kvm_lapic_enabled(vcpu)) {
>  			if (!lowest)
>  				lowest = vcpu;
> @@ -98,7 +98,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>  	}
>  
>  	if (lowest)
> -		r = kvm_apic_set_irq(lowest, irq);
> +		r = kvm_apic_set_irq(lowest, irq, dest_map);
>  
>  	return r;
>  }
> @@ -130,7 +130,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
>  
>  	kvm_set_msi_irq(e, &irq);
>  
> -	return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
> +	return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
>  }
>  
>  
> @@ -142,7 +142,7 @@ static int kvm_set_msi_inatomic(struct kvm_kernel_irq_routing_entry *e,
>  
>  	kvm_set_msi_irq(e, &irq);
>  
> -	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r))
> +	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL))
>  		return r;
>  	else
>  		return -EWOULDBLOCK;
> -- 
> 1.7.1

--
			Gleb.

RE: [PATCH v5 3/6] KVM : Calculate destination vcpu on interrupt injection

[Zhang, Yang Z]

Gleb Natapov wrote on 2013-03-21:
> On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> Add a new parameter to know vcpus who received the interrupt.
>> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>> ---
>>  arch/x86/kvm/lapic.c |   21 ++++++++++++++++-----
>>  arch/x86/kvm/lapic.h |    5 +++--
>>  virt/kvm/ioapic.c    |    2 +-
>>  virt/kvm/ioapic.h    |    2 +-
>>  virt/kvm/irq_comm.c  |   12 ++++++------
>>  5 files changed, 27 insertions(+), 15 deletions(-)
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index d3e322a..5f6b1d0 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu
> *vcpu)
>>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>>  			     int vector, int level, int trig_mode);
>> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
>> +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long
>> *dest_map) +{ +	if (!kvm_lapic_enabled(vcpu)) +		return;
> Why this check here?
The vcpu who didn't enable apic should not account as destination vcpu. 
Without this check, if broadcast interrupt, all cpus will treat as destination vcpu, but only those who enabled apic will receive the interrupt.
There are same check in __apic_accept_irq():
if (unlikely(!apic_enabled(apic)))
     break;

>> +	__set_bit(vcpu->vcpu_id, dest_map);
>> +}
>> +
>> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
>> +		unsigned long *dest_map)
>>  {
>>  	struct kvm_lapic *apic = vcpu->arch.apic;
>> +	if (dest_map)
>> +		kvm_set_irq_dest_map(vcpu, dest_map);
>> +
>>  	return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
>>  			irq->level, irq->trig_mode);
>>  }
>> @@ -611,7 +622,7 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu,
> struct kvm_lapic *source,
>>  }
>>  
>>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
>> -		struct kvm_lapic_irq *irq, int *r)
>> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
>>  {
>>  	struct kvm_apic_map *map;
>>  	unsigned long bitmap = 1;
>> @@ -622,7 +633,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm,
> struct kvm_lapic *src,
>>  	*r = -1;
>>  
>>  	if (irq->shorthand == APIC_DEST_SELF) {
>> -		*r = kvm_apic_set_irq(src->vcpu, irq);
>> +		*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
>>  		return true;
>>  	}
>> @@ -667,7 +678,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm,
> struct kvm_lapic *src,
>>  			continue;
>>  		if (*r < 0)
>>  			*r = 0;
>> -		*r += kvm_apic_set_irq(dst[i]->vcpu, irq);
>> +		*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
>>  	}
>>  
>>  	ret = true; @@ -852,7 +863,7 @@ static void apic_send_ipi(struct
>>  kvm_lapic *apic) 		   irq.trig_mode, irq.level, irq.dest_mode,
>>  irq.delivery_mode, 		   irq.vector);
>> -	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq);
>> +	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
>>  }
>>  
>>  static u32 apic_get_tmcct(struct kvm_lapic *apic)
>> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
>> index 2c721b9..967519c 100644
>> --- a/arch/x86/kvm/lapic.h
>> +++ b/arch/x86/kvm/lapic.h
>> @@ -55,11 +55,12 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
>> 
>>  int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
>>  int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
>> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq);
>> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
>> +		unsigned long *dest_map);
>>  int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
>>  
>>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
>> -		struct kvm_lapic_irq *irq, int *r);
>> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
>> 
>>  u64 kvm_get_apic_base(struct kvm_vcpu *vcpu);
>>  void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
>> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
>> index ed6f111..4767fa6 100644
>> --- a/virt/kvm/ioapic.c
>> +++ b/virt/kvm/ioapic.c
>> @@ -217,7 +217,7 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int
> irq)
>>  	irqe.level = 1;
>>  	irqe.shorthand = 0;
>> -	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
>> +	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
>>  }
>>  
>>  int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
>> diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
>> index 6e5c88f..14e5289 100644
>> --- a/virt/kvm/ioapic.h
>> +++ b/virt/kvm/ioapic.h
>> @@ -88,7 +88,7 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq,
> int irq_source_id,
>>  void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
>>  void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
>>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>> -		struct kvm_lapic_irq *irq);
>> +		struct kvm_lapic_irq *irq, unsigned long *dest_map);
>>  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);
>> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
>> index e9073cf..2f07d2e 100644
>> --- a/virt/kvm/irq_comm.c
>> +++ b/virt/kvm/irq_comm.c
>> @@ -63,7 +63,7 @@ inline static bool kvm_is_dm_lowest_prio(struct
> kvm_lapic_irq *irq)
>>  }
>>  
>>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>> -		struct kvm_lapic_irq *irq)
>> +		struct kvm_lapic_irq *irq, unsigned long *dest_map)
>>  {
>>  	int i, r = -1;
>>  	struct kvm_vcpu *vcpu, *lowest = NULL;
>> @@ -74,7 +74,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> kvm_lapic *src,
>>  		irq->delivery_mode = APIC_DM_FIXED;
>>  	}
>> -	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r))
>> +	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r, dest_map))
>>  		return r;
>>  
>>  	kvm_for_each_vcpu(i, vcpu, kvm) {
>> @@ -88,7 +88,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> kvm_lapic *src,
>>  		if (!kvm_is_dm_lowest_prio(irq)) {
>>  			if (r < 0)
>>  				r = 0;
>> -			r += kvm_apic_set_irq(vcpu, irq);
>> +			r += kvm_apic_set_irq(vcpu, irq, dest_map);
>>  		} else if (kvm_lapic_enabled(vcpu)) {
>>  			if (!lowest)
>>  				lowest = vcpu;
>> @@ -98,7 +98,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> kvm_lapic *src,
>>  	}
>>  
>>  	if (lowest)
>> -		r = kvm_apic_set_irq(lowest, irq);
>> +		r = kvm_apic_set_irq(lowest, irq, dest_map);
>> 
>>  	return r;
>>  }
>> @@ -130,7 +130,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry
>> *e,
>> 
>>  	kvm_set_msi_irq(e, &irq);
>> -	return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
>> +	return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
>>  }
>> 
>> @@ -142,7 +142,7 @@ static int kvm_set_msi_inatomic(struct
>> kvm_kernel_irq_routing_entry *e,
>> 
>>  	kvm_set_msi_irq(e, &irq);
>> -	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r))
>> +	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL))
>>  		return r;
>>  	else
>>  		return -EWOULDBLOCK;
>> --
>> 1.7.1
> 
> --
> 			Gleb.


Best regards,
Yang

Re: [PATCH v5 3/6] KVM : Calculate destination vcpu on interrupt injection

[Gleb Natapov]

On Thu, Mar 21, 2013 at 11:56:05AM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2013-03-21:
> > On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote:
> >> From: Yang Zhang <yang.z.zhang@Intel.com>
> >> 
> >> Add a new parameter to know vcpus who received the interrupt.
> >> 
> >> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> >> ---
> >>  arch/x86/kvm/lapic.c |   21 ++++++++++++++++-----
> >>  arch/x86/kvm/lapic.h |    5 +++--
> >>  virt/kvm/ioapic.c    |    2 +-
> >>  virt/kvm/ioapic.h    |    2 +-
> >>  virt/kvm/irq_comm.c  |   12 ++++++------
> >>  5 files changed, 27 insertions(+), 15 deletions(-)
> >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> >> index d3e322a..5f6b1d0 100644
> >> --- a/arch/x86/kvm/lapic.c
> >> +++ b/arch/x86/kvm/lapic.c
> >> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu
> > *vcpu)
> >>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> >>  			     int vector, int level, int trig_mode);
> >> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
> >> +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long
> >> *dest_map) +{ +	if (!kvm_lapic_enabled(vcpu)) +		return;
> > Why this check here?
> The vcpu who didn't enable apic should not account as destination vcpu. 
> Without this check, if broadcast interrupt, all cpus will treat as destination vcpu, but only those who enabled apic will receive the interrupt.
> There are same check in __apic_accept_irq():
> if (unlikely(!apic_enabled(apic)))
>      break;
I see, but you use more strict check that also checks that apic is
emulated by the kernel and we wouldn't be here if it wasn't. Anyway lets
move bitmap update into __apic_accept_irq().

> 
> >> +	__set_bit(vcpu->vcpu_id, dest_map);
> >> +}
> >> +
> >> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> >> +		unsigned long *dest_map)
> >>  {
> >>  	struct kvm_lapic *apic = vcpu->arch.apic;
> >> +	if (dest_map)
> >> +		kvm_set_irq_dest_map(vcpu, dest_map);
> >> +
> >>  	return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
> >>  			irq->level, irq->trig_mode);
> >>  }
> >> @@ -611,7 +622,7 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu,
> > struct kvm_lapic *source,
> >>  }
> >>  
> >>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> >> -		struct kvm_lapic_irq *irq, int *r)
> >> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
> >>  {
> >>  	struct kvm_apic_map *map;
> >>  	unsigned long bitmap = 1;
> >> @@ -622,7 +633,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm,
> > struct kvm_lapic *src,
> >>  	*r = -1;
> >>  
> >>  	if (irq->shorthand == APIC_DEST_SELF) {
> >> -		*r = kvm_apic_set_irq(src->vcpu, irq);
> >> +		*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
> >>  		return true;
> >>  	}
> >> @@ -667,7 +678,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm,
> > struct kvm_lapic *src,
> >>  			continue;
> >>  		if (*r < 0)
> >>  			*r = 0;
> >> -		*r += kvm_apic_set_irq(dst[i]->vcpu, irq);
> >> +		*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
> >>  	}
> >>  
> >>  	ret = true; @@ -852,7 +863,7 @@ static void apic_send_ipi(struct
> >>  kvm_lapic *apic) 		   irq.trig_mode, irq.level, irq.dest_mode,
> >>  irq.delivery_mode, 		   irq.vector);
> >> -	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq);
> >> +	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
> >>  }
> >>  
> >>  static u32 apic_get_tmcct(struct kvm_lapic *apic)
> >> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> >> index 2c721b9..967519c 100644
> >> --- a/arch/x86/kvm/lapic.h
> >> +++ b/arch/x86/kvm/lapic.h
> >> @@ -55,11 +55,12 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
> >> 
> >>  int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
> >>  int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
> >> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq);
> >> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> >> +		unsigned long *dest_map);
> >>  int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
> >>  
> >>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> >> -		struct kvm_lapic_irq *irq, int *r);
> >> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
> >> 
> >>  u64 kvm_get_apic_base(struct kvm_vcpu *vcpu);
> >>  void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
> >> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> >> index ed6f111..4767fa6 100644
> >> --- a/virt/kvm/ioapic.c
> >> +++ b/virt/kvm/ioapic.c
> >> @@ -217,7 +217,7 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int
> > irq)
> >>  	irqe.level = 1;
> >>  	irqe.shorthand = 0;
> >> -	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
> >> +	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
> >>  }
> >>  
> >>  int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
> >> diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
> >> index 6e5c88f..14e5289 100644
> >> --- a/virt/kvm/ioapic.h
> >> +++ b/virt/kvm/ioapic.h
> >> @@ -88,7 +88,7 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq,
> > int irq_source_id,
> >>  void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
> >>  void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
> >>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> >> -		struct kvm_lapic_irq *irq);
> >> +		struct kvm_lapic_irq *irq, unsigned long *dest_map);
> >>  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);
> >> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
> >> index e9073cf..2f07d2e 100644
> >> --- a/virt/kvm/irq_comm.c
> >> +++ b/virt/kvm/irq_comm.c
> >> @@ -63,7 +63,7 @@ inline static bool kvm_is_dm_lowest_prio(struct
> > kvm_lapic_irq *irq)
> >>  }
> >>  
> >>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> >> -		struct kvm_lapic_irq *irq)
> >> +		struct kvm_lapic_irq *irq, unsigned long *dest_map)
> >>  {
> >>  	int i, r = -1;
> >>  	struct kvm_vcpu *vcpu, *lowest = NULL;
> >> @@ -74,7 +74,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> > kvm_lapic *src,
> >>  		irq->delivery_mode = APIC_DM_FIXED;
> >>  	}
> >> -	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r))
> >> +	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r, dest_map))
> >>  		return r;
> >>  
> >>  	kvm_for_each_vcpu(i, vcpu, kvm) {
> >> @@ -88,7 +88,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> > kvm_lapic *src,
> >>  		if (!kvm_is_dm_lowest_prio(irq)) {
> >>  			if (r < 0)
> >>  				r = 0;
> >> -			r += kvm_apic_set_irq(vcpu, irq);
> >> +			r += kvm_apic_set_irq(vcpu, irq, dest_map);
> >>  		} else if (kvm_lapic_enabled(vcpu)) {
> >>  			if (!lowest)
> >>  				lowest = vcpu;
> >> @@ -98,7 +98,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> > kvm_lapic *src,
> >>  	}
> >>  
> >>  	if (lowest)
> >> -		r = kvm_apic_set_irq(lowest, irq);
> >> +		r = kvm_apic_set_irq(lowest, irq, dest_map);
> >> 
> >>  	return r;
> >>  }
> >> @@ -130,7 +130,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry
> >> *e,
> >> 
> >>  	kvm_set_msi_irq(e, &irq);
> >> -	return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
> >> +	return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
> >>  }
> >> 
> >> @@ -142,7 +142,7 @@ static int kvm_set_msi_inatomic(struct
> >> kvm_kernel_irq_routing_entry *e,
> >> 
> >>  	kvm_set_msi_irq(e, &irq);
> >> -	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r))
> >> +	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL))
> >>  		return r;
> >>  	else
> >>  		return -EWOULDBLOCK;
> >> --
> >> 1.7.1
> > 
> > --
> > 			Gleb.
> 
> 
> Best regards,
> Yang
> 

--
			Gleb.

Re: [PATCH v5 2/6] KVM: Introduce struct rtc_status

[Gleb Natapov]

On Thu, Mar 21, 2013 at 06:49:20PM +0800, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> ---
>  virt/kvm/ioapic.h |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
> index 2fc61a5..6e5c88f 100644
> --- a/virt/kvm/ioapic.h
> +++ b/virt/kvm/ioapic.h
> @@ -34,6 +34,11 @@ struct kvm_vcpu;
>  #define	IOAPIC_INIT			0x5
>  #define	IOAPIC_EXTINT			0x7
>  
> +struct rtc_status {
> +	int pending_eoi;
> +	DECLARE_BITMAP(dest_map, KVM_MAX_VCPUS);
> +};
> +
>  struct kvm_ioapic {
>  	u64 base_address;
>  	u32 ioregsel;
> @@ -47,6 +52,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
IA64 KVM is almost dead, but we still add CONFIG_X86 everywhere in these
patches. Lets drop all CONFIG_X86 throughout the patches and instead leave
only one:
#ifdef CONFIG_X86
#define RTC_GSI 8
else
#define RTC_GSI 255
#endif

Then use RTC_GSI instead of 8 everywhere and the code will be effectively
disabled on IA64.

>  };
>  
>  #ifdef DEBUG
> -- 
> 1.7.1

--
			Gleb.

RE: [PATCH v5 3/6] KVM : Calculate destination vcpu on interrupt injection

[Zhang, Yang Z]

Gleb Natapov wrote on 2013-03-21:
> On Thu, Mar 21, 2013 at 11:56:05AM +0000, Zhang, Yang Z wrote:
>> Gleb Natapov wrote on 2013-03-21:
>>> On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote:
>>>> From: Yang Zhang <yang.z.zhang@Intel.com>
>>>> 
>>>> Add a new parameter to know vcpus who received the interrupt.
>>>> 
>>>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>>>> ---
>>>>  arch/x86/kvm/lapic.c |   21 ++++++++++++++++-----
>>>>  arch/x86/kvm/lapic.h |    5 +++--
>>>>  virt/kvm/ioapic.c    |    2 +-
>>>>  virt/kvm/ioapic.h    |    2 +-
>>>>  virt/kvm/irq_comm.c  |   12 ++++++------
>>>>  5 files changed, 27 insertions(+), 15 deletions(-)
>>>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>>>> index d3e322a..5f6b1d0 100644
>>>> --- a/arch/x86/kvm/lapic.c
>>>> +++ b/arch/x86/kvm/lapic.c
>>>> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu
>>> *vcpu)
>>>>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>>>>  			     int vector, int level, int trig_mode);
>>>> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
>>>> +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long
>>>> *dest_map) +{ +	if (!kvm_lapic_enabled(vcpu)) +		return;
>>> Why this check here?
>> The vcpu who didn't enable apic should not account as destination vcpu.
>> Without this check, if broadcast interrupt, all cpus will treat as
>> destination vcpu, but only those who enabled apic will receive the
>> interrupt. There are same check in __apic_accept_irq(): if
>> (unlikely(!apic_enabled(apic)))
>>      break;
> I see, but you use more strict check that also checks that apic is
> emulated by the kernel and we wouldn't be here if it wasn't. Anyway lets
Do you mean the check add in here will block "userspace apic"? Shouldn't only in-kernel apic will get here?

> move bitmap update into __apic_accept_irq().
Sure.

> 
>> 
>>>> +	__set_bit(vcpu->vcpu_id, dest_map);
>>>> +}
>>>> +
>>>> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
>>>> +		unsigned long *dest_map)
>>>>  {
>>>>  	struct kvm_lapic *apic = vcpu->arch.apic;
>>>> +	if (dest_map)
>>>> +		kvm_set_irq_dest_map(vcpu, dest_map);
>>>> +
>>>>  	return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
>>>>  			irq->level, irq->trig_mode);
>>>>  }
>>>> @@ -611,7 +622,7 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu,
>>> struct kvm_lapic *source,
>>>>  }
>>>>  
>>>>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic
> *src,
>>>> -		struct kvm_lapic_irq *irq, int *r)
>>>> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
>>>>  {
>>>>  	struct kvm_apic_map *map;
>>>>  	unsigned long bitmap = 1;
>>>> @@ -622,7 +633,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm
> *kvm,
>>> struct kvm_lapic *src,
>>>>  	*r = -1;
>>>>  
>>>>  	if (irq->shorthand == APIC_DEST_SELF) {
>>>> -		*r = kvm_apic_set_irq(src->vcpu, irq);
>>>> +		*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
>>>>  		return true;
>>>>  	}
>>>> @@ -667,7 +678,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm
> *kvm,
>>> struct kvm_lapic *src,
>>>>  			continue;
>>>>  		if (*r < 0)
>>>>  			*r = 0;
>>>> -		*r += kvm_apic_set_irq(dst[i]->vcpu, irq);
>>>> +		*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
>>>>  	}
>>>>  
>>>>  	ret = true; @@ -852,7 +863,7 @@ static void apic_send_ipi(struct
>>>>  kvm_lapic *apic) 		   irq.trig_mode, irq.level, irq.dest_mode,
>>>>  irq.delivery_mode, 		   irq.vector);
>>>> -	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq);
>>>> +	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
>>>>  }
>>>>  
>>>>  static u32 apic_get_tmcct(struct kvm_lapic *apic)
>>>> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
>>>> index 2c721b9..967519c 100644
>>>> --- a/arch/x86/kvm/lapic.h
>>>> +++ b/arch/x86/kvm/lapic.h
>>>> @@ -55,11 +55,12 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
>>>> 
>>>>  int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
>>>>  int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
>>>> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq);
>>>> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
>>>> +		unsigned long *dest_map);
>>>>  int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
>>>>  
>>>>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic
> *src,
>>>> -		struct kvm_lapic_irq *irq, int *r);
>>>> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
>>>> 
>>>>  u64 kvm_get_apic_base(struct kvm_vcpu *vcpu);
>>>>  void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
>>>> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
>>>> index ed6f111..4767fa6 100644
>>>> --- a/virt/kvm/ioapic.c
>>>> +++ b/virt/kvm/ioapic.c
>>>> @@ -217,7 +217,7 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int
>>> irq)
>>>>  	irqe.level = 1;
>>>>  	irqe.shorthand = 0;
>>>> -	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
>>>> +	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
>>>>  }
>>>>  
>>>>  int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
>>>> diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
>>>> index 6e5c88f..14e5289 100644
>>>> --- a/virt/kvm/ioapic.h
>>>> +++ b/virt/kvm/ioapic.h
>>>> @@ -88,7 +88,7 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int
> irq,
>>> int irq_source_id,
>>>>  void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
>>>>  void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
>>>>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>>>> -		struct kvm_lapic_irq *irq);
>>>> +		struct kvm_lapic_irq *irq, unsigned long *dest_map);
>>>>  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);
>>>> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
>>>> index e9073cf..2f07d2e 100644
>>>> --- a/virt/kvm/irq_comm.c
>>>> +++ b/virt/kvm/irq_comm.c
>>>> @@ -63,7 +63,7 @@ inline static bool kvm_is_dm_lowest_prio(struct
>>> kvm_lapic_irq *irq)
>>>>  }
>>>>  
>>>>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>>>> -		struct kvm_lapic_irq *irq)
>>>> +		struct kvm_lapic_irq *irq, unsigned long *dest_map)
>>>>  {
>>>>  	int i, r = -1;
>>>>  	struct kvm_vcpu *vcpu, *lowest = NULL;
>>>> @@ -74,7 +74,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
>>> kvm_lapic *src,
>>>>  		irq->delivery_mode = APIC_DM_FIXED;
>>>>  	}
>>>> -	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r))
>>>> +	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r, dest_map))
>>>>  		return r;
>>>>  
>>>>  	kvm_for_each_vcpu(i, vcpu, kvm) {
>>>> @@ -88,7 +88,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
>>> kvm_lapic *src,
>>>>  		if (!kvm_is_dm_lowest_prio(irq)) {
>>>>  			if (r < 0)
>>>>  				r = 0;
>>>> -			r += kvm_apic_set_irq(vcpu, irq);
>>>> +			r += kvm_apic_set_irq(vcpu, irq, dest_map);
>>>>  		} else if (kvm_lapic_enabled(vcpu)) {
>>>>  			if (!lowest)
>>>>  				lowest = vcpu;
>>>> @@ -98,7 +98,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
>>> kvm_lapic *src,
>>>>  	}
>>>>  
>>>>  	if (lowest)
>>>> -		r = kvm_apic_set_irq(lowest, irq);
>>>> +		r = kvm_apic_set_irq(lowest, irq, dest_map);
>>>> 
>>>>  	return r;
>>>>  }
>>>> @@ -130,7 +130,7 @@ int kvm_set_msi(struct
>>>> kvm_kernel_irq_routing_entry *e,
>>>> 
>>>>  	kvm_set_msi_irq(e, &irq);
>>>> -	return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
>>>> +	return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
>>>>  }
>>>> @@ -142,7 +142,7 @@ static int kvm_set_msi_inatomic(struct
>>>> kvm_kernel_irq_routing_entry *e,
>>>> 
>>>>  	kvm_set_msi_irq(e, &irq);
>>>> -	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r))
>>>> +	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL))
>>>>  		return r;
>>>>  	else
>>>>  		return -EWOULDBLOCK;
>>>> --
>>>> 1.7.1
>>> 
>>> --
>>> 			Gleb.
>> 
>> 
>> Best regards,
>> Yang
>> 
> 
> --
> 			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

RE: [PATCH v5 2/6] KVM: Introduce struct rtc_status

[Zhang, Yang Z]

Gleb Natapov wrote on 2013-03-21:
> On Thu, Mar 21, 2013 at 06:49:20PM +0800, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>> ---
>>  virt/kvm/ioapic.h |    8 ++++++++
>>  1 files changed, 8 insertions(+), 0 deletions(-)
>> diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
>> index 2fc61a5..6e5c88f 100644
>> --- a/virt/kvm/ioapic.h
>> +++ b/virt/kvm/ioapic.h
>> @@ -34,6 +34,11 @@ struct kvm_vcpu;
>>  #define	IOAPIC_INIT			0x5
>>  #define	IOAPIC_EXTINT			0x7
>> +struct rtc_status {
>> +	int pending_eoi;
>> +	DECLARE_BITMAP(dest_map, KVM_MAX_VCPUS);
>> +};
>> +
>>  struct kvm_ioapic { 	u64 base_address; 	u32 ioregsel; @@ -47,6 +52,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
> IA64 KVM is almost dead, but we still add CONFIG_X86 everywhere in these
> patches. Lets drop all CONFIG_X86 throughout the patches and instead leave
> only one:
> #ifdef CONFIG_X86
> #define RTC_GSI 8
> else
> #define RTC_GSI 255
> #endif
> 
> Then use RTC_GSI instead of 8 everywhere and the code will be effectively
> disabled on IA64.
Nice idea!

>>  };
>>  
>>  #ifdef DEBUG
>> --
>> 1.7.1
> 
> --
> 			Gleb.


Best regards,
Yang

Re: [PATCH v5 3/6] KVM : Calculate destination vcpu on interrupt injection

[Gleb Natapov]

On Thu, Mar 21, 2013 at 12:12:06PM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2013-03-21:
> > On Thu, Mar 21, 2013 at 11:56:05AM +0000, Zhang, Yang Z wrote:
> >> Gleb Natapov wrote on 2013-03-21:
> >>> On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote:
> >>>> From: Yang Zhang <yang.z.zhang@Intel.com>
> >>>> 
> >>>> Add a new parameter to know vcpus who received the interrupt.
> >>>> 
> >>>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> >>>> ---
> >>>>  arch/x86/kvm/lapic.c |   21 ++++++++++++++++-----
> >>>>  arch/x86/kvm/lapic.h |    5 +++--
> >>>>  virt/kvm/ioapic.c    |    2 +-
> >>>>  virt/kvm/ioapic.h    |    2 +-
> >>>>  virt/kvm/irq_comm.c  |   12 ++++++------
> >>>>  5 files changed, 27 insertions(+), 15 deletions(-)
> >>>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> >>>> index d3e322a..5f6b1d0 100644
> >>>> --- a/arch/x86/kvm/lapic.c
> >>>> +++ b/arch/x86/kvm/lapic.c
> >>>> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu
> >>> *vcpu)
> >>>>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> >>>>  			     int vector, int level, int trig_mode);
> >>>> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
> >>>> +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long
> >>>> *dest_map) +{ +	if (!kvm_lapic_enabled(vcpu)) +		return;
> >>> Why this check here?
> >> The vcpu who didn't enable apic should not account as destination vcpu.
> >> Without this check, if broadcast interrupt, all cpus will treat as
> >> destination vcpu, but only those who enabled apic will receive the
> >> interrupt. There are same check in __apic_accept_irq(): if
> >> (unlikely(!apic_enabled(apic)))
> >>      break;
> > I see, but you use more strict check that also checks that apic is
> > emulated by the kernel and we wouldn't be here if it wasn't. Anyway lets
> Do you mean the check add in here will block "userspace apic"? Shouldn't only in-kernel apic will get here?
> 
No, it will not block. It checks for in kernel apic needlessly. Since we
patch all those checks out anyway using jump labels it is not really
affects performance, but I prefer to make only necessary checks for
consistency.

> > move bitmap update into __apic_accept_irq().
> Sure.
> 

--
			Gleb.

RE: [PATCH v5 3/6] KVM : Calculate destination vcpu on interrupt injection

[Zhang, Yang Z]

Gleb Natapov wrote on 2013-03-21:
> On Thu, Mar 21, 2013 at 12:12:06PM +0000, Zhang, Yang Z wrote:
>> Gleb Natapov wrote on 2013-03-21:
>>> On Thu, Mar 21, 2013 at 11:56:05AM +0000, Zhang, Yang Z wrote:
>>>> Gleb Natapov wrote on 2013-03-21:
>>>>> On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote:
>>>>>> From: Yang Zhang <yang.z.zhang@Intel.com>
>>>>>> 
>>>>>> Add a new parameter to know vcpus who received the interrupt.
>>>>>> 
>>>>>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>>>>>> ---
>>>>>>  arch/x86/kvm/lapic.c |   21 ++++++++++++++++-----
>>>>>>  arch/x86/kvm/lapic.h |    5 +++--
>>>>>>  virt/kvm/ioapic.c    |    2 +-
>>>>>>  virt/kvm/ioapic.h    |    2 +-
>>>>>>  virt/kvm/irq_comm.c  |   12 ++++++------
>>>>>>  5 files changed, 27 insertions(+), 15 deletions(-)
>>>>>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>>>>>> index d3e322a..5f6b1d0 100644
>>>>>> --- a/arch/x86/kvm/lapic.c
>>>>>> +++ b/arch/x86/kvm/lapic.c
>>>>>> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu
>>>>> *vcpu)
>>>>>>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>>>>>>  			     int vector, int level, int trig_mode);
>>>>>> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq
>>>>>> *irq) +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu,
>>>>>> unsigned long *dest_map) +{ +	if (!kvm_lapic_enabled(vcpu))
>>>>>> +		return;
>>>>> Why this check here?
>>>> The vcpu who didn't enable apic should not account as destination vcpu.
>>>> Without this check, if broadcast interrupt, all cpus will treat as
>>>> destination vcpu, but only those who enabled apic will receive the
>>>> interrupt. There are same check in __apic_accept_irq(): if
>>>> (unlikely(!apic_enabled(apic)))
>>>>      break;
>>> I see, but you use more strict check that also checks that apic is
>>> emulated by the kernel and we wouldn't be here if it wasn't. Anyway lets
>> Do you mean the check add in here will block "userspace apic"?
>> Shouldn't only in-kernel apic will get here?
>> 
> No, it will not block. It checks for in kernel apic needlessly. Since we
> patch all those checks out anyway using jump labels it is not really
> affects performance, but I prefer to make only necessary checks for
> consistency.
Make sense.
 
>>> move bitmap update into __apic_accept_irq().
>> Sure.
>> 
> 
> --
> 			Gleb.


Best regards,
Yang

Re: [PATCH v5 4/6] KVM: Add reset/restore rtc_status support

[Marcelo Tosatti]

On Thu, Mar 21, 2013 at 06:49:22PM +0800, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> ---
>  arch/x86/kvm/lapic.c |    8 ++++++++
>  arch/x86/kvm/lapic.h |    2 ++
>  virt/kvm/ioapic.c    |   35 +++++++++++++++++++++++++++++++++++
>  3 files changed, 45 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 5f6b1d0..158e0a3 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 967519c..004d2ad 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -170,4 +170,6 @@ static inline bool kvm_apic_has_events(struct kvm_vcpu *vcpu)
>  	return vcpu->arch.apic->pending_events;
>  }
>  
> +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 4767fa6..8f9c62b 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -87,6 +87,39 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic,
>  	return result;
>  }
>  
> +#ifdef CONFIG_X86
> +static void rtc_irq_reset(struct kvm_ioapic *ioapic)
> +{
> +	ioapic->rtc_status.pending_eoi = 0;
> +	bitmap_zero(ioapic->rtc_status.dest_map, KVM_MAX_VCPUS);
> +}
> +
> +static void rtc_irq_restore(struct kvm_ioapic *ioapic)
> +{
> +	struct kvm_vcpu *vcpu;
> +	int vector, i, pending_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)) {
> +			pending_eoi++;
> +			set_bit(vcpu->vcpu_id, ioapic->rtc_status.dest_map);
> +		}
> +	}
> +	ioapic->rtc_status.pending_eoi = pending_eoi;
> +}

Userspace can load the IOAPIC before loading VCPUS LAPIC via
KVM_SET_LAPIC. So kvm_lapic_reset / kvm_apic_post_state_restore should
also update ioapic->rtc_status.dest_map (checking whether ioapic is
initialized, of course).

Please add a comment explaining why dest_map is necessary (or explain it
in the changelog).

RE: [PATCH v5 4/6] KVM: Add reset/restore rtc_status support

[Zhang, Yang Z]

Marcelo Tosatti wrote on 2013-03-22:
> On Thu, Mar 21, 2013 at 06:49:22PM +0800, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>> ---
>>  arch/x86/kvm/lapic.c |    8 ++++++++
>>  arch/x86/kvm/lapic.h |    2 ++
>>  virt/kvm/ioapic.c    |   35 +++++++++++++++++++++++++++++++++++
>>  3 files changed, 45 insertions(+), 0 deletions(-)
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index 5f6b1d0..158e0a3 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 967519c..004d2ad 100644
>> --- a/arch/x86/kvm/lapic.h
>> +++ b/arch/x86/kvm/lapic.h
>> @@ -170,4 +170,6 @@ static inline bool kvm_apic_has_events(struct
> kvm_vcpu *vcpu)
>>  	return vcpu->arch.apic->pending_events;
>>  }
>> +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 4767fa6..8f9c62b 100644
>> --- a/virt/kvm/ioapic.c
>> +++ b/virt/kvm/ioapic.c
>> @@ -87,6 +87,39 @@ static unsigned long ioapic_read_indirect(struct
> kvm_ioapic *ioapic,
>>  	return result;
>>  }
>> +#ifdef CONFIG_X86
>> +static void rtc_irq_reset(struct kvm_ioapic *ioapic)
>> +{
>> +	ioapic->rtc_status.pending_eoi = 0;
>> +	bitmap_zero(ioapic->rtc_status.dest_map, KVM_MAX_VCPUS);
>> +}
>> +
>> +static void rtc_irq_restore(struct kvm_ioapic *ioapic)
>> +{
>> +	struct kvm_vcpu *vcpu;
>> +	int vector, i, pending_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)) {
>> +			pending_eoi++;
>> +			set_bit(vcpu->vcpu_id, ioapic->rtc_status.dest_map);
>> +		}
>> +	}
>> +	ioapic->rtc_status.pending_eoi = pending_eoi;
>> +}
> 
> Userspace can load the IOAPIC before loading VCPUS LAPIC via
> KVM_SET_LAPIC. So kvm_lapic_reset / kvm_apic_post_state_restore should
> also update ioapic->rtc_status.dest_map (checking whether ioapic is
> initialized, of course).
Yes, this does happen.

>
> Please add a comment explaining why dest_map is necessary (or explain it
> in the changelog).
Sure.

Best regards,
Yang