On 5/15/25 21:08, Steven Sistare wrote:
On 5/15/2025 9:42 AM, Cédric Le Goater wrote:
On 5/12/25 17:32, Steve Sistare wrote:
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.
The listener calls a special dma_map handler that passes the new VA of each
section to the kernel using VFIO_DMA_MAP_FLAG_VADDR. Restore the normal
handler at the end.
Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
---
hw/vfio/container.c | 15 +++++++++++++--
hw/vfio/cpr-legacy.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index a554683..0e02726 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -137,6 +137,8 @@ static int vfio_legacy_dma_unmap_one(const
VFIOContainerBase *bcontainer,
int ret;
Error *local_err = NULL;
+ assert(!container->cpr.reused);
+
if (iotlb && vfio_container_dirty_tracking_is_started(bcontainer)) {
if (!vfio_container_devices_dirty_tracking_is_supported(bcontainer) &&
bcontainer->dirty_pages_supported) {
@@ -691,8 +693,17 @@ static bool vfio_container_connect(VFIOGroup *group,
AddressSpace *as,
}
group_was_added = true;
- if (!vfio_listener_register(bcontainer, errp)) {
- goto fail;
+ /*
+ * 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
+ * dma_map to supply the new vaddr, and the calls will match the mappings
+ * remembered by the kernel.
+ */
+ if (!cpr_reused) {
+ if (!vfio_listener_register(bcontainer, errp)) {
+ goto fail;
+ }
hmm, I am starting to think we should have a vfio_cpr_container_connect
routine too.
I think that would obscure rather than clarify the code, since the normal
non-cpr action of calling vfio_listener_register would be buried in a
cpr flavored function name.
}
bcontainer->initialized = true;
diff --git a/hw/vfio/cpr-legacy.c b/hw/vfio/cpr-legacy.c
index 519d772..bbcf71e 100644
--- a/hw/vfio/cpr-legacy.c
+++ b/hw/vfio/cpr-legacy.c
@@ -11,11 +11,13 @@
#include "hw/vfio/vfio-container.h"
#include "hw/vfio/vfio-cpr.h"
#include "hw/vfio/vfio-device.h"
+#include "hw/vfio/vfio-listener.h"
#include "migration/blocker.h"
#include "migration/cpr.h"
#include "migration/migration.h"
#include "migration/vmstate.h"
#include "qapi/error.h"
+#include "qemu/error-report.h"
static bool vfio_dma_unmap_vaddr_all(VFIOContainer *container, Error **errp)
{
@@ -32,6 +34,34 @@ static bool vfio_dma_unmap_vaddr_all(VFIOContainer
*container, Error **errp)
return true;
}
+/*
+ * Set the new @vaddr for any mappings registered during cpr load.
+ * Reused is cleared thereafter.
+ */
+static int vfio_legacy_cpr_dma_map(const VFIOContainerBase *bcontainer,
+ hwaddr iova, ram_addr_t size, void *vaddr,
+ bool readonly)
+{
+ const VFIOContainer *container = container_of(bcontainer, VFIOContainer,
+ bcontainer);
+ struct vfio_iommu_type1_dma_map map = {
+ .argsz = sizeof(map),
+ .flags = VFIO_DMA_MAP_FLAG_VADDR,
+ .vaddr = (__u64)(uintptr_t)vaddr,
+ .iova = iova,
+ .size = size,
+ };
+
+ assert(container->cpr.reused);
+
+ if (ioctl(container->fd, VFIO_IOMMU_MAP_DMA, &map)) {
+ error_report("vfio_legacy_cpr_dma_map (iova %lu, size %ld, va %p): %s",
+ iova, size, vaddr, strerror(errno));
Callers should also report the error. No need to do it here.
This function has the same signature as the dma_map class method,
which does not return an error message. It's existing implementations
use error_report.
backends .dma_map handlers : vfio_legacy_dma_map(), iommufd_backend_map_dma()
don't report errors. vfio_container_dma_map() doesn't either.
callers of vfio_container_dma_map() : vfio_iommu_map_notify(),
vfio_listener_region_add() report errors.
Thanks,
C.
+ return -errno;
+ }
+
+ return 0;
+}
static bool vfio_cpr_supported(VFIOContainer *container, Error **errp)
{
@@ -63,12 +93,24 @@ static int vfio_container_pre_save(void *opaque)
static int vfio_container_post_load(void *opaque, int version_id)
{
VFIOContainer *container = opaque;
+ VFIOContainerBase *bcontainer = &container->bcontainer;
VFIOGroup *group;
VFIODevice *vbasedev;
+ Error *err = NULL;
+
+ if (!vfio_listener_register(bcontainer, &err)) {
+ error_report_err(err);
+ return -1;
+ }
container->cpr.reused = false;
QLIST_FOREACH(group, &container->group_list, container_next) {
+ VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
+
+ /* Restore original dma_map function */
+ vioc->dma_map = vfio_legacy_dma_map;
+
QLIST_FOREACH(vbasedev, &group->device_list, next) {
vbasedev->cpr.reused = false;
}
@@ -80,6 +122,7 @@ static const VMStateDescription vfio_container_vmstate = {
.name = "vfio-container",
.version_id = 0,
.minimum_version_id = 0,
+ .priority = MIG_PRI_LOW, /* Must happen after devices and groups */
.pre_save = vfio_container_pre_save,
.post_load = vfio_container_post_load,
.needed = cpr_needed_for_reuse,
@@ -104,6 +147,11 @@ 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 (container->cpr.reused) {
+ VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
+ vioc->dma_map = vfio_legacy_cpr_dma_map;
You could backup the previous dma_map() handler in a static variable or,
better, under container->cpr.
OK.
- Steve
