* [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes
@ 2017-03-06 13:17 David Hildenbrand
2017-03-06 13:17 ` [PATCH RFC 01/21] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP David Hildenbrand
` (20 more replies)
0 siblings, 21 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:17 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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(-)
--
2.9.3
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH RFC 01/21] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
@ 2017-03-06 13:17 ` David Hildenbrand
2017-03-06 13:17 ` [PATCH RFC 02/21] KVM: x86: check against irqchip_mode in kvm_set_routing_entry() David Hildenbrand
` (19 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:17 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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 c6b7aff..9cd6a34 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -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;
--
2.9.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 02/21] KVM: x86: check against irqchip_mode in kvm_set_routing_entry()
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
2017-03-06 13:17 ` [PATCH RFC 01/21] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP David Hildenbrand
@ 2017-03-06 13:17 ` David Hildenbrand
2017-03-06 18:08 ` Paolo Bonzini
2017-03-06 13:17 ` [PATCH RFC 03/21] KVM: x86: check against irqchip_mode in pic_in_kernel() David Hildenbrand
` (18 subsequent siblings)
20 siblings, 1 reply; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:17 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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 <david@redhat.com>
---
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(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 74ef58c..c8cdcc3 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_KERNEL_INIT, /* KVM_CREATE_IRQCHIP in progress */
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..9ebb6f5 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -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();
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index b96d389..4e4a67a 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -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;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b2a4b11..c69940c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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);
--
2.9.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 03/21] KVM: x86: check against irqchip_mode in pic_in_kernel()
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
2017-03-06 13:17 ` [PATCH RFC 01/21] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP David Hildenbrand
2017-03-06 13:17 ` [PATCH RFC 02/21] KVM: x86: check against irqchip_mode in kvm_set_routing_entry() David Hildenbrand
@ 2017-03-06 13:17 ` David Hildenbrand
2017-03-06 13:17 ` [PATCH RFC 04/21] KVM: x86: check against irqchip_mode in ioapic_in_kernel() David Hildenbrand
` (17 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:17 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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 9ebb6f5..150c3b3 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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 04/21] KVM: x86: check against irqchip_mode in ioapic_in_kernel()
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (2 preceding siblings ...)
2017-03-06 13:17 ` [PATCH RFC 03/21] KVM: x86: check against irqchip_mode in pic_in_kernel() David Hildenbrand
@ 2017-03-06 13:17 ` David Hildenbrand
2017-03-06 13:17 ` [PATCH RFC 05/21] KVM: x86: get rid of pic_irqchip() David Hildenbrand
` (16 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:17 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 05/21] KVM: x86: get rid of pic_irqchip()
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (3 preceding siblings ...)
2017-03-06 13:17 ` [PATCH RFC 04/21] KVM: x86: check against irqchip_mode in ioapic_in_kernel() David Hildenbrand
@ 2017-03-06 13:17 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 06/21] KVM: x86: get rid of ioapic_irqchip() David Hildenbrand
` (15 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:17 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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 150c3b3..d083876 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 4e4a67a..99f68c2 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -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);
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c69940c..52525e4 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 06/21] KVM: x86: get rid of ioapic_irqchip()
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (4 preceding siblings ...)
2017-03-06 13:17 ` [PATCH RFC 05/21] KVM: x86: get rid of pic_irqchip() David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 07/21] KVM: x86: use ioapic_in_kernel() to check for ioapic existence David Hildenbrand
` (14 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 07/21] KVM: x86: use ioapic_in_kernel() to check for ioapic existence
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (5 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 06/21] KVM: x86: get rid of ioapic_irqchip() David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 08/21] KVM: x86: remove duplicate checks for ioapic David Hildenbrand
` (13 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 08/21] KVM: x86: remove duplicate checks for ioapic
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (6 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 07/21] KVM: x86: use ioapic_in_kernel() to check for ioapic existence David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 09/21] KVM: x86: convert kvm_(set|get)_ioapic() into void David Hildenbrand
` (12 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 09/21] KVM: x86: convert kvm_(set|get)_ioapic() into void
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (7 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 08/21] KVM: x86: remove duplicate checks for ioapic David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 10/21] KVM: x86: don't take kvm->irq_lock when creating IRQCHIP David Hildenbrand
` (11 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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 52525e4..95372e3 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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;
--
2.9.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 10/21] KVM: x86: don't take kvm->irq_lock when creating IRQCHIP
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (8 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 09/21] KVM: x86: convert kvm_(set|get)_ioapic() into void David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 11/21] KVM: x86: push usage of slots_lock down David Hildenbrand
` (10 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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 95372e3..6ac81f0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 11/21] KVM: x86: push usage of slots_lock down
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (9 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 10/21] KVM: x86: don't take kvm->irq_lock when creating IRQCHIP David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 12/21] KVM: x86: KVM_IRQCHIP_PIC_MASTER only has 8 pins David Hildenbrand
` (9 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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 6ac81f0..7219b18 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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. */
--
2.9.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 12/21] KVM: x86: KVM_IRQCHIP_PIC_MASTER only has 8 pins
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (10 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 11/21] KVM: x86: push usage of slots_lock down David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 13/21] KVM: x86: remove all-vcpu request from kvm_ioapic_init() David Hildenbrand
` (8 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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 99f68c2..7535965 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -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;
--
2.9.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 13/21] KVM: x86: remove all-vcpu request from kvm_ioapic_init()
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (11 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 12/21] KVM: x86: KVM_IRQCHIP_PIC_MASTER only has 8 pins David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 14/21] KVM: x86: directly call kvm_make_scan_ioapic_request() in ioapic.c David Hildenbrand
` (7 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 14/21] KVM: x86: directly call kvm_make_scan_ioapic_request() in ioapic.c
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (12 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 13/21] KVM: x86: remove all-vcpu request from kvm_ioapic_init() David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 15/21] KVM: x86: rename kvm_vcpu_request_scan_ioapic() David Hildenbrand
` (6 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 15/21] KVM: x86: rename kvm_vcpu_request_scan_ioapic()
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (13 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 14/21] KVM: x86: directly call kvm_make_scan_ioapic_request() in ioapic.c David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 16/21] KVM: x86: drop goto label in kvm_set_routing_entry() David Hildenbrand
` (5 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 16/21] KVM: x86: drop goto label in kvm_set_routing_entry()
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (14 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 15/21] KVM: x86: rename kvm_vcpu_request_scan_ioapic() David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 17/21] KVM: x86: simplify pic_unlock() David Hildenbrand
` (4 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
No need for the goto label + local variable "r".
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/x86/kvm/irq_comm.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 7535965..04c3561 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -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,
--
2.9.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 17/21] KVM: x86: simplify pic_unlock()
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (15 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 16/21] KVM: x86: drop goto label in kvm_set_routing_entry() David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 18/21] KVM: x86: make kvm_pic_reset() static David Hildenbrand
` (3 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 18/21] KVM: x86: make kvm_pic_reset() static
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (16 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 17/21] KVM: x86: simplify pic_unlock() David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 19/21] KVM: x86: drop picdev_in_range() David Hildenbrand
` (2 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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 d083876..ebb5893 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -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);
--
2.9.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 19/21] KVM: x86: drop picdev_in_range()
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (17 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 18/21] KVM: x86: make kvm_pic_reset() static David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 20/21] KVM: x86: set data directly in picdev_read() David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 21/21] KVM: x86: simplify pic_ioport_read() David Hildenbrand
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 20/21] KVM: x86: set data directly in picdev_read()
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (18 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 19/21] KVM: x86: drop picdev_in_range() David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 21/21] KVM: x86: simplify pic_ioport_read() David Hildenbrand
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH RFC 21/21] KVM: x86: simplify pic_ioport_read()
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (19 preceding siblings ...)
2017-03-06 13:18 ` [PATCH RFC 20/21] KVM: x86: set data directly in picdev_read() David Hildenbrand
@ 2017-03-06 13:18 ` David Hildenbrand
20 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-06 13:18 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, david
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH RFC 02/21] KVM: x86: check against irqchip_mode in kvm_set_routing_entry()
2017-03-06 13:17 ` [PATCH RFC 02/21] KVM: x86: check against irqchip_mode in kvm_set_routing_entry() David Hildenbrand
@ 2017-03-06 18:08 ` Paolo Bonzini
2017-03-07 9:55 ` David Hildenbrand
0 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2017-03-06 18:08 UTC (permalink / raw)
To: David Hildenbrand, kvm; +Cc: rkrcmar
On 06/03/2017 14:17, David Hildenbrand wrote:
> KVM_IRQCHIP_NONE,
> + KVM_IRQCHIP_KERNEL_INIT, /* KVM_CREATE_IRQCHIP in progress */
Maybe KVM_IRQCHIP_INIT_IN_PROGRESS?
> - 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.
> /* 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 b96d389..4e4a67a 100644
> --- a/arch/x86/kvm/irq_comm.c
> +++ b/arch/x86/kvm/irq_comm.c
> @@ -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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH RFC 02/21] KVM: x86: check against irqchip_mode in kvm_set_routing_entry()
2017-03-06 18:08 ` Paolo Bonzini
@ 2017-03-07 9:55 ` David Hildenbrand
2017-03-07 10:53 ` Paolo Bonzini
0 siblings, 1 reply; 27+ messages in thread
From: David Hildenbrand @ 2017-03-07 9:55 UTC (permalink / raw)
To: Paolo Bonzini, kvm; +Cc: rkrcmar
Am 06.03.2017 um 19:08 schrieb Paolo Bonzini:
>
>
> On 06/03/2017 14:17, David Hildenbrand wrote:
>> 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.
>
>> - 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?
>
>> /* 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 b96d389..4e4a67a 100644
>> --- a/arch/x86/kvm/irq_comm.c
>> +++ b/arch/x86/kvm/irq_comm.c
>> @@ -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.
irqchip_in_kernel in its current form would allow KVM_IRQCHIP_SPLIT,
which is not what we want here, or am I missing something?
>
> 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.
> Paolo
>
Thanks!
David
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH RFC 02/21] KVM: x86: check against irqchip_mode in kvm_set_routing_entry()
2017-03-07 9:55 ` David Hildenbrand
@ 2017-03-07 10:53 ` Paolo Bonzini
2017-03-07 14:40 ` Radim Krčmář
0 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2017-03-07 10:53 UTC (permalink / raw)
To: David Hildenbrand, kvm; +Cc: rkrcmar
On 07/03/2017 10:55, David Hildenbrand wrote:
> Am 06.03.2017 um 19:08 schrieb Paolo Bonzini:
>>> - 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?
I don't think we want to add new modes.
>>
>>> /* 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 b96d389..4e4a67a 100644
>>> --- a/arch/x86/kvm/irq_comm.c
>>> +++ b/arch/x86/kvm/irq_comm.c
>>> @@ -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.
>
> 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?
>> 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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH RFC 02/21] KVM: x86: check against irqchip_mode in kvm_set_routing_entry()
2017-03-07 10:53 ` Paolo Bonzini
@ 2017-03-07 14:40 ` Radim Krčmář
2017-03-07 15:32 ` David Hildenbrand
0 siblings, 1 reply; 27+ messages in thread
From: Radim Krčmář @ 2017-03-07 14:40 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: David Hildenbrand, kvm
2017-03-07 11:53+0100, Paolo Bonzini:
> On 07/03/2017 10:55, David Hildenbrand wrote:
>> Am 06.03.2017 um 19:08 schrieb Paolo Bonzini:
>>>> - 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.
>> 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).
>>>> /* 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 b96d389..4e4a67a 100644
>>>> --- a/arch/x86/kvm/irq_comm.c
>>>> +++ b/arch/x86/kvm/irq_comm.c
>>>> @@ -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.
>>
>> 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.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH RFC 02/21] KVM: x86: check against irqchip_mode in kvm_set_routing_entry()
2017-03-07 14:40 ` Radim Krčmář
@ 2017-03-07 15:32 ` David Hildenbrand
0 siblings, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2017-03-07 15:32 UTC (permalink / raw)
To: Radim Krčmář, Paolo Bonzini; +Cc: kvm
>>> 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.
>
> 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.
>
>>>>> /* 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 b96d389..4e4a67a 100644
>>>>> --- a/arch/x86/kvm/irq_comm.c
>>>>> +++ b/arch/x86/kvm/irq_comm.c
>>>>> @@ -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.
>>>
>>> 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!
--
Thanks,
David
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2017-03-07 18:37 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-06 13:17 [PATCH RFC 00/21] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
2017-03-06 13:17 ` [PATCH RFC 01/21] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP David Hildenbrand
2017-03-06 13:17 ` [PATCH RFC 02/21] KVM: x86: check against irqchip_mode in kvm_set_routing_entry() David Hildenbrand
2017-03-06 18:08 ` Paolo Bonzini
2017-03-07 9:55 ` David Hildenbrand
2017-03-07 10:53 ` Paolo Bonzini
2017-03-07 14:40 ` Radim Krčmář
2017-03-07 15:32 ` David Hildenbrand
2017-03-06 13:17 ` [PATCH RFC 03/21] KVM: x86: check against irqchip_mode in pic_in_kernel() David Hildenbrand
2017-03-06 13:17 ` [PATCH RFC 04/21] KVM: x86: check against irqchip_mode in ioapic_in_kernel() David Hildenbrand
2017-03-06 13:17 ` [PATCH RFC 05/21] KVM: x86: get rid of pic_irqchip() David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 06/21] KVM: x86: get rid of ioapic_irqchip() David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 07/21] KVM: x86: use ioapic_in_kernel() to check for ioapic existence David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 08/21] KVM: x86: remove duplicate checks for ioapic David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 09/21] KVM: x86: convert kvm_(set|get)_ioapic() into void David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 10/21] KVM: x86: don't take kvm->irq_lock when creating IRQCHIP David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 11/21] KVM: x86: push usage of slots_lock down David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 12/21] KVM: x86: KVM_IRQCHIP_PIC_MASTER only has 8 pins David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 13/21] KVM: x86: remove all-vcpu request from kvm_ioapic_init() David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 14/21] KVM: x86: directly call kvm_make_scan_ioapic_request() in ioapic.c David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 15/21] KVM: x86: rename kvm_vcpu_request_scan_ioapic() David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 16/21] KVM: x86: drop goto label in kvm_set_routing_entry() David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 17/21] KVM: x86: simplify pic_unlock() David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 18/21] KVM: x86: make kvm_pic_reset() static David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 19/21] KVM: x86: drop picdev_in_range() David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 20/21] KVM: x86: set data directly in picdev_read() David Hildenbrand
2017-03-06 13:18 ` [PATCH RFC 21/21] KVM: x86: simplify pic_ioport_read() David Hildenbrand
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.