>-----Original Message-----
>From: Steven Sistare <steven.sist...@oracle.com>
>Subject: Re: [PATCH V6 03/21] migration: close kvm after cpr
>
>On 7/4/2025 5:50 AM, Duan, Zhenzhong wrote:
>>> -----Original Message-----
>>> From: Steven Sistare <steven.sist...@oracle.com>
>>> Subject: Re: [PATCH V6 03/21] migration: close kvm after cpr
>>>
>>> cc Paolo.
>>>
>>> After incorporating Peter's feedback, IMO this version reads well:
>>> * kvm exports kvm_close
>>> * vfio exports vfio_kvm_device_close
>>> * vfio-cpr registers a notifier that calls vfio_kvm_device_close
>>>
>>> - Steve
>>>
>>> On 7/2/2025 5:58 PM, Steve Sistare wrote:
>>>> cpr-transfer breaks vfio network connectivity to and from the guest, and
>>>> the host system log shows:
>>>> irq bypass consumer (token 00000000a03c32e5) registration fails:
>-16
>>>> which is EBUSY. This occurs because KVM descriptors are still open in
>>>> the old QEMU process. Close them.
>>>>
>>>> Cc: Paolo Bonzini <pbonz...@redhat.com>
>>>> Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
>>>> Reviewed-by: Fabiano Rosas <faro...@suse.de>
>>>> ---
>>>> include/hw/vfio/vfio-cpr.h | 2 ++
>>>> include/hw/vfio/vfio-device.h | 2 ++
>>>> include/system/kvm.h | 1 +
>>>> accel/kvm/kvm-all.c | 32
>>> ++++++++++++++++++++++++++++++++
>>>> hw/vfio/cpr-legacy.c | 2 ++
>>>> hw/vfio/cpr.c | 21 +++++++++++++++++++++
>>>> hw/vfio/helpers.c | 11 +++++++++++
>>>> 7 files changed, 71 insertions(+)
>>>>
>>>> diff --git a/include/hw/vfio/vfio-cpr.h b/include/hw/vfio/vfio-cpr.h
>>>> index 25e74ee..099d54f 100644
>>>> --- a/include/hw/vfio/vfio-cpr.h
>>>> +++ b/include/hw/vfio/vfio-cpr.h
>>>> @@ -62,4 +62,6 @@ void vfio_cpr_delete_vector_fd(struct
>VFIOPCIDevice
>>> *vdev, const char *name,
>>>>
>>>> extern const VMStateDescription vfio_cpr_pci_vmstate;
>>>>
>>>> +void vfio_cpr_add_kvm_notifier(void);
>>>> +
>>>> #endif /* HW_VFIO_VFIO_CPR_H */
>>>> diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
>>>> index c616652..f503837 100644
>>>> --- a/include/hw/vfio/vfio-device.h
>>>> +++ b/include/hw/vfio/vfio-device.h
>>>> @@ -283,4 +283,6 @@ void vfio_device_set_fd(VFIODevice *vbasedev,
>>> const char *str, Error **errp);
>>>> void vfio_device_init(VFIODevice *vbasedev, int type, VFIODeviceOps
>>> *ops,
>>>> DeviceState *dev, bool ram_discard);
>>>> int vfio_device_get_aw_bits(VFIODevice *vdev);
>>>> +
>>>> +void vfio_kvm_device_close(void);
>>>> #endif /* HW_VFIO_VFIO_COMMON_H */
>>>> diff --git a/include/system/kvm.h b/include/system/kvm.h
>>>> index 7cc60d2..4896a3c 100644
>>>> --- a/include/system/kvm.h
>>>> +++ b/include/system/kvm.h
>>>> @@ -195,6 +195,7 @@ bool kvm_has_sync_mmu(void);
>>>> int kvm_has_vcpu_events(void);
>>>> int kvm_max_nested_state_length(void);
>>>> int kvm_has_gsi_routing(void);
>>>> +void kvm_close(void);
>>>>
>>>> /**
>>>> * kvm_arm_supports_user_irq
>>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>>>> index d095d1b..8141854 100644
>>>> --- a/accel/kvm/kvm-all.c
>>>> +++ b/accel/kvm/kvm-all.c
>>>> @@ -515,16 +515,23 @@ static int do_kvm_destroy_vcpu(CPUState
>*cpu)
>>>> goto err;
>>>> }
>>>>
>>>> + /* If I am the CPU that created coalesced_mmio_ring, then discard
>it
>>> */
>>>> + if (s->coalesced_mmio_ring == (void *)cpu->kvm_run + PAGE_SIZE)
>{
>>>> + s->coalesced_mmio_ring = NULL;
>>>> + }
>>>> +
>>>> ret = munmap(cpu->kvm_run, mmap_size);
>>>> if (ret < 0) {
>>>> goto err;
>>>> }
>>>> + cpu->kvm_run = NULL;
>>>>
>>>> if (cpu->kvm_dirty_gfns) {
>>>> ret = munmap(cpu->kvm_dirty_gfns,
>s->kvm_dirty_ring_bytes);
>>>> if (ret < 0) {
>>>> goto err;
>>>> }
>>>> + cpu->kvm_dirty_gfns = NULL;
>>>> }
>>>>
>>>> kvm_park_vcpu(cpu);
>>>> @@ -608,6 +615,31 @@ err:
>>>> return ret;
>>>> }
>>>>
>>>> +void kvm_close(void)
>>>> +{
>>>> + CPUState *cpu;
>>>> +
>>>> + if (!kvm_state || kvm_state->fd == -1) {
>>>> + return;
>>>> + }
>>>> +
>>>> + CPU_FOREACH(cpu) {
>>>> + cpu_remove_sync(cpu);
>>>> + close(cpu->kvm_fd);
>>>> + cpu->kvm_fd = -1;
>>>> + close(cpu->kvm_vcpu_stats_fd);
>>>> + cpu->kvm_vcpu_stats_fd = -1;
>>>> + }
>>>> +
>>>> + if (kvm_state && kvm_state->fd != -1) {
>>>> + close(kvm_state->vmfd);
>>>> + kvm_state->vmfd = -1;
>>>> + close(kvm_state->fd);
>>>> + kvm_state->fd = -1;
>>>> + }
>>>> + kvm_state = NULL;
>>>> +}
>>>> +
>>>> /*
>>>> * dirty pages logging control
>>>> */
>>>> diff --git a/hw/vfio/cpr-legacy.c b/hw/vfio/cpr-legacy.c
>>>> index a84c324..daa3523 100644
>>>> --- a/hw/vfio/cpr-legacy.c
>>>> +++ b/hw/vfio/cpr-legacy.c
>>>> @@ -177,6 +177,8 @@ bool
>>> vfio_legacy_cpr_register_container(VFIOContainer *container, Error
>**errp)
>>>>
>>> MIG_MODE_CPR_TRANSFER, -1) == 0;
>>>> }
>>>>
>>>> + vfio_cpr_add_kvm_notifier();
>>
>> Hi Steven, I just noticed this, do we need to do same for iommufd?
>
>Yes, and that call is added in patch
> "vfio/iommufd: register container for cpr"
>
>> Do we need to delete notifier when all VFIO devices hot unplugged?
>
>No need. The notifier will be called, and close the kvm descriptors and
>vfio_kvm_device_fd. Not strictly necessary if vfio devices are no longer
>present, but not harmful either.
Clear, no problem.
Thanks
Zhenzhong
