* [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes
@ 2017-03-23 14:13 David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 01/24] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP David Hildenbrand
` (23 more replies)
0 siblings, 24 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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.
v1 -> v2:
- Changed commit message of "KVM: x86: race between KVM_SET_GSI_ROUTING and
KVM_CREATE_IRQCHIP" to also include a hint about SPLIT IRQCHIP
- Split "KVM: x86: check against irqchip_mode in kvm_set_routing_entry()"
- return; instead of break; in "KVM: x86: simplify pic_unlock()"
- Added "KVM: x86: cleanup return handling in setup_routing_entry()"
David Hildenbrand (24):
KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP
KVM: x86: new irqchip mode KVM_IRQCHIP_INIT_IN_PROGRESS
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: cleanup return handling in setup_routing_entry()
KVM: x86: simplify pic_unlock()
KVM: x86: make kvm_pic_reset() static
KVM: x86: drop picdev_in_range()
KVM: x86: set data directly in picdev_read()
KVM: x86: simplify pic_ioport_read()
KVM: x86: use irqchip_kernel() to check for pic+ioapic
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/i8259.c | 72 ++++++++++++++++-------------------------
arch/x86/kvm/ioapic.c | 28 ++++++----------
arch/x86/kvm/ioapic.h | 14 ++------
arch/x86/kvm/irq.c | 2 +-
arch/x86/kvm/irq.h | 14 ++------
arch/x86/kvm/irq_comm.c | 43 +++++++++++-------------
arch/x86/kvm/x86.c | 41 +++++++++++------------
include/linux/kvm_host.h | 4 +--
virt/kvm/eventfd.c | 4 +--
virt/kvm/irqchip.c | 11 +++----
virt/kvm/kvm_main.c | 3 ++
12 files changed, 93 insertions(+), 144 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 01/24] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 02/24] KVM: x86: new irqchip mode KVM_IRQCHIP_INIT_IN_PROGRESS David Hildenbrand
` (22 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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.
Also, as a side effect, this patch makes sure that KVM_SET_GSI_ROUTING
and KVM_CAP_SPLIT_IRQCHIP cannot run in parallel.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
virt/kvm/kvm_main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index a17d787..ad0f8b2 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3079,8 +3079,11 @@ static long kvm_vm_ioctl(struct file *filp,
routing.nr * sizeof(*entries)))
goto out_free_irq_routing;
}
+ /* avoid races with KVM_CREATE_IRQCHIP on x86 */
+ mutex_lock(&kvm->lock);
r = kvm_set_irq_routing(kvm, entries, routing.nr,
routing.flags);
+ mutex_unlock(&kvm->lock);
out_free_irq_routing:
vfree(entries);
break;
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 02/24] KVM: x86: new irqchip mode KVM_IRQCHIP_INIT_IN_PROGRESS
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 01/24] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 03/24] KVM: x86: check against irqchip_mode in kvm_set_routing_entry() David Hildenbrand
` (21 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, david
Let's add a new mode and set it while we create the irqchip via
KVM_CREATE_IRQCHIP and KVM_CAP_SPLIT_IRQCHIP.
This mode will be used later to test if adding routes
(in kvm_set_routing_entry()) is already allowed.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/irq.h | 2 +-
arch/x86/kvm/x86.c | 7 ++++++-
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 74ef58c..164e544 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -727,6 +727,7 @@ struct kvm_hv {
enum kvm_irqchip_mode {
KVM_IRQCHIP_NONE,
+ KVM_IRQCHIP_INIT_IN_PROGRESS, /* temporarily set during creation */
KVM_IRQCHIP_KERNEL, /* created with KVM_CREATE_IRQCHIP */
KVM_IRQCHIP_SPLIT, /* created with KVM_CAP_SPLIT_IRQCHIP */
};
diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 40d5b2c..7fa2480 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -103,7 +103,7 @@ static inline int irqchip_kernel(struct kvm *kvm)
static inline int irqchip_in_kernel(struct kvm *kvm)
{
- bool ret = kvm->arch.irqchip_mode != KVM_IRQCHIP_NONE;
+ bool ret = kvm->arch.irqchip_mode > KVM_IRQCHIP_INIT_IN_PROGRESS;
/* Matches with wmb after initializing kvm->irq_routing. */
smp_rmb();
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1faf620..fa8cceb 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3934,9 +3934,12 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
goto split_irqchip_unlock;
if (kvm->created_vcpus)
goto split_irqchip_unlock;
+ kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
r = kvm_setup_empty_irq_routing(kvm);
- if (r)
+ if (r) {
+ kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
goto split_irqchip_unlock;
+ }
/* Pairs with irqchip_in_kernel. */
smp_wmb();
kvm->arch.irqchip_mode = KVM_IRQCHIP_SPLIT;
@@ -4024,8 +4027,10 @@ long kvm_arch_vm_ioctl(struct file *filp,
goto create_irqchip_unlock;
}
+ kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
r = kvm_setup_default_irq_routing(kvm);
if (r) {
+ kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
mutex_lock(&kvm->slots_lock);
mutex_lock(&kvm->irq_lock);
kvm_ioapic_destroy(kvm);
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 03/24] KVM: x86: check against irqchip_mode in kvm_set_routing_entry()
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 01/24] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 02/24] KVM: x86: new irqchip mode KVM_IRQCHIP_INIT_IN_PROGRESS David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 04/24] KVM: x86: check against irqchip_mode in pic_in_kernel() David Hildenbrand
` (20 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, david
Let's replace the checks for pic_in_kernel() and ioapic_in_kernel() by
checks against irqchip_mode.
Also make sure that creation of any route is only possible if we have
an lapic in kernel (irqchip_in_kernel()) or if we are currently
inititalizing 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/kvm/irq_comm.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 6825cd3..0a026f8 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -282,24 +282,24 @@ int kvm_set_routing_entry(struct kvm *kvm,
int delta;
unsigned max_pin;
+ /* also allow creation of routes during KVM_IRQCHIP_INIT_IN_PROGRESS */
+ if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE)
+ goto out;
+
switch (ue->type) {
case KVM_IRQ_ROUTING_IRQCHIP:
+ if (irqchip_split(kvm))
+ goto out;
delta = 0;
switch (ue->u.irqchip.irqchip) {
case KVM_IRQCHIP_PIC_SLAVE:
delta = 8;
/* fall through */
case KVM_IRQCHIP_PIC_MASTER:
- if (!pic_in_kernel(kvm))
- goto out;
-
e->set = kvm_set_pic_irq;
max_pin = PIC_NUM_PINS;
break;
case KVM_IRQCHIP_IOAPIC:
- if (!ioapic_in_kernel(kvm))
- goto out;
-
max_pin = KVM_IOAPIC_NUM_PINS;
e->set = kvm_set_ioapic_irq;
break;
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 04/24] KVM: x86: check against irqchip_mode in pic_in_kernel()
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (2 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 03/24] KVM: x86: check against irqchip_mode in kvm_set_routing_entry() David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 05/24] KVM: x86: check against irqchip_mode in ioapic_in_kernel() David Hildenbrand
` (19 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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 7fa2480..184306e 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -85,10 +85,7 @@ static inline struct kvm_pic *pic_irqchip(struct kvm *kvm)
static inline int pic_in_kernel(struct kvm *kvm)
{
- int ret;
-
- ret = (pic_irqchip(kvm) != NULL);
- return ret;
+ return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL;
}
static inline int irqchip_split(struct kvm *kvm)
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 05/24] KVM: x86: check against irqchip_mode in ioapic_in_kernel()
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (3 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 04/24] KVM: x86: check against irqchip_mode in pic_in_kernel() David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 06/24] KVM: x86: get rid of pic_irqchip() David Hildenbrand
` (18 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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] 29+ messages in thread
* [PATCH v2 06/24] KVM: x86: get rid of pic_irqchip()
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (4 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 05/24] KVM: x86: check against irqchip_mode in ioapic_in_kernel() David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 07/24] KVM: x86: get rid of ioapic_irqchip() David Hildenbrand
` (17 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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 184306e..2455f16 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -78,11 +78,6 @@ void kvm_pic_destroy(struct kvm *kvm);
int kvm_pic_read_irq(struct kvm *kvm);
void kvm_pic_update_irq(struct kvm_pic *s);
-static inline struct kvm_pic *pic_irqchip(struct kvm *kvm)
-{
- return kvm->arch.vpic;
-}
-
static inline int pic_in_kernel(struct kvm *kvm)
{
return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL;
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 0a026f8..45ab43e 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -42,7 +42,7 @@ static int kvm_set_pic_irq(struct kvm_kernel_irq_routing_entry *e,
struct kvm *kvm, int irq_source_id, int level,
bool line_status)
{
- struct kvm_pic *pic = pic_irqchip(kvm);
+ struct kvm_pic *pic = kvm->arch.vpic;
return kvm_pic_set_irq(pic, e->irqchip.pin, irq_source_id, level);
}
@@ -236,7 +236,7 @@ void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id)
goto unlock;
kvm_ioapic_clear_all(kvm->arch.vioapic, irq_source_id);
- kvm_pic_clear_all(pic_irqchip(kvm), irq_source_id);
+ kvm_pic_clear_all(kvm->arch.vpic, irq_source_id);
unlock:
mutex_unlock(&kvm->irq_lock);
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fa8cceb..079bf6b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3721,18 +3721,17 @@ static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm)
static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
{
+ struct kvm_pic *pic = kvm->arch.vpic;
int r;
r = 0;
switch (chip->chip_id) {
case KVM_IRQCHIP_PIC_MASTER:
- memcpy(&chip->chip.pic,
- &pic_irqchip(kvm)->pics[0],
+ memcpy(&chip->chip.pic, &pic->pics[0],
sizeof(struct kvm_pic_state));
break;
case KVM_IRQCHIP_PIC_SLAVE:
- memcpy(&chip->chip.pic,
- &pic_irqchip(kvm)->pics[1],
+ memcpy(&chip->chip.pic, &pic->pics[1],
sizeof(struct kvm_pic_state));
break;
case KVM_IRQCHIP_IOAPIC:
@@ -3747,23 +3746,22 @@ static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
{
+ struct kvm_pic *pic = kvm->arch.vpic;
int r;
r = 0;
switch (chip->chip_id) {
case KVM_IRQCHIP_PIC_MASTER:
- spin_lock(&pic_irqchip(kvm)->lock);
- memcpy(&pic_irqchip(kvm)->pics[0],
- &chip->chip.pic,
+ spin_lock(&pic->lock);
+ memcpy(&pic->pics[0], &chip->chip.pic,
sizeof(struct kvm_pic_state));
- spin_unlock(&pic_irqchip(kvm)->lock);
+ spin_unlock(&pic->lock);
break;
case KVM_IRQCHIP_PIC_SLAVE:
- spin_lock(&pic_irqchip(kvm)->lock);
- memcpy(&pic_irqchip(kvm)->pics[1],
- &chip->chip.pic,
+ spin_lock(&pic->lock);
+ memcpy(&pic->pics[1], &chip->chip.pic,
sizeof(struct kvm_pic_state));
- spin_unlock(&pic_irqchip(kvm)->lock);
+ spin_unlock(&pic->lock);
break;
case KVM_IRQCHIP_IOAPIC:
r = kvm_set_ioapic(kvm, &chip->chip.ioapic);
@@ -3772,7 +3770,7 @@ static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
r = -EINVAL;
break;
}
- kvm_pic_update_irq(pic_irqchip(kvm));
+ kvm_pic_update_irq(pic);
return r;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 07/24] KVM: x86: get rid of ioapic_irqchip()
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (5 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 06/24] KVM: x86: get rid of pic_irqchip() David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 08/24] KVM: x86: use ioapic_in_kernel() to check for ioapic existence David Hildenbrand
` (16 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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] 29+ messages in thread
* [PATCH v2 08/24] KVM: x86: use ioapic_in_kernel() to check for ioapic existence
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (6 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 07/24] KVM: x86: get rid of ioapic_irqchip() David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 09/24] KVM: x86: remove duplicate checks for ioapic David Hildenbrand
` (15 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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] 29+ messages in thread
* [PATCH v2 09/24] KVM: x86: remove duplicate checks for ioapic
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (7 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 08/24] KVM: x86: use ioapic_in_kernel() to check for ioapic existence David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 10/24] KVM: x86: convert kvm_(set|get)_ioapic() into void David Hildenbrand
` (14 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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] 29+ messages in thread
* [PATCH v2 10/24] KVM: x86: convert kvm_(set|get)_ioapic() into void
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (8 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 09/24] KVM: x86: remove duplicate checks for ioapic David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 11/24] KVM: x86: don't take kvm->irq_lock when creating IRQCHIP David Hildenbrand
` (13 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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 079bf6b..1636da0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3735,7 +3735,7 @@ static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
sizeof(struct kvm_pic_state));
break;
case KVM_IRQCHIP_IOAPIC:
- r = kvm_get_ioapic(kvm, &chip->chip.ioapic);
+ kvm_get_ioapic(kvm, &chip->chip.ioapic);
break;
default:
r = -EINVAL;
@@ -3764,7 +3764,7 @@ static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
spin_unlock(&pic->lock);
break;
case KVM_IRQCHIP_IOAPIC:
- r = kvm_set_ioapic(kvm, &chip->chip.ioapic);
+ kvm_set_ioapic(kvm, &chip->chip.ioapic);
break;
default:
r = -EINVAL;
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 11/24] KVM: x86: don't take kvm->irq_lock when creating IRQCHIP
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (9 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 10/24] KVM: x86: convert kvm_(set|get)_ioapic() into void David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-28 8:28 ` Paolo Bonzini
2017-03-23 14:13 ` [PATCH v2 12/24] KVM: x86: push usage of slots_lock down David Hildenbrand
` (12 subsequent siblings)
23 siblings, 1 reply; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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 1636da0..db1a5ff 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4030,10 +4030,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
if (r) {
kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
mutex_lock(&kvm->slots_lock);
- mutex_lock(&kvm->irq_lock);
kvm_ioapic_destroy(kvm);
kvm_pic_destroy(kvm);
- mutex_unlock(&kvm->irq_lock);
mutex_unlock(&kvm->slots_lock);
goto create_irqchip_unlock;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 12/24] KVM: x86: push usage of slots_lock down
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (10 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 11/24] KVM: x86: don't take kvm->irq_lock when creating IRQCHIP David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 13/24] KVM: x86: KVM_IRQCHIP_PIC_MASTER only has 8 pins David Hildenbrand
` (11 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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 db1a5ff..8d6418e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4019,9 +4019,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = kvm_ioapic_init(kvm);
if (r) {
- mutex_lock(&kvm->slots_lock);
kvm_pic_destroy(kvm);
- mutex_unlock(&kvm->slots_lock);
goto create_irqchip_unlock;
}
@@ -4029,10 +4027,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = kvm_setup_default_irq_routing(kvm);
if (r) {
kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
- mutex_lock(&kvm->slots_lock);
kvm_ioapic_destroy(kvm);
kvm_pic_destroy(kvm);
- mutex_unlock(&kvm->slots_lock);
goto create_irqchip_unlock;
}
/* Write kvm->irq_routing before enabling irqchip_in_kernel. */
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 13/24] KVM: x86: KVM_IRQCHIP_PIC_MASTER only has 8 pins
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (11 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 12/24] KVM: x86: push usage of slots_lock down David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 14/24] KVM: x86: remove all-vcpu request from kvm_ioapic_init() David Hildenbrand
` (10 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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 45ab43e..34acbec 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -279,8 +279,6 @@ int kvm_set_routing_entry(struct kvm *kvm,
const struct kvm_irq_routing_entry *ue)
{
int r = -EINVAL;
- int delta;
- unsigned max_pin;
/* also allow creation of routes during KVM_IRQCHIP_INIT_IN_PROGRESS */
if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE)
@@ -290,26 +288,25 @@ int kvm_set_routing_entry(struct kvm *kvm,
case KVM_IRQ_ROUTING_IRQCHIP:
if (irqchip_split(kvm))
goto out;
- delta = 0;
+ e->irqchip.pin = ue->u.irqchip.pin;
switch (ue->u.irqchip.irqchip) {
case KVM_IRQCHIP_PIC_SLAVE:
- delta = 8;
+ e->irqchip.pin += PIC_NUM_PINS / 2;
/* fall through */
case KVM_IRQCHIP_PIC_MASTER:
+ if (ue->u.irqchip.pin >= PIC_NUM_PINS / 2)
+ goto out;
e->set = kvm_set_pic_irq;
- max_pin = PIC_NUM_PINS;
break;
case KVM_IRQCHIP_IOAPIC:
- max_pin = KVM_IOAPIC_NUM_PINS;
+ if (ue->u.irqchip.pin >= KVM_IOAPIC_NUM_PINS)
+ goto out;
e->set = kvm_set_ioapic_irq;
break;
default:
goto out;
}
e->irqchip.irqchip = ue->u.irqchip.irqchip;
- e->irqchip.pin = ue->u.irqchip.pin + delta;
- if (e->irqchip.pin >= max_pin)
- goto out;
break;
case KVM_IRQ_ROUTING_MSI:
e->set = kvm_set_msi;
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 14/24] KVM: x86: remove all-vcpu request from kvm_ioapic_init()
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (12 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 13/24] KVM: x86: KVM_IRQCHIP_PIC_MASTER only has 8 pins David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 15/24] KVM: x86: directly call kvm_make_scan_ioapic_request() in ioapic.c David Hildenbrand
` (9 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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] 29+ messages in thread
* [PATCH v2 15/24] KVM: x86: directly call kvm_make_scan_ioapic_request() in ioapic.c
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (13 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 14/24] KVM: x86: remove all-vcpu request from kvm_ioapic_init() David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 16/24] KVM: x86: rename kvm_vcpu_request_scan_ioapic() David Hildenbrand
` (8 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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] 29+ messages in thread
* [PATCH v2 16/24] KVM: x86: rename kvm_vcpu_request_scan_ioapic()
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (14 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 15/24] KVM: x86: directly call kvm_make_scan_ioapic_request() in ioapic.c David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 17/24] KVM: x86: drop goto label in kvm_set_routing_entry() David Hildenbrand
` (7 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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] 29+ messages in thread
* [PATCH v2 17/24] KVM: x86: drop goto label in kvm_set_routing_entry()
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (15 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 16/24] KVM: x86: rename kvm_vcpu_request_scan_ioapic() David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 18/24] KVM: x86: cleanup return handling in setup_routing_entry() David Hildenbrand
` (6 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, david
No need for the goto label + local variable "r".
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/x86/kvm/irq_comm.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 34acbec..1728ff1 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -278,16 +278,14 @@ int kvm_set_routing_entry(struct kvm *kvm,
struct kvm_kernel_irq_routing_entry *e,
const struct kvm_irq_routing_entry *ue)
{
- int r = -EINVAL;
-
/* also allow creation of routes during KVM_IRQCHIP_INIT_IN_PROGRESS */
if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE)
- goto out;
+ return -EINVAL;
switch (ue->type) {
case KVM_IRQ_ROUTING_IRQCHIP:
if (irqchip_split(kvm))
- goto out;
+ return -EINVAL;
e->irqchip.pin = ue->u.irqchip.pin;
switch (ue->u.irqchip.irqchip) {
case KVM_IRQCHIP_PIC_SLAVE:
@@ -295,16 +293,16 @@ int kvm_set_routing_entry(struct kvm *kvm,
/* fall through */
case KVM_IRQCHIP_PIC_MASTER:
if (ue->u.irqchip.pin >= PIC_NUM_PINS / 2)
- goto out;
+ return -EINVAL;
e->set = kvm_set_pic_irq;
break;
case KVM_IRQCHIP_IOAPIC:
if (ue->u.irqchip.pin >= KVM_IOAPIC_NUM_PINS)
- goto out;
+ return -EINVAL;
e->set = kvm_set_ioapic_irq;
break;
default:
- goto out;
+ return -EINVAL;
}
e->irqchip.irqchip = ue->u.irqchip.irqchip;
break;
@@ -315,7 +313,7 @@ int kvm_set_routing_entry(struct kvm *kvm,
e->msi.data = ue->u.msi.data;
if (kvm_msi_route_invalid(kvm, e))
- goto out;
+ return -EINVAL;
break;
case KVM_IRQ_ROUTING_HV_SINT:
e->set = kvm_hv_set_sint;
@@ -323,12 +321,10 @@ int kvm_set_routing_entry(struct kvm *kvm,
e->hv_sint.sint = ue->u.hv_sint.sint;
break;
default:
- goto out;
+ return -EINVAL;
}
- r = 0;
-out:
- return r;
+ return 0;
}
bool kvm_intr_is_single_vcpu(struct kvm *kvm, struct kvm_lapic_irq *irq,
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 18/24] KVM: x86: cleanup return handling in setup_routing_entry()
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (16 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 17/24] KVM: x86: drop goto label in kvm_set_routing_entry() David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 19/24] KVM: x86: simplify pic_unlock() David Hildenbrand
` (5 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, david
Let's drop the goto and return the error value directly.
Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
virt/kvm/irqchip.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index 3bcc999..cc30d01 100644
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -142,8 +142,8 @@ static int setup_routing_entry(struct kvm *kvm,
struct kvm_kernel_irq_routing_entry *e,
const struct kvm_irq_routing_entry *ue)
{
- int r = -EINVAL;
struct kvm_kernel_irq_routing_entry *ei;
+ int r;
/*
* Do not allow GSI to be mapped to the same irqchip more than once.
@@ -153,20 +153,19 @@ static int setup_routing_entry(struct kvm *kvm,
if (ei->type != KVM_IRQ_ROUTING_IRQCHIP ||
ue->type != KVM_IRQ_ROUTING_IRQCHIP ||
ue->u.irqchip.irqchip == ei->irqchip.irqchip)
- return r;
+ return -EINVAL;
e->gsi = ue->gsi;
e->type = ue->type;
r = kvm_set_routing_entry(kvm, e, ue);
if (r)
- goto out;
+ return r;
if (e->type == KVM_IRQ_ROUTING_IRQCHIP)
rt->chip[e->irqchip.irqchip][e->irqchip.pin] = e->gsi;
hlist_add_head(&e->link, &rt->map[e->gsi]);
- r = 0;
-out:
- return r;
+
+ return 0;
}
void __attribute__((weak)) kvm_arch_irq_routing_update(struct kvm *kvm)
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 19/24] KVM: x86: simplify pic_unlock()
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (17 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 18/24] KVM: x86: cleanup return handling in setup_routing_entry() David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 20/24] KVM: x86: make kvm_pic_reset() static David Hildenbrand
` (4 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, david
We can easily compact this code and get rid of one local variable.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/x86/kvm/i8259.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index e77e8c3..34fa982 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;
- break;
+ kvm_make_request(KVM_REQ_EVENT, vcpu);
+ kvm_vcpu_kick(vcpu);
+ return;
}
}
-
- if (!found)
- return;
-
- kvm_make_request(KVM_REQ_EVENT, found);
- kvm_vcpu_kick(found);
}
}
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 20/24] KVM: x86: make kvm_pic_reset() static
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (18 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 19/24] KVM: x86: simplify pic_unlock() David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 21/24] KVM: x86: drop picdev_in_range() David Hildenbrand
` (3 subsequent siblings)
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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 34fa982..a871e08 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -268,7 +268,7 @@ int kvm_pic_read_irq(struct kvm *kvm)
return intno;
}
-void kvm_pic_reset(struct kvm_kpic_state *s)
+static void kvm_pic_reset(struct kvm_kpic_state *s)
{
int irq, i;
struct kvm_vcpu *vcpu;
diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 2455f16..ae29332 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -102,8 +102,6 @@ static inline int irqchip_in_kernel(struct kvm *kvm)
return ret;
}
-void kvm_pic_reset(struct kvm_kpic_state *s);
-
void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu);
void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu);
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 21/24] KVM: x86: drop picdev_in_range()
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (19 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 20/24] KVM: x86: make kvm_pic_reset() static David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
[not found] ` <CABayD+fZxXb+Z0wjfaf7h3-yZGOZqC-5L4kKdDYsL6p+sDTyMw@mail.gmail.com>
2017-03-23 14:13 ` [PATCH v2 22/24] KVM: x86: set data directly in picdev_read() David Hildenbrand
` (2 subsequent siblings)
23 siblings, 1 reply; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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 a871e08..dda4abd 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] 29+ messages in thread
* [PATCH v2 22/24] KVM: x86: set data directly in picdev_read()
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (20 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 21/24] KVM: x86: drop picdev_in_range() David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 23/24] KVM: x86: simplify pic_ioport_read() David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 24/24] KVM: x86: use irqchip_kernel() to check for pic+ioapic David Hildenbrand
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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 dda4abd..3a8be26 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] 29+ messages in thread
* [PATCH v2 23/24] KVM: x86: simplify pic_ioport_read()
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (21 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 22/24] KVM: x86: set data directly in picdev_read() David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 24/24] KVM: x86: use irqchip_kernel() to check for pic+ioapic David Hildenbrand
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, 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 3a8be26..d07a629 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] 29+ messages in thread
* [PATCH v2 24/24] KVM: x86: use irqchip_kernel() to check for pic+ioapic
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
` (22 preceding siblings ...)
2017-03-23 14:13 ` [PATCH v2 23/24] KVM: x86: simplify pic_ioport_read() David Hildenbrand
@ 2017-03-23 14:13 ` David Hildenbrand
23 siblings, 0 replies; 29+ messages in thread
From: David Hildenbrand @ 2017-03-23 14:13 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, rkrcmar, Peter Xu, david
Although the current check is not wrong, this check explicitly includes
the pic.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/x86/kvm/irq_comm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 1728ff1..5494395 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -232,7 +232,7 @@ void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id)
goto unlock;
}
clear_bit(irq_source_id, &kvm->arch.irq_sources_bitmap);
- if (!ioapic_in_kernel(kvm))
+ if (!irqchip_kernel(kvm))
goto unlock;
kvm_ioapic_clear_all(kvm->arch.vioapic, irq_source_id);
--
2.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 11/24] KVM: x86: don't take kvm->irq_lock when creating IRQCHIP
2017-03-23 14:13 ` [PATCH v2 11/24] KVM: x86: don't take kvm->irq_lock when creating IRQCHIP David Hildenbrand
@ 2017-03-28 8:28 ` Paolo Bonzini
2017-04-03 7:27 ` David Hildenbrand
0 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2017-03-28 8:28 UTC (permalink / raw)
To: David Hildenbrand, kvm; +Cc: rkrcmar, Peter Xu
On 23/03/2017 15:13, David Hildenbrand wrote:
> 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.
Do more readers of irqchip_mode need memory barriers now? Definitely we need an smp_wmb() after setting INIT_IN_PROGRESS.
Something like this:
diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
index 3eb140ba57b6..b1df4d53fad1 100644
--- a/arch/x86/kvm/ioapic.h
+++ b/arch/x86/kvm/ioapic.h
@@ -107,7 +107,9 @@ struct kvm_ioapic {
static inline int ioapic_in_kernel(struct kvm *kvm)
{
- return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL;
+ int mode = kvm->arch.irqchip_mode;
+ smp_rmb();
+ return mode == KVM_IRQCHIP_KERNEL;
}
void kvm_rtc_eoi_tracking_restore_one(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 2455f16ee98a..6f7b3fb92dd4 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -80,26 +80,30 @@ struct kvm_pic {
static inline int pic_in_kernel(struct kvm *kvm)
{
- return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL;
+ int mode = kvm->arch.irqchip_mode;
+ smp_rmb();
+ return mode == KVM_IRQCHIP_KERNEL;
}
static inline int irqchip_split(struct kvm *kvm)
{
- return kvm->arch.irqchip_mode == KVM_IRQCHIP_SPLIT;
+ int mode = kvm->arch.irqchip_mode;
+ smp_rmb();
+ return mode == KVM_IRQCHIP_SPLIT;
}
static inline int irqchip_kernel(struct kvm *kvm)
{
- return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL;
+ int mode = kvm->arch.irqchip_mode;
+ smp_rmb();
+ return mode == KVM_IRQCHIP_KERNEL;
}
static inline int irqchip_in_kernel(struct kvm *kvm)
{
- bool ret = kvm->arch.irqchip_mode > KVM_IRQCHIP_INIT_IN_PROGRESS;
-
- /* Matches with wmb after initializing kvm->irq_routing. */
+ int mode = kvm->arch.irqchip_mode;
smp_rmb();
- return ret;
+ return mode > KVM_IRQCHIP_INIT_IN_PROGRESS;
}
void kvm_pic_reset(struct kvm_kpic_state *s);
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 45ab43e5fbc5..e8306eeb5613 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -286,6 +286,7 @@ int kvm_set_routing_entry(struct kvm *kvm,
if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE)
goto out;
+ smp_rmb();
switch (ue->type) {
case KVM_IRQ_ROUTING_IRQCHIP:
if (irqchip_split(kvm))
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5dca5aed9c93..ed9143f305f9 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3933,6 +3933,8 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
if (kvm->created_vcpus)
goto split_irqchip_unlock;
kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
+ /* Pairs with pic_in_kernel/ioapic_in_kernel/etc. */
+ smp_wmb();
r = kvm_setup_empty_irq_routing(kvm);
if (r) {
kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
@@ -4026,6 +4028,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
}
kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
+ /* Pairs with pic_in_kernel/ioapic_in_kernel/etc. */
+ smp_wmb();
r = kvm_setup_default_irq_routing(kvm);
if (r) {
kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
but with each part properly squashed into the right patch.
Paolo
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> arch/x86/kvm/x86.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1636da0..db1a5ff 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4030,10 +4030,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
> if (r) {
> kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
> mutex_lock(&kvm->slots_lock);
> - mutex_lock(&kvm->irq_lock);
> kvm_ioapic_destroy(kvm);
> kvm_pic_destroy(kvm);
> - mutex_unlock(&kvm->irq_lock);
> mutex_unlock(&kvm->slots_lock);
> goto create_irqchip_unlock;
> }
>
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 21/24] KVM: x86: drop picdev_in_range()
[not found] ` <CABayD+fZxXb+Z0wjfaf7h3-yZGOZqC-5L4kKdDYsL6p+sDTyMw@mail.gmail.com>
@ 2017-03-29 8:58 ` Paolo Bonzini
0 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2017-03-29 8:58 UTC (permalink / raw)
To: Steve Rutherford, David Hildenbrand
Cc: KVM list, Radim Krčmář, Peter Xu
On 29/03/2017 02:28, Steve Rutherford wrote:
> Not sure this matters, but this changes behavior slightly. This reorders
> validation of the length and and validation of the operation.
This would change behavior if for example:
- KVM registered port 0x22
- the guest wrote 4 bytes to port 0x1f
Before, picdev_write would return -EOPNOTSUPP and KVM would invoke the
handler for port 0x22; now it would return 0.
Since the PIT ports are far from anything else registered by the kernel,
this should not matter.
Paolo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 11/24] KVM: x86: don't take kvm->irq_lock when creating IRQCHIP
2017-03-28 8:28 ` Paolo Bonzini
@ 2017-04-03 7:27 ` David Hildenbrand
2017-04-03 8:58 ` Paolo Bonzini
0 siblings, 1 reply; 29+ messages in thread
From: David Hildenbrand @ 2017-04-03 7:27 UTC (permalink / raw)
To: Paolo Bonzini, kvm; +Cc: rkrcmar, Peter Xu
On 28.03.2017 10:28, Paolo Bonzini wrote:
>
>
> On 23/03/2017 15:13, David Hildenbrand wrote:
>> 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.
>
> Do more readers of irqchip_mode need memory barriers now? Definitely we need an smp_wmb() after setting INIT_IN_PROGRESS.
>
My assumption was that if vpic/vioapic checks worked until now without
memory barriers, the same should be true when checking against
irqchip_mode. But as we are dropping some implicit barriers and
irqchip_mode is updated more frequently, you may be right.
Also, using rmb/wmb in a uniform fashion with irqchip_mode looks
cleaner, as we are removing all implicit "cannot happen because of XXX"
special cases.
> Something like this:
>
> diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
> index 3eb140ba57b6..b1df4d53fad1 100644
> --- a/arch/x86/kvm/ioapic.h
> +++ b/arch/x86/kvm/ioapic.h
> @@ -107,7 +107,9 @@ struct kvm_ioapic {
>
> static inline int ioapic_in_kernel(struct kvm *kvm)
> {
> - return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL;
> + int mode = kvm->arch.irqchip_mode;
> + smp_rmb();
> + return mode == KVM_IRQCHIP_KERNEL;
> }
>
> void kvm_rtc_eoi_tracking_restore_one(struct kvm_vcpu *vcpu);
> diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
> index 2455f16ee98a..6f7b3fb92dd4 100644
> --- a/arch/x86/kvm/irq.h
> +++ b/arch/x86/kvm/irq.h
> @@ -80,26 +80,30 @@ struct kvm_pic {
>
> static inline int pic_in_kernel(struct kvm *kvm)
> {
> - return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL;
> + int mode = kvm->arch.irqchip_mode;
> + smp_rmb();
> + return mode == KVM_IRQCHIP_KERNEL;
> }
>
> static inline int irqchip_split(struct kvm *kvm)
> {
> - return kvm->arch.irqchip_mode == KVM_IRQCHIP_SPLIT;
> + int mode = kvm->arch.irqchip_mode;
> + smp_rmb();
> + return mode == KVM_IRQCHIP_SPLIT;
> }
>
> static inline int irqchip_kernel(struct kvm *kvm)
> {
> - return kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL;
> + int mode = kvm->arch.irqchip_mode;
> + smp_rmb();
> + return mode == KVM_IRQCHIP_KERNEL;
> }
>
> static inline int irqchip_in_kernel(struct kvm *kvm)
> {
> - bool ret = kvm->arch.irqchip_mode > KVM_IRQCHIP_INIT_IN_PROGRESS;
> -
> - /* Matches with wmb after initializing kvm->irq_routing. */
> + int mode = kvm->arch.irqchip_mode;
> smp_rmb();
> - return ret;
> + return mode > KVM_IRQCHIP_INIT_IN_PROGRESS;
> }
>
> void kvm_pic_reset(struct kvm_kpic_state *s);
> diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
> index 45ab43e5fbc5..e8306eeb5613 100644
> --- a/arch/x86/kvm/irq_comm.c
> +++ b/arch/x86/kvm/irq_comm.c
> @@ -286,6 +286,7 @@ int kvm_set_routing_entry(struct kvm *kvm,
> if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE)
> goto out;
>
> + smp_rmb();
> switch (ue->type) {
> case KVM_IRQ_ROUTING_IRQCHIP:
> if (irqchip_split(kvm))
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5dca5aed9c93..ed9143f305f9 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3933,6 +3933,8 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> if (kvm->created_vcpus)
> goto split_irqchip_unlock;
> kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
> + /* Pairs with pic_in_kernel/ioapic_in_kernel/etc. */
> + smp_wmb();
> r = kvm_setup_empty_irq_routing(kvm);
> if (r) {
> kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
> @@ -4026,6 +4028,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
> }
>
> kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
> + /* Pairs with pic_in_kernel/ioapic_in_kernel/etc. */
> + smp_wmb();
> r = kvm_setup_default_irq_routing(kvm);
> if (r) {
> kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
>
> but with each part properly squashed into the right patch.
Sure, thanks!
>
> Paolo
--
Thanks,
David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 11/24] KVM: x86: don't take kvm->irq_lock when creating IRQCHIP
2017-04-03 7:27 ` David Hildenbrand
@ 2017-04-03 8:58 ` Paolo Bonzini
0 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2017-04-03 8:58 UTC (permalink / raw)
To: David Hildenbrand, kvm; +Cc: rkrcmar, Peter Xu
On 03/04/2017 09:27, David Hildenbrand wrote:
>>> 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.
>> Do more readers of irqchip_mode need memory barriers now? Definitely we need an smp_wmb() after setting INIT_IN_PROGRESS.
>>
> My assumption was that if vpic/vioapic checks worked until now without
> memory barriers, the same should be true when checking against
> irqchip_mode.
All that was missing previously was smp_read_barrier_depends(). While a
compiler _can_ do things that are prohibited by
smp_read_barrier_depends(), it's much harder to trigger than for
smp_rmb(). For smp_rmb(), the scheduler's whims may cause it to break
the intended semantics.
Paolo
> But as we are dropping some implicit barriers and
> irqchip_mode is updated more frequently, you may be right.
>
> Also, using rmb/wmb in a uniform fashion with irqchip_mode looks
> cleaner, as we are removing all implicit "cannot happen because of XXX"
> special cases.
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2017-04-03 8:58 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-23 14:13 [PATCH v2 00/24] pic/ioapic/irqchip cleanups + minor fixes David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 01/24] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 02/24] KVM: x86: new irqchip mode KVM_IRQCHIP_INIT_IN_PROGRESS David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 03/24] KVM: x86: check against irqchip_mode in kvm_set_routing_entry() David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 04/24] KVM: x86: check against irqchip_mode in pic_in_kernel() David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 05/24] KVM: x86: check against irqchip_mode in ioapic_in_kernel() David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 06/24] KVM: x86: get rid of pic_irqchip() David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 07/24] KVM: x86: get rid of ioapic_irqchip() David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 08/24] KVM: x86: use ioapic_in_kernel() to check for ioapic existence David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 09/24] KVM: x86: remove duplicate checks for ioapic David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 10/24] KVM: x86: convert kvm_(set|get)_ioapic() into void David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 11/24] KVM: x86: don't take kvm->irq_lock when creating IRQCHIP David Hildenbrand
2017-03-28 8:28 ` Paolo Bonzini
2017-04-03 7:27 ` David Hildenbrand
2017-04-03 8:58 ` Paolo Bonzini
2017-03-23 14:13 ` [PATCH v2 12/24] KVM: x86: push usage of slots_lock down David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 13/24] KVM: x86: KVM_IRQCHIP_PIC_MASTER only has 8 pins David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 14/24] KVM: x86: remove all-vcpu request from kvm_ioapic_init() David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 15/24] KVM: x86: directly call kvm_make_scan_ioapic_request() in ioapic.c David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 16/24] KVM: x86: rename kvm_vcpu_request_scan_ioapic() David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 17/24] KVM: x86: drop goto label in kvm_set_routing_entry() David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 18/24] KVM: x86: cleanup return handling in setup_routing_entry() David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 19/24] KVM: x86: simplify pic_unlock() David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 20/24] KVM: x86: make kvm_pic_reset() static David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 21/24] KVM: x86: drop picdev_in_range() David Hildenbrand
[not found] ` <CABayD+fZxXb+Z0wjfaf7h3-yZGOZqC-5L4kKdDYsL6p+sDTyMw@mail.gmail.com>
2017-03-29 8:58 ` Paolo Bonzini
2017-03-23 14:13 ` [PATCH v2 22/24] KVM: x86: set data directly in picdev_read() David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 23/24] KVM: x86: simplify pic_ioport_read() David Hildenbrand
2017-03-23 14:13 ` [PATCH v2 24/24] KVM: x86: use irqchip_kernel() to check for pic+ioapic 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.