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

[Steven Sistare]

On 6/23/2025 6:22 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.d...@intel.com>
---
  include/hw/vfio/vfio-cpr.h |  1 -
  hw/vfio/cpr-legacy.c       | 20 +++++++-------------
  2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/include/hw/vfio/vfio-cpr.h b/include/hw/vfio/vfio-cpr.h
index aef542e93c..71a1c1a70c 100644
--- a/include/hw/vfio/vfio-cpr.h
+++ b/include/hw/vfio/vfio-cpr.h
@@ -25,7 +25,6 @@ typedef struct VFIOContainerCPR {
      bool vaddr_unmapped;
      NotifierWithReturn transfer_notifier;
      MemoryListener remap_listener;
-    DMA_MAP_FUNC saved_dma_map;
  } VFIOContainerCPR;

typedef struct VFIODeviceCPR {

diff --git a/hw/vfio/cpr-legacy.c b/hw/vfio/cpr-legacy.c
index 100a8db74d..ff45a5128f 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_FUNC 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;
  }

@@ -180,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. */

-    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;
-    }
-
      migration_add_notifier_mode(&container->cpr.transfer_notifier,
                                  vfio_cpr_fail_notifier,
                                  MIG_MODE_CPR_TRANSFER);

Good fix, thanks. All looks good.  I will wait to see the "local save" parts
of patch 3 squashed into patch 4 before sending RB.

One nit about DMA_MAP_FUNC -- the majority of typedefs for function signatures
use lower case.  See
  git grep typedef | fgrep '(*'

- Steve