On 3/7/25 11:57, Maciej S. Szmigiero wrote:
From: "Maciej S. Szmigiero" <maciej.szmigi...@oracle.com>
There's already a max in-flight VFIO device state buffers *count* limit,
no. there isn't. Do we need both ?
add also max queued buffers *size* limit.
Signed-off-by: Maciej S. Szmigiero <maciej.szmigi...@oracle.com>
---
docs/devel/migration/vfio.rst | 8 +++++---
hw/vfio/migration-multifd.c | 21 +++++++++++++++++++--
hw/vfio/pci.c | 9 +++++++++
include/hw/vfio/vfio-common.h | 1 +
4 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/docs/devel/migration/vfio.rst b/docs/devel/migration/vfio.rst
index 7c9cb7bdbf87..127a1db35949 100644
--- a/docs/devel/migration/vfio.rst
+++ b/docs/devel/migration/vfio.rst
@@ -254,12 +254,14 @@ This means that a malicious QEMU source could
theoretically cause the target
QEMU to allocate unlimited amounts of memory for such buffers-in-flight.
The "x-migration-max-queued-buffers" property allows capping the maximum count
-of these VFIO device state buffers queued at the destination.
+of these VFIO device state buffers queued at the destination while
+"x-migration-max-queued-buffers-size" property allows capping their total
queued
+size.
Because a malicious QEMU source causing OOM on the target is not expected to be
a realistic threat in most of VFIO live migration use cases and the right
value
-depends on the particular setup by default this queued buffers limit is
-disabled by setting it to UINT64_MAX.
+depends on the particular setup by default these queued buffers limits are
+disabled by setting them to UINT64_MAX.
Some host platforms (like ARM64) require that VFIO device config is loaded only
after all iterables were loaded.
diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
index dccd763d7c39..a9d41b9f1cb1 100644
--- a/hw/vfio/migration-multifd.c
+++ b/hw/vfio/migration-multifd.c
@@ -83,6 +83,7 @@ typedef struct VFIOMultifd {
uint32_t load_buf_idx;
uint32_t load_buf_idx_last;
uint32_t load_buf_queued_pending_buffers;
'load_buf_queued_pending_buffers' is not in mainline. Please rebase.
Thanks,
C.
+ size_t load_buf_queued_pending_buffers_size;
} VFIOMultifd;
static void vfio_state_buffer_clear(gpointer data)
@@ -139,6 +140,7 @@ static bool vfio_load_state_buffer_insert(VFIODevice
*vbasedev,
VFIOMigration *migration = vbasedev->migration;
VFIOMultifd *multifd = migration->multifd;
VFIOStateBuffer *lb;
+ size_t data_size = packet_total_size - sizeof(*packet);
vfio_state_buffers_assert_init(&multifd->load_bufs);
if (packet->idx >= vfio_state_buffers_size_get(&multifd->load_bufs)) {
@@ -165,8 +167,19 @@ static bool vfio_load_state_buffer_insert(VFIODevice
*vbasedev,
return false;
}
- lb->data = g_memdup2(&packet->data, packet_total_size - sizeof(*packet));
- lb->len = packet_total_size - sizeof(*packet);
+ multifd->load_buf_queued_pending_buffers_size += data_size;
+ if (multifd->load_buf_queued_pending_buffers_size >
+ vbasedev->migration_max_queued_buffers_size) {
+ error_setg(errp,
+ "%s: queuing state buffer %" PRIu32
+ " would exceed the size max of %" PRIu64,
+ vbasedev->name, packet->idx,
+ vbasedev->migration_max_queued_buffers_size);
+ return false;
+ }
+
+ lb->data = g_memdup2(&packet->data, data_size);
+ lb->len = data_size;
lb->is_present = true;
return true;
@@ -346,6 +359,9 @@ static bool vfio_load_state_buffer_write(VFIODevice
*vbasedev,
assert(wr_ret <= buf_len);
buf_len -= wr_ret;
buf_cur += wr_ret;
+
+ assert(multifd->load_buf_queued_pending_buffers_size >= wr_ret);
+ multifd->load_buf_queued_pending_buffers_size -= wr_ret;
}
trace_vfio_load_state_device_buffer_load_end(vbasedev->name,
@@ -519,6 +535,7 @@ static VFIOMultifd *vfio_multifd_new(void)
multifd->load_buf_idx = 0;
multifd->load_buf_idx_last = UINT32_MAX;
multifd->load_buf_queued_pending_buffers = 0;
+ multifd->load_buf_queued_pending_buffers_size = 0;
qemu_cond_init(&multifd->load_bufs_buffer_ready_cond);
multifd->load_bufs_iter_done = false;
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 02f784c1b2a3..8abf73f810ee 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3392,6 +3392,8 @@ static const Property vfio_pci_dev_properties[] = {
ON_OFF_AUTO_AUTO),
DEFINE_PROP_UINT64("x-migration-max-queued-buffers", VFIOPCIDevice,
vbasedev.migration_max_queued_buffers, UINT64_MAX),
+ DEFINE_PROP_SIZE("x-migration-max-queued-buffers-size", VFIOPCIDevice,
+ vbasedev.migration_max_queued_buffers_size, UINT64_MAX),
DEFINE_PROP_BOOL("migration-events", VFIOPCIDevice,
vbasedev.migration_events, false),
DEFINE_PROP_BOOL("x-no-mmap", VFIOPCIDevice, vbasedev.no_mmap, false),
@@ -3581,6 +3583,13 @@ static void vfio_pci_dev_class_init(ObjectClass *klass,
void *data)
"destination when doing live "
"migration of device state via "
"multifd channels");
+ object_class_property_set_description(klass, /* 10.0 */
+
"x-migration-max-queued-buffers-size",
+ "Maximum size of in-flight VFIO "
+ "device state buffers queued at the "
+ "destination when doing live "
+ "migration of device state via "
+ "multifd channels");
}
static const TypeInfo vfio_pci_dev_info = {
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index c8ff4252e24a..fff2f35754b2 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -158,6 +158,7 @@ typedef struct VFIODevice {
OnOffAuto migration_multifd_transfer;
OnOffAuto migration_load_config_after_iter;
uint64_t migration_max_queued_buffers;
+ uint64_t migration_max_queued_buffers_size;
bool migration_events;
VFIODeviceOps *ops;
unsigned int num_irqs;
