On 2/29/2024 3:35 AM, Cédric Le Goater wrote:
> Hello Steve,
>
> On 2/22/24 18:28, Steve Sistare wrote:
>> Define entry points to perform per-container cpr-specific initialization
>> and teardown.
>>
>> Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
>> ---
>> hw/vfio/container.c | 11 ++++++++++-
>> hw/vfio/cpr.c | 19 +++++++++++++++++++
>> hw/vfio/iommufd.c | 6 ++++++
>> hw/vfio/meson.build | 1 +
>> include/hw/vfio/vfio-common.h | 3 +++
>> 5 files changed, 39 insertions(+), 1 deletion(-)
>> create mode 100644 hw/vfio/cpr.c
>>
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index bd25b9f..096d77e 100644
>> --- a/hw/vfio/container.c
>> +++ b/hw/vfio/container.c
>> @@ -621,10 +621,15 @@ static int vfio_connect_container(VFIOGroup *group,
>> AddressSpace *as,
>> goto free_container_exit;
>> }
>> + ret = vfio_cpr_register_container(bcontainer, errp);
>> + if (ret) {
>> + goto free_container_exit;
>> + }
>> +
>> ret = vfio_ram_block_discard_disable(container, true);
>> if (ret) {
>> error_setg_errno(errp, -ret, "Cannot set discarding of RAM
>> broken");
>> - goto free_container_exit;
>> + goto unregister_container_exit;
>> }
>> assert(bcontainer->ops->setup);
>> @@ -667,6 +672,9 @@ listener_release_exit:
>> enable_discards_exit:
>> vfio_ram_block_discard_disable(container, false);
>> +unregister_container_exit:
>> + vfio_cpr_unregister_container(bcontainer);
>> +
>> free_container_exit:
>> g_free(container);
>> @@ -710,6 +718,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
>> vfio_container_destroy(bcontainer);
>> trace_vfio_disconnect_container(container->fd);
>> + vfio_cpr_unregister_container(bcontainer);
>> close(container->fd);
>> g_free(container);
>> diff --git a/hw/vfio/cpr.c b/hw/vfio/cpr.c
>> new file mode 100644
>> index 0000000..3bede54
>> --- /dev/null
>> +++ b/hw/vfio/cpr.c
>> @@ -0,0 +1,19 @@
>> +/*
>> + * Copyright (c) 2021-2024 Oracle and/or its affiliates.
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/vfio/vfio-common.h"
>> +#include "qapi/error.h"
>> +
>> +int vfio_cpr_register_container(VFIOContainerBase *bcontainer, Error **errp)
>> +{
>> + return 0;
>> +}
>> +
>> +void vfio_cpr_unregister_container(VFIOContainerBase *bcontainer)
>> +{
>> +}
>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>> index 9bfddc1..e1be224 100644
>> --- a/hw/vfio/iommufd.c
>> +++ b/hw/vfio/iommufd.c
>> @@ -411,6 +411,11 @@ found_container:
>> goto err_listener_register;
>> }
>> + ret = vfio_cpr_register_container(bcontainer, errp);
>> + if (ret) {
>> + goto err_listener_register;
>> + }
>> +
>> /*
>> * TODO: examine RAM_BLOCK_DISCARD stuff, should we do group level
>> * for discarding incompatibility check as well?
>> @@ -461,6 +466,7 @@ static void iommufd_cdev_detach(VFIODevice *vbasedev)
>> iommufd_cdev_ram_block_discard_disable(false);
>> }
>> + vfio_cpr_unregister_container(bcontainer);
>> iommufd_cdev_detach_container(vbasedev, container);
>> iommufd_cdev_container_destroy(container);
>> vfio_put_address_space(space);
>> diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
>> index bb98493..bba776f 100644
>> --- a/hw/vfio/meson.build
>> +++ b/hw/vfio/meson.build
>> @@ -5,6 +5,7 @@ vfio_ss.add(files(
>> 'container-base.c',
>> 'container.c',
>> 'migration.c',
>> + 'cpr.c',
>> ))
>> vfio_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr.c'))
>> vfio_ss.add(when: 'CONFIG_IOMMUFD', if_true: files(
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>> index 4a6c262..b9da6c0 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -205,6 +205,9 @@ void vfio_detach_device(VFIODevice *vbasedev);
>> int vfio_kvm_device_add_fd(int fd, Error **errp);
>> int vfio_kvm_device_del_fd(int fd, Error **errp);
>> +int vfio_cpr_register_container(VFIOContainerBase *bcontainer, Error
>> **errp);
>
> Should we return bool since we have an errp ? the returned value
> is not an errno AFAICT.
>
> Anyhow,
>
> Reviewed-by: Cédric Le Goater <c...@redhat.com>
Hi Cedric, vfio_cpr_register_container sometimes returns the value returned
from
migrate_add_blocker_modes, which is an errno.
Thanks for reviewing these!
- Steve
>> +void vfio_cpr_unregister_container(VFIOContainerBase *bcontainer);
>> +
>> extern const MemoryRegionOps vfio_region_ops;
>> typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList;
>> typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
>
