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

Daniel Paziyski posted 1 patch 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260726183008.160498-1-danielpaziyski@gmail.com
Maintainers: Keith Busch <kbusch@kernel.org>, Klaus Jensen <its@irrelevant.dk>, Jesper Devantier <foss@defmacro.it>
hw/nvme/ctrl.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH] hw/nvme: fix assertion failure on subregion removal when removing a nvme controller with a msix exclusive bar
Posted by Daniel Paziyski 2 months ago
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 <danielpaziyski@gmail.com>
---
 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
Re: [PATCH] hw/nvme: fix assertion failure on subregion removal when removing a nvme controller with a msix exclusive bar
Posted by Klaus Jensen 2 months ago
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 <danielpaziyski@gmail.com>
> ---
>  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 <k.jensen@samsung.com>