Re: [PATCH V1 06/26] vfio/container: preserve DMA mappings
On 2/3/2025 1:25 PM, Cédric Le Goater wrote:
On 1/29/25 15:43, Steve Sistare wrote:
Preserve DMA mappings during cpr-transfer.
In the container pre_save handler, suspend the use of virtual addresses
in DMA mappings with VFIO_DMA_UNMAP_FLAG_VADDR, because guest RAM will
be remapped at a different VA after exec. DMA to already-mapped pages
continues.
Because the vaddr is temporarily invalid, mediated devices cannot be
supported, so add a blocker for them. This restriction will not apply
to iommufd containers when CPR is added for them in a future patch.
In new QEMU, do not register the memory listener at device creation time.
Register it later, in the container post_load handler, after all vmstate
that may affect regions and mapping boundaries has been loaded. The
post_load registration will cause the listener to invoke its callback on
each flat section, and the calls will match the mappings remembered by the
kernel. Modify vfio_dma_map (which is called by the listener) to pass the
new VA to the kernel using VFIO_DMA_MAP_FLAG_VADDR.
Signed-off-by: Steve Sistare
---
hw/vfio/container.c | 44 +++
hw/vfio/cpr-legacy.c | 32 +++
include/hw/vfio/vfio-common.h | 3 +++
3 files changed, 75 insertions(+), 4 deletions(-)
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 81d0ccc..2b5125e 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -32,6 +32,7 @@
#include "trace.h"
#include "qapi/error.h"
#include "migration/cpr.h"
+#include "migration/blocker.h"
#include "pci.h"
VFIOGroupList vfio_group_list =
@@ -132,6 +133,8 @@ static int vfio_legacy_dma_unmap(const VFIOContainerBase
*bcontainer,
int ret;
Error *local_err = NULL;
+ assert(!container->reused);
+
if (iotlb && vfio_devices_all_dirty_tracking_started(bcontainer)) {
if (!vfio_devices_all_device_dirty_tracking(bcontainer) &&
bcontainer->dirty_pages_supported) {
@@ -183,12 +186,24 @@ static int vfio_legacy_dma_map(const VFIOContainerBase
*bcontainer, hwaddr iova,
bcontainer);
struct vfio_iommu_type1_dma_map map = {
.argsz = sizeof(map),
- .flags = VFIO_DMA_MAP_FLAG_READ,
.vaddr = (__u64)(uintptr_t)vaddr,
.iova = iova,
.size = size,
};
+ /*
+ * Set the new vaddr for any mappings registered during cpr load.
+ * Reused is cleared thereafter.
+ */
+ if (container->reused) {
+ map.flags = VFIO_DMA_MAP_FLAG_VADDR;
+ if (ioctl(container->fd, VFIO_IOMMU_MAP_DMA, &map)) {
+ goto fail;
+ }
+ return 0;
+ }
This is a bit ugly.
When reaching routine vfio_attach_device(), could we detect that CPR is
in progress and replace the 'VFIOIOMMUClass *' temporarily with a set of
CPR specific handlers ?
Good idea, I'll try it. I wrote this code years ago before the dma
map and unmap functions were defined in an ops vector.
+
+ map.flags = VFIO_DMA_MAP_FLAG_READ;
if (!readonly) {
map.flags |= VFIO_DMA_MAP_FLAG_WRITE;
}
@@ -205,7 +220,11 @@ static int vfio_legacy_dma_map(const VFIOContainerBase
*bcontainer, hwaddr iova,
return 0;
}
- error_report("VFIO_MAP_DMA failed: %s", strerror(errno));
+fail:
+ error_report("vfio_dma_map %s (iova %lu, size %ld, va %p): %s",
+ (container->reused ? "VADDR" : ""), iova, size, vaddr,
+ strerror(errno));
+
FYI, I am currently trying to remove this error report.
return -errno;
}
@@ -689,8 +708,17 @@ static bool vfio_connect_container(VFIOGroup *group,
AddressSpace *as,
group->container = container;
QLIST_INSERT_HEAD(&container->group_list, group, container_next);
- bcontainer->listener = vfio_memory_listener;
- memory_listener_register(&bcontainer->listener, bcontainer->space->as);
+ /*
+ * If reused, register the listener later, after all state that may
+ * affect regions and mapping boundaries has been cpr load'ed. Later,
+ * the listener will invoke its callback on each flat section and call
+ * vfio_dma_map to supply the new vaddr, and the calls will match the
+ * mappings remembered by the kernel.
+ */
+ if (!reused) {
+ bcontainer->listener = vfio_memory_listener;
+ memory_listener_register(&bcontainer->listener, bcontainer->space->as);
+ }
oh ! This is an important change. Please move in its own patch.
OK.
if (bcontainer->error) {
error_propagate_prepend(errp, bcontainer->error,
@@ -1002,6 +1030,13 @@ static bool vfio_legacy_attach_device(const char *name,
VFIODevice *vbasedev,
return false;
}
+ if (vbasedev->mdev) {
+ error_setg(&vbasedev->cpr_mdev_blocker,
+ "CPR does not support vfio mdev %s", vbasedev->name);
+ migrate_add_blocker_modes(&vbasedev->cpr_mdev_blocker, &error_fatal
Re: [PATCH V1 06/26] vfio/container: preserve DMA mappings
On 1/29/25 15:43, Steve Sistare wrote:
Preserve DMA mappings during cpr-transfer.
In the container pre_save handler, suspend the use of virtual addresses
in DMA mappings with VFIO_DMA_UNMAP_FLAG_VADDR, because guest RAM will
be remapped at a different VA after exec. DMA to already-mapped pages
continues.
Because the vaddr is temporarily invalid, mediated devices cannot be
supported, so add a blocker for them. This restriction will not apply
to iommufd containers when CPR is added for them in a future patch.
In new QEMU, do not register the memory listener at device creation time.
Register it later, in the container post_load handler, after all vmstate
that may affect regions and mapping boundaries has been loaded. The
post_load registration will cause the listener to invoke its callback on
each flat section, and the calls will match the mappings remembered by the
kernel. Modify vfio_dma_map (which is called by the listener) to pass the
new VA to the kernel using VFIO_DMA_MAP_FLAG_VADDR.
Signed-off-by: Steve Sistare
---
hw/vfio/container.c | 44 +++
hw/vfio/cpr-legacy.c | 32 +++
include/hw/vfio/vfio-common.h | 3 +++
3 files changed, 75 insertions(+), 4 deletions(-)
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 81d0ccc..2b5125e 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -32,6 +32,7 @@
#include "trace.h"
#include "qapi/error.h"
#include "migration/cpr.h"
+#include "migration/blocker.h"
#include "pci.h"
VFIOGroupList vfio_group_list =
@@ -132,6 +133,8 @@ static int vfio_legacy_dma_unmap(const VFIOContainerBase
*bcontainer,
int ret;
Error *local_err = NULL;
+assert(!container->reused);
+
if (iotlb && vfio_devices_all_dirty_tracking_started(bcontainer)) {
if (!vfio_devices_all_device_dirty_tracking(bcontainer) &&
bcontainer->dirty_pages_supported) {
@@ -183,12 +186,24 @@ static int vfio_legacy_dma_map(const VFIOContainerBase
*bcontainer, hwaddr iova,
bcontainer);
struct vfio_iommu_type1_dma_map map = {
.argsz = sizeof(map),
-.flags = VFIO_DMA_MAP_FLAG_READ,
.vaddr = (__u64)(uintptr_t)vaddr,
.iova = iova,
.size = size,
};
+/*
+ * Set the new vaddr for any mappings registered during cpr load.
+ * Reused is cleared thereafter.
+ */
+if (container->reused) {
+map.flags = VFIO_DMA_MAP_FLAG_VADDR;
+if (ioctl(container->fd, VFIO_IOMMU_MAP_DMA, &map)) {
+goto fail;
+}
+return 0;
+}
This is a bit ugly.
When reaching routine vfio_attach_device(), could we detect that CPR is
in progress and replace the 'VFIOIOMMUClass *' temporarily with a set of
CPR specific handlers ?
+
+map.flags = VFIO_DMA_MAP_FLAG_READ;
if (!readonly) {
map.flags |= VFIO_DMA_MAP_FLAG_WRITE;
}
@@ -205,7 +220,11 @@ static int vfio_legacy_dma_map(const VFIOContainerBase
*bcontainer, hwaddr iova,
return 0;
}
-error_report("VFIO_MAP_DMA failed: %s", strerror(errno));
+fail:
+error_report("vfio_dma_map %s (iova %lu, size %ld, va %p): %s",
+(container->reused ? "VADDR" : ""), iova, size, vaddr,
+strerror(errno));
+
FYI, I am currently trying to remove this error report.
return -errno;
}
@@ -689,8 +708,17 @@ static bool vfio_connect_container(VFIOGroup *group, AddressSpace *as,
group->container = container;
QLIST_INSERT_HEAD(&container->group_list, group, container_next);
-bcontainer->listener = vfio_memory_listener;
-memory_listener_register(&bcontainer->listener, bcontainer->space->as);
+/*
+ * If reused, register the listener later, after all state that may
+ * affect regions and mapping boundaries has been cpr load'ed. Later,
+ * the listener will invoke its callback on each flat section and call
+ * vfio_dma_map to supply the new vaddr, and the calls will match the
+ * mappings remembered by the kernel.
+ */
+if (!reused) {
+bcontainer->listener = vfio_memory_listener;
+memory_listener_register(&bcontainer->listener, bcontainer->space->as);
+}
oh ! This is an important change. Please move in its own patch.
if (bcontainer->error) {
error_propagate_prepend(errp, bcontainer->error,
@@ -1002,6 +1030,13 @@ static bool vfio_legacy_attach_device(const char *name,
VFIODevice *vbasedev,
return false;
}
+if (vbasedev->mdev) {
+error_setg(&vbasedev->cpr_mdev_blocker,
+ "CPR does not support vfio mdev %s", vbasedev->name);
+migrate_add_blocker_modes(&vbasedev->cpr_mdev_blocker, &error_fatal,
+ MIG_MODE_CPR_TRANSFER, -1);
+}
same here, the cpr blocker for mdev devices should be in its own patch.
[PATCH V1 06/26] vfio/container: preserve DMA mappings
Preserve DMA mappings during cpr-transfer.
In the container pre_save handler, suspend the use of virtual addresses
in DMA mappings with VFIO_DMA_UNMAP_FLAG_VADDR, because guest RAM will
be remapped at a different VA after exec. DMA to already-mapped pages
continues.
Because the vaddr is temporarily invalid, mediated devices cannot be
supported, so add a blocker for them. This restriction will not apply
to iommufd containers when CPR is added for them in a future patch.
In new QEMU, do not register the memory listener at device creation time.
Register it later, in the container post_load handler, after all vmstate
that may affect regions and mapping boundaries has been loaded. The
post_load registration will cause the listener to invoke its callback on
each flat section, and the calls will match the mappings remembered by the
kernel. Modify vfio_dma_map (which is called by the listener) to pass the
new VA to the kernel using VFIO_DMA_MAP_FLAG_VADDR.
Signed-off-by: Steve Sistare
---
hw/vfio/container.c | 44 +++
hw/vfio/cpr-legacy.c | 32 +++
include/hw/vfio/vfio-common.h | 3 +++
3 files changed, 75 insertions(+), 4 deletions(-)
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 81d0ccc..2b5125e 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -32,6 +32,7 @@
#include "trace.h"
#include "qapi/error.h"
#include "migration/cpr.h"
+#include "migration/blocker.h"
#include "pci.h"
VFIOGroupList vfio_group_list =
@@ -132,6 +133,8 @@ static int vfio_legacy_dma_unmap(const VFIOContainerBase
*bcontainer,
int ret;
Error *local_err = NULL;
+assert(!container->reused);
+
if (iotlb && vfio_devices_all_dirty_tracking_started(bcontainer)) {
if (!vfio_devices_all_device_dirty_tracking(bcontainer) &&
bcontainer->dirty_pages_supported) {
@@ -183,12 +186,24 @@ static int vfio_legacy_dma_map(const VFIOContainerBase
*bcontainer, hwaddr iova,
bcontainer);
struct vfio_iommu_type1_dma_map map = {
.argsz = sizeof(map),
-.flags = VFIO_DMA_MAP_FLAG_READ,
.vaddr = (__u64)(uintptr_t)vaddr,
.iova = iova,
.size = size,
};
+/*
+ * Set the new vaddr for any mappings registered during cpr load.
+ * Reused is cleared thereafter.
+ */
+if (container->reused) {
+map.flags = VFIO_DMA_MAP_FLAG_VADDR;
+if (ioctl(container->fd, VFIO_IOMMU_MAP_DMA, &map)) {
+goto fail;
+}
+return 0;
+}
+
+map.flags = VFIO_DMA_MAP_FLAG_READ;
if (!readonly) {
map.flags |= VFIO_DMA_MAP_FLAG_WRITE;
}
@@ -205,7 +220,11 @@ static int vfio_legacy_dma_map(const VFIOContainerBase
*bcontainer, hwaddr iova,
return 0;
}
-error_report("VFIO_MAP_DMA failed: %s", strerror(errno));
+fail:
+error_report("vfio_dma_map %s (iova %lu, size %ld, va %p): %s",
+(container->reused ? "VADDR" : ""), iova, size, vaddr,
+strerror(errno));
+
return -errno;
}
@@ -689,8 +708,17 @@ static bool vfio_connect_container(VFIOGroup *group,
AddressSpace *as,
group->container = container;
QLIST_INSERT_HEAD(&container->group_list, group, container_next);
-bcontainer->listener = vfio_memory_listener;
-memory_listener_register(&bcontainer->listener, bcontainer->space->as);
+/*
+ * If reused, register the listener later, after all state that may
+ * affect regions and mapping boundaries has been cpr load'ed. Later,
+ * the listener will invoke its callback on each flat section and call
+ * vfio_dma_map to supply the new vaddr, and the calls will match the
+ * mappings remembered by the kernel.
+ */
+if (!reused) {
+bcontainer->listener = vfio_memory_listener;
+memory_listener_register(&bcontainer->listener, bcontainer->space->as);
+}
if (bcontainer->error) {
error_propagate_prepend(errp, bcontainer->error,
@@ -1002,6 +1030,13 @@ static bool vfio_legacy_attach_device(const char *name,
VFIODevice *vbasedev,
return false;
}
+if (vbasedev->mdev) {
+error_setg(&vbasedev->cpr_mdev_blocker,
+ "CPR does not support vfio mdev %s", vbasedev->name);
+migrate_add_blocker_modes(&vbasedev->cpr_mdev_blocker, &error_fatal,
+ MIG_MODE_CPR_TRANSFER, -1);
+}
+
bcontainer = &group->container->bcontainer;
vbasedev->bcontainer = bcontainer;
QLIST_INSERT_HEAD(&bcontainer->device_list, vbasedev, container_next);
@@ -1018,6 +1053,7 @@ static void vfio_legacy_detach_device(VFIODevice
*vbasedev)
QLIST_REMOVE(vbasedev, container_next);
vbasedev->bcontainer = NULL;
trace_vfio_detach_device(vbasedev->name, group->groupid);
+migrate_del_blocker(&vbasedev->cpr_mdev_blocker);
vfio_put_base_device(vbasedev);
