[PATCH v2 0/2] Some trivial live update fixes

[PATCH v2 0/2] Some trivial live update fixes

[Zhenzhong Duan]

Hi

These are trivial VFIO live update fixes in corner cases.

1) potential SIGSEGV when unmap-all-vaddr failed
2) potential vfio_container_post_load failure

Thanks
Zhenzhong

Changelog:
v2:
- drop patch1,2 in v1 as they are merged
- squashed "local save" parts of patch3 into patch4 (Steve)
- s/DMA_MAP_FUNC/dma_map_fn (Steve)

Zhenzhong Duan (2):
  vfio/container: Fix potential SIGSEGV when recover from
    unmap-all-vaddr failure
  vfio/container: Fix vfio_container_post_load()

 include/hw/vfio/vfio-cpr.h |  7 ++++---
 hw/vfio/cpr-legacy.c       | 23 +++++++++--------------
 2 files changed, 13 insertions(+), 17 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] vfio/container: Fix potential SIGSEGV when recover from unmap-all-vaddr failure

[Zhenzhong Duan]

CPR overrides then restores dma_map in both outgoing and incoming QEMU, for
different reasons. But it only sets saved_dma_map in the target.

Fix it by always setting saved_dma_map.

Fixes: eba1f657cbb1 ("vfio/container: recover from unmap-all-vaddr failure")
Suggested-by: Steven Sistare <steven.sistare@oracle.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/cpr-legacy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/cpr-legacy.c b/hw/vfio/cpr-legacy.c
index a84c3247b7..0a5d1bd480 100644
--- a/hw/vfio/cpr-legacy.c
+++ b/hw/vfio/cpr-legacy.c
@@ -180,9 +180,9 @@ bool vfio_legacy_cpr_register_container(VFIOContainer *container, Error **errp)
     vmstate_register(NULL, -1, &vfio_container_vmstate, container);
 
     /* During incoming CPR, divert calls to dma_map. */
+    VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
+    container->cpr.saved_dma_map = vioc->dma_map;
     if (cpr_is_incoming()) {
-        VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
-        container->cpr.saved_dma_map = vioc->dma_map;
         vioc->dma_map = vfio_legacy_cpr_dma_map;
     }
 
-- 
2.34.1

[PATCH v2 2/2] vfio/container: Fix vfio_container_post_load()

[Zhenzhong Duan]

When there are multiple VFIO containers, vioc->dma_map is restored
multiple times, this made only first container work and remaining
containers using vioc->dma_map restored by first container.

Fix it by save and restore vioc->dma_map locally. saved_dma_map in
VFIOContainerCPR becomes useless and is removed.

Fixes: 7e9f21411302 ("vfio/container: restore DMA vaddr")
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/vfio/vfio-cpr.h |  7 ++++---
 hw/vfio/cpr-legacy.c       | 23 +++++++++--------------
 2 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/include/hw/vfio/vfio-cpr.h b/include/hw/vfio/vfio-cpr.h
index 8bf85b9f4e..dbb2a16b7a 100644
--- a/include/hw/vfio/vfio-cpr.h
+++ b/include/hw/vfio/vfio-cpr.h
@@ -16,14 +16,15 @@ struct VFIOContainer;
 struct VFIOContainerBase;
 struct VFIOGroup;
 
+typedef int (*dma_map_fn)(const struct VFIOContainerBase *bcontainer,
+                          hwaddr iova, ram_addr_t size, void *vaddr,
+                          bool readonly, MemoryRegion *mr);
+
 typedef struct VFIOContainerCPR {
     Error *blocker;
     bool vaddr_unmapped;
     NotifierWithReturn transfer_notifier;
     MemoryListener remap_listener;
-    int (*saved_dma_map)(const struct VFIOContainerBase *bcontainer,
-                         hwaddr iova, ram_addr_t size,
-                         void *vaddr, bool readonly, MemoryRegion *mr);
 } VFIOContainerCPR;
 
 typedef struct VFIODeviceCPR {
diff --git a/hw/vfio/cpr-legacy.c b/hw/vfio/cpr-legacy.c
index 0a5d1bd480..1216717546 100644
--- a/hw/vfio/cpr-legacy.c
+++ b/hw/vfio/cpr-legacy.c
@@ -99,20 +99,21 @@ static int vfio_container_post_load(void *opaque, int version_id)
 {
     VFIOContainer *container = opaque;
     VFIOContainerBase *bcontainer = &container->bcontainer;
-    VFIOGroup *group;
+    VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
+    dma_map_fn saved_dma_map = vioc->dma_map;
     Error *local_err = NULL;
 
+    /* During incoming CPR, divert calls to dma_map. */
+    vioc->dma_map = vfio_legacy_cpr_dma_map;
+
     if (!vfio_listener_register(bcontainer, &local_err)) {
         error_report_err(local_err);
         return -1;
     }
 
-    QLIST_FOREACH(group, &container->group_list, container_next) {
-        VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
+    /* Restore original dma_map function */
+    vioc->dma_map = saved_dma_map;
 
-        /* Restore original dma_map function */
-        vioc->dma_map = container->cpr.saved_dma_map;
-    }
     return 0;
 }
 
@@ -148,6 +149,7 @@ static int vfio_cpr_fail_notifier(NotifierWithReturn *notifier,
          */
 
         VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
+        dma_map_fn saved_dma_map = vioc->dma_map;
         vioc->dma_map = vfio_legacy_cpr_dma_map;
 
         container->cpr.remap_listener = (MemoryListener) {
@@ -158,7 +160,7 @@ static int vfio_cpr_fail_notifier(NotifierWithReturn *notifier,
                                  bcontainer->space->as);
         memory_listener_unregister(&container->cpr.remap_listener);
         container->cpr.vaddr_unmapped = false;
-        vioc->dma_map = container->cpr.saved_dma_map;
+        vioc->dma_map = saved_dma_map;
     }
     return 0;
 }
@@ -179,13 +181,6 @@ bool vfio_legacy_cpr_register_container(VFIOContainer *container, Error **errp)
 
     vmstate_register(NULL, -1, &vfio_container_vmstate, container);
 
-    /* During incoming CPR, divert calls to dma_map. */
-    VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
-    container->cpr.saved_dma_map = vioc->dma_map;
-    if (cpr_is_incoming()) {
-        vioc->dma_map = vfio_legacy_cpr_dma_map;
-    }
-
     migration_add_notifier_mode(&container->cpr.transfer_notifier,
                                 vfio_cpr_fail_notifier,
                                 MIG_MODE_CPR_TRANSFER);
-- 
2.34.1

Re: [PATCH v2 1/2] vfio/container: Fix potential SIGSEGV when recover from unmap-all-vaddr failure

[Steven Sistare]

On 6/27/2025 2:33 AM, Zhenzhong Duan wrote:
> CPR overrides then restores dma_map in both outgoing and incoming QEMU, for
> different reasons. But it only sets saved_dma_map in the target.
> 
> Fix it by always setting saved_dma_map.
> 
> Fixes: eba1f657cbb1 ("vfio/container: recover from unmap-all-vaddr failure")
> Suggested-by: Steven Sistare <steven.sistare@oracle.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/vfio/cpr-legacy.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/vfio/cpr-legacy.c b/hw/vfio/cpr-legacy.c
> index a84c3247b7..0a5d1bd480 100644
> --- a/hw/vfio/cpr-legacy.c
> +++ b/hw/vfio/cpr-legacy.c
> @@ -180,9 +180,9 @@ bool vfio_legacy_cpr_register_container(VFIOContainer *container, Error **errp)
>       vmstate_register(NULL, -1, &vfio_container_vmstate, container);
>   
>       /* During incoming CPR, divert calls to dma_map. */
> +    VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
> +    container->cpr.saved_dma_map = vioc->dma_map;
>       if (cpr_is_incoming()) {
> -        VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
> -        container->cpr.saved_dma_map = vioc->dma_map;
>           vioc->dma_map = vfio_legacy_cpr_dma_map;
>       }  

Reviewed-by: Steve Sistare <steven.sistare@oracle.com>

Re: [PATCH v2 2/2] vfio/container: Fix vfio_container_post_load()

[Steven Sistare]

On 6/27/2025 2:33 AM, Zhenzhong Duan wrote:
> When there are multiple VFIO containers, vioc->dma_map is restored
> multiple times, this made only first container work and remaining
> containers using vioc->dma_map restored by first container.
> 
> Fix it by save and restore vioc->dma_map locally. saved_dma_map in
> VFIOContainerCPR becomes useless and is removed.
> 
> Fixes: 7e9f21411302 ("vfio/container: restore DMA vaddr")
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   include/hw/vfio/vfio-cpr.h |  7 ++++---
>   hw/vfio/cpr-legacy.c       | 23 +++++++++--------------
>   2 files changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/include/hw/vfio/vfio-cpr.h b/include/hw/vfio/vfio-cpr.h
> index 8bf85b9f4e..dbb2a16b7a 100644
> --- a/include/hw/vfio/vfio-cpr.h
> +++ b/include/hw/vfio/vfio-cpr.h
> @@ -16,14 +16,15 @@ struct VFIOContainer;
>   struct VFIOContainerBase;
>   struct VFIOGroup;
>   
> +typedef int (*dma_map_fn)(const struct VFIOContainerBase *bcontainer,
> +                          hwaddr iova, ram_addr_t size, void *vaddr,
> +                          bool readonly, MemoryRegion *mr);
> +
>   typedef struct VFIOContainerCPR {
>       Error *blocker;
>       bool vaddr_unmapped;
>       NotifierWithReturn transfer_notifier;
>       MemoryListener remap_listener;
> -    int (*saved_dma_map)(const struct VFIOContainerBase *bcontainer,
> -                         hwaddr iova, ram_addr_t size,
> -                         void *vaddr, bool readonly, MemoryRegion *mr);
>   } VFIOContainerCPR;
>   
>   typedef struct VFIODeviceCPR {
> diff --git a/hw/vfio/cpr-legacy.c b/hw/vfio/cpr-legacy.c
> index 0a5d1bd480..1216717546 100644
> --- a/hw/vfio/cpr-legacy.c
> +++ b/hw/vfio/cpr-legacy.c
> @@ -99,20 +99,21 @@ static int vfio_container_post_load(void *opaque, int version_id)
>   {
>       VFIOContainer *container = opaque;
>       VFIOContainerBase *bcontainer = &container->bcontainer;
> -    VFIOGroup *group;
> +    VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
> +    dma_map_fn saved_dma_map = vioc->dma_map;
>       Error *local_err = NULL;
>   
> +    /* During incoming CPR, divert calls to dma_map. */
> +    vioc->dma_map = vfio_legacy_cpr_dma_map;
> +
>       if (!vfio_listener_register(bcontainer, &local_err)) {
>           error_report_err(local_err);
>           return -1;
>       }
>   
> -    QLIST_FOREACH(group, &container->group_list, container_next) {
> -        VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
> +    /* Restore original dma_map function */
> +    vioc->dma_map = saved_dma_map;
>   
> -        /* Restore original dma_map function */
> -        vioc->dma_map = container->cpr.saved_dma_map;
> -    }
>       return 0;
>   }
>   
> @@ -148,6 +149,7 @@ static int vfio_cpr_fail_notifier(NotifierWithReturn *notifier,
>            */
>   
>           VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
> +        dma_map_fn saved_dma_map = vioc->dma_map;
>           vioc->dma_map = vfio_legacy_cpr_dma_map;
>   
>           container->cpr.remap_listener = (MemoryListener) {
> @@ -158,7 +160,7 @@ static int vfio_cpr_fail_notifier(NotifierWithReturn *notifier,
>                                    bcontainer->space->as);
>           memory_listener_unregister(&container->cpr.remap_listener);
>           container->cpr.vaddr_unmapped = false;
> -        vioc->dma_map = container->cpr.saved_dma_map;
> +        vioc->dma_map = saved_dma_map;
>       }
>       return 0;
>   }
> @@ -179,13 +181,6 @@ bool vfio_legacy_cpr_register_container(VFIOContainer *container, Error **errp)
>   
>       vmstate_register(NULL, -1, &vfio_container_vmstate, container);
>   
> -    /* During incoming CPR, divert calls to dma_map. */
> -    VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
> -    container->cpr.saved_dma_map = vioc->dma_map;
> -    if (cpr_is_incoming()) {
> -        vioc->dma_map = vfio_legacy_cpr_dma_map;
> -    }
> -
>       migration_add_notifier_mode(&container->cpr.transfer_notifier,
>                                   vfio_cpr_fail_notifier,
>                                   MIG_MODE_CPR_TRANSFER);

Reviewed-by: Steve Sistare <steven.sistare@oracle.com>

Re: [PATCH v2 0/2] Some trivial live update fixes

[Cédric Le Goater]

On 6/27/25 08:33, Zhenzhong Duan wrote:
> Hi
> 
> These are trivial VFIO live update fixes in corner cases.
> 
> 1) potential SIGSEGV when unmap-all-vaddr failed
> 2) potential vfio_container_post_load failure
> 
> Thanks
> Zhenzhong
> 
> Changelog:
> v2:
> - drop patch1,2 in v1 as they are merged
> - squashed "local save" parts of patch3 into patch4 (Steve)
> - s/DMA_MAP_FUNC/dma_map_fn (Steve)
> 
> Zhenzhong Duan (2):
>    vfio/container: Fix potential SIGSEGV when recover from
>      unmap-all-vaddr failure
>    vfio/container: Fix vfio_container_post_load()
> 
>   include/hw/vfio/vfio-cpr.h |  7 ++++---
>   hw/vfio/cpr-legacy.c       | 23 +++++++++--------------
>   2 files changed, 13 insertions(+), 17 deletions(-)
> 


Applied to vfio-next.

Thanks,

C.