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

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

[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 <yjwei@cn.fujitsu.com>
---
 arch/x86/kvm/i8254.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 6a74246..6c90091 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -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;
-- 
1.6.3.3

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

[Wei Yongjun]

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

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
 arch/x86/kvm/i8254.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 6c90091..e6500ed 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -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;
-- 
1.6.3.3

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

[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 <yjwei@cn.fujitsu.com>
---
 virt/kvm/ioapic.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index a2edfd1..e13f529 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -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;
 }
-- 
1.6.3.3

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

[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 <yjwei@cn.fujitsu.com>
---
 arch/ia64/kvm/kvm-ia64.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 0618898..0cfc15d 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -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;
-- 
1.6.3.3

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

[Wei Yongjun]

Wei Yongjun wrote:
> 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 <yjwei@cn.fujitsu.com>
> ---
>  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.

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

[Marcelo Tosatti]

On Mon, Feb 08, 2010 at 05:36:45PM +0800, Wei Yongjun wrote:
> 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 <yjwei@cn.fujitsu.com>
> ---
>  virt/kvm/ioapic.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index a2edfd1..e13f529 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -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;
>  }

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.

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

[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 <yjwei@cn.fujitsu.com>
---
 virt/kvm/ioapic.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index a2edfd1..f3d0693 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -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;
 }
-- 
1.6.3.3

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

[Wei Yongjun]

Wei Yongjun wrote:
> kvm->arch.vioapic should be NULL in case of kvm_ioapic_init() failure
> due to cannot register io dev.
>
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> ---
>  virt/kvm/ioapic.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
>
>   

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

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

[Marcelo Tosatti]

On Mon, Feb 08, 2010 at 05:03:51PM +0800, Wei Yongjun wrote:
> If fail to create pit, we should unregister kvm irq notifier
> which register in kvm_create_pit().
> 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> ---
>  arch/x86/kvm/i8254.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)

ACK

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

[Avi Kivity]

On 02/08/2010 11:03 AM, Wei Yongjun wrote:
> If fail to create pit, we should unregister kvm irq notifier
> which register in kvm_create_pit().
>   

Applied, thanks.

-- 
error compiling committee.c: too many arguments to function

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

[Marcelo Tosatti]

On Mon, Feb 08, 2010 at 05:01:15PM +0800, Wei Yongjun wrote:
> 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 <yjwei@cn.fujitsu.com>

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

> ---
>  arch/x86/kvm/i8254.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
> index 6a74246..6c90091 100644
> --- a/arch/x86/kvm/i8254.c
> +++ b/arch/x86/kvm/i8254.c
> @@ -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;