[PATCH v1 00/22] pic/ioapic/irqchip cleanups + minor fixes

[PATCH v1 00/22] pic/ioapic/irqchip cleanups + minor fixes

[David Hildenbrand]

My attempt to further cleanup pic/ioapic checks using irqchip_mode + other
cleanups in that area.

The goal is to only check against irqchip_mode and not to rely on
vpic/vioapic variables anymore to test for existence of pic/ioapic. This
will avoid any possible races when creating the kernel irqchip fails.

RFC -> v1:
- renamed KVM_IRQCHIP_KERNEL_INIT to KVM_IRQCHIP_INIT_IN_PROGRESS
- set KVM_IRQCHIP_INIT_IN_PROGRESS also during KVM_CAP_SPLIT_IRQCHIP
- don't intoduce another helper to check against KVM_IRQCHIP_NONE, but
  perform the check in kvm_set_routing_entry() explicitly
- changed irqchip_in_kernel() to only perform one check
- added "KVM: x86: use irqchip_kernel() to check for pic+ioapic"

David Hildenbrand (22):
  KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP
  KVM: x86: check against irqchip_mode in kvm_set_routing_entry()
  KVM: x86: check against irqchip_mode in pic_in_kernel()
  KVM: x86: check against irqchip_mode in ioapic_in_kernel()
  KVM: x86: get rid of pic_irqchip()
  KVM: x86: get rid of ioapic_irqchip()
  KVM: x86: use ioapic_in_kernel() to check for ioapic existence
  KVM: x86: remove duplicate checks for ioapic
  KVM: x86: convert kvm_(set|get)_ioapic() into void
  KVM: x86: don't take kvm->irq_lock when creating IRQCHIP
  KVM: x86: push usage of slots_lock down
  KVM: x86: KVM_IRQCHIP_PIC_MASTER only has 8 pins
  KVM: x86: remove all-vcpu request from kvm_ioapic_init()
  KVM: x86: directly call kvm_make_scan_ioapic_request() in ioapic.c
  KVM: x86: rename kvm_vcpu_request_scan_ioapic()
  KVM: x86: drop goto label in kvm_set_routing_entry()
  KVM: x86: simplify pic_unlock()
  KVM: x86: make kvm_pic_reset() static
  KVM: x86: drop picdev_in_range()
  KVM: x86: set data directly in picdev_read()
  KVM: x86: simplify pic_ioport_read()
  KVM: x86: use irqchip_kernel() to check for pic+ioapic

 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/i8259.c            | 70 +++++++++++++++--------------------------
 arch/x86/kvm/ioapic.c           | 28 ++++++-----------
 arch/x86/kvm/ioapic.h           | 14 ++-------
 arch/x86/kvm/irq.c              |  2 +-
 arch/x86/kvm/irq.h              | 14 ++-------
 arch/x86/kvm/irq_comm.c         | 43 +++++++++++--------------
 arch/x86/kvm/x86.c              | 41 +++++++++++-------------
 include/linux/kvm_host.h        |  4 +--
 virt/kvm/eventfd.c              |  4 +--
 virt/kvm/kvm_main.c             |  3 ++
 11 files changed, 87 insertions(+), 137 deletions(-)

-- 
2.9.3

[PATCH v1 01/22] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP

[David Hildenbrand]

Avoid races between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP by taking
the kvm->lock when setting up routes.

If KVM_CREATE_IRQCHIP fails, KVM_SET_GSI_ROUTING could have already set
up routes pointing at pic/ioapic, being silently removed already.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 virt/kvm/kvm_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index a17d787..ad0f8b2 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3079,8 +3079,11 @@ static long kvm_vm_ioctl(struct file *filp,
 					   routing.nr * sizeof(*entries)))
 				goto out_free_irq_routing;
 		}
+		/* avoid races with KVM_CREATE_IRQCHIP on x86 */
+		mutex_lock(&kvm->lock);
 		r = kvm_set_irq_routing(kvm, entries, routing.nr,
 					routing.flags);
+		mutex_unlock(&kvm->lock);
 out_free_irq_routing:
 		vfree(entries);
 		break;
-- 
2.9.3

[PATCH v1 02/22] KVM: x86: check against irqchip_mode in kvm_set_routing_entry()

[David Hildenbrand]

Let's replace the checks for pic_in_kernel() and ioapic_in_kernel() by
checks against irqchip_mode. Add another state, indicating that the
caller is currently initializing the irqchip.

This is necessary to switch pic_in_kernel() and ioapic_in_kernel() to
irqchip_mode, too.

Also make sure that creation of any route is only possible if we have
a lapic in kernel (irqchip_in_kernel()) or if we are currently
inititalizing the irqchip.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/irq.h              |  2 +-
 arch/x86/kvm/irq_comm.c         | 12 ++++++------
 arch/x86/kvm/x86.c              |  7 ++++++-
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 74ef58c..164e544 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -727,6 +727,7 @@ struct kvm_hv {
 
 enum kvm_irqchip_mode {
 	KVM_IRQCHIP_NONE,
+	KVM_IRQCHIP_INIT_IN_PROGRESS, /* temporarily set during creation */
 	KVM_IRQCHIP_KERNEL,       /* created with KVM_CREATE_IRQCHIP */
 	KVM_IRQCHIP_SPLIT,        /* created with KVM_CAP_SPLIT_IRQCHIP */
 };
diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 40d5b2c..7fa2480 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -103,7 +103,7 @@ static inline int irqchip_kernel(struct kvm *kvm)
 
 static inline int irqchip_in_kernel(struct kvm *kvm)
 {
-	bool ret = kvm->arch.irqchip_mode != KVM_IRQCHIP_NONE;
+	bool ret = kvm->arch.irqchip_mode > KVM_IRQCHIP_INIT_IN_PROGRESS;
 
 	/* Matches with wmb after initializing kvm->irq_routing. */
 	smp_rmb();
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 6825cd3..0a026f8 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -282,24 +282,24 @@ int kvm_set_routing_entry(struct kvm *kvm,
 	int delta;
 	unsigned max_pin;
 
+	/* also allow creation of routes during KVM_IRQCHIP_INIT_IN_PROGRESS */
+	if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE)
+		goto out;
+
 	switch (ue->type) {
 	case KVM_IRQ_ROUTING_IRQCHIP:
+		if (irqchip_split(kvm))
+			goto out;
 		delta = 0;
 		switch (ue->u.irqchip.irqchip) {
 		case KVM_IRQCHIP_PIC_SLAVE:
 			delta = 8;
 			/* fall through */
 		case KVM_IRQCHIP_PIC_MASTER:
-			if (!pic_in_kernel(kvm))
-				goto out;
-
 			e->set = kvm_set_pic_irq;
 			max_pin = PIC_NUM_PINS;
 			break;
 		case KVM_IRQCHIP_IOAPIC:
-			if (!ioapic_in_kernel(kvm))
-				goto out;
-
 			max_pin = KVM_IOAPIC_NUM_PINS;
 			e->set = kvm_set_ioapic_irq;
 			break;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1faf620..fa8cceb 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3934,9 +3934,12 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 			goto split_irqchip_unlock;
 		if (kvm->created_vcpus)
 			goto split_irqchip_unlock;
+		kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
 		r = kvm_setup_empty_irq_routing(kvm);
-		if (r)
+		if (r) {
+			kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
 			goto split_irqchip_unlock;
+		}
 		/* Pairs with irqchip_in_kernel. */
 		smp_wmb();
 		kvm->arch.irqchip_mode = KVM_IRQCHIP_SPLIT;
@@ -4024,8 +4027,10 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto create_irqchip_unlock;
 		}
 
+		kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
 		r = kvm_setup_default_irq_routing(kvm);
 		if (r) {
+			kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
 			mutex_lock(&kvm->slots_lock);
 			mutex_lock(&kvm->irq_lock);
 			kvm_ioapic_destroy(kvm);
-- 
2.9.3

[PATCH v1 03/22] KVM: x86: check against irqchip_mode in pic_in_kernel()

[David Hildenbrand]

Let's avoid checking against kvm->arch.vpic. We have kvm->arch.irqchip_mode
for that now.

KVM_IRQCHIP_KERNEL implies a fully inititalized pic, while kvm->arch.vpic
might temporarily be set but invalidated again if e.g. kvm_ioapic_init()
fails when setting KVM_CREATE_IRQCHIP. Although current users seem to be
fine, this avoids future bugs.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/irq.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 7fa2480..184306e 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -85,10 +85,7 @@ static inline struct kvm_pic *pic_irqchip(struct kvm *kvm)
 
 static inline int pic_in_kernel(struct kvm *kvm)
 {
-	int ret;
-
-	ret = (pic_irqchip(kvm) != NULL);
-	return ret;
+	return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL;
 }
 
 static inline int irqchip_split(struct kvm *kvm)
-- 
2.9.3

[PATCH v1 04/22] KVM: x86: check against irqchip_mode in ioapic_in_kernel()

[David Hildenbrand]

KVM_IRQCHIP_KERNEL implies a fully inititalized ioapic, while
kvm->arch.vioapic might temporarily be set but invalidated again if e.g.
setting of default routing fails when setting KVM_CREATE_IRQCHIP.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/ioapic.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
index 1cc6e54..9a62fe1 100644
--- a/arch/x86/kvm/ioapic.h
+++ b/arch/x86/kvm/ioapic.h
@@ -112,10 +112,7 @@ static inline struct kvm_ioapic *ioapic_irqchip(struct kvm *kvm)
 
 static inline int ioapic_in_kernel(struct kvm *kvm)
 {
-	int ret;
-
-	ret = (ioapic_irqchip(kvm) != NULL);
-	return ret;
+	return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL;
 }
 
 void kvm_rtc_eoi_tracking_restore_one(struct kvm_vcpu *vcpu);
-- 
2.9.3

[PATCH v1 05/22] KVM: x86: get rid of pic_irqchip()

[David Hildenbrand]

It seemed like a nice idea to encapsulate access to kvm->arch.vpic. But
as the usage is already mixed, internal locks are taken outside of i8259.c
and grepping for "vpic" only is much easier, let's just get rid of
pic_irqchip().

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/i8259.c    |  4 ++--
 arch/x86/kvm/irq.c      |  2 +-
 arch/x86/kvm/irq.h      |  5 -----
 arch/x86/kvm/irq_comm.c |  4 ++--
 arch/x86/kvm/x86.c      | 24 +++++++++++-------------
 5 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index 73ea24d..64e4b92 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -239,7 +239,7 @@ static inline void pic_intack(struct kvm_kpic_state *s, int irq)
 int kvm_pic_read_irq(struct kvm *kvm)
 {
 	int irq, irq2, intno;
-	struct kvm_pic *s = pic_irqchip(kvm);
+	struct kvm_pic *s = kvm->arch.vpic;
 
 	s->output = 0;
 
@@ -576,7 +576,7 @@ static int picdev_eclr_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
  */
 static void pic_irq_request(struct kvm *kvm, int level)
 {
-	struct kvm_pic *s = pic_irqchip(kvm);
+	struct kvm_pic *s = kvm->arch.vpic;
 
 	if (!s->output)
 		s->wakeup_needed = true;
diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c
index 60d91c9..5c24811 100644
--- a/arch/x86/kvm/irq.c
+++ b/arch/x86/kvm/irq.c
@@ -60,7 +60,7 @@ static int kvm_cpu_has_extint(struct kvm_vcpu *v)
 		if (irqchip_split(v->kvm))
 			return pending_userspace_extint(v);
 		else
-			return pic_irqchip(v->kvm)->output;
+			return v->kvm->arch.vpic->output;
 	} else
 		return 0;
 }
diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 184306e..2455f16 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -78,11 +78,6 @@ void kvm_pic_destroy(struct kvm *kvm);
 int kvm_pic_read_irq(struct kvm *kvm);
 void kvm_pic_update_irq(struct kvm_pic *s);
 
-static inline struct kvm_pic *pic_irqchip(struct kvm *kvm)
-{
-	return kvm->arch.vpic;
-}
-
 static inline int pic_in_kernel(struct kvm *kvm)
 {
 	return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL;
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 0a026f8..45ab43e 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -42,7 +42,7 @@ static int kvm_set_pic_irq(struct kvm_kernel_irq_routing_entry *e,
 			   struct kvm *kvm, int irq_source_id, int level,
 			   bool line_status)
 {
-	struct kvm_pic *pic = pic_irqchip(kvm);
+	struct kvm_pic *pic = kvm->arch.vpic;
 	return kvm_pic_set_irq(pic, e->irqchip.pin, irq_source_id, level);
 }
 
@@ -236,7 +236,7 @@ void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id)
 		goto unlock;
 
 	kvm_ioapic_clear_all(kvm->arch.vioapic, irq_source_id);
-	kvm_pic_clear_all(pic_irqchip(kvm), irq_source_id);
+	kvm_pic_clear_all(kvm->arch.vpic, irq_source_id);
 unlock:
 	mutex_unlock(&kvm->irq_lock);
 }
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fa8cceb..079bf6b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3721,18 +3721,17 @@ static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm)
 
 static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
 {
+	struct kvm_pic *pic = kvm->arch.vpic;
 	int r;
 
 	r = 0;
 	switch (chip->chip_id) {
 	case KVM_IRQCHIP_PIC_MASTER:
-		memcpy(&chip->chip.pic,
-			&pic_irqchip(kvm)->pics[0],
+		memcpy(&chip->chip.pic, &pic->pics[0],
 			sizeof(struct kvm_pic_state));
 		break;
 	case KVM_IRQCHIP_PIC_SLAVE:
-		memcpy(&chip->chip.pic,
-			&pic_irqchip(kvm)->pics[1],
+		memcpy(&chip->chip.pic, &pic->pics[1],
 			sizeof(struct kvm_pic_state));
 		break;
 	case KVM_IRQCHIP_IOAPIC:
@@ -3747,23 +3746,22 @@ static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
 
 static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
 {
+	struct kvm_pic *pic = kvm->arch.vpic;
 	int r;
 
 	r = 0;
 	switch (chip->chip_id) {
 	case KVM_IRQCHIP_PIC_MASTER:
-		spin_lock(&pic_irqchip(kvm)->lock);
-		memcpy(&pic_irqchip(kvm)->pics[0],
-			&chip->chip.pic,
+		spin_lock(&pic->lock);
+		memcpy(&pic->pics[0], &chip->chip.pic,
 			sizeof(struct kvm_pic_state));
-		spin_unlock(&pic_irqchip(kvm)->lock);
+		spin_unlock(&pic->lock);
 		break;
 	case KVM_IRQCHIP_PIC_SLAVE:
-		spin_lock(&pic_irqchip(kvm)->lock);
-		memcpy(&pic_irqchip(kvm)->pics[1],
-			&chip->chip.pic,
+		spin_lock(&pic->lock);
+		memcpy(&pic->pics[1], &chip->chip.pic,
 			sizeof(struct kvm_pic_state));
-		spin_unlock(&pic_irqchip(kvm)->lock);
+		spin_unlock(&pic->lock);
 		break;
 	case KVM_IRQCHIP_IOAPIC:
 		r = kvm_set_ioapic(kvm, &chip->chip.ioapic);
@@ -3772,7 +3770,7 @@ static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
 		r = -EINVAL;
 		break;
 	}
-	kvm_pic_update_irq(pic_irqchip(kvm));
+	kvm_pic_update_irq(pic);
 	return r;
 }
 
-- 
2.9.3

[PATCH v1 06/22] KVM: x86: get rid of ioapic_irqchip()

[David Hildenbrand]

Let's just use kvm->arch.vioapic directly.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/ioapic.c | 4 ++--
 arch/x86/kvm/ioapic.h | 5 -----
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 6e219e5..3d9dc98 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -643,7 +643,7 @@ void kvm_ioapic_destroy(struct kvm *kvm)
 
 int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
 {
-	struct kvm_ioapic *ioapic = ioapic_irqchip(kvm);
+	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
 	if (!ioapic)
 		return -EINVAL;
 
@@ -656,7 +656,7 @@ int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
 
 int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
 {
-	struct kvm_ioapic *ioapic = ioapic_irqchip(kvm);
+	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
 	if (!ioapic)
 		return -EINVAL;
 
diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
index 9a62fe1..717d98a 100644
--- a/arch/x86/kvm/ioapic.h
+++ b/arch/x86/kvm/ioapic.h
@@ -105,11 +105,6 @@ do {									\
 #define ASSERT(x) do { } while (0)
 #endif
 
-static inline struct kvm_ioapic *ioapic_irqchip(struct kvm *kvm)
-{
-	return kvm->arch.vioapic;
-}
-
 static inline int ioapic_in_kernel(struct kvm *kvm)
 {
 	return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL;
-- 
2.9.3

[PATCH v1 07/22] KVM: x86: use ioapic_in_kernel() to check for ioapic existence

[David Hildenbrand]

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/ioapic.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 3d9dc98..4fd234d 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -268,9 +268,7 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, ulong *ioapic_handled_vectors)
 
 void kvm_vcpu_request_scan_ioapic(struct kvm *kvm)
 {
-	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
-
-	if (!ioapic)
+	if (!ioapic_in_kernel(kvm))
 		return;
 	kvm_make_scan_ioapic_request(kvm);
 }
-- 
2.9.3

[PATCH v1 08/22] KVM: x86: remove duplicate checks for ioapic

[David Hildenbrand]

When handling KVM_GET_IRQCHIP, we already check irqchip_kernel(), which
implies a fully inititalized ioapic.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/ioapic.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 4fd234d..9b2a7f0 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -642,8 +642,6 @@ void kvm_ioapic_destroy(struct kvm *kvm)
 int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
 {
 	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
-	if (!ioapic)
-		return -EINVAL;
 
 	spin_lock(&ioapic->lock);
 	memcpy(state, ioapic, sizeof(struct kvm_ioapic_state));
@@ -655,8 +653,6 @@ int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
 int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
 {
 	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
-	if (!ioapic)
-		return -EINVAL;
 
 	spin_lock(&ioapic->lock);
 	memcpy(ioapic, state, sizeof(struct kvm_ioapic_state));
-- 
2.9.3

[PATCH v1 09/22] KVM: x86: convert kvm_(set|get)_ioapic() into void

[David Hildenbrand]

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/ioapic.c | 6 ++----
 arch/x86/kvm/ioapic.h | 4 ++--
 arch/x86/kvm/x86.c    | 4 ++--
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 9b2a7f0..427e5f9 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -639,7 +639,7 @@ void kvm_ioapic_destroy(struct kvm *kvm)
 	kfree(ioapic);
 }
 
-int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
+void kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
 {
 	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
 
@@ -647,10 +647,9 @@ int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
 	memcpy(state, ioapic, sizeof(struct kvm_ioapic_state));
 	state->irr &= ~ioapic->irr_delivered;
 	spin_unlock(&ioapic->lock);
-	return 0;
 }
 
-int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
+void kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
 {
 	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
 
@@ -661,5 +660,4 @@ int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
 	kvm_vcpu_request_scan_ioapic(kvm);
 	kvm_ioapic_inject_all(ioapic, state->irr);
 	spin_unlock(&ioapic->lock);
-	return 0;
 }
diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
index 717d98a..3eb140b 100644
--- a/arch/x86/kvm/ioapic.h
+++ b/arch/x86/kvm/ioapic.h
@@ -124,8 +124,8 @@ void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
 int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 			     struct kvm_lapic_irq *irq,
 			     struct dest_map *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_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
+void kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
 void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu,
 			   ulong *ioapic_handled_vectors);
 void kvm_scan_ioapic_routes(struct kvm_vcpu *vcpu,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 079bf6b..1636da0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3735,7 +3735,7 @@ static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
 			sizeof(struct kvm_pic_state));
 		break;
 	case KVM_IRQCHIP_IOAPIC:
-		r = kvm_get_ioapic(kvm, &chip->chip.ioapic);
+		kvm_get_ioapic(kvm, &chip->chip.ioapic);
 		break;
 	default:
 		r = -EINVAL;
@@ -3764,7 +3764,7 @@ static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
 		spin_unlock(&pic->lock);
 		break;
 	case KVM_IRQCHIP_IOAPIC:
-		r = kvm_set_ioapic(kvm, &chip->chip.ioapic);
+		kvm_set_ioapic(kvm, &chip->chip.ioapic);
 		break;
 	default:
 		r = -EINVAL;
-- 
2.9.3

[PATCH v1 10/22] KVM: x86: don't take kvm->irq_lock when creating IRQCHIP

[David Hildenbrand]

I don't see any reason any more for this lock, seemed to be used to protect
removal of kvm->arch.vpic / kvm->arch.vioapic when already partially
inititalized, now access is properly protected using kvm->arch.irqchip_mode
and this shouldn't be necessary anymore.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/x86.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1636da0..db1a5ff 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4030,10 +4030,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 		if (r) {
 			kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
 			mutex_lock(&kvm->slots_lock);
-			mutex_lock(&kvm->irq_lock);
 			kvm_ioapic_destroy(kvm);
 			kvm_pic_destroy(kvm);
-			mutex_unlock(&kvm->irq_lock);
 			mutex_unlock(&kvm->slots_lock);
 			goto create_irqchip_unlock;
 		}
-- 
2.9.3

[PATCH v1 11/22] KVM: x86: push usage of slots_lock down

[David Hildenbrand]

Let's just move it to the place where it is actually needed.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/i8259.c  | 2 ++
 arch/x86/kvm/ioapic.c | 2 ++
 arch/x86/kvm/x86.c    | 4 ----
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index 64e4b92..e77e8c3 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -657,9 +657,11 @@ void kvm_pic_destroy(struct kvm *kvm)
 {
 	struct kvm_pic *vpic = kvm->arch.vpic;
 
+	mutex_lock(&kvm->slots_lock);
 	kvm_io_bus_unregister_dev(vpic->kvm, KVM_PIO_BUS, &vpic->dev_master);
 	kvm_io_bus_unregister_dev(vpic->kvm, KVM_PIO_BUS, &vpic->dev_slave);
 	kvm_io_bus_unregister_dev(vpic->kvm, KVM_PIO_BUS, &vpic->dev_eclr);
+	mutex_unlock(&kvm->slots_lock);
 
 	kvm->arch.vpic = NULL;
 	kfree(vpic);
diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 427e5f9..1a30320 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -634,7 +634,9 @@ void kvm_ioapic_destroy(struct kvm *kvm)
 	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
 
 	cancel_delayed_work_sync(&ioapic->eoi_inject);
+	mutex_lock(&kvm->slots_lock);
 	kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, &ioapic->dev);
+	mutex_unlock(&kvm->slots_lock);
 	kvm->arch.vioapic = NULL;
 	kfree(ioapic);
 }
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index db1a5ff..8d6418e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4019,9 +4019,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 
 		r = kvm_ioapic_init(kvm);
 		if (r) {
-			mutex_lock(&kvm->slots_lock);
 			kvm_pic_destroy(kvm);
-			mutex_unlock(&kvm->slots_lock);
 			goto create_irqchip_unlock;
 		}
 
@@ -4029,10 +4027,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 		r = kvm_setup_default_irq_routing(kvm);
 		if (r) {
 			kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
-			mutex_lock(&kvm->slots_lock);
 			kvm_ioapic_destroy(kvm);
 			kvm_pic_destroy(kvm);
-			mutex_unlock(&kvm->slots_lock);
 			goto create_irqchip_unlock;
 		}
 		/* Write kvm->irq_routing before enabling irqchip_in_kernel. */
-- 
2.9.3

[PATCH v1 12/22] KVM: x86: KVM_IRQCHIP_PIC_MASTER only has 8 pins

[David Hildenbrand]

Currently, one could set pin 8-15, implicitly referring to
KVM_IRQCHIP_PIC_SLAVE.

Get rid of the two local variables max_pin and delta on the way.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/irq_comm.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 45ab43e..34acbec 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -279,8 +279,6 @@ int kvm_set_routing_entry(struct kvm *kvm,
 			  const struct kvm_irq_routing_entry *ue)
 {
 	int r = -EINVAL;
-	int delta;
-	unsigned max_pin;
 
 	/* also allow creation of routes during KVM_IRQCHIP_INIT_IN_PROGRESS */
 	if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE)
@@ -290,26 +288,25 @@ int kvm_set_routing_entry(struct kvm *kvm,
 	case KVM_IRQ_ROUTING_IRQCHIP:
 		if (irqchip_split(kvm))
 			goto out;
-		delta = 0;
+		e->irqchip.pin = ue->u.irqchip.pin;
 		switch (ue->u.irqchip.irqchip) {
 		case KVM_IRQCHIP_PIC_SLAVE:
-			delta = 8;
+			e->irqchip.pin += PIC_NUM_PINS / 2;
 			/* fall through */
 		case KVM_IRQCHIP_PIC_MASTER:
+			if (ue->u.irqchip.pin >= PIC_NUM_PINS / 2)
+				goto out;
 			e->set = kvm_set_pic_irq;
-			max_pin = PIC_NUM_PINS;
 			break;
 		case KVM_IRQCHIP_IOAPIC:
-			max_pin = KVM_IOAPIC_NUM_PINS;
+			if (ue->u.irqchip.pin >= KVM_IOAPIC_NUM_PINS)
+				goto out;
 			e->set = kvm_set_ioapic_irq;
 			break;
 		default:
 			goto out;
 		}
 		e->irqchip.irqchip = ue->u.irqchip.irqchip;
-		e->irqchip.pin = ue->u.irqchip.pin + delta;
-		if (e->irqchip.pin >= max_pin)
-			goto out;
 		break;
 	case KVM_IRQ_ROUTING_MSI:
 		e->set = kvm_set_msi;
-- 
2.9.3

[PATCH v1 13/22] KVM: x86: remove all-vcpu request from kvm_ioapic_init()

[David Hildenbrand]

kvm_ioapic_init() is guaranteed to be called without any created VCPUs,
so doing an all-vcpu request results in a NOP.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/ioapic.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 1a30320..0eb7847 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -622,10 +622,8 @@ int kvm_ioapic_init(struct kvm *kvm)
 	if (ret < 0) {
 		kvm->arch.vioapic = NULL;
 		kfree(ioapic);
-		return ret;
 	}
 
-	kvm_vcpu_request_scan_ioapic(kvm);
 	return ret;
 }
 
-- 
2.9.3

[PATCH v1 14/22] KVM: x86: directly call kvm_make_scan_ioapic_request() in ioapic.c

[David Hildenbrand]

We know there is an ioapic, so let's call it directly.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/ioapic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 0eb7847..d1c30c0 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -313,7 +313,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, false);
-		kvm_vcpu_request_scan_ioapic(ioapic->kvm);
+		kvm_make_scan_ioapic_request(ioapic->kvm);
 		break;
 	}
 }
@@ -657,7 +657,7 @@ void kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
 	memcpy(ioapic, state, sizeof(struct kvm_ioapic_state));
 	ioapic->irr = 0;
 	ioapic->irr_delivered = 0;
-	kvm_vcpu_request_scan_ioapic(kvm);
+	kvm_make_scan_ioapic_request(kvm);
 	kvm_ioapic_inject_all(ioapic, state->irr);
 	spin_unlock(&ioapic->lock);
 }
-- 
2.9.3

[PATCH v1 15/22] KVM: x86: rename kvm_vcpu_request_scan_ioapic()

[David Hildenbrand]

Let's rename it into a proper arch specific callback.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/ioapic.c    | 2 +-
 include/linux/kvm_host.h | 4 ++--
 virt/kvm/eventfd.c       | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index d1c30c0..58c7993 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -266,7 +266,7 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, ulong *ioapic_handled_vectors)
 	spin_unlock(&ioapic->lock);
 }
 
-void kvm_vcpu_request_scan_ioapic(struct kvm *kvm)
+void kvm_arch_post_irq_ack_notifier_list_update(struct kvm *kvm)
 {
 	if (!ioapic_in_kernel(kvm))
 		return;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 2c14ad9..7aa2f9b 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -502,10 +502,10 @@ int __must_check vcpu_load(struct kvm_vcpu *vcpu);
 void vcpu_put(struct kvm_vcpu *vcpu);
 
 #ifdef __KVM_HAVE_IOAPIC
-void kvm_vcpu_request_scan_ioapic(struct kvm *kvm);
+void kvm_arch_post_irq_ack_notifier_list_update(struct kvm *kvm);
 void kvm_arch_post_irq_routing_update(struct kvm *kvm);
 #else
-static inline void kvm_vcpu_request_scan_ioapic(struct kvm *kvm)
+static inline void kvm_arch_post_irq_ack_notifier_list_update(struct kvm *kvm)
 {
 }
 static inline void kvm_arch_post_irq_routing_update(struct kvm *kvm)
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index a29786d..c2fbecb 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -490,7 +490,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_vcpu_request_scan_ioapic(kvm);
+	kvm_arch_post_irq_ack_notifier_list_update(kvm);
 }
 
 void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
@@ -500,7 +500,7 @@ void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 	hlist_del_init_rcu(&kian->link);
 	mutex_unlock(&kvm->irq_lock);
 	synchronize_srcu(&kvm->irq_srcu);
-	kvm_vcpu_request_scan_ioapic(kvm);
+	kvm_arch_post_irq_ack_notifier_list_update(kvm);
 }
 #endif
 
-- 
2.9.3

[PATCH v1 16/22] KVM: x86: drop goto label in kvm_set_routing_entry()

[David Hildenbrand]

No need for the goto label + local variable "r".

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/irq_comm.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 34acbec..1728ff1 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -278,16 +278,14 @@ int kvm_set_routing_entry(struct kvm *kvm,
 			  struct kvm_kernel_irq_routing_entry *e,
 			  const struct kvm_irq_routing_entry *ue)
 {
-	int r = -EINVAL;
-
 	/* also allow creation of routes during KVM_IRQCHIP_INIT_IN_PROGRESS */
 	if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE)
-		goto out;
+		return -EINVAL;
 
 	switch (ue->type) {
 	case KVM_IRQ_ROUTING_IRQCHIP:
 		if (irqchip_split(kvm))
-			goto out;
+			return -EINVAL;
 		e->irqchip.pin = ue->u.irqchip.pin;
 		switch (ue->u.irqchip.irqchip) {
 		case KVM_IRQCHIP_PIC_SLAVE:
@@ -295,16 +293,16 @@ int kvm_set_routing_entry(struct kvm *kvm,
 			/* fall through */
 		case KVM_IRQCHIP_PIC_MASTER:
 			if (ue->u.irqchip.pin >= PIC_NUM_PINS / 2)
-				goto out;
+				return -EINVAL;
 			e->set = kvm_set_pic_irq;
 			break;
 		case KVM_IRQCHIP_IOAPIC:
 			if (ue->u.irqchip.pin >= KVM_IOAPIC_NUM_PINS)
-				goto out;
+				return -EINVAL;
 			e->set = kvm_set_ioapic_irq;
 			break;
 		default:
-			goto out;
+			return -EINVAL;
 		}
 		e->irqchip.irqchip = ue->u.irqchip.irqchip;
 		break;
@@ -315,7 +313,7 @@ int kvm_set_routing_entry(struct kvm *kvm,
 		e->msi.data = ue->u.msi.data;
 
 		if (kvm_msi_route_invalid(kvm, e))
-			goto out;
+			return -EINVAL;
 		break;
 	case KVM_IRQ_ROUTING_HV_SINT:
 		e->set = kvm_hv_set_sint;
@@ -323,12 +321,10 @@ int kvm_set_routing_entry(struct kvm *kvm,
 		e->hv_sint.sint = ue->u.hv_sint.sint;
 		break;
 	default:
-		goto out;
+		return -EINVAL;
 	}
 
-	r = 0;
-out:
-	return r;
+	return 0;
 }
 
 bool kvm_intr_is_single_vcpu(struct kvm *kvm, struct kvm_lapic_irq *irq,
-- 
2.9.3

[PATCH v1 17/22] KVM: x86: simplify pic_unlock()

[David Hildenbrand]

We can easily compact this code and get rid of one local variable.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/i8259.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index e77e8c3..c50b839 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -49,7 +49,7 @@ static void pic_unlock(struct kvm_pic *s)
 	__releases(&s->lock)
 {
 	bool wakeup = s->wakeup_needed;
-	struct kvm_vcpu *vcpu, *found = NULL;
+	struct kvm_vcpu *vcpu;
 	int i;
 
 	s->wakeup_needed = false;
@@ -59,16 +59,11 @@ static void pic_unlock(struct kvm_pic *s)
 	if (wakeup) {
 		kvm_for_each_vcpu(i, vcpu, s->kvm) {
 			if (kvm_apic_accept_pic_intr(vcpu)) {
-				found = vcpu;
+				kvm_make_request(KVM_REQ_EVENT, vcpu);
+				kvm_vcpu_kick(vcpu);
 				break;
 			}
 		}
-
-		if (!found)
-			return;
-
-		kvm_make_request(KVM_REQ_EVENT, found);
-		kvm_vcpu_kick(found);
 	}
 }
 
-- 
2.9.3

[PATCH v1 18/22] KVM: x86: make kvm_pic_reset() static

[David Hildenbrand]

Not used outside of i8259.c, so let's make it static.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/i8259.c | 2 +-
 arch/x86/kvm/irq.h   | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index c50b839..c29a7e1 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -268,7 +268,7 @@ int kvm_pic_read_irq(struct kvm *kvm)
 	return intno;
 }
 
-void kvm_pic_reset(struct kvm_kpic_state *s)
+static void kvm_pic_reset(struct kvm_kpic_state *s)
 {
 	int irq, i;
 	struct kvm_vcpu *vcpu;
diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 2455f16..ae29332 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -102,8 +102,6 @@ static inline int irqchip_in_kernel(struct kvm *kvm)
 	return ret;
 }
 
-void kvm_pic_reset(struct kvm_kpic_state *s);
-
 void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
 void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu);
 void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu);
-- 
2.9.3

[PATCH v1 19/22] KVM: x86: drop picdev_in_range()

[David Hildenbrand]

We already have the exact same checks a couple of lines below.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/i8259.c | 35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index c29a7e1..53c4c6a 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -451,46 +451,33 @@ static u32 elcr_ioport_read(void *opaque, u32 addr1)
 	return s->elcr;
 }
 
-static int picdev_in_range(gpa_t addr)
-{
-	switch (addr) {
-	case 0x20:
-	case 0x21:
-	case 0xa0:
-	case 0xa1:
-	case 0x4d0:
-	case 0x4d1:
-		return 1;
-	default:
-		return 0;
-	}
-}
-
 static int picdev_write(struct kvm_pic *s,
 			 gpa_t addr, int len, const void *val)
 {
 	unsigned char data = *(unsigned char *)val;
-	if (!picdev_in_range(addr))
-		return -EOPNOTSUPP;
 
 	if (len != 1) {
 		pr_pic_unimpl("non byte write\n");
 		return 0;
 	}
-	pic_lock(s);
 	switch (addr) {
 	case 0x20:
 	case 0x21:
 	case 0xa0:
 	case 0xa1:
+		pic_lock(s);
 		pic_ioport_write(&s->pics[addr >> 7], addr, data);
+		pic_unlock(s);
 		break;
 	case 0x4d0:
 	case 0x4d1:
+		pic_lock(s);
 		elcr_ioport_write(&s->pics[addr & 1], addr, data);
+		pic_unlock(s);
 		break;
+	default:
+		return -EOPNOTSUPP;
 	}
-	pic_unlock(s);
 	return 0;
 }
 
@@ -498,29 +485,31 @@ static int picdev_read(struct kvm_pic *s,
 		       gpa_t addr, int len, void *val)
 {
 	unsigned char data = 0;
-	if (!picdev_in_range(addr))
-		return -EOPNOTSUPP;
 
 	if (len != 1) {
 		memset(val, 0, len);
 		pr_pic_unimpl("non byte read\n");
 		return 0;
 	}
-	pic_lock(s);
 	switch (addr) {
 	case 0x20:
 	case 0x21:
 	case 0xa0:
 	case 0xa1:
+		pic_lock(s);
 		data = pic_ioport_read(&s->pics[addr >> 7], addr);
+		pic_unlock(s);
 		break;
 	case 0x4d0:
 	case 0x4d1:
+		pic_lock(s);
 		data = elcr_ioport_read(&s->pics[addr & 1], addr);
+		pic_unlock(s);
 		break;
+	default:
+		return -EOPNOTSUPP;
 	}
 	*(unsigned char *)val = data;
-	pic_unlock(s);
 	return 0;
 }
 
-- 
2.9.3

[PATCH v1 20/22] KVM: x86: set data directly in picdev_read()

[David Hildenbrand]

Now it looks almost as picdev_write().

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/i8259.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index 53c4c6a..52d21f2 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -484,7 +484,7 @@ static int picdev_write(struct kvm_pic *s,
 static int picdev_read(struct kvm_pic *s,
 		       gpa_t addr, int len, void *val)
 {
-	unsigned char data = 0;
+	unsigned char *data = (unsigned char *)val;
 
 	if (len != 1) {
 		memset(val, 0, len);
@@ -497,19 +497,18 @@ static int picdev_read(struct kvm_pic *s,
 	case 0xa0:
 	case 0xa1:
 		pic_lock(s);
-		data = pic_ioport_read(&s->pics[addr >> 7], addr);
+		*data = pic_ioport_read(&s->pics[addr >> 7], addr);
 		pic_unlock(s);
 		break;
 	case 0x4d0:
 	case 0x4d1:
 		pic_lock(s);
-		data = elcr_ioport_read(&s->pics[addr & 1], addr);
+		*data = elcr_ioport_read(&s->pics[addr & 1], addr);
 		pic_unlock(s);
 		break;
 	default:
 		return -EOPNOTSUPP;
 	}
-	*(unsigned char *)val = data;
 	return 0;
 }
 
-- 
2.9.3

[PATCH v1 21/22] KVM: x86: simplify pic_ioport_read()

[David Hildenbrand]

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/i8259.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index 52d21f2..1abd110 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -417,19 +417,16 @@ static u32 pic_poll_read(struct kvm_kpic_state *s, u32 addr1)
 	return ret;
 }
 
-static u32 pic_ioport_read(void *opaque, u32 addr1)
+static u32 pic_ioport_read(void *opaque, u32 addr)
 {
 	struct kvm_kpic_state *s = opaque;
-	unsigned int addr;
 	int ret;
 
-	addr = addr1;
-	addr &= 1;
 	if (s->poll) {
-		ret = pic_poll_read(s, addr1);
+		ret = pic_poll_read(s, addr);
 		s->poll = 0;
 	} else
-		if (addr == 0)
+		if ((addr & 1) == 0)
 			if (s->read_reg_select)
 				ret = s->isr;
 			else
-- 
2.9.3

[PATCH v1 22/22] KVM: x86: use irqchip_kernel() to check for pic+ioapic

[David Hildenbrand]

Although the current check is not wrong, this check explicitly includes
the pic.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/irq_comm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 1728ff1..5494395 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -232,7 +232,7 @@ void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id)
 		goto unlock;
 	}
 	clear_bit(irq_source_id, &kvm->arch.irq_sources_bitmap);
-	if (!ioapic_in_kernel(kvm))
+	if (!irqchip_kernel(kvm))
 		goto unlock;
 
 	kvm_ioapic_clear_all(kvm->arch.vioapic, irq_source_id);
-- 
2.9.3

Re: [PATCH v1 01/22] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP

[Peter Xu]

On Tue, Mar 14, 2017 at 02:34:29PM +0100, David Hildenbrand wrote:
> Avoid races between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP by taking
> the kvm->lock when setting up routes.
> 
> If KVM_CREATE_IRQCHIP fails, KVM_SET_GSI_ROUTING could have already set
> up routes pointing at pic/ioapic, being silently removed already.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  virt/kvm/kvm_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index a17d787..ad0f8b2 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3079,8 +3079,11 @@ static long kvm_vm_ioctl(struct file *filp,
>  					   routing.nr * sizeof(*entries)))
>  				goto out_free_irq_routing;
>  		}
> +		/* avoid races with KVM_CREATE_IRQCHIP on x86 */
> +		mutex_lock(&kvm->lock);
>  		r = kvm_set_irq_routing(kvm, entries, routing.nr,
>  					routing.flags);
> +		mutex_unlock(&kvm->lock);
>  out_free_irq_routing:
>  		vfree(entries);
>  		break;
> -- 
> 2.9.3
> 

Out of my curiousity: do we have a use case that these two operations
might collapse (KVM_CREATE_IRQCHIP and KVM_SET_GSI_ROUTING)? Or is
this patch only for the sake of security?

Another thing to mention is that, I guess adding this lock will
benefit KVM_CAP_SPLIT_IRQCHIP as well, maybe nice to mention it too in
the commit message. No worth a repost for this single reason though.

Thanks,

-- peterx

Re: [PATCH v1 02/22] KVM: x86: check against irqchip_mode in kvm_set_routing_entry()

[Peter Xu]

On Tue, Mar 14, 2017 at 02:34:30PM +0100, David Hildenbrand wrote:
> Let's replace the checks for pic_in_kernel() and ioapic_in_kernel() by
> checks against irqchip_mode. Add another state, indicating that the
> caller is currently initializing the irqchip.
> 
> This is necessary to switch pic_in_kernel() and ioapic_in_kernel() to
> irqchip_mode, too.
> 
> Also make sure that creation of any route is only possible if we have
> a lapic in kernel (irqchip_in_kernel()) or if we are currently
> inititalizing the irqchip.

The subject of the patch is:

  KVM: x86: check against irqchip_mode in kvm_set_routing_entry()

IMHO the most significant change this patch brought is the new irqchip
mode, but we just didn't mention it in the subject...

If we really want this new mode, maybe split current patch into two?
One for the new mode, one for the change in kvm_set_routing_entry()?

Thanks,

> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  1 +
>  arch/x86/kvm/irq.h              |  2 +-
>  arch/x86/kvm/irq_comm.c         | 12 ++++++------
>  arch/x86/kvm/x86.c              |  7 ++++++-
>  4 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 74ef58c..164e544 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -727,6 +727,7 @@ struct kvm_hv {
>  
>  enum kvm_irqchip_mode {
>  	KVM_IRQCHIP_NONE,
> +	KVM_IRQCHIP_INIT_IN_PROGRESS, /* temporarily set during creation */
>  	KVM_IRQCHIP_KERNEL,       /* created with KVM_CREATE_IRQCHIP */
>  	KVM_IRQCHIP_SPLIT,        /* created with KVM_CAP_SPLIT_IRQCHIP */
>  };
> diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
> index 40d5b2c..7fa2480 100644
> --- a/arch/x86/kvm/irq.h
> +++ b/arch/x86/kvm/irq.h
> @@ -103,7 +103,7 @@ static inline int irqchip_kernel(struct kvm *kvm)
>  
>  static inline int irqchip_in_kernel(struct kvm *kvm)
>  {
> -	bool ret = kvm->arch.irqchip_mode != KVM_IRQCHIP_NONE;
> +	bool ret = kvm->arch.irqchip_mode > KVM_IRQCHIP_INIT_IN_PROGRESS;
>  
>  	/* Matches with wmb after initializing kvm->irq_routing. */
>  	smp_rmb();
> diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
> index 6825cd3..0a026f8 100644
> --- a/arch/x86/kvm/irq_comm.c
> +++ b/arch/x86/kvm/irq_comm.c
> @@ -282,24 +282,24 @@ int kvm_set_routing_entry(struct kvm *kvm,
>  	int delta;
>  	unsigned max_pin;
>  
> +	/* also allow creation of routes during KVM_IRQCHIP_INIT_IN_PROGRESS */
> +	if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE)
> +		goto out;
> +
>  	switch (ue->type) {
>  	case KVM_IRQ_ROUTING_IRQCHIP:
> +		if (irqchip_split(kvm))
> +			goto out;
>  		delta = 0;
>  		switch (ue->u.irqchip.irqchip) {
>  		case KVM_IRQCHIP_PIC_SLAVE:
>  			delta = 8;
>  			/* fall through */
>  		case KVM_IRQCHIP_PIC_MASTER:
> -			if (!pic_in_kernel(kvm))
> -				goto out;
> -
>  			e->set = kvm_set_pic_irq;
>  			max_pin = PIC_NUM_PINS;
>  			break;
>  		case KVM_IRQCHIP_IOAPIC:
> -			if (!ioapic_in_kernel(kvm))
> -				goto out;
> -
>  			max_pin = KVM_IOAPIC_NUM_PINS;
>  			e->set = kvm_set_ioapic_irq;
>  			break;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1faf620..fa8cceb 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3934,9 +3934,12 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>  			goto split_irqchip_unlock;
>  		if (kvm->created_vcpus)
>  			goto split_irqchip_unlock;
> +		kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
>  		r = kvm_setup_empty_irq_routing(kvm);
> -		if (r)
> +		if (r) {
> +			kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
>  			goto split_irqchip_unlock;
> +		}
>  		/* Pairs with irqchip_in_kernel. */
>  		smp_wmb();
>  		kvm->arch.irqchip_mode = KVM_IRQCHIP_SPLIT;
> @@ -4024,8 +4027,10 @@ long kvm_arch_vm_ioctl(struct file *filp,
>  			goto create_irqchip_unlock;
>  		}
>  
> +		kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
>  		r = kvm_setup_default_irq_routing(kvm);
>  		if (r) {
> +			kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
>  			mutex_lock(&kvm->slots_lock);
>  			mutex_lock(&kvm->irq_lock);
>  			kvm_ioapic_destroy(kvm);
> -- 
> 2.9.3
> 

-- peterx

Re: [PATCH v1 16/22] KVM: x86: drop goto label in kvm_set_routing_entry()

[Peter Xu]

On Tue, Mar 14, 2017 at 02:34:44PM +0100, David Hildenbrand wrote:
> No need for the goto label + local variable "r".
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

(Maybe setup_routing_entry() can be cleaned up as well?)

-- peterx

Re: [PATCH v1 17/22] KVM: x86: simplify pic_unlock()

[Peter Xu]

On Tue, Mar 14, 2017 at 02:34:45PM +0100, David Hildenbrand wrote:
> We can easily compact this code and get rid of one local variable.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  arch/x86/kvm/i8259.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
> index e77e8c3..c50b839 100644
> --- a/arch/x86/kvm/i8259.c
> +++ b/arch/x86/kvm/i8259.c
> @@ -49,7 +49,7 @@ static void pic_unlock(struct kvm_pic *s)
>  	__releases(&s->lock)
>  {
>  	bool wakeup = s->wakeup_needed;
> -	struct kvm_vcpu *vcpu, *found = NULL;
> +	struct kvm_vcpu *vcpu;
>  	int i;
>  
>  	s->wakeup_needed = false;
> @@ -59,16 +59,11 @@ static void pic_unlock(struct kvm_pic *s)
>  	if (wakeup) {
>  		kvm_for_each_vcpu(i, vcpu, s->kvm) {
>  			if (kvm_apic_accept_pic_intr(vcpu)) {
> -				found = vcpu;
> +				kvm_make_request(KVM_REQ_EVENT, vcpu);
> +				kvm_vcpu_kick(vcpu);
>  				break;

Maybe "return" directly here? Besides:

Reviewed-by: Peter Xu <peterx@redhat.com>

>  			}
>  		}
> -
> -		if (!found)
> -			return;
> -
> -		kvm_make_request(KVM_REQ_EVENT, found);
> -		kvm_vcpu_kick(found);
>  	}
>  }
>  
> -- 
> 2.9.3
> 

-- peterx

Re: [PATCH v1 17/22] KVM: x86: simplify pic_unlock()

[David Hildenbrand]

Am 15.03.2017 um 09:27 schrieb Peter Xu:
> On Tue, Mar 14, 2017 at 02:34:45PM +0100, David Hildenbrand wrote:
>> We can easily compact this code and get rid of one local variable.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  arch/x86/kvm/i8259.c | 11 +++--------
>>  1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
>> index e77e8c3..c50b839 100644
>> --- a/arch/x86/kvm/i8259.c
>> +++ b/arch/x86/kvm/i8259.c
>> @@ -49,7 +49,7 @@ static void pic_unlock(struct kvm_pic *s)
>>  	__releases(&s->lock)
>>  {
>>  	bool wakeup = s->wakeup_needed;
>> -	struct kvm_vcpu *vcpu, *found = NULL;
>> +	struct kvm_vcpu *vcpu;
>>  	int i;
>>  
>>  	s->wakeup_needed = false;
>> @@ -59,16 +59,11 @@ static void pic_unlock(struct kvm_pic *s)
>>  	if (wakeup) {
>>  		kvm_for_each_vcpu(i, vcpu, s->kvm) {
>>  			if (kvm_apic_accept_pic_intr(vcpu)) {
>> -				found = vcpu;
>> +				kvm_make_request(KVM_REQ_EVENT, vcpu);
>> +				kvm_vcpu_kick(vcpu);
>>  				break;
> 
> Maybe "return" directly here? Besides:
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks, turned that break into return and added your rb!

-- 
Thanks,

David

Re: [PATCH v1 16/22] KVM: x86: drop goto label in kvm_set_routing_entry()

[David Hildenbrand]

Am 15.03.2017 um 09:23 schrieb Peter Xu:
> On Tue, Mar 14, 2017 at 02:34:44PM +0100, David Hildenbrand wrote:
>> No need for the goto label + local variable "r".
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> (Maybe setup_routing_entry() can be cleaned up as well?)
> 

Indeed, will add a patch to this queue, thanks!

> -- peterx
> 


-- 
Thanks,

David

Re: [PATCH v1 01/22] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP

[David Hildenbrand]

Am 15.03.2017 um 07:24 schrieb Peter Xu:
> On Tue, Mar 14, 2017 at 02:34:29PM +0100, David Hildenbrand wrote:
>> Avoid races between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP by taking
>> the kvm->lock when setting up routes.
>>
>> If KVM_CREATE_IRQCHIP fails, KVM_SET_GSI_ROUTING could have already set
>> up routes pointing at pic/ioapic, being silently removed already.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  virt/kvm/kvm_main.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index a17d787..ad0f8b2 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -3079,8 +3079,11 @@ static long kvm_vm_ioctl(struct file *filp,
>>  					   routing.nr * sizeof(*entries)))
>>  				goto out_free_irq_routing;
>>  		}
>> +		/* avoid races with KVM_CREATE_IRQCHIP on x86 */
>> +		mutex_lock(&kvm->lock);
>>  		r = kvm_set_irq_routing(kvm, entries, routing.nr,
>>  					routing.flags);
>> +		mutex_unlock(&kvm->lock);
>>  out_free_irq_routing:
>>  		vfree(entries);
>>  		break;
>> -- 
>> 2.9.3
>>
> 
> Out of my curiousity: do we have a use case that these two operations
> might collapse (KVM_CREATE_IRQCHIP and KVM_SET_GSI_ROUTING)? Or is
> this patch only for the sake of security?

Just for the sake of security. I think, in general such calls are not
supposed to be done in parallel. But of course, user space can.

> 
> Another thing to mention is that, I guess adding this lock will
> benefit KVM_CAP_SPLIT_IRQCHIP as well, maybe nice to mention it too in
> the commit message. No worth a repost for this single reason though.
> 

I added

"Also, as a side effect, this patch makes sure that KVM_SET_GSI_ROUTING
and KVM_CAP_SPLIT_IRQCHIP cannot run in parallel."

Thanks!

> Thanks,
> 
> -- peterx
> 


-- 
Thanks,

David

Re: [PATCH v1 01/22] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP

[Peter Xu]

On Wed, Mar 15, 2017 at 10:19:53AM +0100, David Hildenbrand wrote:
> Am 15.03.2017 um 07:24 schrieb Peter Xu:
> > On Tue, Mar 14, 2017 at 02:34:29PM +0100, David Hildenbrand wrote:
> >> Avoid races between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP by taking
> >> the kvm->lock when setting up routes.
> >>
> >> If KVM_CREATE_IRQCHIP fails, KVM_SET_GSI_ROUTING could have already set
> >> up routes pointing at pic/ioapic, being silently removed already.
> >>
> >> Signed-off-by: David Hildenbrand <david@redhat.com>
> >> ---
> >>  virt/kvm/kvm_main.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> >> index a17d787..ad0f8b2 100644
> >> --- a/virt/kvm/kvm_main.c
> >> +++ b/virt/kvm/kvm_main.c
> >> @@ -3079,8 +3079,11 @@ static long kvm_vm_ioctl(struct file *filp,
> >>  					   routing.nr * sizeof(*entries)))
> >>  				goto out_free_irq_routing;
> >>  		}
> >> +		/* avoid races with KVM_CREATE_IRQCHIP on x86 */
> >> +		mutex_lock(&kvm->lock);
> >>  		r = kvm_set_irq_routing(kvm, entries, routing.nr,
> >>  					routing.flags);
> >> +		mutex_unlock(&kvm->lock);
> >>  out_free_irq_routing:
> >>  		vfree(entries);
> >>  		break;
> >> -- 
> >> 2.9.3
> >>
> > 
> > Out of my curiousity: do we have a use case that these two operations
> > might collapse (KVM_CREATE_IRQCHIP and KVM_SET_GSI_ROUTING)? Or is
> > this patch only for the sake of security?
> 
> Just for the sake of security. I think, in general such calls are not
> supposed to be done in parallel. But of course, user space can.

I see.

> 
> > 
> > Another thing to mention is that, I guess adding this lock will
> > benefit KVM_CAP_SPLIT_IRQCHIP as well, maybe nice to mention it too in
> > the commit message. No worth a repost for this single reason though.
> > 
> 
> I added
> 
> "Also, as a side effect, this patch makes sure that KVM_SET_GSI_ROUTING
> and KVM_CAP_SPLIT_IRQCHIP cannot run in parallel."

Yeah it looks better. Thanks!

-- peterx

Re: [PATCH v1 02/22] KVM: x86: check against irqchip_mode in kvm_set_routing_entry()

[David Hildenbrand]

Am 15.03.2017 um 07:33 schrieb Peter Xu:
> On Tue, Mar 14, 2017 at 02:34:30PM +0100, David Hildenbrand wrote:
>> Let's replace the checks for pic_in_kernel() and ioapic_in_kernel() by
>> checks against irqchip_mode. Add another state, indicating that the
>> caller is currently initializing the irqchip.
>>
>> This is necessary to switch pic_in_kernel() and ioapic_in_kernel() to
>> irqchip_mode, too.
>>
>> Also make sure that creation of any route is only possible if we have
>> a lapic in kernel (irqchip_in_kernel()) or if we are currently
>> inititalizing the irqchip.
> 
> The subject of the patch is:
> 
>   KVM: x86: check against irqchip_mode in kvm_set_routing_entry()
> 
> IMHO the most significant change this patch brought is the new irqchip
> mode, but we just didn't mention it in the subject...
> 
> If we really want this new mode, maybe split current patch into two?
> One for the new mode, one for the change in kvm_set_routing_entry()?
> 

I split that up, thanks!

-- 

Thanks,

David