Re: [PATCH 2/2] vfio/migration: Use BE byte order for device state wire packets

[Avihai Horon]

On 07/03/2025 12:57, Maciej S. Szmigiero wrote:
External email: Use caution opening links or attachments


From: "Maciej S. Szmigiero" <maciej.szmigi...@oracle.com>

Wire data commonly use BE byte order (including in the existing migration
protocol), use it also for for VFIO device state packets.

Nit: should we add a sentence about the motivation? Something like:

This will allow VFIO multifd device state transfer between hosts with 
different endianness.
Although currently there is no such use case, it's good to have it now 
for completeness.


Fixes: 3228d311ab18 ("vfio/migration: Multifd device state transfer support - 
received buffers queuing")
Fixes: 6d644baef203 ("vfio/migration: Multifd device state transfer support - send 
side")
Signed-off-by: Maciej S. Szmigiero <maciej.szmigi...@oracle.com>

Reviewed-by: Avihai Horon <avih...@nvidia.com>

Thanks.

---
  hw/vfio/migration-multifd.c | 15 ++++++++++-----
  1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
index a9d41b9f1cb1..e816461e1652 100644
--- a/hw/vfio/migration-multifd.c
+++ b/hw/vfio/migration-multifd.c
@@ -13,6 +13,7 @@
  #include "hw/vfio/vfio-common.h"
  #include "migration/misc.h"
  #include "qapi/error.h"
+#include "qemu/bswap.h"
  #include "qemu/error-report.h"
  #include "qemu/lockable.h"
  #include "qemu/main-loop.h"
@@ -208,12 +209,16 @@ bool vfio_multifd_load_state_buffer(void *opaque, char 
*data, size_t data_size,
          return false;
      }

+    packet->version = be32_to_cpu(packet->version);
      if (packet->version != VFIO_DEVICE_STATE_PACKET_VER_CURRENT) {
          error_setg(errp, "%s: packet has unknown version %" PRIu32,
                     vbasedev->name, packet->version);
          return false;
      }

+    packet->idx = be32_to_cpu(packet->idx);
+    packet->flags = be32_to_cpu(packet->flags);
+
      if (packet->idx == UINT32_MAX) {
          error_setg(errp, "%s: packet index is invalid", vbasedev->name);
          return false;
@@ -682,9 +687,9 @@ vfio_save_complete_precopy_thread_config_state(VFIODevice 
*vbasedev,

      packet_len = sizeof(*packet) + bioc->usage;
      packet = g_malloc0(packet_len);
-    packet->version = VFIO_DEVICE_STATE_PACKET_VER_CURRENT;
-    packet->idx = idx;
-    packet->flags = VFIO_DEVICE_STATE_CONFIG_STATE;
+    packet->version = cpu_to_be32(VFIO_DEVICE_STATE_PACKET_VER_CURRENT);
+    packet->idx = cpu_to_be32(idx);
+    packet->flags = cpu_to_be32(VFIO_DEVICE_STATE_CONFIG_STATE);
      memcpy(&packet->data, bioc->data, bioc->usage);

      if (!multifd_queue_device_state(idstr, instance_id,
@@ -734,7 +739,7 @@ 
vfio_multifd_save_complete_precopy_thread(SaveLiveCompletePrecopyThreadData *d,
      }

      packet = g_malloc0(sizeof(*packet) + migration->data_buffer_size);
-    packet->version = VFIO_DEVICE_STATE_PACKET_VER_CURRENT;
+    packet->version = cpu_to_be32(VFIO_DEVICE_STATE_PACKET_VER_CURRENT);

      for (idx = 0; ; idx++) {
          ssize_t data_size;
@@ -755,7 +760,7 @@ 
vfio_multifd_save_complete_precopy_thread(SaveLiveCompletePrecopyThreadData *d,
              break;
          }

-        packet->idx = idx;
+        packet->idx = cpu_to_be32(idx);
          packet_size = sizeof(*packet) + data_size;

          if (!multifd_queue_device_state(d->idstr, d->instance_id,