Re: [PATCH] hw/nvme: fix assertion failure on subregion removal when removing a nvme controller with a msix exclusive bar

Klaus Jensen <[email protected]>
Newsgroups gmane.comp.emulators.qemu.block,gmane.comp.emulators.qemu
Message-ID <[email protected]>
On Jul 26 20:30, Daniel Paziyski wrote:
> When a controller is created with a MSI-X exclusive BAR, the bar0 memory region
> is not used at all, and so, the iomem region is not added as a subregion of it.
> However, when removing a NVMe controller, the iomem region is unconditionally
> removed as a subregion of bar0, causing an assertion failure. Remove the iomem
> memory region as a subregion of bar0 only if not using a MSI-X exclusive BAR.
> 
> QEMU options (requires a hotunplug-aware OS):
> 
>         -M q35 -device pcie-root-port,id=rp0 \
>         -device nvme,serial=ctrl0,id=ctrl0,bus=rp0,msix-exclusive-bar=on
> 
> In the QEMU monitor, or by causing an ejection from the OS:
> 
>         device_del ctrl0
> 
> Message in stderr:
> 
> qemu-system-x86_64: ../system/memory.c:2617: memory_region_del_subregion: Assertion `subregion->container == mr' failed.
> 
> Fixes: fa905f65c554 ("hw/nvme: add machine compatibility parameter to enable msix exclusive bar")
> Fixes: 9162f1012576 ("hw/nvme: fix msix_uninit with exclusive bar")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4090
> Signed-off-by: Daniel Paziyski <[email protected]>
> ---
>  hw/nvme/ctrl.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index a67e159889..142fbe033e 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -9710,10 +9710,9 @@ static void nvme_exit(PCIDevice *pci_dev)
>          msix_uninit_exclusive_bar(pci_dev);
>      } else {
>          msix_uninit(pci_dev, &n->bar0, &n->bar0);
> +        memory_region_del_subregion(&n->bar0, &n->iomem);
>      }
>  
> -    memory_region_del_subregion(&n->bar0, &n->iomem);
> -
>      migrate_del_blocker(&n->migration_blocker);
>  }
>  
> -- 
> 2.55.0
> 
> 

LGTM.

Reviewed-by: Klaus Jensen <[email protected]>
signature.asc (application/pgp-signature, 488 B)
-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEUigzqnXi3OaiR2bATeGvMW1PDekFAmpoXkAACgkQTeGvMW1P
DelWrAf/UBiPJfTdQvFLJrlo3vThuXkmOHgH3JUMtHsjyPS3zOCKW4fk+zmhPrTi
lt+jPRw315ebQzwKJx3Zo9dH0TP8RckEq5nuR/T4oSRhH44GFuKfk2htWZBmjpdX
/UD5ymNkhP9n4N55Umpto4obSWvOVds4dxfNl4kCFl04KrAClhXwO31n5W1GixR+
DkSPo53mKIIlhCZeZCFUZKJySDIVgTTRH3pkJF8bY/bzr0stQAjCefKdSw2BC3x7
+LfZye/CNf2aCl4AdaQkmZg3SXse4RXmUMCrEt9B7HwB2aBvmh1J1Cx9lAu4dInk
m/73BeTKzh6WXc8UqrImJEaFKU+mzQ==
=VYN0
-----END PGP SIGNATURE-----
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.