* [PATCH V3 1/3] vfio/container: ram discard disable helper
2025-05-02 14:22 [PATCH V3 0/3] vfio cleanup, pre-cpr Steve Sistare
@ 2025-05-02 14:22 ` Steve Sistare
2025-05-02 14:22 ` [PATCH V3 2/3] vfio/container: reform vfio_container_connect cleanup Steve Sistare
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Steve Sistare @ 2025-05-02 14:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Williamson, Cedric Le Goater, Steve Sistare
Define a helper to set ram discard disable, generate error messages,
and cleanup on failure. The second vfio_ram_block_discard_disable
call site now performs VFIO_GROUP_UNSET_CONTAINER immediately on failure,
instead of relying on the close of the container fd to do so in the kernel,
but this is equivalent.
Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Reviewed-by: Cedric Le Goater <clg@redhat.com>
---
hw/vfio/container.c | 48 +++++++++++++++++++++++++++---------------------
1 file changed, 27 insertions(+), 21 deletions(-)
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 77ff56b..bf4bbd2 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -511,16 +511,10 @@ static bool vfio_legacy_setup(VFIOContainerBase *bcontainer, Error **errp)
return true;
}
-static bool vfio_container_connect(VFIOGroup *group, AddressSpace *as,
- Error **errp)
+static bool vfio_attach_discard_disable(VFIOContainer *container,
+ VFIOGroup *group, Error **errp)
{
- VFIOContainer *container;
- VFIOContainerBase *bcontainer;
- int ret, fd;
- VFIOAddressSpace *space;
- VFIOIOMMUClass *vioc;
-
- space = vfio_address_space_get(as);
+ int ret;
/*
* VFIO is currently incompatible with discarding of RAM insofar as the
@@ -553,18 +547,32 @@ static bool vfio_container_connect(VFIOGroup *group, AddressSpace *as,
* details once we know which type of IOMMU we are using.
*/
+ ret = vfio_ram_block_discard_disable(container, true);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Cannot set discarding of RAM broken");
+ if (ioctl(group->fd, VFIO_GROUP_UNSET_CONTAINER, &container->fd)) {
+ error_report("vfio: error disconnecting group %d from"
+ " container", group->groupid);
+ }
+ }
+ return !ret;
+}
+
+static bool vfio_container_connect(VFIOGroup *group, AddressSpace *as,
+ Error **errp)
+{
+ VFIOContainer *container;
+ VFIOContainerBase *bcontainer;
+ int ret, fd;
+ VFIOAddressSpace *space;
+ VFIOIOMMUClass *vioc;
+
+ space = vfio_address_space_get(as);
+
QLIST_FOREACH(bcontainer, &space->containers, next) {
container = container_of(bcontainer, VFIOContainer, bcontainer);
if (!ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &container->fd)) {
- ret = vfio_ram_block_discard_disable(container, true);
- if (ret) {
- error_setg_errno(errp, -ret,
- "Cannot set discarding of RAM broken");
- if (ioctl(group->fd, VFIO_GROUP_UNSET_CONTAINER,
- &container->fd)) {
- error_report("vfio: error disconnecting group %d from"
- " container", group->groupid);
- }
+ if (!vfio_attach_discard_disable(container, group, errp)) {
return false;
}
group->container = container;
@@ -596,9 +604,7 @@ static bool vfio_container_connect(VFIOGroup *group, AddressSpace *as,
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");
+ if (!vfio_attach_discard_disable(container, group, errp)) {
goto unregister_container_exit;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH V3 2/3] vfio/container: reform vfio_container_connect cleanup
2025-05-02 14:22 [PATCH V3 0/3] vfio cleanup, pre-cpr Steve Sistare
2025-05-02 14:22 ` [PATCH V3 1/3] vfio/container: ram discard disable helper Steve Sistare
@ 2025-05-02 14:22 ` Steve Sistare
2025-05-02 14:22 ` [PATCH V3 3/3] vfio/container: vfio_container_group_add Steve Sistare
2025-05-05 8:45 ` [PATCH V3 0/3] vfio cleanup, pre-cpr Cédric Le Goater
3 siblings, 0 replies; 5+ messages in thread
From: Steve Sistare @ 2025-05-02 14:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Williamson, Cedric Le Goater, Steve Sistare
Replace the proliferation of exit labels in vfio_container_connect with
conditionals for cleaning each piece of state. No functional change.
Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Reviewed-by: Cedric Le Goater <clg@redhat.com>
---
hw/vfio/container.c | 60 +++++++++++++++++++++++++++++------------------------
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index bf4bbd2..26433ae 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -563,9 +563,12 @@ static bool vfio_container_connect(VFIOGroup *group, AddressSpace *as,
{
VFIOContainer *container;
VFIOContainerBase *bcontainer;
- int ret, fd;
+ int ret, fd = -1;
VFIOAddressSpace *space;
- VFIOIOMMUClass *vioc;
+ VFIOIOMMUClass *vioc = NULL;
+ bool new_container = false;
+ bool group_was_added = false;
+ bool discard_disabled = false;
space = vfio_address_space_get(as);
@@ -584,35 +587,37 @@ static bool vfio_container_connect(VFIOGroup *group, AddressSpace *as,
fd = qemu_open("/dev/vfio/vfio", O_RDWR, errp);
if (fd < 0) {
- goto put_space_exit;
+ goto fail;
}
ret = ioctl(fd, VFIO_GET_API_VERSION);
if (ret != VFIO_API_VERSION) {
error_setg(errp, "supported vfio version: %d, "
"reported version: %d", VFIO_API_VERSION, ret);
- goto close_fd_exit;
+ goto fail;
}
container = vfio_create_container(fd, group, errp);
if (!container) {
- goto close_fd_exit;
+ goto fail;
}
+ new_container = true;
bcontainer = &container->bcontainer;
if (!vfio_cpr_register_container(bcontainer, errp)) {
- goto free_container_exit;
+ goto fail;
}
if (!vfio_attach_discard_disable(container, group, errp)) {
- goto unregister_container_exit;
+ goto fail;
}
+ discard_disabled = true;
vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
assert(vioc->setup);
if (!vioc->setup(bcontainer, errp)) {
- goto enable_discards_exit;
+ goto fail;
}
vfio_group_add_kvm_device(group);
@@ -621,35 +626,36 @@ static bool vfio_container_connect(VFIOGroup *group, AddressSpace *as,
group->container = container;
QLIST_INSERT_HEAD(&container->group_list, group, container_next);
+ group_was_added = true;
if (!vfio_listener_register(bcontainer, errp)) {
- goto listener_release_exit;
+ goto fail;
}
bcontainer->initialized = true;
return true;
-listener_release_exit:
- QLIST_REMOVE(group, container_next);
- vfio_group_del_kvm_device(group);
+
+fail:
vfio_listener_unregister(bcontainer);
- if (vioc->release) {
+
+ if (group_was_added) {
+ QLIST_REMOVE(group, container_next);
+ vfio_group_del_kvm_device(group);
+ }
+ if (vioc && vioc->release) {
vioc->release(bcontainer);
}
-
-enable_discards_exit:
- vfio_ram_block_discard_disable(container, false);
-
-unregister_container_exit:
- vfio_cpr_unregister_container(bcontainer);
-
-free_container_exit:
- object_unref(container);
-
-close_fd_exit:
- close(fd);
-
-put_space_exit:
+ if (discard_disabled) {
+ vfio_ram_block_discard_disable(container, false);
+ }
+ if (new_container) {
+ vfio_cpr_unregister_container(bcontainer);
+ object_unref(container);
+ }
+ if (fd >= 0) {
+ close(fd);
+ }
vfio_address_space_put(space);
return false;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH V3 3/3] vfio/container: vfio_container_group_add
2025-05-02 14:22 [PATCH V3 0/3] vfio cleanup, pre-cpr Steve Sistare
2025-05-02 14:22 ` [PATCH V3 1/3] vfio/container: ram discard disable helper Steve Sistare
2025-05-02 14:22 ` [PATCH V3 2/3] vfio/container: reform vfio_container_connect cleanup Steve Sistare
@ 2025-05-02 14:22 ` Steve Sistare
2025-05-05 8:45 ` [PATCH V3 0/3] vfio cleanup, pre-cpr Cédric Le Goater
3 siblings, 0 replies; 5+ messages in thread
From: Steve Sistare @ 2025-05-02 14:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Williamson, Cedric Le Goater, Steve Sistare
Add vfio_container_group_add to de-dup some code. No functional change.
Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Reviewed-by: Cedric Le Goater <clg@redhat.com>
---
hw/vfio/container.c | 47 +++++++++++++++++++++++++----------------------
1 file changed, 25 insertions(+), 22 deletions(-)
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 26433ae..d0b8dfc 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -558,6 +558,26 @@ static bool vfio_attach_discard_disable(VFIOContainer *container,
return !ret;
}
+static bool vfio_container_group_add(VFIOContainer *container, VFIOGroup *group,
+ Error **errp)
+{
+ if (!vfio_attach_discard_disable(container, group, errp)) {
+ return false;
+ }
+ group->container = container;
+ QLIST_INSERT_HEAD(&container->group_list, group, container_next);
+ vfio_group_add_kvm_device(group);
+ return true;
+}
+
+static void vfio_container_group_del(VFIOContainer *container, VFIOGroup *group)
+{
+ QLIST_REMOVE(group, container_next);
+ group->container = NULL;
+ vfio_group_del_kvm_device(group);
+ vfio_ram_block_discard_disable(container, false);
+}
+
static bool vfio_container_connect(VFIOGroup *group, AddressSpace *as,
Error **errp)
{
@@ -568,20 +588,13 @@ static bool vfio_container_connect(VFIOGroup *group, AddressSpace *as,
VFIOIOMMUClass *vioc = NULL;
bool new_container = false;
bool group_was_added = false;
- bool discard_disabled = false;
space = vfio_address_space_get(as);
QLIST_FOREACH(bcontainer, &space->containers, next) {
container = container_of(bcontainer, VFIOContainer, bcontainer);
if (!ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &container->fd)) {
- if (!vfio_attach_discard_disable(container, group, errp)) {
- return false;
- }
- group->container = container;
- QLIST_INSERT_HEAD(&container->group_list, group, container_next);
- vfio_group_add_kvm_device(group);
- return true;
+ return vfio_container_group_add(container, group, errp);
}
}
@@ -608,11 +621,6 @@ static bool vfio_container_connect(VFIOGroup *group, AddressSpace *as,
goto fail;
}
- if (!vfio_attach_discard_disable(container, group, errp)) {
- goto fail;
- }
- discard_disabled = true;
-
vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
assert(vioc->setup);
@@ -620,12 +628,11 @@ static bool vfio_container_connect(VFIOGroup *group, AddressSpace *as,
goto fail;
}
- vfio_group_add_kvm_device(group);
-
vfio_address_space_insert(space, bcontainer);
- group->container = container;
- QLIST_INSERT_HEAD(&container->group_list, group, container_next);
+ if (!vfio_container_group_add(container, group, errp)) {
+ goto fail;
+ }
group_was_added = true;
if (!vfio_listener_register(bcontainer, errp)) {
@@ -640,15 +647,11 @@ fail:
vfio_listener_unregister(bcontainer);
if (group_was_added) {
- QLIST_REMOVE(group, container_next);
- vfio_group_del_kvm_device(group);
+ vfio_container_group_del(container, group);
}
if (vioc && vioc->release) {
vioc->release(bcontainer);
}
- if (discard_disabled) {
- vfio_ram_block_discard_disable(container, false);
- }
if (new_container) {
vfio_cpr_unregister_container(bcontainer);
object_unref(container);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH V3 0/3] vfio cleanup, pre-cpr
2025-05-02 14:22 [PATCH V3 0/3] vfio cleanup, pre-cpr Steve Sistare
` (2 preceding siblings ...)
2025-05-02 14:22 ` [PATCH V3 3/3] vfio/container: vfio_container_group_add Steve Sistare
@ 2025-05-05 8:45 ` Cédric Le Goater
3 siblings, 0 replies; 5+ messages in thread
From: Cédric Le Goater @ 2025-05-05 8:45 UTC (permalink / raw)
To: Steve Sistare, qemu-devel; +Cc: Alex Williamson
On 5/2/25 16:22, Steve Sistare wrote:
> Cleanup a few vfio functions prior to the introduction of CPR.
>
> These were extracted from
> https://lore.kernel.org/qemu-devel/1739542467-226739-1-git-send-email-steven.sistare@oracle.com/
>
> Changes in V3:
> * update to master
>
> Steve Sistare (3):
> vfio/container: ram discard disable helper
> vfio/container: reform vfio_container_connect cleanup
> vfio/container: vfio_container_group_add
>
> hw/vfio/container.c | 131 +++++++++++++++++++++++++++++-----------------------
> 1 file changed, 73 insertions(+), 58 deletions(-)
>
> base-commit: 73d29ea2417b58ca55fba1aa468ba38e3607b583
>
Applied to vfio-next with this change :
vfio_attach_discard_disable() -> vfio_container_attach_discard_disable()
Thanks,
C.
^ permalink raw reply [flat|nested] 5+ messages in thread