* [PATCH] vfio-user: Do not delete the subregion
@ 2025-10-10 9:34 Akihiko Odaki
2025-10-10 9:54 ` John Levon
0 siblings, 1 reply; 3+ messages in thread
From: Akihiko Odaki @ 2025-10-10 9:34 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Peter Xu,
David Hildenbrand, Philippe Mathieu-Daudé, Richard Henderson,
Helge Deller, Marc-André Lureau, Michael S. Tsirkin,
Gerd Hoffmann, John Snow, qemu-block, Keith Busch, Klaus Jensen,
Jesper Devantier, Marcel Apfelbaum, Nicholas Piggin, qemu-ppc,
John Levon, Thanos Makatos, Yanan Wang, BALATON Zoltan,
Jiaxun Yang, Daniel Henrique Barboza, David Gibson,
Harsh Prateek Bora, Alexey Kardashevskiy, Alex Bennée,
Fabiano Rosas, Thomas Huth, Laurent Vivier, Peter Maydell,
Aurelien Jarno, Aleksandar Rikalo, Max Filippov,
Hervé Poussineau, Mark Cave-Ayland, Artyom Tarasenko,
Akihiko Odaki
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>
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] vfio-user: Do not delete the subregion
2025-10-10 9:34 [PATCH] vfio-user: Do not delete the subregion Akihiko Odaki
@ 2025-10-10 9:54 ` John Levon
2025-10-10 9:58 ` Akihiko Odaki
0 siblings, 1 reply; 3+ messages in thread
From: John Levon @ 2025-10-10 9:54 UTC (permalink / raw)
To: Akihiko Odaki
Cc: qemu-devel, Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Peter Xu,
David Hildenbrand, Philippe Mathieu-Daudé, Richard Henderson,
Helge Deller, Marc-André Lureau, Michael S. Tsirkin,
Gerd Hoffmann, John Snow, qemu-block, Keith Busch, Klaus Jensen,
Jesper Devantier, Marcel Apfelbaum, Nicholas Piggin, qemu-ppc,
Thanos Makatos, Yanan Wang, BALATON Zoltan, Jiaxun Yang,
Daniel Henrique Barboza, David Gibson, Harsh Prateek Bora,
Alexey Kardashevskiy, Alex Bennée, Fabiano Rosas,
Thomas Huth, Laurent Vivier, Peter Maydell, Aurelien Jarno,
Aleksandar Rikalo, Max Filippov, Hervé Poussineau,
Mark Cave-Ayland, Artyom Tarasenko
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] vfio-user: Do not delete the subregion
2025-10-10 9:54 ` John Levon
@ 2025-10-10 9:58 ` Akihiko Odaki
0 siblings, 0 replies; 3+ messages in thread
From: Akihiko Odaki @ 2025-10-10 9:58 UTC (permalink / raw)
To: John Levon
Cc: qemu-devel, Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Peter Xu,
David Hildenbrand, Philippe Mathieu-Daudé, Richard Henderson,
Helge Deller, Marc-André Lureau, Michael S. Tsirkin,
Gerd Hoffmann, John Snow, qemu-block, Keith Busch, Klaus Jensen,
Jesper Devantier, Marcel Apfelbaum, Nicholas Piggin, qemu-ppc,
Thanos Makatos, Yanan Wang, BALATON Zoltan, Jiaxun Yang,
Daniel Henrique Barboza, David Gibson, Harsh Prateek Bora,
Alexey Kardashevskiy, Alex Bennée, Fabiano Rosas,
Thomas Huth, Laurent Vivier, Peter Maydell, Aurelien Jarno,
Aleksandar Rikalo, Max Filippov, Hervé Poussineau,
Mark Cave-Ayland, Artyom Tarasenko
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-10-10 10:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-10 9:34 [PATCH] vfio-user: Do not delete the subregion Akihiko Odaki
2025-10-10 9:54 ` John Levon
2025-10-10 9:58 ` Akihiko Odaki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).