[PATCH v4 6/7] vfio: Do not unparent in instance_finalize()

Akihiko Odaki posted 7 patches 4 days, 10 hours ago
Maintainers: Alistair Francis <alistair@alistair23.me>, "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Bin Meng <bmeng.cn@gmail.com>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, Stefano Stabellini <sstabellini@kernel.org>, Anthony PERARD <anthony@xenproject.org>, Paul Durrant <paul@xen.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
[PATCH v4 6/7] vfio: Do not unparent in instance_finalize()
Posted by Akihiko Odaki 4 days, 10 hours ago
Children are automatically unparented so manually unparenting is
unnecessary.

Worse, automatic unparenting happens before the instance_finalize()
callback of the parent gets called, so object_unparent() calls in
the callback will refer to objects that are already unparented, which
is semantically incorrect.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/vfio/pci-quirks.c | 9 +--------
 hw/vfio/region.c     | 3 ---
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index c97606dbf194..b5da6afbf5b0 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1159,15 +1159,12 @@ void vfio_vga_quirk_exit(VFIOPCIDevice *vdev)
 
 void vfio_vga_quirk_finalize(VFIOPCIDevice *vdev)
 {
-    int i, j;
+    int i;
 
     for (i = 0; i < ARRAY_SIZE(vdev->vga->region); i++) {
         while (!QLIST_EMPTY(&vdev->vga->region[i].quirks)) {
             VFIOQuirk *quirk = QLIST_FIRST(&vdev->vga->region[i].quirks);
             QLIST_REMOVE(quirk, next);
-            for (j = 0; j < quirk->nr_mem; j++) {
-                object_unparent(OBJECT(&quirk->mem[j]));
-            }
             g_free(quirk->mem);
             g_free(quirk->data);
             g_free(quirk);
@@ -1207,14 +1204,10 @@ void vfio_bar_quirk_exit(VFIOPCIDevice *vdev, int nr)
 void vfio_bar_quirk_finalize(VFIOPCIDevice *vdev, int nr)
 {
     VFIOBAR *bar = &vdev->bars[nr];
-    int i;
 
     while (!QLIST_EMPTY(&bar->quirks)) {
         VFIOQuirk *quirk = QLIST_FIRST(&bar->quirks);
         QLIST_REMOVE(quirk, next);
-        for (i = 0; i < quirk->nr_mem; i++) {
-            object_unparent(OBJECT(&quirk->mem[i]));
-        }
         g_free(quirk->mem);
         g_free(quirk->data);
         g_free(quirk);
diff --git a/hw/vfio/region.c b/hw/vfio/region.c
index d04c57db630f..b165ab0b9378 100644
--- a/hw/vfio/region.c
+++ b/hw/vfio/region.c
@@ -365,12 +365,9 @@ void vfio_region_finalize(VFIORegion *region)
     for (i = 0; i < region->nr_mmaps; i++) {
         if (region->mmaps[i].mmap) {
             munmap(region->mmaps[i].mmap, region->mmaps[i].size);
-            object_unparent(OBJECT(&region->mmaps[i].mem));
         }
     }
 
-    object_unparent(OBJECT(region->mem));
-
     g_free(region->mem);
     g_free(region->mmaps);
 

-- 
2.51.0


Re: [PATCH v4 6/7] vfio: Do not unparent in instance_finalize()
Posted by Cédric Le Goater 4 days, 3 hours ago
On 9/24/25 06:37, Akihiko Odaki wrote:
> Children are automatically unparented so manually unparenting is
> unnecessary.
> 
> Worse, automatic unparenting happens before the instance_finalize()
> callback of the parent gets called, so object_unparent() calls in
> the callback will refer to objects that are already unparented, which
> is semantically incorrect.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   hw/vfio/pci-quirks.c | 9 +--------
>   hw/vfio/region.c     | 3 ---
>   2 files changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index c97606dbf194..b5da6afbf5b0 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -1159,15 +1159,12 @@ void vfio_vga_quirk_exit(VFIOPCIDevice *vdev)
>   
>   void vfio_vga_quirk_finalize(VFIOPCIDevice *vdev)
>   {
> -    int i, j;
> +    int i;
>   
>       for (i = 0; i < ARRAY_SIZE(vdev->vga->region); i++) {
>           while (!QLIST_EMPTY(&vdev->vga->region[i].quirks)) {
>               VFIOQuirk *quirk = QLIST_FIRST(&vdev->vga->region[i].quirks);
>               QLIST_REMOVE(quirk, next);
> -            for (j = 0; j < quirk->nr_mem; j++) {
> -                object_unparent(OBJECT(&quirk->mem[j]));
> -            }
>               g_free(quirk->mem);
>               g_free(quirk->data);
>               g_free(quirk);
> @@ -1207,14 +1204,10 @@ void vfio_bar_quirk_exit(VFIOPCIDevice *vdev, int nr)
>   void vfio_bar_quirk_finalize(VFIOPCIDevice *vdev, int nr)
>   {
>       VFIOBAR *bar = &vdev->bars[nr];
> -    int i;
>   
>       while (!QLIST_EMPTY(&bar->quirks)) {
>           VFIOQuirk *quirk = QLIST_FIRST(&bar->quirks);
>           QLIST_REMOVE(quirk, next);
> -        for (i = 0; i < quirk->nr_mem; i++) {
> -            object_unparent(OBJECT(&quirk->mem[i]));
> -        }
>           g_free(quirk->mem);
>           g_free(quirk->data);
>           g_free(quirk);
> diff --git a/hw/vfio/region.c b/hw/vfio/region.c
> index d04c57db630f..b165ab0b9378 100644
> --- a/hw/vfio/region.c
> +++ b/hw/vfio/region.c
> @@ -365,12 +365,9 @@ void vfio_region_finalize(VFIORegion *region)
>       for (i = 0; i < region->nr_mmaps; i++) {
>           if (region->mmaps[i].mmap) {
>               munmap(region->mmaps[i].mmap, region->mmaps[i].size);
> -            object_unparent(OBJECT(&region->mmaps[i].mem));
>           }
>       }
>   
> -    object_unparent(OBJECT(region->mem));
> -
>       g_free(region->mem);
>       g_free(region->mmaps);
>   
> 

What about vfio_subregion_unmap() calling object_unparent() too ?

C.