Removing the PBA memory region from its container in
vfio_user_msix_teardown() is semantically incorrect as the reference to
the region is already deleted when the function is called.

The operation is unnecessary in the first place since the PCI code
removes all BARs during unrealization, so stop removing the PBA memory
region in vfio_user_msix_teardown().

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/vfio-user/pci.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/hw/vfio-user/pci.c b/hw/vfio-user/pci.c
index b53ed3b456f9..1a3741a29a1a 100644
--- a/hw/vfio-user/pci.c
+++ b/hw/vfio-user/pci.c
@@ -74,12 +74,6 @@ static void vfio_user_msix_setup(VFIOPCIDevice *vdev)
 
 static void vfio_user_msix_teardown(VFIOPCIDevice *vdev)
 {
-    MemoryRegion *mr, *sub;
-
-    mr = vdev->bars[vdev->msix->pba_bar].mr;
-    sub = vdev->msix->pba_region;
-    memory_region_del_subregion(mr, sub);
-
     g_free(vdev->msix->pba_region);
     vdev->msix->pba_region = NULL;
 }

---
base-commit: 94474a7733a57365d5a27efc28c05462e90e8944
change-id: 20251010-vfio-af834e5f3807

Best regards,
--  
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

On Fri, Oct 10, 2025 at 06:34:38PM +0900, Akihiko Odaki wrote:

> Removing the PBA memory region from its container in
> vfio_user_msix_teardown() is semantically incorrect as the reference to
> the region is already deleted when the function is called.

Can you clue me in?

378     if (vdev->msix != NULL) {
379         vfio_user_msix_teardown(vdev);
380     }
381
382     vfio_pci_put_device(vdev);

vfio_pci_put_device() -> vfio_bars_finalize() -> vfio_region_finalize()

Where is the "reference to the region is already deleted" code that runs prior
to line :379 that you're referring to?

What about vfio_pci_teardown_msi() - isn't that the same?

> The operation is unnecessary in the first place since the PCI code
> removes all BARs during unrealization

Might be worth mentioning vfio_pci_bars_exit() (which is what I presume you're
referring to).

regards
john

On 2025/10/10 18:54, John Levon wrote:
> On Fri, Oct 10, 2025 at 06:34:38PM +0900, Akihiko Odaki wrote:
> 
>> Removing the PBA memory region from its container in
>> vfio_user_msix_teardown() is semantically incorrect as the reference to
>> the region is already deleted when the function is called.
> 
> Can you clue me in?
> 
> 378     if (vdev->msix != NULL) {
> 379         vfio_user_msix_teardown(vdev);
> 380     }
> 381
> 382     vfio_pci_put_device(vdev);
> 
> vfio_pci_put_device() -> vfio_bars_finalize() -> vfio_region_finalize()
> 
> Where is the "reference to the region is already deleted" code that runs prior
> to line :379 that you're referring to?
> 
> What about vfio_pci_teardown_msi() - isn't that the same?
> 
>> The operation is unnecessary in the first place since the PCI code
>> removes all BARs during unrealization
> 
> Might be worth mentioning vfio_pci_bars_exit() (which is what I presume you're
> referring to).
I referred to the object_property_del_all() call in object_finalize(). 
This removes the references to all children. I was too lazy to mention 
that but I agree it's worthwhile.

Regards,
Akihiko Odaki