Re: [RFC PATCH] KVM: Fix missing lock for kvm_io_bus_unregister_dev()

[Takuya Yoshikawa <takuya.yoshikawa@gmail.com>]

Marcelo Tosatti <mtosatti@redhat.com> wrote:
> > --- a/arch/x86/kvm/i8254.c
> > +++ b/arch/x86/kvm/i8254.c
> > @@ -744,9 +744,11 @@ void kvm_free_pit(struct kvm *kvm)
> >  	struct hrtimer *timer;
> >  
> >  	if (kvm->arch.vpit) {
> > +		mutex_lock(&kvm->slots_lock);
> >  		kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &kvm->arch.vpit->dev);
> >  		kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS,
> >  					      &kvm->arch.vpit->speaker_dev);
> > +		mutex_unlock(&kvm->slots_lock);
> >  		kvm_unregister_irq_mask_notifier(kvm, 0,
> >  					       &kvm->arch.vpit->mask_notifier);
> >  		kvm_unregister_irq_ack_notifier(kvm,
> 
> This is supposedly safe because this is only called from vm destroy
> context, when dropping the last reference.
> 

I see. Drop this from the next version.

> 
> It seems the best way to fix is to move irq_lock and slots_lock
> acquision from kvm_set_irq_routing/kvm_ioapic_destroy/kvm_destroy_pic to
> their callers.
> 

Then, the lock acquisition for registeration and unregisteration may be
centralized to vm_ioctl.

I'm now checking these to reformat the patch.

Thanks,
  Takuya