Skip to contentSkip to conversation

KVM: PIT: remove useless code from kvm_create_pit()

PATCH Applied Wei Yongjun 3 participants 11 messages Feb 8, 2010 – Feb 9, 2010

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

Wei Yongjun

author
#0 lore raw

PATCH 1/2 KVM: PIT: remove useless code from kvm_create_pit()

kvm->arch.vpit will be set to the return value of kvm_create_pit(), so we do not need to set kvm->arch.vpit in kvm_create_pit(). This patch remove the useless code.

Signed-off-by Wei Yongjun <[email protected]>
---
 arch/x86/kvm/i8254.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
arch/x86/kvm/i8254.c +0 -1
index 6a74246..6c90091 100644
@@ -626,7 +626,6 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
 	mutex_lock(&pit->pit_state.lock);
 	spin_lock_init(&pit->pit_state.inject_lock);
 
-	kvm->arch.vpit = pit;
 	pit->kvm = kvm;
 
 	pit_state = &pit->pit_state;
Signature · 1 line
1.6.3.3
author
#1 lore raw

PATCH 2/2 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]>
---
 arch/x86/kvm/i8254.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
arch/x86/kvm/i8254.c +3 -2
index 6c90091..e6500ed 100644
@@ -662,8 +662,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
#2 lore raw

PATCH 1/1 KVM: only set kvm->arch.vioapic when success to init ioapic

kvm->arch.vioapic is always set either kvm_ioapic_init() is success or fail. If kvm_ioapic_init() is fail, the kvm->arch.vioapic may point a freed memory.

Signed-off-by Wei Yongjun <[email protected]>
---
 virt/kvm/ioapic.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
virt/kvm/ioapic.c +2 -1
index a2edfd1..e13f529 100644
@@ -386,7 +386,6 @@ int kvm_ioapic_init(struct kvm *kvm)
 	if (!ioapic)
 		return -ENOMEM;
 	mutex_init(&ioapic->lock);
-	kvm->arch.vioapic = ioapic;
 	kvm_ioapic_reset(ioapic);
 	kvm_iodevice_init(&ioapic->dev, &ioapic_mmio_ops);
 	ioapic->kvm = kvm;

@@ -395,6 +394,8 @@ int kvm_ioapic_init(struct kvm *kvm)
 	mutex_unlock(&kvm->slots_lock);
 	if (ret < 0)
 		kfree(ioapic);
+	else
+		kvm->arch.vioapic = ioapic;
 
 	return ret;
 }
Signature · 1 line
1.6.3.3
author
#3 lore raw

PATCH 1/1 KVM: ia64: clear kvm->arch.vioapic if fail to create irq chip

If fail to create irq chip due to fail of kvm_setup_default_irq_routing(), 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 |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
arch/ia64/kvm/kvm-ia64.c +1 -0
index 0618898..0cfc15d 100644
@@ -969,6 +969,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 		r = kvm_setup_default_irq_routing(kvm);
 		if (r) {
 			kfree(kvm->arch.vioapic);
+			kvm->arch.vioapic = NULL;
 			goto out;
 		}
 		break;
Signature · 1 line
1.6.3.3
#5 lore raw

Re: [PATCH] KVM: only set kvm->arch.vioapic when success to init ioapic

In reply to Wei Yongjun
Quoted 22 lines · Wei Yongjun

kvm->arch.vioapic is always set either kvm_ioapic_init() is success or fail. If kvm_ioapic_init() is fail, the kvm->arch.vioapic may point a freed memory.

Signed-off-by Wei Yongjun <[email protected]>
---
 virt/kvm/ioapic.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
virt/kvm/ioapic.c +0 -1
index a2edfd1..e13f529 100644
@@ -386,7 +386,6 @@ int kvm_ioapic_init(struct kvm *kvm)
 	if (!ioapic)
 		return -ENOMEM;
 	mutex_init(&ioapic->lock);
-	kvm->arch.vioapic = ioapic;
 	kvm_ioapic_reset(ioapic);
 	kvm_iodevice_init(&ioapic->dev, &ioapic_mmio_ops);
 	ioapic->kvm = kvm;

virt/kvm/ioapic.c int kvm_ioapic_init(struct kvm *kvm) · line 401

Quoted 8 lines
virt/kvm/ioapic.c +2 -0
@@ -395,6 +394,8 @@ int kvm_ioapic_init(struct kvm *kvm)
 	mutex_unlock(&kvm->slots_lock);
 	if (ret < 0)
 		kfree(ioapic);
+	else
+		kvm->arch.vioapic = ioapic;
 
 	return ret;
 }

Same thing as with the PIT issue, kvm->arch.vioapic must be set before kvm_io_bus_register_dev.

But you're right that kvm->arch.vioapic should be NULLified in case of failure.

#8 lore raw

Re: [PATCH 2/2] KVM: PIT: unregister kvm irq notifier if fail to create pit

In reply to Wei Yongjun
Quoted 7 lines · Wei Yongjun

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

Signed-off-by Wei Yongjun <[email protected]>
---
 arch/x86/kvm/i8254.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

ACK

#10 lore raw
In reply to Wei Yongjun
Quoted 5 lines · Wei Yongjun

kvm->arch.vpit will be set to the return value of kvm_create_pit(), so we do not need to set kvm->arch.vpit in kvm_create_pit(). This patch remove the useless code.

Signed-off-by Wei Yongjun <[email protected]>

The device could be accessed between kvm_io_bus_register_dev and kvm->arch.vpit assignment in the ioctl handler.

Quoted 16 lines
---
 arch/x86/kvm/i8254.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
arch/x86/kvm/i8254.c +0 -1
index 6a74246..6c90091 100644
@@ -626,7 +626,6 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
 	mutex_lock(&pit->pit_state.lock);
 	spin_lock_init(&pit->pit_state.inject_lock);
 
-	kvm->arch.vpit = pit;
 	pit->kvm = kvm;
 
 	pit_state = &pit->pit_state;
author
#6 lore raw

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

In reply to Marcelo Tosatti

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
#7 lore raw

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

Quoted 10 lines · Wei Yongjun

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(-)

Please ignore this patch, I will send in other thread, thanks.

author
#4 lore raw

Re: [PATCH] KVM: ia64: clear kvm->arch.vioapic if fail to create irq chip

In reply to Wei Yongjun
Quoted 11 lines · Wei Yongjun

If fail to create irq chip due to fail of kvm_setup_default_irq_routing(), 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 |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Please ignore this patch, I will send a update patch in other thread, thanks.

Applied
#9 lore raw

Re: [PATCH 2/2] KVM: PIT: unregister kvm irq notifier if fail to create pit

In reply to Wei Yongjun
Quoted 3 lines · Wei Yongjun

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

Applied, thanks.

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