Re: [PATCH v2 net] LoongArch: KVM: Add missing slots_lock for IO bus device register/unregister
From: Huacai Chen
Date: Mon May 25 2026 - 05:59:15 EST
On Mon, May 25, 2026 at 5:53 PM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
>
>
>
> On 2026/5/25 下午5:26, Zeng Chi wrote:
> > From: Zeng Chi <zengchi@xxxxxxxxxx>
> >
> > kvm_io_bus_register_dev() and kvm_io_bus_unregister_dev() should be
> > called under kvm->slots_lock. The unregister calls in ipi.c, pch_pic.c
> > and eiointc.c were missing this protection. Add it to match the
> > register side.
> one small nit, the mail tile is [PATCH v2 net], I think it should be
> [PATCH v2] :)
>
> >
> > Signed-off-by: Zeng Chi <zengchi@xxxxxxxxxx>
> > ---
> > arch/loongarch/kvm/intc/eiointc.c | 6 ++++++
> > arch/loongarch/kvm/intc/ipi.c | 2 ++
> > arch/loongarch/kvm/intc/pch_pic.c | 2 ++
> > 3 files changed, 10 insertions(+)
> >
> > diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
> > index 2ab7fafa86d5..2b14485d14a7 100644
> > --- a/arch/loongarch/kvm/intc/eiointc.c
> > +++ b/arch/loongarch/kvm/intc/eiointc.c
> > @@ -645,10 +645,14 @@ static int kvm_eiointc_create(struct kvm_device *dev, u32 type)
> >
> > device = &s->device_vext;
> > kvm_iodevice_init(device, &kvm_eiointc_virt_ops);
> > + mutex_lock(&kvm->slots_lock);
> > ret = kvm_io_bus_register_dev(kvm, KVM_IOCSR_BUS,
> > EIOINTC_VIRT_BASE, EIOINTC_VIRT_SIZE, device);
> > + mutex_unlock(&kvm->slots_lock);
> > if (ret < 0) {
> > + mutex_lock(&kvm->slots_lock);
> > kvm_io_bus_unregister_dev(kvm, KVM_IOCSR_BUS, &s->device);
> > + mutex_unlock(&kvm->slots_lock);
> > kfree(s);
> > return ret;
> > }
>
> I prefer something like this, however it is only my personal option.
> --- a/arch/loongarch/kvm/intc/eiointc.c
> +++ b/arch/loongarch/kvm/intc/eiointc.c
> @@ -645,13 +645,16 @@ static int kvm_eiointc_create(struct kvm_device
> *dev, u32 type)
>
> device = &s->device_vext;
> kvm_iodevice_init(device, &kvm_eiointc_virt_ops);
> + mutex_lock(&kvm->slots_lock);
> ret = kvm_io_bus_register_dev(kvm, KVM_IOCSR_BUS,
> EIOINTC_VIRT_BASE, EIOINTC_VIRT_SIZE, device);
> if (ret < 0) {
> kvm_io_bus_unregister_dev(kvm, KVM_IOCSR_BUS, &s->device);
> + mutex_unlock(&kvm->slots_lock);
> kfree(s);
> return ret;
> }
> + mutex_unlock(&kvm->slots_lock);
> kvm->arch.eiointc = s;
>
> @chenhuacai what is your option?
Both are OK for me. If we want to minimize the critical section, then
this patch is a little better.
Huacai
>
> The others look good to me.
>
> Regards
> Bibo Mao
> > @@ -667,8 +671,10 @@ static void kvm_eiointc_destroy(struct kvm_device *dev)
> >
> > kvm = dev->kvm;
> > eiointc = kvm->arch.eiointc;
> > + mutex_lock(&kvm->slots_lock);
> > kvm_io_bus_unregister_dev(kvm, KVM_IOCSR_BUS, &eiointc->device);
> > kvm_io_bus_unregister_dev(kvm, KVM_IOCSR_BUS, &eiointc->device_vext);
> > + mutex_unlock(&kvm->slots_lock);
> > kfree(eiointc);
> > kfree(dev);
> > }
> > diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
> > index 1f6ebbd0af5c..4fa0897d7bdb 100644
> > --- a/arch/loongarch/kvm/intc/ipi.c
> > +++ b/arch/loongarch/kvm/intc/ipi.c
> > @@ -447,7 +447,9 @@ static void kvm_ipi_destroy(struct kvm_device *dev)
> >
> > kvm = dev->kvm;
> > ipi = kvm->arch.ipi;
> > + mutex_lock(&kvm->slots_lock);
> > kvm_io_bus_unregister_dev(kvm, KVM_IOCSR_BUS, &ipi->device);
> > + mutex_unlock(&kvm->slots_lock);
> > kfree(ipi);
> > kfree(dev);
> > }
> > diff --git a/arch/loongarch/kvm/intc/pch_pic.c b/arch/loongarch/kvm/intc/pch_pic.c
> > index aa0ed59ae8cf..175a630aceb4 100644
> > --- a/arch/loongarch/kvm/intc/pch_pic.c
> > +++ b/arch/loongarch/kvm/intc/pch_pic.c
> > @@ -481,7 +481,9 @@ static void kvm_pch_pic_destroy(struct kvm_device *dev)
> > kvm = dev->kvm;
> > s = kvm->arch.pch_pic;
> > /* unregister pch pic device and free it's memory */
> > + mutex_lock(&kvm->slots_lock);
> > kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, &s->device);
> > + mutex_unlock(&kvm->slots_lock);
> > kfree(s);
> > kfree(dev);
> > }
> >
>