Skip to contentSkip to conversation

pic/ioapic/irqchip cleanups + minor fixes

RFC PATCH In review David Hildenbrand 3 participants 27 messages Mar 6, 2017 – Mar 7, 2017

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

David Hildenbrand

author
#0 lore raw

cover pic/ioapic/irqchip cleanups + minor fixes

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.

Feel free to nack if a a certain cleanup is not worth it.

David Hildenbrand (21):
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()

 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              | 20 +++++-------
 arch/x86/kvm/irq_comm.c         | 39 +++++++++--------------
 arch/x86/kvm/x86.c              | 36 +++++++++------------
 include/linux/kvm_host.h        |  4 +--
 virt/kvm/eventfd.c              |  4 +--
 virt/kvm/kvm_main.c             |  3 ++
 11 files changed, 85 insertions(+), 136 deletions(-)
Signature · 1 line
2.9.3
author
#1 lore raw

RFC PATCH 1/21 KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP

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 <[email protected]>
---
 virt/kvm/kvm_main.c | 3 +++
 1 file changed, 3 insertions(+)
virt/kvm/kvm_main.c +3 -0
index c6b7aff..9cd6a34 100644
@@ -3077,8 +3077,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;
Signature · 1 line
2.9.3
author
#2 lore raw

RFC PATCH 2/21 KVM: x86: check against irqchip_mode in kvm_set_routing_entry()

In reply to 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.

Signed-off-by David Hildenbrand <[email protected]>
---
 arch/x86/include/asm/kvm_host.h | 1 +
 arch/x86/kvm/irq.h              | 8 +++++++-
 arch/x86/kvm/irq_comm.c         | 8 ++------
 arch/x86/kvm/x86.c              | 2 ++
 4 files changed, 12 insertions(+), 7 deletions(-)
arch/x86/include/asm/kvm_host.h +1 -0
index 74ef58c..c8cdcc3 100644
@@ -727,6 +727,7 @@ struct kvm_hv {
 
 enum kvm_irqchip_mode {
 	KVM_IRQCHIP_NONE,
+	KVM_IRQCHIP_KERNEL_INIT,  /* KVM_CREATE_IRQCHIP in progress */
 	KVM_IRQCHIP_KERNEL,       /* created with KVM_CREATE_IRQCHIP */
 	KVM_IRQCHIP_SPLIT,        /* created with KVM_CAP_SPLIT_IRQCHIP */
 };
arch/x86/kvm/irq.h +7 -1
index 40d5b2c..9ebb6f5 100644
@@ -96,6 +96,11 @@ static inline int irqchip_split(struct kvm *kvm)
 	return kvm->arch.irqchip_mode == KVM_IRQCHIP_SPLIT;
 }
 
+static inline int irqchip_kernel_init(struct kvm *kvm)
+{
+	return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL_INIT;
+}
+
 static inline int irqchip_kernel(struct kvm *kvm)
 {
 	return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL;

@@ -103,7 +108,8 @@ 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_KERNEL ||
+		   kvm->arch.irqchip_mode == KVM_IRQCHIP_SPLIT;
 
 	/* Matches with wmb after initializing kvm->irq_routing. */
 	smp_rmb();
arch/x86/kvm/irq_comm.c +2 -6
index b96d389..4e4a67a 100644
@@ -282,22 +282,18 @@ int kvm_set_routing_entry(struct kvm *kvm,
 
 	switch (ue->type) {
 	case KVM_IRQ_ROUTING_IRQCHIP:
+		if (!irqchip_kernel(kvm) && !irqchip_kernel_init(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;
arch/x86/kvm/x86.c +2 -0
index b2a4b11..c69940c 100644
@@ -4022,8 +4022,10 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto create_irqchip_unlock;
 		}
 
+		kvm->arch.irqchip_mode = KVM_IRQCHIP_KERNEL_INIT;
 		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);
Signature · 1 line
2.9.3
maintainer
#3 lore raw

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

arch/x86/include/asm/kvm_host.h struct kvm_hv { · line 730

Quoted 2 lines · David Hildenbrand
arch/x86/include/asm/kvm_host.h +1 -0
 	KVM_IRQCHIP_NONE,
+	KVM_IRQCHIP_KERNEL_INIT,  /* KVM_CREATE_IRQCHIP in progress */

Maybe KVM_IRQCHIP_INIT_IN_PROGRESS?

arch/x86/kvm/irq.h static inline int irqchip_kernel(struct kvm *kvm) · line 112

Quoted 3 lines
arch/x86/kvm/irq.h +2 -1
-	bool ret = kvm->arch.irqchip_mode != KVM_IRQCHIP_NONE;
+	bool ret = kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL ||
+		   kvm->arch.irqchip_mode == KVM_IRQCHIP_SPLIT;

I suspect that if you phrase it the other way round (!= NONE && != KERNEL_INIT) you'll get infinitesimally better code, because it can be compiled to an unsigned comparison with 1.

arch/x86/kvm/irq_comm.c int kvm_set_routing_entry(struct kvm *kvm, · line 287

Quoted 13 lines
arch/x86/kvm/irq.h +0 -0
 	/* Matches with wmb after initializing kvm->irq_routing. */
 	smp_rmb();
arch/x86/kvm/irq_comm.c +2 -0
index b96d389..4e4a67a 100644
@@ -282,22 +282,18 @@ int kvm_set_routing_entry(struct kvm *kvm,
 
 	switch (ue->type) {
 	case KVM_IRQ_ROUTING_IRQCHIP:
+		if (!irqchip_kernel(kvm) && !irqchip_kernel_init(kvm))
+			goto out;
 		delta = 0;

This can be irqchip_in_kernel, after which irqchip_kernel_init can be removed.

Should the code to enable split irqchip also use KVM_IRQCHIP_KERNEL_INIT?

Paolo

author
#4 lore raw

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

Am 06.03.2017 um 19:08 schrieb Paolo Bonzini:

Quoted 7 lines · David Hildenbrand

arch/x86/include/asm/kvm_host.h struct kvm_hv { · line 730

Quoted 2 lines
arch/x86/include/asm/kvm_host.h +1 -0
 	KVM_IRQCHIP_NONE,
+	KVM_IRQCHIP_KERNEL_INIT,  /* KVM_CREATE_IRQCHIP in progress */

Maybe KVM_IRQCHIP_INIT_IN_PROGRESS?

I tried to make it short but I agree, that is more self explaining.

Quoted 8 lines

arch/x86/kvm/irq.h static inline int irqchip_kernel(struct kvm *kvm) · line 112

Quoted 4 lines
arch/x86/kvm/irq.h +2 -1
-	bool ret = kvm->arch.irqchip_mode != KVM_IRQCHIP_NONE;
+	bool ret = kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL ||
+		   kvm->arch.irqchip_mode == KVM_IRQCHIP_SPLIT;

I suspect that if you phrase it the other way round (!= NONE && != KERNEL_INIT) you'll get infinitesimally better code, because it can be compiled to an unsigned comparison with 1.

However, adding new modes can silently make this check wrong (e.g. grepping for KVM_IRQCHIP_KERNEL will no longer identify all users). Do you think the optimization is worth it?

Quoted 9 lines
Quoted 9 lines
arch/x86/kvm/irq.h +0 -0
 	/* Matches with wmb after initializing kvm->irq_routing. */
 	smp_rmb();
arch/x86/kvm/irq_comm.c +0 -0
index b96d389..4e4a67a 100644
@@ -282,22 +282,18 @@ int kvm_set_routing_entry(struct kvm *kvm,
Quoted 8 lines

arch/x86/kvm/irq_comm.c int kvm_set_routing_entry(struct kvm *kvm, · line 287

Quoted 5 lines
arch/x86/kvm/irq_comm.c +2 -0
 	switch (ue->type) {
 	case KVM_IRQ_ROUTING_IRQCHIP:
+		if (!irqchip_kernel(kvm) && !irqchip_kernel_init(kvm))
+			goto out;
 		delta = 0;

This can be irqchip_in_kernel, after which irqchip_kernel_init can be removed.

irqchip_in_kernel in its current form would allow KVM_IRQCHIP_SPLIT, which is not what we want here, or am I missing something?

Quoted 3 lines

Should the code to enable split irqchip also use KVM_IRQCHIP_KERNEL_INIT?

a) checking against KVM_IRQCHIP_KERNEL_INIT shouldn't be necessary due to the kvm->lock (this code path will never see KVM_IRQCHIP_KERNEL_INIT).

b) setting KVM_IRQCHIP_KERNEL_INIT could be done. We only initialize an empty irq routing. But also that should never be allowed to set up routings targeted at pic/ioapic. However that would in its current form never happen.

Quoted 2 lines

Paolo

Thanks!

David

maintainer
#5 lore raw

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

Quoted 9 lines · David Hildenbrand

Am 06.03.2017 um 19:08 schrieb Paolo Bonzini:

Quoted 7 lines

arch/x86/kvm/irq.h static inline int irqchip_kernel(struct kvm *kvm) · line 112

Quoted 3 lines
arch/x86/kvm/irq.h +2 -1
-	bool ret = kvm->arch.irqchip_mode != KVM_IRQCHIP_NONE;
+	bool ret = kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL ||
+		   kvm->arch.irqchip_mode == KVM_IRQCHIP_SPLIT;

I suspect that if you phrase it the other way round (!= NONE && != KERNEL_INIT) you'll get infinitesimally better code, because it can be compiled to an unsigned comparison with 1.

Quoted 3 lines

However, adding new modes can silently make this check wrong (e.g. grepping for KVM_IRQCHIP_KERNEL will no longer identify all users). Do you think the optimization is worth it?

I don't think we want to add new modes.

Quoted 18 lines
Quoted 9 lines
Quoted 9 lines
arch/x86/kvm/irq.h +0 -0
 	/* Matches with wmb after initializing kvm->irq_routing. */
 	smp_rmb();
arch/x86/kvm/irq_comm.c +0 -0
index b96d389..4e4a67a 100644
@@ -282,22 +282,18 @@ int kvm_set_routing_entry(struct kvm *kvm,
Quoted 8 lines

arch/x86/kvm/irq_comm.c int kvm_set_routing_entry(struct kvm *kvm, · line 287

Quoted 5 lines
arch/x86/kvm/irq_comm.c +2 -0
 	switch (ue->type) {
 	case KVM_IRQ_ROUTING_IRQCHIP:
+		if (!irqchip_kernel(kvm) && !irqchip_kernel_init(kvm))
+			goto out;
 		delta = 0;

This can be irqchip_in_kernel, after which irqchip_kernel_init can be removed.

Quoted 2 lines

irqchip_in_kernel in its current form would allow KVM_IRQCHIP_SPLIT, which is not what we want here, or am I missing something?

Hmm, perhaps we can split the checks to rule out KVM_IRQCHIP_NONE outside the switch, and KVM_IRQCHIP_SPLIT here?

Quoted 9 lines
Quoted 1 line

Should the code to enable split irqchip also use KVM_IRQCHIP_KERNEL_INIT?

a) checking against KVM_IRQCHIP_KERNEL_INIT shouldn't be necessary due to the kvm->lock (this code path will never see KVM_IRQCHIP_KERNEL_INIT).

b) setting KVM_IRQCHIP_KERNEL_INIT could be done. We only initialize an empty irq routing. But also that should never be allowed to set up routings targeted at pic/ioapic. However that would in its current form never happen.

You're right, it'd be just a matter of keeping the code similar between the two cases. You can try it and decide when you post the next version.

Thanks!

Paolo

#6 lore raw

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

2017-03-07 11:53+0100, Paolo Bonzini:

Quoted 9 lines · David Hildenbrand
Quoted 8 lines

Am 06.03.2017 um 19:08 schrieb Paolo Bonzini:

Quoted 7 lines

arch/x86/kvm/irq.h static inline int irqchip_kernel(struct kvm *kvm) · line 112

Quoted 3 lines
arch/x86/kvm/irq.h +2 -1
-	bool ret = kvm->arch.irqchip_mode != KVM_IRQCHIP_NONE;
+	bool ret = kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL ||
+		   kvm->arch.irqchip_mode == KVM_IRQCHIP_SPLIT;

I suspect that if you phrase it the other way round (!= NONE && != KERNEL_INIT) you'll get infinitesimally better code, because it can be compiled to an unsigned comparison with 1.

Yes, it seems that the compiler must assume that an enum can also be a value that is not enumerated.

Quoted 5 lines
Quoted 3 lines

However, adding new modes can silently make this check wrong (e.g. grepping for KVM_IRQCHIP_KERNEL will no longer identify all users). Do you think the optimization is worth it?

I don't think we want to add new modes.

I would prefer to write it like:

kvm->arch.irqchip_mode > KVM_IRQCHIP_KERNEL_INIT;

Same assembly with simpler code. Setting KVM_IRQCHIP_KERNEL_INIT before KVM_IRQCHIP_SPLIT would make it a bit more descriprive (and would allow the check below as well).

Quoted 17 lines
Quoted 17 lines
Quoted 16 lines

arch/x86/kvm/irq_comm.c int kvm_set_routing_entry(struct kvm *kvm, · line 287

Quoted 13 lines
arch/x86/kvm/irq.h +0 -0
 	/* Matches with wmb after initializing kvm->irq_routing. */
 	smp_rmb();
arch/x86/kvm/irq_comm.c +2 -0
index b96d389..4e4a67a 100644
@@ -282,22 +282,18 @@ int kvm_set_routing_entry(struct kvm *kvm,
 
 	switch (ue->type) {
 	case KVM_IRQ_ROUTING_IRQCHIP:
+		if (!irqchip_kernel(kvm) && !irqchip_kernel_init(kvm))
+			goto out;
 		delta = 0;

This can be irqchip_in_kernel, after which irqchip_kernel_init can be removed.

Quoted 5 lines
Quoted 2 lines

irqchip_in_kernel in its current form would allow KVM_IRQCHIP_SPLIT, which is not what we want here, or am I missing something?

Hmm, perhaps we can split the checks to rule out KVM_IRQCHIP_NONE outside the switch, and KVM_IRQCHIP_SPLIT here?

Right, we don't want MSI or HV without LAPIC in kernel either.

author
#7 lore raw

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

Quoted 10 lines
Quoted 6 lines
Quoted 4 lines

However, adding new modes can silently make this check wrong (e.g. grepping for KVM_IRQCHIP_KERNEL will no longer identify all users). Do you think the optimization is worth it?

I don't think we want to add new modes.

I would prefer to write it like:

kvm->arch.irqchip_mode > KVM_IRQCHIP_KERNEL_INIT;

Agreed.

Quoted 4 lines

Same assembly with simpler code. Setting KVM_IRQCHIP_KERNEL_INIT before KVM_IRQCHIP_SPLIT would make it a bit more descriprive (and would allow the check below as well).

Okay, I added that.

Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 17 lines
Quoted 8 lines
arch/x86/kvm/irq.h +0 -0
 	/* Matches with wmb after initializing kvm->irq_routing. */
 	smp_rmb();
arch/x86/kvm/irq_comm.c +0 -0
index b96d389..4e4a67a 100644

arch/x86/kvm/irq_comm.c int kvm_set_routing_entry(struct kvm *kvm, · line 287

Quoted 6 lines
arch/x86/kvm/irq_comm.c +2 -0
@@ -282,22 +282,18 @@ int kvm_set_routing_entry(struct kvm *kvm,
 
 	switch (ue->type) {
 	case KVM_IRQ_ROUTING_IRQCHIP:
+		if (!irqchip_kernel(kvm) && !irqchip_kernel_init(kvm))
+			goto out;
 		delta = 0;

This can be irqchip_in_kernel, after which irqchip_kernel_init can be removed.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

irqchip_in_kernel in its current form would allow KVM_IRQCHIP_SPLIT, which is not what we want here, or am I missing something?

Hmm, perhaps we can split the checks to rule out KVM_IRQCHIP_NONE outside the switch, and KVM_IRQCHIP_SPLIT here?

Right, we don't want MSI or HV without LAPIC in kernel either.

Ok, I will see how this turns out!

Thanks!

Signature · 3 lines
Thanks,

David
author
#8 lore raw

RFC PATCH 3/21 KVM: x86: check against irqchip_mode in pic_in_kernel()

In reply to 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 <[email protected]>
---
 arch/x86/kvm/irq.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
arch/x86/kvm/irq.h +1 -4
index 9ebb6f5..150c3b3 100644
@@ -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)
Signature · 1 line
2.9.3
author
#9 lore raw

RFC PATCH 4/21 KVM: x86: check against irqchip_mode in ioapic_in_kernel()

In reply to 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 <[email protected]>
---
 arch/x86/kvm/ioapic.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
arch/x86/kvm/ioapic.h +1 -4
index 1cc6e54..9a62fe1 100644
@@ -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);
Signature · 1 line
2.9.3
author
#10 lore raw

RFC PATCH 5/21 KVM: x86: get rid of pic_irqchip()

In reply to 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 <[email protected]>
---
 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(-)
arch/x86/kvm/i8259.c +2 -2
index 73ea24d..64e4b92 100644
@@ -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;
arch/x86/kvm/irq.c +1 -1
index 60d91c9..5c24811 100644
@@ -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;
 }
arch/x86/kvm/irq.h +0 -5
index 150c3b3..d083876 100644
@@ -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;
arch/x86/kvm/irq_comm.c +2 -2
index 4e4a67a..99f68c2 100644
@@ -40,7 +40,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);
 }
 

@@ -234,7 +234,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);
 }
arch/x86/kvm/x86.c +11 -13
index c69940c..52525e4 100644
@@ -3719,18 +3719,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:

@@ -3745,23 +3744,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);

@@ -3770,7 +3768,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;
 }
Signature · 1 line
2.9.3
author
#11 lore raw

RFC PATCH 6/21 KVM: x86: get rid of ioapic_irqchip()

In reply to David Hildenbrand

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

Signed-off-by David Hildenbrand <[email protected]>
---
 arch/x86/kvm/ioapic.c | 4 ++--
 arch/x86/kvm/ioapic.h | 5 -----
 2 files changed, 2 insertions(+), 7 deletions(-)
arch/x86/kvm/ioapic.c +2 -2
index 6e219e5..3d9dc98 100644
@@ -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;
arch/x86/kvm/ioapic.h +0 -5
index 9a62fe1..717d98a 100644
@@ -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;
Signature · 1 line
2.9.3
author
#12 lore raw

RFC PATCH 7/21 KVM: x86: use ioapic_in_kernel() to check for ioapic existence

In reply to David Hildenbrand
Signed-off-by David Hildenbrand <[email protected]>
---
 arch/x86/kvm/ioapic.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
arch/x86/kvm/ioapic.c +1 -3
index 3d9dc98..4fd234d 100644
@@ -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);
 }
Signature · 1 line
2.9.3
author
#13 lore raw

RFC PATCH 8/21 KVM: x86: remove duplicate checks for ioapic

In reply to David Hildenbrand

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

Signed-off-by David Hildenbrand <[email protected]>
---
 arch/x86/kvm/ioapic.c | 4 ----
 1 file changed, 4 deletions(-)
arch/x86/kvm/ioapic.c +0 -4
index 4fd234d..9b2a7f0 100644
@@ -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));
Signature · 1 line
2.9.3
author
#14 lore raw

RFC PATCH 9/21 KVM: x86: convert kvm_(set|get)_ioapic() into void

In reply to David Hildenbrand
Signed-off-by David Hildenbrand <[email protected]>
---
 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(-)
arch/x86/kvm/ioapic.c +2 -4
index 9b2a7f0..427e5f9 100644
@@ -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;
 }
arch/x86/kvm/ioapic.h +2 -2
index 717d98a..3eb140b 100644
@@ -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,
arch/x86/kvm/x86.c +2 -2
index 52525e4..95372e3 100644
@@ -3733,7 +3733,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;

@@ -3762,7 +3762,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;
Signature · 1 line
2.9.3
author
#15 lore raw

RFC PATCH 10/21 KVM: x86: don't take kvm->irq_lock when creating IRQCHIP

In reply to 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 <[email protected]>
---
 arch/x86/kvm/x86.c | 2 --
 1 file changed, 2 deletions(-)
arch/x86/kvm/x86.c +0 -2
index 95372e3..6ac81f0 100644
@@ -4025,10 +4025,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;
 		}
Signature · 1 line
2.9.3
author
#16 lore raw

RFC PATCH 11/21 KVM: x86: push usage of slots_lock down

In reply to David Hildenbrand

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

Signed-off-by David Hildenbrand <[email protected]>
---
 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(-)
arch/x86/kvm/i8259.c +2 -0
index 64e4b92..e77e8c3 100644
@@ -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);
arch/x86/kvm/ioapic.c +2 -0
index 427e5f9..1a30320 100644
@@ -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);
 }
arch/x86/kvm/x86.c +0 -4
index 6ac81f0..7219b18 100644
@@ -4014,9 +4014,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;
 		}
 

@@ -4024,10 +4022,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. */
Signature · 1 line
2.9.3
author
#17 lore raw

RFC PATCH 12/21 KVM: x86: KVM_IRQCHIP_PIC_MASTER only has 8 pins

In reply to 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 <[email protected]>
---
 arch/x86/kvm/irq_comm.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)
arch/x86/kvm/irq_comm.c +6 -9
index 99f68c2..7535965 100644
@@ -277,33 +277,30 @@ int kvm_set_routing_entry(struct kvm *kvm,
 			  const struct kvm_irq_routing_entry *ue)
 {
 	int r = -EINVAL;
-	int delta;
-	unsigned max_pin;
 
 	switch (ue->type) {
 	case KVM_IRQ_ROUTING_IRQCHIP:
 		if (!irqchip_kernel(kvm) && !irqchip_kernel_init(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;
Signature · 1 line
2.9.3
author
#18 lore raw

RFC PATCH 13/21 KVM: x86: remove all-vcpu request from kvm_ioapic_init()

In reply to 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 <[email protected]>
---
 arch/x86/kvm/ioapic.c | 2 --
 1 file changed, 2 deletions(-)
arch/x86/kvm/ioapic.c +0 -2
index 1a30320..0eb7847 100644
@@ -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;
 }
Signature · 1 line
2.9.3
author
#19 lore raw

RFC PATCH 14/21 KVM: x86: directly call kvm_make_scan_ioapic_request() in ioapic.c

In reply to David Hildenbrand

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

Signed-off-by David Hildenbrand <[email protected]>
---
 arch/x86/kvm/ioapic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kvm/ioapic.c +2 -2
index 0eb7847..d1c30c0 100644
@@ -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);
 }
Signature · 1 line
2.9.3
author
#20 lore raw

RFC PATCH 15/21 KVM: x86: rename kvm_vcpu_request_scan_ioapic()

In reply to David Hildenbrand

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

Signed-off-by David Hildenbrand <[email protected]>
---
 arch/x86/kvm/ioapic.c    | 2 +-
 include/linux/kvm_host.h | 4 ++--
 virt/kvm/eventfd.c       | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)
arch/x86/kvm/ioapic.c +1 -1
index d1c30c0..58c7993 100644
@@ -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;
include/linux/kvm_host.h +2 -2
index 2c14ad9..7aa2f9b 100644
@@ -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)
virt/kvm/eventfd.c +2 -2
index a29786d..c2fbecb 100644
@@ -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
Signature · 1 line
2.9.3
author
#21 lore raw

RFC PATCH 16/21 KVM: x86: drop goto label in kvm_set_routing_entry()

In reply to David Hildenbrand

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

Signed-off-by David Hildenbrand <[email protected]>
---
 arch/x86/kvm/irq_comm.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)
arch/x86/kvm/irq_comm.c +7 -11
index 7535965..04c3561 100644
@@ -276,12 +276,10 @@ 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;
-
 	switch (ue->type) {
 	case KVM_IRQ_ROUTING_IRQCHIP:
 		if (!irqchip_kernel(kvm) && !irqchip_kernel_init(kvm))
-			goto out;
+			return -EINVAL;
 		e->irqchip.pin = ue->u.irqchip.pin;
 		switch (ue->u.irqchip.irqchip) {
 		case KVM_IRQCHIP_PIC_SLAVE:

@@ -289,16 +287,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;

@@ -309,7 +307,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;

@@ -317,12 +315,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,
Signature · 1 line
2.9.3
author
#22 lore raw

RFC PATCH 17/21 KVM: x86: simplify pic_unlock()

In reply to David Hildenbrand

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

Signed-off-by David Hildenbrand <[email protected]>
---
 arch/x86/kvm/i8259.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)
arch/x86/kvm/i8259.c +3 -8
index e77e8c3..c50b839 100644
@@ -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);
 	}
 }
Signature · 1 line
2.9.3
author
#23 lore raw

RFC PATCH 18/21 KVM: x86: make kvm_pic_reset() static

In reply to David Hildenbrand

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

Signed-off-by David Hildenbrand <[email protected]>
---
 arch/x86/kvm/i8259.c | 2 +-
 arch/x86/kvm/irq.h   | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)
arch/x86/kvm/i8259.c +1 -1
index c50b839..c29a7e1 100644
@@ -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;
arch/x86/kvm/irq.h +0 -2
index d083876..ebb5893 100644
@@ -108,8 +108,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);
Signature · 1 line
2.9.3
author
#24 lore raw

RFC PATCH 19/21 KVM: x86: drop picdev_in_range()

In reply to David Hildenbrand

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

Signed-off-by David Hildenbrand <[email protected]>
---
 arch/x86/kvm/i8259.c | 35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)
arch/x86/kvm/i8259.c +12 -23
index c29a7e1..53c4c6a 100644
@@ -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;
 }
Signature · 1 line
2.9.3
author
#25 lore raw

RFC PATCH 20/21 KVM: x86: set data directly in picdev_read()

In reply to David Hildenbrand

Now it looks almost as picdev_write().

Signed-off-by David Hildenbrand <[email protected]>
---
 arch/x86/kvm/i8259.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
arch/x86/kvm/i8259.c +3 -4
index 53c4c6a..52d21f2 100644
@@ -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;
 }
Signature · 1 line
2.9.3
author
#26 lore raw

RFC PATCH 21/21 KVM: x86: simplify pic_ioport_read()

In reply to David Hildenbrand
Signed-off-by David Hildenbrand <[email protected]>
---
 arch/x86/kvm/i8259.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
arch/x86/kvm/i8259.c +3 -6
index 52d21f2..1abd110 100644
@@ -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
Signature · 1 line
2.9.3