Re: [PATCH v4 3/3] LoongArch: KVM: Reject repeated PCH-PIC CTRL_INIT

[Tao Cui]

Hi，Bibo

在 2026/10/16 9:31, Bibo Mao 写道:
> 
> 
> On 2026/10/16 上午7:46, Tao Cui wrote:
>> From: Tao Cui <cuitao@xxxxxxxxxx>
>>
>> KVM_DEV_LOONGARCH_PCH_PIC_CTRL_INIT has no guard against repeated
>> invocation: every call overwrites pch_pic_base and registers the same
>> kvm_io_device on the MMIO bus at the new address, while
>> kvm_pch_pic_destroy() unregisters only one bus range.  After a repeated
>> init, MMIO to the stale ranges computes its register offset against the
>> new base and silently reads 0 / drops writes, and the leftover bus
>> entries persist until the VM is destroyed.
>>
>> Reject repeated initialization with -EEXIST, using the initialized
>> state of the kvm_io_device as the indicator.  The check, the MMIO base
>> update and the bus registration are serialized by slots_lock, and the
>> real registration error is propagated instead of being replaced with
>> -EFAULT.
>>
>> Fixes: d206d9514873 ("LoongArch: KVM: Add PCHPIC user mode read and write functions")
>> Signed-off-by: Tao Cui <cuitao@xxxxxxxxxx>
>> ---
>>   arch/loongarch/kvm/intc/pch_pic.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/loongarch/kvm/intc/pch_pic.c b/arch/loongarch/kvm/intc/pch_pic.c
>> index 51e4a6a39dc0..a4c35ed704be 100644
>> --- a/arch/loongarch/kvm/intc/pch_pic.c
>> +++ b/arch/loongarch/kvm/intc/pch_pic.c
>> @@ -281,16 +281,21 @@ static int kvm_pch_pic_init(struct kvm_device *dev, u64 addr)
>>       struct kvm_io_device *device;
>>       struct loongarch_pch_pic *s = dev->kvm->arch.pch_pic;
>>   -    s->pch_pic_base = addr;
>>       device = &s->device;
>> +    mutex_lock(&kvm->slots_lock);
>> +    if (s->device.ops) {
>> +        ret = -EEXIST;
>> +        goto out;
>> +    }
>> +    s->pch_pic_base = addr;
>>       /* init device by pch pic writing and reading ops */
>>       kvm_iodevice_init(device, &kvm_pch_pic_ops);
>> -    mutex_lock(&kvm->slots_lock);
>>       /* register pch pic device */
>>       ret = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS, addr, PCH_PIC_SIZE, device);
>> +out:
>>       mutex_unlock(&kvm->slots_lock);
> one small nit, what to do if there is failure with kvm_io_bus_register_dev()?  clear s->pch_pic_base and  s->device.ops?
> 

I missed this.  You're right. On the failure path I should undo the
state set before kvm_io_bus_register_dev(). I'll fix that in the next
revision.

Thanks，
Tao

> Regards
> Bibo Mao
>>   -    return (ret < 0) ? -EFAULT : 0;
>> +    return ret;
>>   }
>>     /* used by user space to get or set pch pic registers */
>>
>