Am 16.12.2016 um 16:10 schrieb Radim Krčmář:
We don't treat kvm->arch.vpic specially anymore, so the setup can look
like ioapic. This gets a bit more information out of return values.
Reviewed-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Signed-off-by: Radim Krčmář <rkrcmar@xxxxxxxxxx>
---
v2: r-b Paolo
---
arch/x86/kvm/i8259.c | 16 +++++++++++-----
arch/x86/kvm/irq.h | 4 ++--
arch/x86/kvm/x86.c | 30 +++++++++++++++---------------
3 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index 7cc2360f1848..73ea24d4f119 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -598,14 +598,14 @@ static const struct kvm_io_device_ops picdev_eclr_ops = {
.write = picdev_eclr_write,
};
-struct kvm_pic *kvm_create_pic(struct kvm *kvm)
+int kvm_pic_init(struct kvm *kvm)
{
struct kvm_pic *s;
int ret;
s = kzalloc(sizeof(struct kvm_pic), GFP_KERNEL);
if (!s)
- return NULL;
+ return -ENOMEM;
spin_lock_init(&s->lock);
s->kvm = kvm;
s->pics[0].elcr_mask = 0xf8;
@@ -635,7 +635,9 @@ struct kvm_pic *kvm_create_pic(struct kvm *kvm)
mutex_unlock(&kvm->slots_lock);
- return s;
+ kvm->arch.vpic = s;
+
+ return 0;
fail_unreg_1:
kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &s->dev_slave);
@@ -648,13 +650,17 @@ struct kvm_pic *kvm_create_pic(struct kvm *kvm)
kfree(s);
- return NULL;
+ return ret;
}
-void kvm_destroy_pic(struct kvm_pic *vpic)
+void kvm_pic_destroy(struct kvm *kvm)
{
+ struct kvm_pic *vpic = kvm->arch.vpic;
+
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);
+
+ kvm->arch.vpic = NULL;
kfree(vpic);
}
diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 79cfc945125c..13248d4d306c 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -73,8 +73,8 @@ struct kvm_pic {
unsigned long irq_states[PIC_NUM_PINS];
};
-struct kvm_pic *kvm_create_pic(struct kvm *kvm);
-void kvm_destroy_pic(struct kvm_pic *vpic);
+int kvm_pic_init(struct kvm *kvm);
+void kvm_pic_destroy(struct kvm *kvm);
int kvm_pic_read_irq(struct kvm *kvm);
void kvm_pic_update_irq(struct kvm_pic *s);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a8dbfb4129c5..2fa004029b37 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3935,33 +3935,34 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = kvm_vm_ioctl_get_nr_mmu_pages(kvm);
break;
case KVM_CREATE_IRQCHIP: {
- struct kvm_pic *vpic;
-
mutex_lock(&kvm->lock);
+
r = -EEXIST;
if (irqchip_in_kernel(kvm))
goto create_irqchip_unlock;
+
r = -EINVAL;
if (kvm->created_vcpus)
goto create_irqchip_unlock;
- r = -ENOMEM;
- vpic = kvm_create_pic(kvm);
- if (vpic) {
- r = kvm_ioapic_init(kvm);
- if (r) {
- mutex_lock(&kvm->slots_lock);
- kvm_destroy_pic(vpic);
- mutex_unlock(&kvm->slots_lock);
- goto create_irqchip_unlock;
- }
- } else
+
+ r = kvm_pic_init(kvm);
+ if (r)
goto create_irqchip_unlock;
+
+ 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;
+ }
+
r = kvm_setup_default_irq_routing(kvm);
if (r) {
mutex_lock(&kvm->slots_lock);
mutex_lock(&kvm->irq_lock);
kvm_ioapic_destroy(kvm);
- kvm_destroy_pic(vpic);
+ kvm_pic_destroy(kvm);
mutex_unlock(&kvm->irq_lock);
mutex_unlock(&kvm->slots_lock);
goto create_irqchip_unlock;
@@ -3969,7 +3970,6 @@ long kvm_arch_vm_ioctl(struct file *filp,
/* Write kvm->irq_routing before enabling irqchip_in_kernel. */
smp_wmb();
kvm->arch.irqchip_mode = KVM_IRQCHIP_KERNEL;
- kvm->arch.vpic = vpic;
This originally saved us from a race condition as far as I can
reconstruct from the commit history. Think the problem was
vpic being set but routes not being set up yet.
commit 71ba994c94a81c37185ef2fb5190844286ba9aca
Author: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Date: Wed Jul 29 12:31:15 2015 +0200
KVM: x86: clean/fix memory barriers in irqchip_in_kernel
The memory barriers are trying to protect against concurrent RCU-based
interrupt injection, but the IRQ routing table is not valid at the time
kvm->arch.vpic is written. Fix this by writing kvm->arch.vpic last.
kvm_destroy_pic then need not set kvm->arch.vpic to NULL; modify it
to take a struct kvm_pic* and reuse it if the IOAPIC creation fails.
Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
I assume that this is now fixed via the irqchip_mode, as it is stored
last? If so, I really like this patch :)
create_irqchip_unlock:
mutex_unlock(&kvm->lock);
break;
--
David
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
