Skip to contentSkip to conversation

KVM: kvm->arch.vioapic should be NULL if kvm_ioapic_init() failure

PATCH Applied Wei Yongjun 2 participants 5 messages Feb 9, 2010

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

Wei Yongjun

author
#0 lore raw

PATCH 1/4 KVM: kvm->arch.vioapic should be NULL if kvm_ioapic_init() failure

kvm->arch.vioapic should be NULL in case of kvm_ioapic_init() failure due to cannot register io dev.

Signed-off-by Wei Yongjun <[email protected]>
---
 virt/kvm/ioapic.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
virt/kvm/ioapic.c +3 -1
index a2edfd1..f3d0693 100644
@@ -393,8 +393,10 @@ int kvm_ioapic_init(struct kvm *kvm)
 	mutex_lock(&kvm->slots_lock);
 	ret = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS, &ioapic->dev);
 	mutex_unlock(&kvm->slots_lock);
-	if (ret < 0)
+	if (ret < 0) {
+		kvm->arch.vioapic = NULL;
 		kfree(ioapic);
+	}
 
 	return ret;
 }
Signature · 1 line
1.6.3.3
author
#1 lore raw

PATCH 2/4 KVM: cleanup the failure path of KVM_CREATE_IRQCHIP ioctrl

If we fail to init ioapic device or the fail to setup the default irq routing, the device register by kvm_create_pic() and kvm_ioapic_init() remain unregister. This patch fixed to do this.

Signed-off-by Wei Yongjun <[email protected]>
---
 arch/x86/kvm/i8259.c |   11 +++++++++++
 arch/x86/kvm/irq.h   |    1 +
 arch/x86/kvm/x86.c   |    8 ++++----
 virt/kvm/ioapic.c    |   11 +++++++++++
 virt/kvm/ioapic.h    |    1 +
 5 files changed, 28 insertions(+), 4 deletions(-)
arch/x86/kvm/i8259.c +11 -0
index d5753a7..a3711f9 100644
@@ -543,3 +543,14 @@ struct kvm_pic *kvm_create_pic(struct kvm *kvm)
 
 	return s;
 }
+
+void kvm_destroy_pic(struct kvm *kvm)
+{
+	struct kvm_pic *vpic = kvm->arch.vpic;
+
+	if (vpic) {
+		kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &vpic->dev);
+		kvm->arch.vpic = NULL;
+		kfree(vpic);
+	}
+}
arch/x86/kvm/irq.h +1 -0
index be399e2..0b71d48 100644
@@ -75,6 +75,7 @@ struct kvm_pic {
 };
 
 struct kvm_pic *kvm_create_pic(struct kvm *kvm);
+void kvm_destroy_pic(struct kvm *kvm);
 int kvm_pic_read_irq(struct kvm *kvm);
 void kvm_pic_update_irq(struct kvm_pic *s);
 void kvm_pic_clear_isr_ack(struct kvm *kvm);
arch/x86/kvm/x86.c +4 -4
index c91007f..db5f1fa 100644
@@ -2771,6 +2771,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 		if (vpic) {
 			r = kvm_ioapic_init(kvm);
 			if (r) {
+				kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS,
+							  &vpic->dev);
 				kfree(vpic);
 				goto create_irqchip_unlock;
 			}

@@ -2782,10 +2784,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 		r = kvm_setup_default_irq_routing(kvm);
 		if (r) {
 			mutex_lock(&kvm->irq_lock);
-			kfree(kvm->arch.vpic);
-			kfree(kvm->arch.vioapic);
-			kvm->arch.vpic = NULL;
-			kvm->arch.vioapic = NULL;
+			kvm_ioapic_destroy(kvm);
+			kvm_destroy_pic(kvm);
 			mutex_unlock(&kvm->irq_lock);
 		}
 	create_irqchip_unlock:
virt/kvm/ioapic.c +11 -0
index f3d0693..3db15a8 100644
@@ -401,6 +401,17 @@ int kvm_ioapic_init(struct kvm *kvm)
 	return ret;
 }
 
+void kvm_ioapic_destroy(struct kvm *kvm)
+{
+	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
+
+	if (ioapic) {
+		kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, &ioapic->dev);
+		kvm->arch.vioapic = NULL;
+		kfree(ioapic);
+	}
+}
+
 int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
 {
 	struct kvm_ioapic *ioapic = ioapic_irqchip(kvm);
virt/kvm/ioapic.h +1 -0
index a505ce9..8a751b7 100644
@@ -72,6 +72,7 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
 int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2);
 void kvm_ioapic_update_eoi(struct kvm *kvm, int vector, int trigger_mode);
 int kvm_ioapic_init(struct kvm *kvm);
+void kvm_ioapic_destroy(struct kvm *kvm);
 int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level);
 void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
 int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
Signature · 1 line
1.6.3.3
author
#2 lore raw

PATCH 3/4 KVM: PIT: unregister kvm irq notifier if fail to create pit

If fail to create pit, we should unregister kvm irq notifier which register in kvm_create_pit().

Signed-off-by Wei Yongjun <[email protected]>
Acked-by Marcelo Tosatti <[email protected]>
---
 arch/x86/kvm/i8254.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
arch/x86/kvm/i8254.c +3 -2
index 6a74246..c9569f2 100644
@@ -663,8 +663,9 @@ fail_unregister:
 	kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &pit->dev);
 
 fail:
-	if (pit->irq_source_id >= 0)
-		kvm_free_irq_source_id(kvm, pit->irq_source_id);
+	kvm_unregister_irq_mask_notifier(kvm, 0, &pit->mask_notifier);
+	kvm_unregister_irq_ack_notifier(kvm, &pit_state->irq_ack_notifier);
+	kvm_free_irq_source_id(kvm, pit->irq_source_id);
 
 	kfree(pit);
 	return NULL;
Signature · 1 line
1.6.3.3
author
#3 lore raw

PATCH 4/4 KVM: ia64: destroy ioapic device if fail to setup default irq routing

If KVM_CREATE_IRQCHIP fail due to kvm_setup_default_irq_routing(), ioapic device is not destroyed and kvm->arch.vioapic is not set to NULL, this may cause KVM_GET_IRQCHIP and KVM_SET_IRQCHIP access to unexcepted memory.

Signed-off-by Wei Yongjun <[email protected]>
---
 arch/ia64/kvm/kvm-ia64.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/ia64/kvm/kvm-ia64.c +1 -1
index 0618898..26e0e08 100644
@@ -968,7 +968,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 		r = kvm_setup_default_irq_routing(kvm);
 		if (r) {
-			kfree(kvm->arch.vioapic);
+			kvm_ioapic_destroy(kvm);
 			goto out;
 		}
 		break;
Signature · 1 line
1.6.3.3
Applied
#4 lore raw
In reply to Wei Yongjun
Quoted 3 lines · Wei Yongjun

kvm->arch.vioapic should be NULL in case of kvm_ioapic_init() failure due to cannot register io dev.

Applied all, thanks.

Signature · 1 line
error compiling committee.c: too many arguments to function