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 | 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);
--
2.34.1
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.duan@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
© 2016 - 2025 Red Hat, Inc.